initial pass at populating the os version label

[rphillips]

- What I did

Enumerates the OS version from /etc/os-release and creates a node.openshift.io/os_version={OS_VERSION} label.

- How to verify it
oc get node [node name] -o yaml

- Description for the changelog
Add an initial OS Version to label (node.openshift.io/os_version={OS_VERSION}) to nodes.

- Some background info
Kubelet --node-labels only get created on kubelet registration, so changes will need to happen with a daemonset. Liggit has a number of comments in upstream saying the kubelet should not update labels, so a custom patch would probably never be merged upstream (IMO). There are differing opinions on the ticket.

This PR is phase 1) create the label with the starting version. The second phase will be to create a daemonset to update the label on RHEL7->RHEL8 upgrades. A final third phase, when RHEL7 is not supported anymore, we can retire the daemonset and delete the code/daemonset.

/cc @jeremyeder @sjenning @smarterclayton @derekwaynecarr

[kikisdeliveryservice]

Some comments in line.

Also is there an open issue somewhere for this? Also since there is a bug fix is there approval to put this in 4.0 because the MCO is putting all new enchancements/features into our master-4.1 branch at this time.

cc: @runcom

[kikisdeliveryservice]

Until there is confirmation that this should go into 4.0

/hold

[rphillips]

@kikisdeliveryservice changed to the 4.1 branch

[rphillips]

There has been some discussion on aos-devel regarding how networking can figure out if the underlying OS is RHEL7 or RHEL8.

[kikisdeliveryservice]

Thanks @rphillips !
/hold cancel

changes made as requested, will allow others to review.

[derekwaynecarr]

this is only needed for 4.1. In 4.0, we know a node not labeled is rhel7, and in 4.1, pods that require rhel8 will have nodeSelectors that will hold scheduling until a label is present. In 4.1, rhel7 workers are new nodes, and would have newly registered labels which should be fine with general approach.

[squeed]

Why not just add /etc/os-release as an EnvironmentFile and pull it out from there?

[rphillips]

that does simplify things. fixed!

[sdodson]

Should we add similar support so that we can differentiate between RHCOS and RHEL Server or other?

[rphillips]

@sdodson which variable would you like to add from os-version?

[ashcrow]

This looks good to me. Just to remind folks, you'll end up getting a version like this on RHCOS:

400.7.20190301.0

Related:

• Design: Change version numbering scheme os#259
• Impl: Update version to be inline with  os#360

[sdodson]

The variable ID from /etc/os-release seems like a good source for the value but we probably want to name the label differently. os_id perhaps?

[rphillips]

@sdodson My general thought was to have os_version correspond to ${VERSION} within the os-release. If we want to add more variables from os-release, then they might have a 1 to 1 mapping. os_id would map to ${ID} within os-release.

[sdodson]

Yes, that's what I meant, an additional label for os_id.

[rphillips]

I think this PR is ready for approval unless there are any further comments.

Edit: I can add that id in.

[rphillips]

@sdodson added the ID

[rphillips]

/retest

[rphillips]

/retest

[rphillips]

/retest

[rphillips]

@kikisdeliveryservice after talking to @sjenning, we need this in 4.0.

I've switched the target branch back to master.

[kikisdeliveryservice]

thanks for the update @rphillips

Could you amend the body of the commit messages to add a little more info? For ex, os version using: "Enumerates the OS version from /etc/os-release and creates a node.openshift.io/os_version={OS_VERSION} label." and something substantive for the id label commit.

[kikisdeliveryservice]

@runcom @ashcrow @cgwalters

Any comments on this PR?

[rphillips]

@kikisdeliveryservice updated the commit message and comment.

[kikisdeliveryservice]

thanks @rphillips !

This looks good. Can verify that labels were created for each node. Checking e2e-aws/artifacts/nodes.json. For ex:

                "labels": {
                    "beta.kubernetes.io/arch": "amd64",
                    "beta.kubernetes.io/instance-type": "m4.xlarge",
                    "beta.kubernetes.io/os": "linux",
                    "failure-domain.beta.kubernetes.io/region": "us-east-1",
                    "failure-domain.beta.kubernetes.io/zone": "us-east-1b",
                    "kubernetes.io/hostname": "ip-10-0-154-8",
                    "node-role.kubernetes.io/master": "",
                    "node.openshift.io/os_id": "rhcos",
                    "node.openshift.io/os_version": "400.7.20190306.0"

[runcom]

code lgtm, I'd love @sjenning to approve

[sjenning]

/lgtm

[kikisdeliveryservice]

/approve

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kikisdeliveryservice, rphillips, sjenning

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

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [kikisdeliveryservice]

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

[vrutkovs]

/test e2e-rhel-scaleup

[vrutkovs]

This breaks RHEL scaleup:

kubelet_node_status.go:92] Unable to register node "ip-10-0-156-42.ec2.internal" with API server: Node "ip-10-0-156-42.ec2.internal" is invalid: metadata.labels: Invalid value: "7 (Core)": a valid label must be an empty string or consist of alphanumeric characters, ''-'', ''_'' or ''.'', and must start and end with an alphanumeric character (e.g. ''MyValue'',  or ''my_value'',  or ''12345'', regex used for validation is ''(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?'')'