Merge pull request kubernetes#128244 from gnufied/fix-fsgroup-behaviour

Apply fsGroup when accessMode is ReadWriteOncePod

Author: k8s-ci-robot

pkg/apis/storage/types.go

@@ -423,7 +423,7 @@ const (
 	// ReadWriteOnceWithFSTypeFSGroupPolicy indicates that each volume will be examined
 	// to determine if the volume ownership and permissions
 	// should be modified. If a fstype is defined and the volume's access mode
-	// contains ReadWriteOnce, then the defined fsGroup will be applied.
+	// contains ReadWriteOnce or ReadWriteOncePod, then the defined fsGroup will be applied.
 	// This mode should be defined if it's expected that the
 	// fsGroup may need to be modified depending on the pod's SecurityPolicy.
 	// This is the default behavior if no other FSGroupPolicy is defined.

pkg/volume/csi/csi_mounter_test.go

@@ -860,6 +860,15 @@ func TestMounterSetUpWithFSGroup(t *testing.T) {
 			setFsGroup: true,
 			fsGroup:    3000,
 		},
+		{
+			name: "fstype, fsgroup, RWOP provided (should apply fsgroup)",
+			accessModes: []corev1.PersistentVolumeAccessMode{
+				corev1.ReadWriteOncePod,
+			},
+			fsType:     "ext4",
+			setFsGroup: true,
+			fsGroup:    3000,
+		},
 		{
 			name: "fstype, fsgroup, RWO provided, FSGroupPolicy ReadWriteOnceWithFSType (should apply fsgroup)",
 			accessModes: []corev1.PersistentVolumeAccessMode{

pkg/volume/csi/csi_util.go

@@ -134,7 +134,8 @@ func hasReadWriteOnce(modes []api.PersistentVolumeAccessMode) bool {
 		return false
 	}
 	for _, mode := range modes {
-		if mode == api.ReadWriteOnce {
+		if mode == api.ReadWriteOnce ||
+			mode == api.ReadWriteOncePod {
 			return true
 		}
 	}

staging/src/k8s.io/api/storage/v1/types.go

@@ -433,7 +433,7 @@ const (
 	// ReadWriteOnceWithFSTypeFSGroupPolicy indicates that each volume will be examined
 	// to determine if the volume ownership and permissions
 	// should be modified. If a fstype is defined and the volume's access mode
-	// contains ReadWriteOnce, then the defined fsGroup will be applied.
+	// contains ReadWriteOnce or ReadWriteOncePod, then the defined fsGroup will be applied.
 	// This mode should be defined if it's expected that the
 	// fsGroup may need to be modified depending on the pod's SecurityPolicy.
 	// This is the default behavior if no other FSGroupPolicy is defined.

test/e2e/storage/testsuites/fsgroupchangepolicy.go

@@ -113,8 +113,6 @@ func (s *fsGroupChangePolicyTestSuite) DefineTests(driver storageframework.TestD
 		l = local{}
 		l.driver = driver
 		l.config = driver.PrepareTest(ctx, f)
-		testVolumeSizeRange := s.GetTestSuiteInfo().SupportedSizeRange
-		l.resource = storageframework.CreateVolumeResource(ctx, l.driver, l.config, pattern, testVolumeSizeRange)
 	}
 
 	cleanup := func(ctx context.Context) {
@@ -129,6 +127,8 @@ func (s *fsGroupChangePolicyTestSuite) DefineTests(driver storageframework.TestD
 		framework.ExpectNoError(errors.NewAggregate(errs), "while cleanup resource")
 	}
 
+	rwopAccessMode := v1.ReadWriteOncePod
+
 	tests := []struct {
 		name                              string // Test case name
 		podfsGroupChangePolicy            string // 'Always' or 'OnRootMismatch'
@@ -143,6 +143,7 @@ func (s *fsGroupChangePolicyTestSuite) DefineTests(driver storageframework.TestD
 		// * OnRootMismatch policy is not supported.
 		// * It may not be possible to chgrp after mounting a volume.
 		supportsVolumeMountGroup bool
+		volumeAccessMode         *v1.PersistentVolumeAccessMode
 	}{
 		// Test cases for 'Always' policy
 		{
@@ -154,6 +155,16 @@ func (s *fsGroupChangePolicyTestSuite) DefineTests(driver storageframework.TestD
 			finalExpectedSubDirFileOwnership:  2000,
 			supportsVolumeMountGroup:          true,
 		},
+		{
+			name:                              "rwop pod created with an initial fsgroup, new pod fsgroup applied to volume contents",
+			podfsGroupChangePolicy:            "Always",
+			initialPodFsGroup:                 1000,
+			secondPodFsGroup:                  2000,
+			finalExpectedRootDirFileOwnership: 2000,
+			finalExpectedSubDirFileOwnership:  2000,
+			supportsVolumeMountGroup:          true,
+			volumeAccessMode:                  &rwopAccessMode,
+		},
 		{
 			name:                              "pod created with an initial fsgroup, volume contents ownership changed via chgrp in first pod, new pod with same fsgroup applied to the volume contents",
 			podfsGroupChangePolicy:            "Always",
@@ -218,6 +229,13 @@ func (s *fsGroupChangePolicyTestSuite) DefineTests(driver storageframework.TestD
 			}
 
 			init(ctx)
+			testVolumeSizeRange := s.GetTestSuiteInfo().SupportedSizeRange
+			if test.volumeAccessMode != nil {
+				accessModes := []v1.PersistentVolumeAccessMode{*test.volumeAccessMode}
+				l.resource = storageframework.CreateVolumeResourceWithAccessModes(ctx, l.driver, l.config, pattern, testVolumeSizeRange, accessModes, nil)
+			} else {
+				l.resource = storageframework.CreateVolumeResource(ctx, l.driver, l.config, pattern, testVolumeSizeRange)
+			}
 			ginkgo.DeferCleanup(cleanup)
 			podConfig := e2epod.Config{
 				NS:                     f.Namespace.Name,