Skip to content

Commit

Permalink
Remove all references to ConfigurableFSGroupPolicy feature gate
Browse files Browse the repository at this point in the history
  • Loading branch information
gnufied committed Nov 10, 2021
1 parent 9d9c300 commit 27d1e9a
Show file tree
Hide file tree
Showing 3 changed files with 0 additions and 56 deletions.
23 changes: 0 additions & 23 deletions pkg/api/pod/util.go
Expand Up @@ -546,8 +546,6 @@ func dropDisabledFields(
})
}

dropDisabledFSGroupFields(podSpec, oldPodSpec)

if !utilfeature.DefaultFeatureGate.Enabled(features.PodOverhead) && !overheadInUse(oldPodSpec) {
// Set Overhead to nil only if the feature is disabled and it is not used
podSpec.Overhead = nil
Expand Down Expand Up @@ -585,16 +583,6 @@ func dropDisabledProcMountField(podSpec, oldPodSpec *api.PodSpec) {
}
}

func dropDisabledFSGroupFields(podSpec, oldPodSpec *api.PodSpec) {
if !utilfeature.DefaultFeatureGate.Enabled(features.ConfigurableFSGroupPolicy) && !fsGroupPolicyInUse(oldPodSpec) {
// if oldPodSpec had no FSGroupChangePolicy set then we should prevent new pod from having this field
// if ConfigurableFSGroupPolicy feature is disabled
if podSpec.SecurityContext != nil {
podSpec.SecurityContext.FSGroupChangePolicy = nil
}
}
}

// dropDisabledCSIVolumeSourceAlphaFields removes disabled alpha fields from []CSIVolumeSource.
// This should be called from PrepareForCreate/PrepareForUpdate for all pod specs resources containing a CSIVolumeSource
func dropDisabledCSIVolumeSourceAlphaFields(podSpec, oldPodSpec *api.PodSpec) {
Expand Down Expand Up @@ -682,17 +670,6 @@ func ephemeralContainersInUse(podSpec *api.PodSpec) bool {
return len(podSpec.EphemeralContainers) > 0
}

func fsGroupPolicyInUse(podSpec *api.PodSpec) bool {
if podSpec == nil {
return false
}
securityContext := podSpec.SecurityContext
if securityContext != nil && securityContext.FSGroupChangePolicy != nil {
return true
}
return false
}

// overheadInUse returns true if the pod spec is non-nil and has Overhead set
func overheadInUse(podSpec *api.PodSpec) bool {
if podSpec == nil {
Expand Down
25 changes: 0 additions & 25 deletions pkg/volume/volume_linux.go
Expand Up @@ -27,9 +27,7 @@ import (
"time"

v1 "k8s.io/api/core/v1"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/klog/v2"
"k8s.io/kubernetes/pkg/features"
"k8s.io/kubernetes/pkg/volume/util/types"
)

Expand All @@ -47,25 +45,11 @@ func SetVolumeOwnership(mounter Mounter, fsGroup *int64, fsGroupChangePolicy *v1
return nil
}

fsGroupPolicyEnabled := utilfeature.DefaultFeatureGate.Enabled(features.ConfigurableFSGroupPolicy)

timer := time.AfterFunc(30*time.Second, func() {
klog.Warningf("Setting volume ownership for %s and fsGroup set. If the volume has a lot of files then setting volume ownership could be slow, see https://github.com/kubernetes/kubernetes/issues/69699", mounter.GetPath())
})
defer timer.Stop()

// This code exists for legacy purposes, so as old behaviour is entirely preserved when feature gate is disabled
// TODO: remove this when ConfigurableFSGroupPolicy turns GA.
if !fsGroupPolicyEnabled {
err := legacyOwnershipChange(mounter, fsGroup)
if completeFunc != nil {
completeFunc(types.CompleteFuncParam{
Err: &err,
})
}
return err
}

if skipPermissionChange(mounter, fsGroup, fsGroupChangePolicy) {
klog.V(3).InfoS("Skipping permission and ownership change for volume", "path", mounter.GetPath())
return nil
Expand All @@ -85,15 +69,6 @@ func SetVolumeOwnership(mounter Mounter, fsGroup *int64, fsGroupChangePolicy *v1
return err
}

func legacyOwnershipChange(mounter Mounter, fsGroup *int64) error {
return filepath.Walk(mounter.GetPath(), func(path string, info os.FileInfo, err error) error {
if err != nil {
return err
}
return changeFilePermission(path, fsGroup, mounter.GetAttributes().ReadOnly, info)
})
}

func changeFilePermission(filename string, fsGroup *int64, readonly bool, info os.FileInfo) error {
err := os.Lchown(filename, -1, int(*fsGroup))
if err != nil {
Expand Down
8 changes: 0 additions & 8 deletions pkg/volume/volume_linux_test.go
Expand Up @@ -27,10 +27,7 @@ import (
"testing"

v1 "k8s.io/api/core/v1"
utilfeature "k8s.io/apiserver/pkg/util/feature"
utiltesting "k8s.io/client-go/util/testing"
featuregatetesting "k8s.io/component-base/featuregate/testing"
"k8s.io/kubernetes/pkg/features"
)

type localFakeMounter struct {
Expand Down Expand Up @@ -192,12 +189,10 @@ func TestSetVolumeOwnershipMode(t *testing.T) {
fsGroupChangePolicy *v1.PodFSGroupChangePolicy
setupFunc func(path string) error
assertFunc func(path string) error
featureGate bool
}{
{
description: "featuregate=on, fsgroupchangepolicy=always",
fsGroupChangePolicy: &always,
featureGate: true,
setupFunc: func(path string) error {
info, err := os.Lstat(path)
if err != nil {
Expand Down Expand Up @@ -230,7 +225,6 @@ func TestSetVolumeOwnershipMode(t *testing.T) {
{
description: "featuregate=on, fsgroupchangepolicy=onrootmismatch,rootdir=validperm",
fsGroupChangePolicy: &onrootMismatch,
featureGate: true,
setupFunc: func(path string) error {
info, err := os.Lstat(path)
if err != nil {
Expand Down Expand Up @@ -262,7 +256,6 @@ func TestSetVolumeOwnershipMode(t *testing.T) {
{
description: "featuregate=on, fsgroupchangepolicy=onrootmismatch,rootdir=invalidperm",
fsGroupChangePolicy: &onrootMismatch,
featureGate: true,
setupFunc: func(path string) error {
// change mode of root folder to be right
err := os.Chmod(path, 0770)
Expand Down Expand Up @@ -291,7 +284,6 @@ func TestSetVolumeOwnershipMode(t *testing.T) {

for _, test := range tests {
t.Run(test.description, func(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ConfigurableFSGroupPolicy, test.featureGate)()
tmpDir, err := utiltesting.MkTmpdir("volume_linux_ownership")
if err != nil {
t.Fatalf("error creating temp dir: %v", err)
Expand Down

0 comments on commit 27d1e9a

Please sign in to comment.