Skip to content

Conversation

@cgwalters
Copy link
Member

This removes the functionality added in
#514
which I don't think really worked for what people needed; a node
selector can't parse the value.

All we care about is currently 7 or 8; this signals things like
iptables vs nftables.

And the presence of this label also signals "RHEL" too;
though we keep the os_id label in case e.g. a daemonset should only
run on RHCOS (or traditional RHEL).

Closes: #582

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 26, 2019
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cgwalters

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-robot openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 26, 2019
@cgwalters
Copy link
Member Author

Haven't done full testing of this yet, submitting WIP for visibility/early review.

@kikisdeliveryservice
Copy link
Contributor

to clarify, if:

  • rhelmajor = 7|8 then os_id = nil
  • os_id != nil then rhelmajor = nil

?

@cgwalters
Copy link
Member Author

to clarify, if:

Mmm, os_id is always set as it was before, I haven't changed that (right?). We will either have os_id=rhcos or os_id=rhel.

@kikisdeliveryservice
Copy link
Contributor

@cgwalters thanks, got it!

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 2, 2019
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 10, 2019
This removes the functionality added in
openshift#514
which I don't think really worked for what people needed; a node
selector can't parse the value.

All we care about is currently 7 or 8; this signals things like
`iptables` vs `nftables`.

And the presence of this label also signals "RHEL" too;
though we keep the `os_id` label in case e.g. a daemonset should only
run on RHCOS (or traditional RHEL).

Closes: openshift#582
@openshift-ci-robot
Copy link
Contributor

@cgwalters: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
ci/prow/e2e-aws a914f1b link /test e2e-aws

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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 Author

OK so I discovered this: --node-labels mapStringString <Warning: Alpha feature> Labels to add when registering the node in the cluster.

In other words, the labels aren't dynamic which is rather problematic for something that has a version number. I think the MCD should probably start managing the labels.

@cgwalters
Copy link
Member Author

Obsoleting in favor of #657

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. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add new node.openshift.io/rhelmajor=7|8 label

3 participants