OCPBUGS-31376: kubelet: restorecon necessary files on kubelet's prestart

[rphillips]

MCD will write the scripts after the kubelet relabel systemd unit is run, effectively resetting the selinux label. Let's restorecon the scripts within the kubelet systemd unit to guarantee the label when the kubelet systemd unit starts.

[openshift-ci-robot]

@rphillips: This pull request references Jira Issue OCPBUGS-31376, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

• bug is open, matching expected state (open)
• bug target version (4.16.0) matches configured target version for branch (4.16.0)
• bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

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 openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci-robot]

@rphillips: This pull request references Jira Issue OCPBUGS-31376, which is valid.

3 validation(s) were run on this bug

• bug is open, matching expected state (open)
• bug target version (4.16.0) matches configured target version for branch (4.16.0)
• bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

In response to this:

MCD will write the scripts after the kubelet relabel systemd unit is run, effectively resetting the selinux label. Let's restorecon the scripts within the kubelet systemd unit to guarantee the label when the kubelet systemd unit starts.

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 openshift-eng/jira-lifecycle-plugin repository.

[rphillips]

/cherry-pick release-4.15

[openshift-cherrypick-robot]

@rphillips: once the present PR merges, I will cherry-pick it on top of release-4.15 in a new PR and assign it to you.

In response to this:

/cherry-pick release-4.15

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.

[haircommander]

/lgtm

I think long-term it makes sense to move the kubenswrapper and kubensenter binaries to the kubelet rpm so we don't need to do this, but this is fine for now

[rphillips]

/approve

[yuqi-zhang]

MCD will write the scripts after the kubelet relabel systemd unit is run

On firstboot ignition writes this, and in subsequent boots the MCD writes the file pre-reboot. Since the old service ran before kubelet-dependencies.target, I would expect it to run in time.

Looking at the bug, I assume this is an install time issue, which, now I thin about it, is probably because of https://issues.redhat.com/browse/MCO-536

I'm fine with fixing it this way but technically I think this is an underlying issue in the MCO (which is a bit harder to fix).

Edit: thinking about this a bit more, I think this fix will end up with the same result. The files gets written without a service restart so it will break regardless

[rphillips]

@yuqi-zhang Thanks for the explanation. Agreed. We are seeing MCD update the files on the node, which then have the wrong selinux label. To fix this with the systemd units, we would have to create a restart dependency chain. Instead, I opted to fix this inline with the kubelet unit file for simplicity. We hope to migrate these scripts to the hyperkube rpm at some point, which would not have this problem.

Kevin is going to work on a openshift/origin integration test to make sure we don't hit this problem in the future.

[yuqi-zhang]

Ok, the understanding here is that the selinux labels can be overwritten after the kubelet already starts running, and that's not a problem. This is only a problem iff the kubelet restarts for some reason and the file has the wrong selinux label at that point, which I think should be covered by this.

/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: haircommander, rphillips, yuqi-zhang

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 [yuqi-zhang]

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]

/retest-required

Remaining retests: 0 against base HEAD 33b202c and 2 for PR HEAD 52cae48 in total

[rphillips]

/retest-required

[openshift-ci]

@rphillips: The following tests 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/okd-scos-e2e-aws-ovn	52cae48	link	false	/test okd-scos-e2e-aws-ovn
ci/prow/e2e-azure-ovn-upgrade-out-of-change	52cae48	link	false	/test e2e-azure-ovn-upgrade-out-of-change

Full PR test history. Your PR dashboard.

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.

[openshift-ci-robot]

/retest-required

Remaining retests: 0 against base HEAD 7f0beb8 and 1 for PR HEAD 52cae48 in total

[openshift-ci-robot]

@rphillips: Jira Issue OCPBUGS-31376: All pull requests linked via external trackers have merged:

• openshift/machine-config-operator#4287

Jira Issue OCPBUGS-31376 has been moved to the MODIFIED state.

In response to this:

MCD will write the scripts after the kubelet relabel systemd unit is run, effectively resetting the selinux label. Let's restorecon the scripts within the kubelet systemd unit to guarantee the label when the kubelet systemd unit starts.

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 openshift-eng/jira-lifecycle-plugin repository.

[openshift-cherrypick-robot]

@rphillips: #4287 failed to apply on top of branch "release-4.15":

Applying: kubelet: restorecon necessary files on kubelet's prestart
Using index info to reconstruct a base tree...
M	templates/master/01-master-kubelet/_base/units/kubelet.service.yaml
M	templates/master/01-master-kubelet/alibabacloud/units/kubelet.service.yaml
M	templates/master/01-master-kubelet/on-prem/units/kubelet.service.yaml
M	templates/worker/01-worker-kubelet/_base/units/kubelet.service.yaml
M	templates/worker/01-worker-kubelet/alibabacloud/units/kubelet.service.yaml
M	templates/worker/01-worker-kubelet/on-prem/units/kubelet.service.yaml
Falling back to patching base and 3-way merge...
Auto-merging templates/worker/01-worker-kubelet/on-prem/units/kubelet.service.yaml
CONFLICT (content): Merge conflict in templates/worker/01-worker-kubelet/on-prem/units/kubelet.service.yaml
Auto-merging templates/worker/01-worker-kubelet/alibabacloud/units/kubelet.service.yaml
CONFLICT (content): Merge conflict in templates/worker/01-worker-kubelet/alibabacloud/units/kubelet.service.yaml
Auto-merging templates/worker/01-worker-kubelet/_base/units/kubelet.service.yaml
CONFLICT (content): Merge conflict in templates/worker/01-worker-kubelet/_base/units/kubelet.service.yaml
Auto-merging templates/master/01-master-kubelet/on-prem/units/kubelet.service.yaml
CONFLICT (content): Merge conflict in templates/master/01-master-kubelet/on-prem/units/kubelet.service.yaml
Auto-merging templates/master/01-master-kubelet/alibabacloud/units/kubelet.service.yaml
CONFLICT (content): Merge conflict in templates/master/01-master-kubelet/alibabacloud/units/kubelet.service.yaml
Auto-merging templates/master/01-master-kubelet/_base/units/kubelet.service.yaml
CONFLICT (content): Merge conflict in templates/master/01-master-kubelet/_base/units/kubelet.service.yaml
Removing templates/common/_base/units/kubelet-selinux-relabel.yaml
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 kubelet: restorecon necessary files on kubelet's prestart
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick release-4.15

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.

[openshift-bot]

[ART PR BUILD NOTIFIER]

This PR has been included in build ose-machine-config-operator-container-v4.16.0-202403290310.p0.ga9112cf.assembly.stream.el8 for distgit ose-machine-config-operator.
All builds following this will include this PR.

[openshift-merge-robot]

Fix included in accepted release 4.16.0-0.nightly-2024-03-30-033956

[cgwalters]

Belated review but and I know this is just moving the original code but there is a giant difference between /usr/local/bin and /usr/bin: It's only the former that the MCO writes.

The files in /usr/bin are written by ostree, and are immutable at runtime and will always have their labels correct (barring corruption).

Whoever added the code originally here probably added the - to the ExecStart because they got EROFS, but we should have never needed to do that restorecon in the first place.

[rphillips]

We have to do the restorecon because MCD is laying down files after boot (about 5 minutes later). The overwritten files (that contain the same content) do not have the correct label written via MCD.

[cgwalters]

I'm not questioning the restorecon for /usr/local/bin (though it is unfortunate). I'm saying the /usr/bin/ one should be unnecessary.