Skip to content

Conversation

@rphillips
Copy link
Contributor

- What I did

- How to verify it

- Description for the changelog

Add version_id to labels

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 10, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 10, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rphillips

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 10, 2022
@cgwalters
Copy link
Member

cgwalters commented May 10, 2022

I thought last time this came up we determined it wouldn't actually work because the labels from kubelet are only applied when the node is first created, so when we do an in-place update the label will become a lie?

Personally, while I see the rationale here I think it's going to be a better pattern to encourage daemonsets and other privileged containers to instead:

  • ship multiple copies of binaries they intend to run on the host, built via e.g. multi-stage build, once for each OS major (e.g. rhel7,rhel8,rhel9)
  • Dynamically inspect the host and copy the right binary
  • Profit

This is how we changed the SDN to work I think.

EDIT: See openshift/containernetworking-plugins@0b71f33 for one example.

@rphillips
Copy link
Contributor Author

I wasn't clear on the context. @sdodson asked me in slack .

@rphillips
Copy link
Contributor Author

I remember the conversation and agree. This is problematic.

@rphillips rphillips closed this May 10, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 10, 2022

@rphillips: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-agnostic-upgrade 188441e link true /test e2e-agnostic-upgrade

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@cgwalters
Copy link
Member

Had to dig it up but #514
which then eventually led to #584 (comment)

History goes in cycles 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants