-
Notifications
You must be signed in to change notification settings - Fork 545
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support & e2e for mountOptions & efficient selinux relabelling support #3902
Merged
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,3 +14,4 @@ spec: | |
attachRequired: false | ||
podInfoOnMount: false | ||
fsGroupPolicy: File | ||
seLinuxMount: true |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,5 +13,6 @@ metadata: | |
spec: | ||
attachRequired: false | ||
fsGroupPolicy: File | ||
seLinuxMount: true | ||
volumeLifecycleModes: | ||
- Persistent |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,4 +13,5 @@ metadata: | |
spec: | ||
attachRequired: true | ||
podInfoOnMount: false | ||
seLinuxMount: true | ||
fsGroupPolicy: File |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,6 +31,7 @@ import ( | |
"k8s.io/kubernetes/pkg/client/conditions" | ||
"k8s.io/kubernetes/test/e2e/framework" | ||
e2epod "k8s.io/kubernetes/test/e2e/framework/pod" | ||
frameworkPod "k8s.io/kubernetes/test/e2e/framework/pod" | ||
) | ||
|
||
const errRWOPConflict = "node has pod using PersistentVolumeClaim with the same name and ReadWriteOncePod access mode." | ||
|
@@ -164,6 +165,28 @@ func execCommandInDaemonsetPod( | |
f *framework.Framework, | ||
c, daemonsetName, nodeName, containerName, ns string, | ||
) (string, error) { | ||
podName, err := getDaemonsetPodOnNode(f, daemonsetName, nodeName, ns) | ||
if err != nil { | ||
return "", err | ||
} | ||
|
||
cmd := []string{"/bin/sh", "-c", c} | ||
podOpt := e2epod.ExecOptions{ | ||
Command: cmd, | ||
Namespace: ns, | ||
PodName: podName, | ||
ContainerName: containerName, | ||
CaptureStdout: true, | ||
CaptureStderr: true, | ||
} | ||
|
||
_ /* stdout */, stderr, err := execWithRetry(f, &podOpt) | ||
|
||
return stderr, err | ||
} | ||
|
||
// getDaemonsetPodOnNode returns the name of a daemonset pod on a particular node. | ||
func getDaemonsetPodOnNode(f *framework.Framework, daemonsetName, nodeName, ns string) (string, error) { | ||
selector, err := getDaemonSetLabelSelector(f, ns, daemonsetName) | ||
if err != nil { | ||
return "", err | ||
|
@@ -187,19 +210,7 @@ func execCommandInDaemonsetPod( | |
return "", fmt.Errorf("%s daemonset pod on node %s in namespace %s not found", daemonsetName, nodeName, ns) | ||
} | ||
|
||
cmd := []string{"/bin/sh", "-c", c} | ||
podOpt := e2epod.ExecOptions{ | ||
Command: cmd, | ||
Namespace: ns, | ||
PodName: podName, | ||
ContainerName: containerName, | ||
CaptureStdout: true, | ||
CaptureStderr: true, | ||
} | ||
|
||
_ /* stdout */, stderr, err := execWithRetry(f, &podOpt) | ||
|
||
return stderr, err | ||
return podName, nil | ||
} | ||
|
||
// listPods returns slice of pods matching given ListOptions and namespace. | ||
|
@@ -542,3 +553,73 @@ func validateRWOPPodCreation( | |
|
||
return nil | ||
} | ||
|
||
// verifySeLinuxMountOption verifies the SeLinux context MountOption added to PV.Spec.MountOption | ||
// is successfully used by nodeplugin during mounting by checking for its presence in the | ||
// nodeplugin container logs. | ||
func verifySeLinuxMountOption( | ||
f *framework.Framework, | ||
pvcPath, appPath, daemonSetName, cn, ns string, | ||
) error { | ||
mountOption := "context=\"system_u:object_r:container_file_t:s0:c0,c1\"" | ||
nixpanic marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// create PVC | ||
pvc, err := loadPVC(pvcPath) | ||
if err != nil { | ||
return fmt.Errorf("failed to load pvc: %w", err) | ||
} | ||
pvc.Namespace = f.UniqueName | ||
err = createPVCAndvalidatePV(f.ClientSet, pvc, deployTimeout) | ||
if err != nil { | ||
return fmt.Errorf("failed to create PVC: %w", err) | ||
} | ||
// modify PV spec.MountOptions | ||
pv, err := getBoundPV(f.ClientSet, pvc) | ||
if err != nil { | ||
return fmt.Errorf("failed to get PV: %w", err) | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Return is missing here. |
||
pv.Spec.MountOptions = []string{mountOption} | ||
|
||
// update PV | ||
_, err = f.ClientSet.CoreV1().PersistentVolumes().Update(context.TODO(), pv, metav1.UpdateOptions{}) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. create one ctx and reuse it. |
||
if err != nil { | ||
return fmt.Errorf("failed to update pv: %w", err) | ||
} | ||
|
||
app, err := loadApp(appPath) | ||
if err != nil { | ||
return fmt.Errorf("failed to load application: %w", err) | ||
} | ||
app.Namespace = f.UniqueName | ||
err = createApp(f.ClientSet, app, deployTimeout) | ||
if err != nil { | ||
return fmt.Errorf("failed to create application: %w", err) | ||
} | ||
|
||
pod, err := f.ClientSet.CoreV1().Pods(f.UniqueName).Get(context.TODO(), app.Name, metav1.GetOptions{}) | ||
if err != nil { | ||
framework.Logf("Error occurred getting pod %s in namespace %s", app.Name, f.UniqueName) | ||
|
||
return fmt.Errorf("failed to get pod: %w", err) | ||
} | ||
|
||
nodepluginPodName, err := getDaemonsetPodOnNode(f, daemonSetName, pod.Spec.NodeName, ns) | ||
if err != nil { | ||
return fmt.Errorf("failed to get daemonset pod on node: %w", err) | ||
} | ||
logs, err := frameworkPod.GetPodLogs(context.TODO(), f.ClientSet, ns, nodepluginPodName, cn) | ||
if err != nil { | ||
return fmt.Errorf("failed to get pod logs from container %s/%s/%s : %w", ns, nodepluginPodName, cn, err) | ||
} | ||
|
||
if !strings.Contains(logs, mountOption) { | ||
return fmt.Errorf("mount option %s not found in logs: %s", mountOption, logs) | ||
} | ||
nixpanic marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
err = deletePVCAndApp("", f, pvc, app) | ||
if err != nil { | ||
return fmt.Errorf("failed to delete PVC and application: %w", err) | ||
} | ||
|
||
return nil | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CephFS upgrade tests pick up cephfs sc from previous release branches which still
have
debug
mountoption in sc yamls.Adding this workaround untill we set upgrade test from 3.9 to devel.
I'll add a note regarding this breaking change in release notes too.
(users will need to recreate cephfs storage without debug option and
remove it from pvs too.)
added issue to track this one #3911
I'll ask re-review requests after all the ci is green.
(:sweat_smile: this pr seems to be giving more ci issues than expected.)