Skip to content
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

Promote DelegateFSGroupToCSIDriver feature to GA #113225

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Expand Up @@ -22,7 +22,7 @@ require (
github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5
github.com/aws/aws-sdk-go v1.44.116
github.com/blang/semver/v4 v4.0.0
github.com/container-storage-interface/spec v1.6.0
github.com/container-storage-interface/spec v1.7.0
github.com/coredns/corefile-migration v1.0.17
github.com/coreos/go-oidc v2.1.0+incompatible
github.com/coreos/go-systemd/v22 v22.3.2
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Expand Up @@ -158,8 +158,8 @@ github.com/cockroachdb/errors v1.2.4 h1:Lap807SXTH5tri2TivECb/4abUkMZC9zRoLarvcK
github.com/cockroachdb/errors v1.2.4/go.mod h1:rQD95gz6FARkaKkQXUksEje/d9a6wBJoCr5oaCLELYA=
github.com/cockroachdb/logtags v0.0.0-20190617123548-eb05cc24525f h1:o/kfcElHqOiXqcou5a3rIlMc7oJbMQkeLk0VQJ7zgqY=
github.com/cockroachdb/logtags v0.0.0-20190617123548-eb05cc24525f/go.mod h1:i/u985jwjWRlyHXQbwatDASoW0RMlZ/3i9yJHE2xLkI=
github.com/container-storage-interface/spec v1.6.0 h1:vwN9uCciKygX/a0toYryoYD5+qI9ZFeAMuhEEKO+JBA=
github.com/container-storage-interface/spec v1.6.0/go.mod h1:8K96oQNkJ7pFcC2R9Z1ynGGBB1I93kcS6PGg3SsOk8s=
github.com/container-storage-interface/spec v1.7.0 h1:gW8eyFQUZWWrMWa8p1seJ28gwDoN5CVJ4uAbQ+Hdycw=
github.com/container-storage-interface/spec v1.7.0/go.mod h1:JYuzLqr9VVNoDJl44xp/8fmCOvWPDKzuGTwCoklhuqk=
github.com/containerd/cgroups v1.0.1 h1:iJnMvco9XGvKUvNQkv88bE4uJXxRQH18efbKo9w5vHQ=
github.com/containerd/cgroups v1.0.1/go.mod h1:0SJrPIenamHDcZhEcJMNBB85rHcUsw4f25ZfBiPYRkU=
github.com/containerd/console v1.0.1/go.mod h1:XUsP6YE/mKtz6bxc+I8UiKKTP04qjQL4qcS3XoQ5xkw=
Expand Down
5 changes: 3 additions & 2 deletions pkg/features/kube_features.go
Expand Up @@ -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.
Expand Down Expand Up @@ -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

Expand Down
16 changes: 7 additions & 9 deletions pkg/volume/csi/csi_attacher.go
Expand Up @@ -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
Expand Down
31 changes: 3 additions & 28 deletions pkg/volume/csi/csi_attacher_test.go
Expand Up @@ -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"
Expand Down Expand Up @@ -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("")
Expand All @@ -1107,7 +1102,6 @@ func TestAttacherMountDevice(t *testing.T) {
stageUnstageSet bool
fsGroup *int64
expectedVolumeMountGroup string
delegateFSGroupFeatureGate bool
driverSupportsVolumeMountGroup bool
shouldFail bool
skipOnWindows bool
Expand Down Expand Up @@ -1222,56 +1216,40 @@ 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,
createAttachment: true,
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,
createAttachment: true,
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 {
Expand All @@ -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()
Expand Down Expand Up @@ -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 {
Expand Down
16 changes: 7 additions & 9 deletions pkg/volume/csi/csi_mounter.go
Expand Up @@ -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
Expand Down
21 changes: 3 additions & 18 deletions pkg/volume/csi/csi_mounter_test.go
Expand Up @@ -784,7 +784,6 @@ func TestMounterSetUpWithFSGroup(t *testing.T) {
fsGroup int64
driverFSGroupPolicy bool
supportMode storage.FSGroupPolicy
delegateFSGroupFeatureGate bool
driverSupportsVolumeMountGroup bool
expectedFSGroupInNodePublish string
}{
Expand Down Expand Up @@ -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)
Expand Down