From 6b9348e878b72bfb376b209bdec07ecda5e170c4 Mon Sep 17 00:00:00 2001 From: Christian Huffman Date: Tue, 20 Oct 2020 16:09:44 -0400 Subject: [PATCH 1/9] Relax validation for CSIVolumeFSGroupPolicy --- pkg/apis/storage/validation/validation.go | 2 +- pkg/apis/storage/validation/validation_test.go | 6 +++++- pkg/registry/storage/csidriver/strategy.go | 3 +-- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/pkg/apis/storage/validation/validation.go b/pkg/apis/storage/validation/validation.go index e8510c152a4..fd4be03c956 100644 --- a/pkg/apis/storage/validation/validation.go +++ b/pkg/apis/storage/validation/validation.go @@ -404,7 +404,7 @@ func ValidateCSIDriver(csiDriver *storage.CSIDriver) field.ErrorList { // ValidateCSIDriverUpdate validates a CSIDriver. func ValidateCSIDriverUpdate(new, old *storage.CSIDriver) field.ErrorList { - allErrs := ValidateCSIDriver(new) + allErrs := apivalidation.ValidateObjectMetaUpdate(&new.ObjectMeta, &old.ObjectMeta, field.NewPath("metadata")) // Spec is read-only // If this ever relaxes in the future, make sure to increment the Generation number in PrepareForUpdate diff --git a/pkg/apis/storage/validation/validation_test.go b/pkg/apis/storage/validation/validation_test.go index 10c071f2240..dd4b8ae21b4 100644 --- a/pkg/apis/storage/validation/validation_test.go +++ b/pkg/apis/storage/validation/validation_test.go @@ -1869,9 +1869,13 @@ func TestCSIDriverValidationUpdate(t *testing.T) { attachNotRequired := false podInfoOnMount := true notPodInfoOnMount := false +<<<<<<< HEAD notRequiresRepublish := false +======= + resourceVersion := "1" +>>>>>>> Relax validation for CSIVolumeFSGroupPolicy old := storage.CSIDriver{ - ObjectMeta: metav1.ObjectMeta{Name: driverName}, + ObjectMeta: metav1.ObjectMeta{Name: driverName, ResourceVersion: resourceVersion}, Spec: storage.CSIDriverSpec{ AttachRequired: &attachNotRequired, PodInfoOnMount: ¬PodInfoOnMount, diff --git a/pkg/registry/storage/csidriver/strategy.go b/pkg/registry/storage/csidriver/strategy.go index 8cf3f06144d..ec600ac4109 100644 --- a/pkg/registry/storage/csidriver/strategy.go +++ b/pkg/registry/storage/csidriver/strategy.go @@ -113,8 +113,7 @@ func (csiDriverStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime. func (csiDriverStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { newCSIDriverObj := obj.(*storage.CSIDriver) oldCSIDriverObj := old.(*storage.CSIDriver) - errorList := validation.ValidateCSIDriver(newCSIDriverObj) - return append(errorList, validation.ValidateCSIDriverUpdate(newCSIDriverObj, oldCSIDriverObj)...) + return validation.ValidateCSIDriverUpdate(newCSIDriverObj, oldCSIDriverObj) } func (csiDriverStrategy) AllowUnconditionalUpdate() bool { From 01f70d69b73ec3aa31c4568ece2bfe6ec1654377 Mon Sep 17 00:00:00 2001 From: Christian Huffman Date: Wed, 21 Oct 2020 13:30:13 -0400 Subject: [PATCH 2/9] Move CSIVolumeFSGroupPolicy to beta --- .../storage/validation/validation_test.go | 44 +++++++++++++++++-- pkg/features/kube_features.go | 3 +- pkg/volume/csi/csi_test.go | 6 +++ pkg/volume/csi/csi_util_test.go | 2 + 4 files changed, 50 insertions(+), 5 deletions(-) diff --git a/pkg/apis/storage/validation/validation_test.go b/pkg/apis/storage/validation/validation_test.go index dd4b8ae21b4..8e692f04973 100644 --- a/pkg/apis/storage/validation/validation_test.go +++ b/pkg/apis/storage/validation/validation_test.go @@ -1873,7 +1873,12 @@ func TestCSIDriverValidationUpdate(t *testing.T) { notRequiresRepublish := false ======= resourceVersion := "1" +<<<<<<< HEAD >>>>>>> Relax validation for CSIVolumeFSGroupPolicy +======= + invalidFSGroupPolicy := storage.ReadWriteOnceWithFSTypeFSGroupPolicy + invalidFSGroupPolicy = "invalid-mode" +>>>>>>> Move CSIVolumeFSGroupPolicy to beta old := storage.CSIDriver{ ObjectMeta: metav1.ObjectMeta{Name: driverName, ResourceVersion: resourceVersion}, Spec: storage.CSIDriverSpec{ @@ -1887,11 +1892,27 @@ func TestCSIDriverValidationUpdate(t *testing.T) { }, } - // Currently there is only one success case: exactly the same - // as the existing object. - successCases := []storage.CSIDriver{old} + // Currently we compare the object against itself + // and ensure updates succeed + successCases := []storage.CSIDriver{ + old, + // An invalid FSGroupPolicy should still pass + { + ObjectMeta: metav1.ObjectMeta{Name: driverName, ResourceVersion: resourceVersion}, + Spec: storage.CSIDriverSpec{ + AttachRequired: &attachNotRequired, + PodInfoOnMount: ¬PodInfoOnMount, + VolumeLifecycleModes: []storage.VolumeLifecycleMode{ + storage.VolumeLifecycleEphemeral, + storage.VolumeLifecyclePersistent, + }, + FSGroupPolicy: &invalidFSGroupPolicy, + }, + }, + } for _, csiDriver := range successCases { - if errs := ValidateCSIDriverUpdate(&csiDriver, &old); len(errs) != 0 { + newDriver := csiDriver.DeepCopy() + if errs := ValidateCSIDriverUpdate(&csiDriver, newDriver); len(errs) != 0 { t.Errorf("expected success for %+v: %v", csiDriver, errs) } } @@ -1967,6 +1988,21 @@ func TestCSIDriverValidationUpdate(t *testing.T) { } }, }, + { + name: "FSGroupPolicy invalidated", + modify: func(new *storage.CSIDriver) { + invalidFSGroupPolicy := storage.ReadWriteOnceWithFSTypeFSGroupPolicy + invalidFSGroupPolicy = "invalid" + new.Spec.FSGroupPolicy = &invalidFSGroupPolicy + }, + }, + { + name: "FSGroupPolicy changed", + modify: func(new *storage.CSIDriver) { + fileFSGroupPolicy := storage.FileFSGroupPolicy + new.Spec.FSGroupPolicy = &fileFSGroupPolicy + }, + }, } for _, test := range errorCases { diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index f79c8d40193..3ae6c65689f 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -432,6 +432,7 @@ const ( // owner: @huffmanca // alpha: v1.19 + // beta: v1.20 // // Determines if a CSI Driver supports applying fsGroup. CSIVolumeFSGroupPolicy featuregate.Feature = "CSIVolumeFSGroupPolicy" @@ -764,7 +765,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS CSIStorageCapacity: {Default: false, PreRelease: featuregate.Alpha}, CSIServiceAccountToken: {Default: false, PreRelease: featuregate.Alpha}, GenericEphemeralVolume: {Default: false, PreRelease: featuregate.Alpha}, - CSIVolumeFSGroupPolicy: {Default: false, PreRelease: featuregate.Alpha}, + CSIVolumeFSGroupPolicy: {Default: true, PreRelease: featuregate.Beta}, RuntimeClass: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.23 NodeLease: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, SCTPSupport: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.22 diff --git a/pkg/volume/csi/csi_test.go b/pkg/volume/csi/csi_test.go index 993a7b35b90..a353532c851 100644 --- a/pkg/volume/csi/csi_test.go +++ b/pkg/volume/csi/csi_test.go @@ -27,6 +27,7 @@ import ( api "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1" storage "k8s.io/api/storage/v1" + storagev1 "k8s.io/api/storage/v1" meta "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -47,6 +48,7 @@ import ( // based on operations from the volume manager/reconciler/operation executor func TestCSI_VolumeAll(t *testing.T) { defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIInlineVolume, true)() + defaultFSGroupPolicy := storagev1.ReadWriteOnceWithFSTypeFSGroupPolicy tests := []struct { name string @@ -87,6 +89,7 @@ func TestCSI_VolumeAll(t *testing.T) { driverSpec: &storage.CSIDriverSpec{ // Required for the driver to be accepted for the persistent volume. VolumeLifecycleModes: []storage.VolumeLifecycleMode{storage.VolumeLifecyclePersistent}, + FSGroupPolicy: &defaultFSGroupPolicy, }, }, { @@ -104,6 +107,7 @@ func TestCSI_VolumeAll(t *testing.T) { driverSpec: &storage.CSIDriverSpec{ // This will cause the volume to be rejected. VolumeLifecycleModes: []storage.VolumeLifecycleMode{storage.VolumeLifecycleEphemeral}, + FSGroupPolicy: &defaultFSGroupPolicy, }, shouldFail: true, }, @@ -122,6 +126,7 @@ func TestCSI_VolumeAll(t *testing.T) { driverSpec: &storage.CSIDriverSpec{ // Required for the driver to be accepted for the inline volume. VolumeLifecycleModes: []storage.VolumeLifecycleMode{storage.VolumeLifecycleEphemeral}, + FSGroupPolicy: &defaultFSGroupPolicy, }, }, { @@ -139,6 +144,7 @@ func TestCSI_VolumeAll(t *testing.T) { driverSpec: &storage.CSIDriverSpec{ // Required for the driver to be accepted for the inline volume. VolumeLifecycleModes: []storage.VolumeLifecycleMode{storage.VolumeLifecyclePersistent, storage.VolumeLifecycleEphemeral}, + FSGroupPolicy: &defaultFSGroupPolicy, }, }, { diff --git a/pkg/volume/csi/csi_util_test.go b/pkg/volume/csi/csi_util_test.go index 3df12babcde..f4d3582b004 100644 --- a/pkg/volume/csi/csi_util_test.go +++ b/pkg/volume/csi/csi_util_test.go @@ -85,6 +85,7 @@ func makeTestVol(name string, driverName string) *api.Volume { } func getTestCSIDriver(name string, podInfoMount *bool, attachable *bool, volumeLifecycleModes []storagev1.VolumeLifecycleMode) *storagev1.CSIDriver { + defaultFSGroupPolicy := storagev1.ReadWriteOnceWithFSTypeFSGroupPolicy return &storagev1.CSIDriver{ ObjectMeta: meta.ObjectMeta{ Name: name, @@ -93,6 +94,7 @@ func getTestCSIDriver(name string, podInfoMount *bool, attachable *bool, volumeL PodInfoOnMount: podInfoMount, AttachRequired: attachable, VolumeLifecycleModes: volumeLifecycleModes, + FSGroupPolicy: &defaultFSGroupPolicy, }, } } From 4d2d063635c86aceb626d88c1d785e0463109df8 Mon Sep 17 00:00:00 2001 From: Christian Huffman Date: Thu, 8 Oct 2020 10:34:17 -0400 Subject: [PATCH 3/9] Included e2e test for CSIDriver FSGroupPolicy --- pkg/volume/csi/csi_test.go | 40 ++++-- test/e2e/storage/csi_mock_volume.go | 179 ++++++++++++++++++++++++++- test/e2e/storage/drivers/csi.go | 4 + test/e2e/storage/utils/deployment.go | 6 + test/e2e/storage/utils/utils.go | 11 ++ 5 files changed, 229 insertions(+), 11 deletions(-) diff --git a/pkg/volume/csi/csi_test.go b/pkg/volume/csi/csi_test.go index a353532c851..125079eddd4 100644 --- a/pkg/volume/csi/csi_test.go +++ b/pkg/volume/csi/csi_test.go @@ -51,15 +51,16 @@ func TestCSI_VolumeAll(t *testing.T) { defaultFSGroupPolicy := storagev1.ReadWriteOnceWithFSTypeFSGroupPolicy tests := []struct { - name string - specName string - driver string - volName string - specFunc func(specName, driver, volName string) *volume.Spec - podFunc func() *api.Pod - isInline bool - shouldFail bool - driverSpec *storage.CSIDriverSpec + name string + specName string + driver string + volName string + specFunc func(specName, driver, volName string) *volume.Spec + podFunc func() *api.Pod + isInline bool + shouldFail bool + disableFSGroupPolicyFeatureGate bool + driverSpec *storage.CSIDriverSpec }{ { name: "PersistentVolume", @@ -92,6 +93,25 @@ func TestCSI_VolumeAll(t *testing.T) { FSGroupPolicy: &defaultFSGroupPolicy, }, }, + { + name: "PersistentVolume with driver info and FSGroup disabled", + specName: "pv2", + driver: "simple-driver", + volName: "vol2", + specFunc: func(specName, driver, volName string) *volume.Spec { + return volume.NewSpecFromPersistentVolume(makeTestPV(specName, 20, driver, volName), false) + }, + podFunc: func() *api.Pod { + podUID := types.UID(fmt.Sprintf("%08X", rand.Uint64())) + return &api.Pod{ObjectMeta: meta.ObjectMeta{UID: podUID, Namespace: testns}} + }, + disableFSGroupPolicyFeatureGate: true, + driverSpec: &storage.CSIDriverSpec{ + // Required for the driver to be accepted for the persistent volume. + VolumeLifecycleModes: []storage.VolumeLifecycleMode{storage.VolumeLifecyclePersistent}, + FSGroupPolicy: &defaultFSGroupPolicy, + }, + }, { name: "PersistentVolume with wrong mode in driver info", specName: "pv2", @@ -227,6 +247,8 @@ func TestCSI_VolumeAll(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIInlineVolume, !test.disableFSGroupPolicyFeatureGate)() + tmpDir, err := utiltesting.MkTmpdir("csi-test") if err != nil { t.Fatalf("can't create temp dir: %v", err) diff --git a/test/e2e/storage/csi_mock_volume.go b/test/e2e/storage/csi_mock_volume.go index 92a196b8673..395041670eb 100644 --- a/test/e2e/storage/csi_mock_volume.go +++ b/test/e2e/storage/csi_mock_volume.go @@ -22,6 +22,7 @@ import ( "encoding/json" "errors" "fmt" + "math/rand" "strconv" "strings" "time" @@ -49,6 +50,7 @@ import ( e2epod "k8s.io/kubernetes/test/e2e/framework/pod" e2epv "k8s.io/kubernetes/test/e2e/framework/pv" e2eskipper "k8s.io/kubernetes/test/e2e/framework/skipper" + e2evolume "k8s.io/kubernetes/test/e2e/framework/volume" "k8s.io/kubernetes/test/e2e/storage/drivers" "k8s.io/kubernetes/test/e2e/storage/testpatterns" "k8s.io/kubernetes/test/e2e/storage/testsuites" @@ -114,6 +116,7 @@ var _ = utils.SIGDescribe("CSI mock volume", func() { javascriptHooks map[string]string tokenRequests []storagev1.TokenRequest requiresRepublish *bool + fsGroupPolicy *storagev1.FSGroupPolicy } type mockDriverSetup struct { @@ -155,6 +158,7 @@ var _ = utils.SIGDescribe("CSI mock volume", func() { JavascriptHooks: tp.javascriptHooks, TokenRequests: tp.tokenRequests, RequiresRepublish: tp.requiresRepublish, + FSGroupPolicy: tp.fsGroupPolicy, } // this just disable resizing on driver, keeping resizing on SC enabled. @@ -229,6 +233,39 @@ var _ = utils.SIGDescribe("CSI mock volume", func() { return pod, err } + createPodWithFSGroup := func(fsGroup *int64) (*storagev1.StorageClass, *v1.PersistentVolumeClaim, *v1.Pod) { + ginkgo.By("Creating pod with fsGroup") + nodeSelection := m.config.ClientNodeSelection + var sc *storagev1.StorageClass + if dDriver, ok := m.driver.(testsuites.DynamicPVTestDriver); ok { + sc = dDriver.GetDynamicProvisionStorageClass(m.config, "") + } + scTest := testsuites.StorageClassTest{ + Name: m.driver.GetDriverInfo().Name, + Provisioner: sc.Provisioner, + Parameters: sc.Parameters, + ClaimSize: "1Gi", + ExpectedSize: "1Gi", + DelayBinding: m.tp.lateBinding, + AllowVolumeExpansion: m.tp.enableResizing, + } + + class, claim, pod := startBusyBoxPod(f.ClientSet, scTest, nodeSelection, m.tp.scName, f.Namespace.Name, fsGroup) + + if class != nil { + m.sc[class.Name] = class + } + if claim != nil { + m.pvcs = append(m.pvcs, claim) + } + + if pod != nil { + m.pods = append(m.pods, pod) + } + + return class, claim, pod + } + cleanup := func() { cs := f.ClientSet var errs []error @@ -1366,6 +1403,86 @@ var _ = utils.SIGDescribe("CSI mock volume", func() { ginkgo.By("Checking CSI driver logs") err = checkPodLogs(m.cs, m.config.DriverNamespace.Name, driverPodName, driverContainerName, pod, false, false, false, test.deployCSIDriverObject && csiServiceAccountTokenEnabled, numNodePublishVolume) framework.ExpectNoError(err) + // These tests *only* work on a cluster which has the CSIVolumeFSGroupPolicy feature enabled. + ginkgo.Context("CSI FSGroupPolicy [LinuxOnly]", func() { + tests := []struct { + name string + fsGroupPolicy storagev1.FSGroupPolicy + modified bool + }{ + { + name: "should modify fsGroup if fsGroupPolicy=default", + fsGroupPolicy: storagev1.ReadWriteOnceWithFSTypeFSGroupPolicy, + modified: true, + }, + { + name: "should modify fsGroup if fsGroupPolicy=File", + fsGroupPolicy: storagev1.FileFSGroupPolicy, + modified: true, + }, + { + name: "should not modify fsGroup if fsGroupPolicy=None", + fsGroupPolicy: storagev1.NoneFSGroupPolicy, + modified: false, + }, + } + for _, t := range tests { + test := t + ginkgo.It(test.name, func() { + if framework.NodeOSDistroIs("windows") { + e2eskipper.Skipf("FSGroupPolicy is only applied on linux nodes -- skipping") + } + init(testParameters{ + disableAttach: true, + registerDriver: true, + fsGroupPolicy: &test.fsGroupPolicy, + }) + defer cleanup() + + // kube-scheduler may need some time before it gets the CSIDriver object. + // Without them, scheduling doesn't run as expected by the test. + syncDelay := 5 * time.Second + time.Sleep(syncDelay) + + fsGroupVal := int64(rand.Int63n(20000) + 1024) + fsGroup := &fsGroupVal + + _, _, pod := createPodWithFSGroup(fsGroup) /* persistent volume */ + + mountPath := pod.Spec.Containers[0].VolumeMounts[0].MountPath + dirName := mountPath + "/" + f.UniqueName + fileName := dirName + "/" + f.UniqueName + + err := e2epod.WaitForPodNameRunningInNamespace(m.cs, pod.Name, pod.Namespace) + framework.ExpectNoError(err, "failed to start pod") + + // Create the subdirectory to ensure that fsGroup propagates + createDirectory := fmt.Sprintf("mkdir %s", dirName) + _, _, err = utils.PodExec(f, pod, createDirectory) + framework.ExpectNoError(err, "failed: creating the directory: %s", err) + + // Inject the contents onto the mount + createFile := fmt.Sprintf("echo '%s' > '%s'; sync", "filecontents", fileName) + _, _, err = utils.PodExec(f, pod, createFile) + framework.ExpectNoError(err, "failed: writing the contents: %s", err) + + // Delete the created file. This step is mandatory, as the mock driver + // won't clean up the contents automatically. + defer func() { + delete := fmt.Sprintf("rm -fr %s", dirName) + _, _, err = utils.PodExec(f, pod, delete) + framework.ExpectNoError(err, "failed: deleting the directory: %s", err) + }() + + // Ensure that the fsGroup matches what we expect + if test.modified { + utils.VerifyFSGroupInPod(f, fileName, strconv.FormatInt(*fsGroup, 10), pod) + } else { + utils.VerifyFSGroupInPod(f, fileName, "root", pod) + } + + // The created resources will be removed by the cleanup() function, + // so need to delete anything here. }) } }) @@ -1505,7 +1622,7 @@ func checkCSINodeForLimits(nodeName string, driverName string, cs clientset.Inte return attachLimit, nil } -func startPausePod(cs clientset.Interface, t testsuites.StorageClassTest, node e2epod.NodeSelection, scName, ns string) (*storagev1.StorageClass, *v1.PersistentVolumeClaim, *v1.Pod) { +func createClaim(cs clientset.Interface, t testsuites.StorageClassTest, node e2epod.NodeSelection, scName, ns string) (*storagev1.StorageClass, *v1.PersistentVolumeClaim) { class := newStorageClass(t, ns, "") if scName != "" { class.Name = scName @@ -1530,9 +1647,21 @@ func startPausePod(cs clientset.Interface, t testsuites.StorageClassTest, node e _, err = e2epv.WaitForPVClaimBoundPhase(cs, pvcClaims, framework.ClaimProvisionTimeout) framework.ExpectNoError(err, "Failed waiting for PVC to be bound: %v", err) } + return class, claim +} + +func startPausePod(cs clientset.Interface, t testsuites.StorageClassTest, node e2epod.NodeSelection, scName, ns string) (*storagev1.StorageClass, *v1.PersistentVolumeClaim, *v1.Pod) { + class, claim := createClaim(cs, t, node, scName, ns) pod, err := startPausePodWithClaim(cs, claim, node, ns) - framework.ExpectNoError(err, "Failed to create pod: %v", err) + framework.ExpectNoError(err, "Failed to create pause pod: %v", err) + return class, claim, pod +} + +func startBusyBoxPod(cs clientset.Interface, t testsuites.StorageClassTest, node e2epod.NodeSelection, scName, ns string, fsGroup *int64) (*storagev1.StorageClass, *v1.PersistentVolumeClaim, *v1.Pod) { + class, claim := createClaim(cs, t, node, scName, ns) + pod, err := startBusyBoxPodWithClaim(cs, claim, node, ns, fsGroup) + framework.ExpectNoError(err, "Failed to create busybox pod: %v", err) return class, claim, pod } @@ -1557,6 +1686,17 @@ func startPausePodWithClaim(cs clientset.Interface, pvc *v1.PersistentVolumeClai node, ns) } +func startBusyBoxPodWithClaim(cs clientset.Interface, pvc *v1.PersistentVolumeClaim, node e2epod.NodeSelection, ns string, fsGroup *int64) (*v1.Pod, error) { + return startBusyBoxPodWithVolumeSource(cs, + v1.VolumeSource{ + PersistentVolumeClaim: &v1.PersistentVolumeClaimVolumeSource{ + ClaimName: pvc.Name, + ReadOnly: false, + }, + }, + node, ns, fsGroup) +} + func startPausePodWithInlineVolume(cs clientset.Interface, inlineVolume *v1.CSIVolumeSource, node e2epod.NodeSelection, ns string) (*v1.Pod, error) { return startPausePodWithVolumeSource(cs, v1.VolumeSource{ @@ -1596,6 +1736,41 @@ func startPausePodWithVolumeSource(cs clientset.Interface, volumeSource v1.Volum return cs.CoreV1().Pods(ns).Create(context.TODO(), pod, metav1.CreateOptions{}) } +func startBusyBoxPodWithVolumeSource(cs clientset.Interface, volumeSource v1.VolumeSource, node e2epod.NodeSelection, ns string, fsGroup *int64) (*v1.Pod, error) { + pod := &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "pvc-volume-tester-", + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "volume-tester", + Image: framework.BusyBoxImage, + VolumeMounts: []v1.VolumeMount{ + { + Name: "my-volume", + MountPath: "/mnt/test", + }, + }, + Command: e2evolume.GenerateScriptCmd("while true ; do sleep 2; done"), + }, + }, + SecurityContext: &v1.PodSecurityContext{ + FSGroup: fsGroup, + }, + RestartPolicy: v1.RestartPolicyNever, + Volumes: []v1.Volume{ + { + Name: "my-volume", + VolumeSource: volumeSource, + }, + }, + }, + } + e2epod.SetNodeSelection(&pod.Spec, node) + return cs.CoreV1().Pods(ns).Create(context.TODO(), pod, metav1.CreateOptions{}) +} + // Dummy structure that parses just volume_attributes and error code out of logged CSI call type mockCSICall struct { json string // full log entry diff --git a/test/e2e/storage/drivers/csi.go b/test/e2e/storage/drivers/csi.go index 1c92cee8d6f..03ec35e557c 100644 --- a/test/e2e/storage/drivers/csi.go +++ b/test/e2e/storage/drivers/csi.go @@ -255,6 +255,7 @@ type mockCSIDriver struct { javascriptHooks map[string]string tokenRequests []storagev1.TokenRequest requiresRepublish *bool + fsGroupPolicy *storagev1.FSGroupPolicy } // CSIMockDriverOpts defines options used for csi driver @@ -271,6 +272,7 @@ type CSIMockDriverOpts struct { JavascriptHooks map[string]string TokenRequests []storagev1.TokenRequest RequiresRepublish *bool + FSGroupPolicy *storagev1.FSGroupPolicy } var _ testsuites.TestDriver = &mockCSIDriver{} @@ -330,6 +332,7 @@ func InitMockCSIDriver(driverOpts CSIMockDriverOpts) testsuites.TestDriver { javascriptHooks: driverOpts.JavascriptHooks, tokenRequests: driverOpts.TokenRequests, requiresRepublish: driverOpts.RequiresRepublish, + fsGroupPolicy: driverOpts.FSGroupPolicy, } } @@ -433,6 +436,7 @@ func (m *mockCSIDriver) PrepareTest(f *framework.Framework) (*testsuites.PerTest }, TokenRequests: m.tokenRequests, RequiresRepublish: m.requiresRepublish, + FSGroupPolicy: m.fsGroupPolicy, } cleanup, err := utils.CreateFromManifests(f, driverNamespace, func(item interface{}) error { return utils.PatchCSIDeployment(f, o, item) diff --git a/test/e2e/storage/utils/deployment.go b/test/e2e/storage/utils/deployment.go index c83b9194da6..0d4c1bc9642 100644 --- a/test/e2e/storage/utils/deployment.go +++ b/test/e2e/storage/utils/deployment.go @@ -138,6 +138,8 @@ func PatchCSIDeployment(f *framework.Framework, o PatchCSIOptions, object interf } if o.RequiresRepublish != nil { object.Spec.RequiresRepublish = o.RequiresRepublish + if o.FSGroupPolicy != nil { + object.Spec.FSGroupPolicy = o.FSGroupPolicy } } @@ -194,4 +196,8 @@ type PatchCSIOptions struct { // field *if* the driver deploys a CSIDriver object. Ignored // otherwise. RequiresRepublish *bool + // If not nil, the value to use for the CSIDriver.Spec.FSGroupPolicy + // field *if* the driver deploys a CSIDriver object. Ignored + // otherwise. + FSGroupPolicy *storagev1.FSGroupPolicy } diff --git a/test/e2e/storage/utils/utils.go b/test/e2e/storage/utils/utils.go index cd0dfdb00ea..340be22c4c5 100644 --- a/test/e2e/storage/utils/utils.go +++ b/test/e2e/storage/utils/utils.go @@ -94,6 +94,17 @@ func VerifyExecInPodSucceed(f *framework.Framework, pod *v1.Pod, shExec string) } } +// VerifyFSGroupInPod verifies that the passed in filePath contains the expectedFSGroup +func VerifyFSGroupInPod(f *framework.Framework, filePath, expectedFSGroup string, pod *v1.Pod) { + cmd := fmt.Sprintf("ls -l %s", filePath) + stdout, stderr, err := PodExec(f, pod, cmd) + framework.ExpectNoError(err) + framework.Logf("pod %s/%s exec for cmd %s, stdout: %s, stderr: %s", pod.Namespace, pod.Name, cmd, stdout, stderr) + fsGroupResult := strings.Fields(stdout)[3] + framework.ExpectEqual(expectedFSGroup, fsGroupResult, + "Expected fsGroup of %s, got %s", expectedFSGroup, fsGroupResult) +} + // VerifyExecInPodFail verifies shell cmd in target pod fail with certain exit code func VerifyExecInPodFail(f *framework.Framework, pod *v1.Pod, shExec string, exitCode int) { stdout, stderr, err := PodExec(f, pod, shExec) From 6385760d9fe2af7e03b5c11333c4c6c018184871 Mon Sep 17 00:00:00 2001 From: Christian Huffman Date: Thu, 12 Nov 2020 10:57:28 -0500 Subject: [PATCH 4/9] Update the mock driver to use 4.0.2 --- .../e2e/testing-manifests/storage-csi/mock/csi-mock-driver.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/testing-manifests/storage-csi/mock/csi-mock-driver.yaml b/test/e2e/testing-manifests/storage-csi/mock/csi-mock-driver.yaml index a5f3c9a7da4..d44ff136825 100644 --- a/test/e2e/testing-manifests/storage-csi/mock/csi-mock-driver.yaml +++ b/test/e2e/testing-manifests/storage-csi/mock/csi-mock-driver.yaml @@ -48,7 +48,7 @@ spec: - mountPath: /registration name: registration-dir - name: mock - image: k8s.gcr.io/sig-storage/mock-driver:v3.1.0 + image: k8s.gcr.io/sig-storage/mock-driver:v4.0.2 args: - "--name=mock.storage.k8s.io" - "--permissive-target-path" # because of https://github.com/kubernetes/kubernetes/issues/75535 From df9ea74a735ddabab68276f391cb003dc9cd0c1c Mon Sep 17 00:00:00 2001 From: Christian Huffman Date: Thu, 12 Nov 2020 10:58:25 -0500 Subject: [PATCH 5/9] Enable logging and drop permissive targets for CSI mock driver --- .../e2e/testing-manifests/storage-csi/mock/csi-mock-driver.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/testing-manifests/storage-csi/mock/csi-mock-driver.yaml b/test/e2e/testing-manifests/storage-csi/mock/csi-mock-driver.yaml index d44ff136825..81b2a22dcd4 100644 --- a/test/e2e/testing-manifests/storage-csi/mock/csi-mock-driver.yaml +++ b/test/e2e/testing-manifests/storage-csi/mock/csi-mock-driver.yaml @@ -51,7 +51,7 @@ spec: image: k8s.gcr.io/sig-storage/mock-driver:v4.0.2 args: - "--name=mock.storage.k8s.io" - - "--permissive-target-path" # because of https://github.com/kubernetes/kubernetes/issues/75535 + - "-v=3" # enabled the gRPC call logging env: - name: CSI_ENDPOINT value: /csi/csi.sock From 3287dbf914b69fbe1f9eb7237069d34856b353ed Mon Sep 17 00:00:00 2001 From: Christian Huffman Date: Thu, 12 Nov 2020 15:05:25 -0500 Subject: [PATCH 6/9] Adjust CSIDriver validation to check objectmeta --- pkg/apis/storage/validation/validation.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/apis/storage/validation/validation.go b/pkg/apis/storage/validation/validation.go index fd4be03c956..74a3684310d 100644 --- a/pkg/apis/storage/validation/validation.go +++ b/pkg/apis/storage/validation/validation.go @@ -393,9 +393,14 @@ func validateCSINodeDriver(driver storage.CSINodeDriver, driverNamesInSpecs sets return allErrs } +// ValidateCSIDriverName checks that a name is appropriate for a +// CSIDriver object. +var ValidateCSIDriverName = apimachineryvalidation.NameIsDNSSubdomain + // ValidateCSIDriver validates a CSIDriver. func ValidateCSIDriver(csiDriver *storage.CSIDriver) field.ErrorList { - allErrs := field.ErrorList{} + allErrs := apivalidation.ValidateObjectMeta(&csiDriver.ObjectMeta, false, ValidateCSIDriverName, field.NewPath("metadata")) + // We have an additional name check to verify the length and lowercase the name allErrs = append(allErrs, apivalidation.ValidateCSIDriverName(csiDriver.Name, field.NewPath("name"))...) allErrs = append(allErrs, validateCSIDriverSpec(&csiDriver.Spec, field.NewPath("spec"))...) From 8444823bed8e59f36252b5901f976f423017c92f Mon Sep 17 00:00:00 2001 From: Christian Huffman Date: Thu, 12 Nov 2020 16:32:36 -0500 Subject: [PATCH 7/9] Remove duplicate CSIDriver name validation --- pkg/apis/storage/validation/validation.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/apis/storage/validation/validation.go b/pkg/apis/storage/validation/validation.go index 74a3684310d..81d74b188b7 100644 --- a/pkg/apis/storage/validation/validation.go +++ b/pkg/apis/storage/validation/validation.go @@ -400,8 +400,6 @@ var ValidateCSIDriverName = apimachineryvalidation.NameIsDNSSubdomain // ValidateCSIDriver validates a CSIDriver. func ValidateCSIDriver(csiDriver *storage.CSIDriver) field.ErrorList { allErrs := apivalidation.ValidateObjectMeta(&csiDriver.ObjectMeta, false, ValidateCSIDriverName, field.NewPath("metadata")) - // We have an additional name check to verify the length and lowercase the name - allErrs = append(allErrs, apivalidation.ValidateCSIDriverName(csiDriver.Name, field.NewPath("name"))...) allErrs = append(allErrs, validateCSIDriverSpec(&csiDriver.Spec, field.NewPath("spec"))...) return allErrs From 701b42ca2b0e7823d2ac03849ca4caef7593fa8d Mon Sep 17 00:00:00 2001 From: Christian Huffman Date: Thu, 12 Nov 2020 16:45:26 -0500 Subject: [PATCH 8/9] Corrected CSIDriver validation rebase issues --- pkg/apis/storage/validation/validation_test.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/pkg/apis/storage/validation/validation_test.go b/pkg/apis/storage/validation/validation_test.go index 8e692f04973..c0e80641422 100644 --- a/pkg/apis/storage/validation/validation_test.go +++ b/pkg/apis/storage/validation/validation_test.go @@ -1869,16 +1869,10 @@ func TestCSIDriverValidationUpdate(t *testing.T) { attachNotRequired := false podInfoOnMount := true notPodInfoOnMount := false -<<<<<<< HEAD notRequiresRepublish := false -======= resourceVersion := "1" -<<<<<<< HEAD ->>>>>>> Relax validation for CSIVolumeFSGroupPolicy -======= invalidFSGroupPolicy := storage.ReadWriteOnceWithFSTypeFSGroupPolicy invalidFSGroupPolicy = "invalid-mode" ->>>>>>> Move CSIVolumeFSGroupPolicy to beta old := storage.CSIDriver{ ObjectMeta: metav1.ObjectMeta{Name: driverName, ResourceVersion: resourceVersion}, Spec: storage.CSIDriverSpec{ From 38071e74cf993b67290e9a458537e0a8460e8846 Mon Sep 17 00:00:00 2001 From: Christian Huffman Date: Thu, 12 Nov 2020 17:09:49 -0500 Subject: [PATCH 9/9] Correct rebase issues --- test/e2e/storage/csi_mock_volume.go | 3 +++ test/e2e/storage/utils/deployment.go | 1 + 2 files changed, 4 insertions(+) diff --git a/test/e2e/storage/csi_mock_volume.go b/test/e2e/storage/csi_mock_volume.go index 395041670eb..c8d5e769894 100644 --- a/test/e2e/storage/csi_mock_volume.go +++ b/test/e2e/storage/csi_mock_volume.go @@ -1403,6 +1403,9 @@ var _ = utils.SIGDescribe("CSI mock volume", func() { ginkgo.By("Checking CSI driver logs") err = checkPodLogs(m.cs, m.config.DriverNamespace.Name, driverPodName, driverContainerName, pod, false, false, false, test.deployCSIDriverObject && csiServiceAccountTokenEnabled, numNodePublishVolume) framework.ExpectNoError(err) + }) + } + }) // These tests *only* work on a cluster which has the CSIVolumeFSGroupPolicy feature enabled. ginkgo.Context("CSI FSGroupPolicy [LinuxOnly]", func() { tests := []struct { diff --git a/test/e2e/storage/utils/deployment.go b/test/e2e/storage/utils/deployment.go index 0d4c1bc9642..4a2622d1f0c 100644 --- a/test/e2e/storage/utils/deployment.go +++ b/test/e2e/storage/utils/deployment.go @@ -138,6 +138,7 @@ func PatchCSIDeployment(f *framework.Framework, o PatchCSIOptions, object interf } if o.RequiresRepublish != nil { object.Spec.RequiresRepublish = o.RequiresRepublish + } if o.FSGroupPolicy != nil { object.Spec.FSGroupPolicy = o.FSGroupPolicy }