Add SELinux labels for kubelet on Fedora CoreOS #95035

harche · 2020-09-24T15:29:25Z

Signed-off-by: Harshal Patil harpatil@redhat.com

What type of PR is this?

/kind failing-test

What this PR does / why we need it:
After extracting the kubelet binaries on the remote host, but just before starting the tests, this PR will try to set the right SELinux labels on the Fedora CoreOS node.

Which issue(s) this PR fixes:

Fixes #95034

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

k8s-ci-robot · 2020-09-24T15:29:33Z

Hi @harche. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

harche · 2020-09-25T14:39:34Z

/assign @harche

mrunalp · 2020-09-25T16:34:18Z

test/e2e_node/remote/node_e2e.go

 		return prependMemcgNotificationFlag(args), nil
+	case strings.Contains(output, "fedora"):


Might as well get rhel and centos, too.

Thanks, I will add those too.

I added the check for,

Fedora CoreOS and Fedora - fedora

CentOS - centos

RHEL - rhel

Red Hat Core OS - rhcos

rphillips · 2020-09-25T17:16:07Z

/ok-to-test

harche · 2020-09-25T18:21:43Z

Go playground for testing the regex in this PR, https://play.golang.org/p/WmAZTuFWsBD

harche · 2020-09-26T00:20:51Z

/test pull-kubernetes-e2e-kind

harche · 2020-09-26T00:58:56Z

/test pull-kubernetes-node-crio-e2e

harche · 2020-09-26T01:13:31Z

/hold

harche · 2020-09-29T06:14:53Z

/test pull-kubernetes-node-crio-e2e

harche · 2020-09-29T06:36:23Z

/test pull-kubernetes-node-crio-e2e

harche · 2020-09-29T07:28:34Z

/test pull-kubernetes-node-crio-e2e

harche · 2020-09-29T10:13:02Z

/test pull-kubernetes-node-crio-cgrpv2-e2e

dims · 2020-09-29T10:26:00Z

/lgtm

reapplying LGTM

oomichi · 2020-09-30T23:08:01Z

test/e2e_node/remote/node_e2e.go

 		return prependMemcgNotificationFlag(args), nil
+	case strings.Contains(output, "fedora"), strings.Contains(output, "rhcos"),


I guess if the output contains "rhcos", the line 112 catches it on strings.Contains(output, "cos") instead of here.
It is better to change the order or check string strictly.

/cc @oomichi

Thanks @oomichi

That's a really good point, if this is order dependent that's a defect waiting to happen in the future.

Signed-off-by: Harshal Patil <harpatil@redhat.com>

k8s-ci-robot · 2020-10-06T09:29:53Z

@harche: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
pull-kubernetes-node-crio-e2e	f6ab6ed5d1d0fbcbc83a567a005eb0a8f6a19995	link	`/test pull-kubernetes-node-crio-e2e`
pull-kubernetes-node-crio-cgrpv2-e2e	f6ab6ed5d1d0fbcbc83a567a005eb0a8f6a19995	link	`/test pull-kubernetes-node-crio-cgrpv2-e2e`

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.

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.

harche · 2020-10-07T12:31:54Z

/test pull-kubernetes-node-e2e

harche · 2020-10-07T12:51:28Z

/test pull-kubernetes-bazel-build

harche · 2020-10-07T12:55:18Z

/hold cancel

rphillips · 2020-10-07T12:57:28Z

/lgtm

rphillips · 2020-10-07T12:58:01Z

/assign @dims @sjenning

MHBauer

Nit, but overall looks like an improvement.

MHBauer · 2020-10-08T16:01:18Z

test/e2e_node/remote/node_e2e.go

 		args = prependMemcgNotificationFlag(args)
 		return prependCOSMounterFlag(args, host, workspace)
-	case strings.Contains(output, "ID=ubuntu"):
+	case strings.Contains(output, "ubuntu"):


why remove the ID=?

Ah, down below a stricter regex to find that and pull it out ahead of time.

MHBauer · 2020-10-08T16:02:32Z

test/e2e_node/remote/node_e2e.go

+		return "", fmt.Errorf("issue detecting node's OS via node's /etc/os-release. Err: %v, Output:\n%s", err, output)
+	}
+
+	var re = regexp.MustCompile(`(?m)^ID="?(\w+)"?`)


ID= used here instead.

MHBauer · 2020-10-08T16:04:25Z

test/e2e_node/remote/node_e2e.go

+		fmt.Sprintf("/usr/bin/chcon -u system_u -r object_r -t bin_t %s", filepath.Join(workspace, "ginkgo")),
+		fmt.Sprintf("/usr/bin/chcon -u system_u -r object_r -t bin_t %s", filepath.Join(workspace, "mounter")),
+		fmt.Sprintf("/usr/bin/chcon -R -u system_u -r object_r -t bin_t %s", filepath.Join(workspace, "cni", "bin/")),
+	)


I am unfamiliar with SELinux, and so checking my understanding.
These binaries need to be added because they're not part of the distribution itself, and they weren't created on the OS, but were extracted/pushed/pulled from somewhere else.
Is that correct?

We had to add those SELinux labels explicitly because otherwise SELinux will prevent the execution of those binaries. By default, when you copy those binaries in certain location on the filesystem will not have those required labels.

MHBauer · 2020-10-08T16:05:15Z

test/e2e_node/remote/node_e2e.go

 		return prependMemcgNotificationFlag(args), nil
 	}
 	return args, nil
 }

+// setKubeletSELinuxLabels set the appropriate SELinux labels for the
+// kubelet on Fedora CoreOS distribution
+func setKubeletSELinuxLabels(host, workspace string) error {


Function name is more than only the Kubelet. Maybe drop Kubelet to setSELinuxLabels?

MHBauer · 2020-10-08T16:05:43Z

test/e2e_node/remote/node_e2e.go

@@ -134,7 +169,7 @@ func (n *NodeE2ERemote) RunTest(host, workspace, results, imageDesc, junitFilePr
 	// Kill any running node processes
 	cleanupNodeProcesses(host)

-	testArgs, err := updateOSSpecificKubeletFlags(testArgs, host, workspace)
+	testArgs, err := osSpecificActions(testArgs, host, workspace)


Probably a better name now.

sjenning · 2020-10-21T03:16:52Z

/approve

k8s-ci-robot · 2020-10-21T03:17:22Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dims, harche, 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:

~~test/e2e_node/OWNERS~~ [sjenning]

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

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Sep 24, 2020

k8s-ci-robot requested review from karan and mattjmcnaughton September 24, 2020 15:30

k8s-ci-robot added area/test sig/node Categorizes an issue or PR as relevant to SIG Node. sig/testing Categorizes an issue or PR as relevant to SIG Testing. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Sep 24, 2020

harche mentioned this pull request Sep 24, 2020

Add SELinux labels to crio related artifacts kubernetes/test-infra#19342

Merged

harche changed the title ~~[WIP] Add SELinux labels for kubelet on Fedora CoreOS~~ Add SELinux labels for kubelet on Fedora CoreOS Sep 25, 2020

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 25, 2020

k8s-ci-robot assigned harche Sep 25, 2020

mrunalp reviewed Sep 25, 2020

View reviewed changes

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Sep 25, 2020

harche force-pushed the selinux_kubelet branch from 3db6f2c to e907883 Compare September 25, 2020 18:33

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 26, 2020

harche force-pushed the selinux_kubelet branch from 4476124 to b400b54 Compare September 29, 2020 06:35

harche force-pushed the selinux_kubelet branch from b400b54 to f6ab6ed Compare September 29, 2020 07:28

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 29, 2020

oomichi reviewed Sep 30, 2020

View reviewed changes

k8s-ci-robot requested a review from oomichi September 30, 2020 23:08

harche force-pushed the selinux_kubelet branch from f6ab6ed to d3a26b6 Compare October 6, 2020 09:16

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 6, 2020

Add SELinux labels for kubelet on Fedora CoreOS

a4cd6f1

Signed-off-by: Harshal Patil <harpatil@redhat.com>

harche force-pushed the selinux_kubelet branch from d3a26b6 to a4cd6f1 Compare October 6, 2020 09:24

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 7, 2020

k8s-ci-robot assigned rphillips Oct 7, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 7, 2020

k8s-ci-robot assigned sjenning Oct 7, 2020

MHBauer reviewed Oct 8, 2020

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 21, 2020

k8s-ci-robot merged commit 5d49a62 into kubernetes:master Oct 21, 2020

k8s-ci-robot added this to the v1.20 milestone Oct 21, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add SELinux labels for kubelet on Fedora CoreOS #95035

Add SELinux labels for kubelet on Fedora CoreOS #95035

harche commented Sep 24, 2020

k8s-ci-robot commented Sep 24, 2020

harche commented Sep 25, 2020

mrunalp Sep 25, 2020 •

edited

harche Sep 25, 2020

harche Sep 25, 2020

rphillips commented Sep 25, 2020

harche commented Sep 25, 2020

harche commented Sep 26, 2020

harche commented Sep 26, 2020

harche commented Sep 26, 2020

harche commented Sep 29, 2020

harche commented Sep 29, 2020

harche commented Sep 29, 2020

harche commented Sep 29, 2020

dims commented Sep 29, 2020

oomichi Sep 30, 2020

harche Oct 6, 2020

MHBauer Oct 8, 2020

k8s-ci-robot commented Oct 6, 2020 •

edited

harche commented Oct 7, 2020

harche commented Oct 7, 2020

harche commented Oct 7, 2020

rphillips commented Oct 7, 2020

rphillips commented Oct 7, 2020

MHBauer left a comment

MHBauer Oct 8, 2020

MHBauer Oct 8, 2020

MHBauer Oct 8, 2020

MHBauer Oct 8, 2020

harche Oct 21, 2020

MHBauer Oct 8, 2020

MHBauer Oct 8, 2020

sjenning commented Oct 21, 2020

k8s-ci-robot commented Oct 21, 2020

		return prependMemcgNotificationFlag(args), nil
		case strings.Contains(output, "fedora"):

		return prependMemcgNotificationFlag(args), nil
		case strings.Contains(output, "fedora"), strings.Contains(output, "rhcos"),

Add SELinux labels for kubelet on Fedora CoreOS #95035

Add SELinux labels for kubelet on Fedora CoreOS #95035

Conversation

harche commented Sep 24, 2020

k8s-ci-robot commented Sep 24, 2020

harche commented Sep 25, 2020

mrunalp Sep 25, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rphillips commented Sep 25, 2020

harche commented Sep 25, 2020

harche commented Sep 26, 2020

harche commented Sep 26, 2020

harche commented Sep 26, 2020

harche commented Sep 29, 2020

harche commented Sep 29, 2020

harche commented Sep 29, 2020

harche commented Sep 29, 2020

dims commented Sep 29, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k8s-ci-robot commented Oct 6, 2020 • edited

harche commented Oct 7, 2020

harche commented Oct 7, 2020

harche commented Oct 7, 2020

rphillips commented Oct 7, 2020

rphillips commented Oct 7, 2020

MHBauer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sjenning commented Oct 21, 2020

k8s-ci-robot commented Oct 21, 2020

mrunalp Sep 25, 2020 •

edited

k8s-ci-robot commented Oct 6, 2020 •

edited