Add multi-pod tests with SELinux mounts #113789

jsafrane · 2022-11-09T13:20:13Z

What type of PR is this?

/kind feature
(?)

What this PR does / why we need it:

Add test to check that a volume is fully unmounted (NodeUnstaged) before kubelet starts a pod with the same volume, but with a different SELinux context.

Does this PR introduce a user-facing change?

NONE

kubernetes/enhancements#3548

- [KEP]: https://github.com/kubernetes/enhancements/tree/master/keps/sig-storage/1710-selinux-relabeling

k8s-ci-robot · 2022-11-09T13:20:21Z

@jsafrane: This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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.

jsafrane · 2022-11-09T13:20:41Z

/sig storage
/priority important-soon

jsafrane · 2022-11-09T13:24:03Z

cc @gnufied

gnufied · 2022-11-11T22:16:18Z

The verify failure looks legit:

test/e2e/storage/csi_mock_volume.go:2165:5: ineffectual assignment to err (ineffassign)
				err = e2eevents.WaitTimeoutForEvent(m.cs, pod2.Namespace, eventSelector, msg, f.Timeouts.PodStart)

dims · 2022-11-14T17:03:16Z

@jsafrane if you need this to land in 1.26, please fix the problem in the verify log and get approval/lgtm!

jsafrane · 2022-11-24T12:31:58Z

marked as WIP, it does not need to catch 1.26

jsafrane · 2022-12-09T17:46:10Z

Rebased & updated & removed WIP

dims · 2022-12-12T15:34:47Z

If you still need this PR then please rebase, if not, please close the PR

jsafrane · 2022-12-13T12:19:16Z

If you still need this PR then please rebase, if not, please close the PR

There is nothing to rebase. I updated the PR 4 days ago and I still want it in 1.27.

/retest

"ResourceQuota with cross namespace pod affinity scope using scope-selectors" flaked

test/e2e/storage/csi_mock_volume.go

jsafrane · 2023-01-04T12:46:10Z

I did non-trivial rebase +

renamed "first pod" to "the initial pod".
added a few comments + rephrased some ExpectNoError contexts.

Check that a volume is fully unmounted (Unstaged) before kubelet starts a pod with the same volume, but with a different SELinux context.

pohly

Error strings look good to me, just some minor comments.

pohly · 2023-01-09T13:56:46Z

test/e2e/storage/csi_mock/base.go

 				mountVolume := nodePublishRequest.GetVolumeCapability().GetMount()
 				if mountVolume != nil {
 					*nodePublishMountOpts = mountVolume.MountFlags
 				}
 			}
+			_, ok = request.(*csipbv1.NodeUnstageVolumeRequest)


Missing error handling?

I think the code is correct, still, I rewrote it to switch request.(type) to be more obvious.

pohly · 2023-01-09T13:56:56Z

test/e2e/storage/csi_mock/base.go

+			if ok {
+				unstageCalls.Add(1)
+			}
+			_, ok = request.(*csipbv1.NodeUnpublishVolumeRequest)


pohly · 2023-01-09T13:58:24Z

test/e2e/storage/csi_mock/csi_selinux_mount.go

 				})
-				ginkgo.DeferCleanup(m.cleanup)
+				defer m.cleanup(ctx)


Why are you reverting to using defer here?

It will not be able to clean up when the test gets interrupted because then ctx will be canceled.

Good catch, using DeferCleaup now

pohly · 2023-01-09T14:02:08Z

test/e2e/storage/csi_mock/csi_selinux_mount.go

+
+				// Skip scheduler, it would block scheduling the second pod with ReadWriteOncePod PV.
+				pod, err = m.cs.CoreV1().Pods(pod.Namespace).Get(ctx, pod.Name, metav1.GetOptions{})
+				framework.ExpectNoError(err, fmt.Sprintf("getting the initial pod"))


No need for fmt.Sprintf: there's nothing to format, and even if there was, ExpectNoError could be passed a format string plus parameters.

Replace it with ginkgo.DeferCleanup

jsafrane · 2023-01-17T14:56:10Z

All three failed this test: [It] [sig-cli] Kubectl logs default container logs the second container is the default-container by annotation should log default container if not specified

/retest

pohly

Looks good to me.

gnufied · 2023-01-18T20:15:03Z

/lgtm
/approve

k8s-ci-robot · 2023-01-18T20:15:11Z

LGTM label has been added.

Git tree hash: 87da4005167590625f91f021f39fae471f56191c

k8s-ci-robot · 2023-01-18T20:15:31Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gnufied, jsafrane

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/storage/OWNERS~~ [gnufied,jsafrane]

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 requested review from humblec and pohly November 9, 2022 13:20

k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. area/test sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Nov 9, 2022

jsafrane changed the title ~~Add multi-pod tests with SELinux mounts~~ WIP: Add multi-pod tests with SELinux mounts Nov 24, 2022

k8s-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 Nov 24, 2022

jsafrane force-pushed the add-selinux-tests branch from eb354ba to b2fd027 Compare December 9, 2022 17:46

jsafrane changed the title ~~WIP: Add multi-pod tests with SELinux mounts~~ Add multi-pod tests with SELinux mounts Dec 9, 2022

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 Dec 9, 2022

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 14, 2022

gnufied mentioned this pull request Dec 14, 2022

Test coverage of volume relabeling is lacking #86080

Open

SergeyKanzhelev reviewed Dec 14, 2022

View reviewed changes

test/e2e/storage/csi_mock_volume.go Outdated Show resolved Hide resolved

SergeyKanzhelev reviewed Dec 14, 2022

View reviewed changes

test/e2e/storage/csi_mock_volume.go Outdated Show resolved Hide resolved

jsafrane force-pushed the add-selinux-tests branch from b2fd027 to 261aa14 Compare January 4, 2023 12:44

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 4, 2023

jsafrane force-pushed the add-selinux-tests branch from 261aa14 to 1b1d4b7 Compare January 4, 2023 12:46

jsafrane force-pushed the add-selinux-tests branch from 1b1d4b7 to 39419d2 Compare January 4, 2023 14:05

Add multi-pod tests with SELinux mounts

e1f62a9

Check that a volume is fully unmounted (Unstaged) before kubelet starts a pod with the same volume, but with a different SELinux context.

jsafrane force-pushed the add-selinux-tests branch from 39419d2 to e1f62a9 Compare January 5, 2023 12:15

Update error context messages

03beb01

pohly suggested changes Jan 9, 2023

View reviewed changes

jsafrane added 3 commits January 17, 2023 11:22

Remove defer

de4ce7b

Replace it with ginkgo.DeferCleanup

Remove useless Sprintf

d2bb866

Rework createSELinuxMountPreHook to switch()

b9c2449

pohly reviewed Jan 18, 2023

View reviewed changes

k8s-ci-robot assigned gnufied Jan 18, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 18, 2023

k8s-ci-robot merged commit dfbe1c0 into kubernetes:master Jan 18, 2023

k8s-ci-robot added this to the v1.27 milestone Jan 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add multi-pod tests with SELinux mounts #113789

Add multi-pod tests with SELinux mounts #113789

jsafrane commented Nov 9, 2022 •

edited

k8s-ci-robot commented Nov 9, 2022

jsafrane commented Nov 9, 2022

jsafrane commented Nov 9, 2022

gnufied commented Nov 11, 2022

dims commented Nov 14, 2022

jsafrane commented Nov 24, 2022

jsafrane commented Dec 9, 2022

dims commented Dec 12, 2022

jsafrane commented Dec 13, 2022

jsafrane commented Jan 4, 2023

pohly left a comment

pohly Jan 9, 2023

jsafrane Jan 17, 2023

pohly Jan 9, 2023

pohly Jan 9, 2023

jsafrane Jan 17, 2023

pohly Jan 9, 2023

jsafrane Jan 17, 2023

jsafrane commented Jan 17, 2023

pohly left a comment

gnufied commented Jan 18, 2023

k8s-ci-robot commented Jan 18, 2023

k8s-ci-robot commented Jan 18, 2023

Add multi-pod tests with SELinux mounts #113789

Add multi-pod tests with SELinux mounts #113789

Conversation

jsafrane commented Nov 9, 2022 • edited

What type of PR is this?

What this PR does / why we need it:

Does this PR introduce a user-facing change?

k8s-ci-robot commented Nov 9, 2022

jsafrane commented Nov 9, 2022

jsafrane commented Nov 9, 2022

gnufied commented Nov 11, 2022

dims commented Nov 14, 2022

jsafrane commented Nov 24, 2022

jsafrane commented Dec 9, 2022

dims commented Dec 12, 2022

jsafrane commented Dec 13, 2022

jsafrane commented Jan 4, 2023

pohly left a comment

Choose a reason for hiding this comment

pohly Jan 9, 2023

Choose a reason for hiding this comment

jsafrane Jan 17, 2023

Choose a reason for hiding this comment

pohly Jan 9, 2023

Choose a reason for hiding this comment

pohly Jan 9, 2023

Choose a reason for hiding this comment

jsafrane Jan 17, 2023

Choose a reason for hiding this comment

pohly Jan 9, 2023

Choose a reason for hiding this comment

jsafrane Jan 17, 2023

Choose a reason for hiding this comment

jsafrane commented Jan 17, 2023

pohly left a comment

Choose a reason for hiding this comment

gnufied commented Jan 18, 2023

k8s-ci-robot commented Jan 18, 2023

k8s-ci-robot commented Jan 18, 2023

jsafrane commented Nov 9, 2022 •

edited