From b19172c58fa2dfc3fe662c9ecf1f7438522d3fe3 Mon Sep 17 00:00:00 2001 From: Fabio Bertinatto Date: Thu, 20 Oct 2022 16:39:03 -0300 Subject: [PATCH] Promote DelegateFSGroupToCSIDriver feature to GA --- pkg/features/kube_features.go | 5 +++-- pkg/volume/csi/csi_attacher.go | 16 +++++++-------- pkg/volume/csi/csi_attacher_test.go | 31 +++-------------------------- pkg/volume/csi/csi_mounter.go | 16 +++++++-------- pkg/volume/csi/csi_mounter_test.go | 21 +++---------------- 5 files changed, 23 insertions(+), 66 deletions(-) diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 18da94ce825..ed4741c551c 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -222,9 +222,10 @@ const ( // DaemonSets allow workloads to maintain availability during update per node DaemonSetUpdateSurge featuregate.Feature = "DaemonSetUpdateSurge" - // owner: @gnufied, @verult + // owner: @gnufied, @verult, @bertinatto // alpha: v1.22 // beta: v1.23 + // GA: v1.26 // If supported by the CSI driver, delegates the role of applying FSGroup to // the driver by passing FSGroup through the NodeStageVolume and // NodePublishVolume calls. @@ -887,7 +888,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS DaemonSetUpdateSurge: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.27 - DelegateFSGroupToCSIDriver: {Default: true, PreRelease: featuregate.Beta}, + DelegateFSGroupToCSIDriver: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.28 DevicePlugins: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // GA in 1.26 diff --git a/pkg/volume/csi/csi_attacher.go b/pkg/volume/csi/csi_attacher.go index cf6b108251b..4187d16fdc7 100644 --- a/pkg/volume/csi/csi_attacher.go +++ b/pkg/volume/csi/csi_attacher.go @@ -383,16 +383,14 @@ func (c *csiAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMo } var nodeStageFSGroupArg *int64 - if utilfeature.DefaultFeatureGate.Enabled(features.DelegateFSGroupToCSIDriver) { - driverSupportsCSIVolumeMountGroup, err := csi.NodeSupportsVolumeMountGroup(ctx) - if err != nil { - return volumetypes.NewTransientOperationFailure(log("attacher.MountDevice failed to determine if the node service has VOLUME_MOUNT_GROUP capability: %v", err)) - } + driverSupportsCSIVolumeMountGroup, err := csi.NodeSupportsVolumeMountGroup(ctx) + if err != nil { + return volumetypes.NewTransientOperationFailure(log("attacher.MountDevice failed to determine if the node service has VOLUME_MOUNT_GROUP capability: %v", err)) + } - if driverSupportsCSIVolumeMountGroup { - klog.V(3).Infof("Driver %s supports applying FSGroup (has VOLUME_MOUNT_GROUP node capability). Delegating FSGroup application to the driver through NodeStageVolume.", csiSource.Driver) - nodeStageFSGroupArg = deviceMounterArgs.FsGroup - } + if driverSupportsCSIVolumeMountGroup { + klog.V(3).Infof("Driver %s supports applying FSGroup (has VOLUME_MOUNT_GROUP node capability). Delegating FSGroup application to the driver through NodeStageVolume.", csiSource.Driver) + nodeStageFSGroupArg = deviceMounterArgs.FsGroup } fsType := csiSource.FSType diff --git a/pkg/volume/csi/csi_attacher_test.go b/pkg/volume/csi/csi_attacher_test.go index 1857ab3ff60..32231ec5aa9 100644 --- a/pkg/volume/csi/csi_attacher_test.go +++ b/pkg/volume/csi/csi_attacher_test.go @@ -37,12 +37,9 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/watch" - utilfeature "k8s.io/apiserver/pkg/util/feature" clientset "k8s.io/client-go/kubernetes" fakeclient "k8s.io/client-go/kubernetes/fake" core "k8s.io/client-go/testing" - featuregatetesting "k8s.io/component-base/featuregate/testing" - "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/volume" fakecsi "k8s.io/kubernetes/pkg/volume/csi/fake" volumetypes "k8s.io/kubernetes/pkg/volume/util/types" @@ -1092,8 +1089,6 @@ func TestAttacherGetDeviceMountPath(t *testing.T) { } func TestAttacherMountDevice(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DelegateFSGroupToCSIDriver, true)() - pvName := "test-pv" var testFSGroup int64 = 3000 nonFinalError := volumetypes.NewUncertainProgressError("") @@ -1107,7 +1102,6 @@ func TestAttacherMountDevice(t *testing.T) { stageUnstageSet bool fsGroup *int64 expectedVolumeMountGroup string - delegateFSGroupFeatureGate bool driverSupportsVolumeMountGroup bool shouldFail bool skipOnWindows bool @@ -1222,12 +1216,11 @@ func TestAttacherMountDevice(t *testing.T) { spec: volume.NewSpecFromPersistentVolume(makeTestPV(pvName, 10, testDriver, "test-vol1"), true), }, { - testName: "fsgroup provided, DelegateFSGroupToCSIDriver feature enabled, driver supports volume mount group; expect fsgroup to be passed to NodeStageVolume", + testName: "fsgroup provided, driver supports volume mount group; expect fsgroup to be passed to NodeStageVolume", volName: "test-vol1", devicePath: "path1", deviceMountPath: "path2", fsGroup: &testFSGroup, - delegateFSGroupFeatureGate: true, driverSupportsVolumeMountGroup: true, expectedVolumeMountGroup: "3000", stageUnstageSet: true, @@ -1235,11 +1228,10 @@ func TestAttacherMountDevice(t *testing.T) { spec: volume.NewSpecFromPersistentVolume(makeTestPV(pvName, 10, testDriver, "test-vol1"), false), }, { - testName: "fsgroup not provided, DelegateFSGroupToCSIDriver feature enabled, driver supports volume mount group; expect fsgroup not to be passed to NodeStageVolume", + testName: "fsgroup not provided, driver supports volume mount group; expect fsgroup not to be passed to NodeStageVolume", volName: "test-vol1", devicePath: "path1", deviceMountPath: "path2", - delegateFSGroupFeatureGate: true, driverSupportsVolumeMountGroup: true, expectedVolumeMountGroup: "", stageUnstageSet: true, @@ -1247,31 +1239,17 @@ func TestAttacherMountDevice(t *testing.T) { spec: volume.NewSpecFromPersistentVolume(makeTestPV(pvName, 10, testDriver, "test-vol1"), false), }, { - testName: "fsgroup provided, DelegateFSGroupToCSIDriver feature enabled, driver does not support volume mount group; expect fsgroup not to be passed to NodeStageVolume", + testName: "fsgroup provided, driver does not support volume mount group; expect fsgroup not to be passed to NodeStageVolume", volName: "test-vol1", devicePath: "path1", deviceMountPath: "path2", fsGroup: &testFSGroup, - delegateFSGroupFeatureGate: true, driverSupportsVolumeMountGroup: false, expectedVolumeMountGroup: "", stageUnstageSet: true, createAttachment: true, spec: volume.NewSpecFromPersistentVolume(makeTestPV(pvName, 10, testDriver, "test-vol1"), false), }, - { - testName: "fsgroup provided, DelegateFSGroupToCSIDriver feature disabled, driver supports volume mount group; expect fsgroup not to be passed to NodeStageVolume", - volName: "test-vol1", - devicePath: "path1", - deviceMountPath: "path2", - fsGroup: &testFSGroup, - delegateFSGroupFeatureGate: false, - driverSupportsVolumeMountGroup: true, - expectedVolumeMountGroup: "", - stageUnstageSet: true, - createAttachment: true, - spec: volume.NewSpecFromPersistentVolume(makeTestPV(pvName, 10, testDriver, "test-vol1"), false), - }, } for _, tc := range testCases { @@ -1289,8 +1267,6 @@ func TestAttacherMountDevice(t *testing.T) { } t.Logf("Running test case: %s", tc.testName) - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DelegateFSGroupToCSIDriver, tc.delegateFSGroupFeatureGate)() - // Setup // Create a new attacher fakeClient := fakeclient.NewSimpleClientset() @@ -1420,7 +1396,6 @@ func TestAttacherMountDevice(t *testing.T) { } func TestAttacherMountDeviceWithInline(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DelegateFSGroupToCSIDriver, true)() pvName := "test-pv" var testFSGroup int64 = 3000 testCases := []struct { diff --git a/pkg/volume/csi/csi_mounter.go b/pkg/volume/csi/csi_mounter.go index b811754dffb..bf9014238d6 100644 --- a/pkg/volume/csi/csi_mounter.go +++ b/pkg/volume/csi/csi_mounter.go @@ -241,16 +241,14 @@ func (c *csiMountMgr) SetUpAt(dir string, mounterArgs volume.MounterArgs) error driverSupportsCSIVolumeMountGroup := false var nodePublishFSGroupArg *int64 - if utilfeature.DefaultFeatureGate.Enabled(features.DelegateFSGroupToCSIDriver) { - driverSupportsCSIVolumeMountGroup, err = csi.NodeSupportsVolumeMountGroup(ctx) - if err != nil { - return volumetypes.NewTransientOperationFailure(log("mounter.SetUpAt failed to determine if the node service has VOLUME_MOUNT_GROUP capability: %v", err)) - } + driverSupportsCSIVolumeMountGroup, err = csi.NodeSupportsVolumeMountGroup(ctx) + if err != nil { + return volumetypes.NewTransientOperationFailure(log("mounter.SetUpAt failed to determine if the node service has VOLUME_MOUNT_GROUP capability: %v", err)) + } - if driverSupportsCSIVolumeMountGroup { - klog.V(3).Infof("Driver %s supports applying FSGroup (has VOLUME_MOUNT_GROUP node capability). Delegating FSGroup application to the driver through NodePublishVolume.", c.driverName) - nodePublishFSGroupArg = mounterArgs.FsGroup - } + if driverSupportsCSIVolumeMountGroup { + klog.V(3).Infof("Driver %s supports applying FSGroup (has VOLUME_MOUNT_GROUP node capability). Delegating FSGroup application to the driver through NodePublishVolume.", c.driverName) + nodePublishFSGroupArg = mounterArgs.FsGroup } var selinuxLabelMount bool diff --git a/pkg/volume/csi/csi_mounter_test.go b/pkg/volume/csi/csi_mounter_test.go index fbbca1d8281..f0e06511d77 100644 --- a/pkg/volume/csi/csi_mounter_test.go +++ b/pkg/volume/csi/csi_mounter_test.go @@ -784,7 +784,6 @@ func TestMounterSetUpWithFSGroup(t *testing.T) { fsGroup int64 driverFSGroupPolicy bool supportMode storage.FSGroupPolicy - delegateFSGroupFeatureGate bool driverSupportsVolumeMountGroup bool expectedFSGroupInNodePublish string }{ @@ -916,47 +915,33 @@ func TestMounterSetUpWithFSGroup(t *testing.T) { supportMode: storage.FileFSGroupPolicy, }, { - name: "fsgroup provided, DelegateFSGroupToCSIDriver feature enabled, driver supports volume mount group; expect fsgroup to be passed to NodePublishVolume", + name: "fsgroup provided, driver supports volume mount group; expect fsgroup to be passed to NodePublishVolume", fsType: "ext4", setFsGroup: true, fsGroup: 3000, - delegateFSGroupFeatureGate: true, driverSupportsVolumeMountGroup: true, expectedFSGroupInNodePublish: "3000", }, { - name: "fsgroup not provided, DelegateFSGroupToCSIDriver feature enabled, driver supports volume mount group; expect fsgroup not to be passed to NodePublishVolume", + name: "fsgroup not provided, driver supports volume mount group; expect fsgroup not to be passed to NodePublishVolume", fsType: "ext4", setFsGroup: false, - delegateFSGroupFeatureGate: true, driverSupportsVolumeMountGroup: true, expectedFSGroupInNodePublish: "", }, { - name: "fsgroup provided, DelegateFSGroupToCSIDriver feature enabled, driver does not support volume mount group; expect fsgroup not to be passed to NodePublishVolume", + name: "fsgroup provided, driver does not support volume mount group; expect fsgroup not to be passed to NodePublishVolume", fsType: "ext4", setFsGroup: true, fsGroup: 3000, - delegateFSGroupFeatureGate: true, driverSupportsVolumeMountGroup: false, expectedFSGroupInNodePublish: "", }, - { - name: "fsgroup provided, DelegateFSGroupToCSIDriver feature disabled, driver supports volume mount group; expect fsgroup not to be passed to NodePublishVolume", - fsType: "ext4", - setFsGroup: true, - fsGroup: 3000, - delegateFSGroupFeatureGate: false, - driverSupportsVolumeMountGroup: true, - expectedFSGroupInNodePublish: "", - }, } for i, tc := range testCases { t.Logf("Running test %s", tc.name) - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DelegateFSGroupToCSIDriver, tc.delegateFSGroupFeatureGate)() - volName := fmt.Sprintf("test-vol-%d", i) registerFakePlugin(testDriver, "endpoint", []string{"1.0.0"}, t) pv := makeTestPV("test-pv", 10, testDriver, volName)