diff --git a/cmd/kube-controller-manager/app/core.go b/cmd/kube-controller-manager/app/core.go index acaea316867..203b2a2e971 100644 --- a/cmd/kube-controller-manager/app/core.go +++ b/cmd/kube-controller-manager/app/core.go @@ -65,7 +65,6 @@ import ( persistentvolumecontroller "k8s.io/kubernetes/pkg/controller/volume/persistentvolume" "k8s.io/kubernetes/pkg/controller/volume/pvcprotection" "k8s.io/kubernetes/pkg/controller/volume/pvprotection" - "k8s.io/kubernetes/pkg/features" quotainstall "k8s.io/kubernetes/pkg/quota/v1/install" "k8s.io/kubernetes/pkg/volume/csimigration" netutils "k8s.io/utils/net" @@ -336,36 +335,34 @@ func startAttachDetachController(ctx context.Context, controllerContext Controll } func startVolumeExpandController(ctx context.Context, controllerContext ControllerContext) (controller.Interface, bool, error) { - if utilfeature.DefaultFeatureGate.Enabled(features.ExpandPersistentVolumes) { - plugins, err := ProbeExpandableVolumePlugins(controllerContext.ComponentConfig.PersistentVolumeBinderController.VolumeConfiguration) - if err != nil { - return nil, true, fmt.Errorf("failed to probe volume plugins when starting volume expand controller: %v", err) - } - csiTranslator := csitrans.New() - filteredDialOptions, err := options.ParseVolumeHostFilters( - controllerContext.ComponentConfig.PersistentVolumeBinderController.VolumeHostCIDRDenylist, - controllerContext.ComponentConfig.PersistentVolumeBinderController.VolumeHostAllowLocalLoopback) - if err != nil { - return nil, true, err - } - expandController, expandControllerErr := expand.NewExpandController( - controllerContext.ClientBuilder.ClientOrDie("expand-controller"), - controllerContext.InformerFactory.Core().V1().PersistentVolumeClaims(), - controllerContext.InformerFactory.Core().V1().PersistentVolumes(), - controllerContext.Cloud, - plugins, - csiTranslator, - csimigration.NewPluginManager(csiTranslator, utilfeature.DefaultFeatureGate), - filteredDialOptions, - ) - - if expandControllerErr != nil { - return nil, true, fmt.Errorf("failed to start volume expand controller: %v", expandControllerErr) - } - go expandController.Run(ctx) - return nil, true, nil + plugins, err := ProbeExpandableVolumePlugins(controllerContext.ComponentConfig.PersistentVolumeBinderController.VolumeConfiguration) + if err != nil { + return nil, true, fmt.Errorf("failed to probe volume plugins when starting volume expand controller: %v", err) } - return nil, false, nil + csiTranslator := csitrans.New() + filteredDialOptions, err := options.ParseVolumeHostFilters( + controllerContext.ComponentConfig.PersistentVolumeBinderController.VolumeHostCIDRDenylist, + controllerContext.ComponentConfig.PersistentVolumeBinderController.VolumeHostAllowLocalLoopback) + if err != nil { + return nil, true, err + } + expandController, expandControllerErr := expand.NewExpandController( + controllerContext.ClientBuilder.ClientOrDie("expand-controller"), + controllerContext.InformerFactory.Core().V1().PersistentVolumeClaims(), + controllerContext.InformerFactory.Core().V1().PersistentVolumes(), + controllerContext.Cloud, + plugins, + csiTranslator, + csimigration.NewPluginManager(csiTranslator, utilfeature.DefaultFeatureGate), + filteredDialOptions, + ) + + if expandControllerErr != nil { + return nil, true, fmt.Errorf("failed to start volume expand controller: %v", expandControllerErr) + } + go expandController.Run(ctx) + return nil, true, nil + } func startEphemeralVolumeController(ctx context.Context, controllerContext ControllerContext) (controller.Interface, bool, error) { diff --git a/pkg/api/persistentvolume/OWNERS b/pkg/api/persistentvolume/OWNERS deleted file mode 100644 index 2152f772fb4..00000000000 --- a/pkg/api/persistentvolume/OWNERS +++ /dev/null @@ -1,5 +0,0 @@ -# See the OWNERS docs at https://go.k8s.io/owners - -reviewers: - - smarterclayton - - jsafrane diff --git a/pkg/api/persistentvolume/util.go b/pkg/api/persistentvolume/util.go deleted file mode 100644 index ad307ba2c95..00000000000 --- a/pkg/api/persistentvolume/util.go +++ /dev/null @@ -1,44 +0,0 @@ -/* -Copyright 2017 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package persistentvolume - -import ( - utilfeature "k8s.io/apiserver/pkg/util/feature" - api "k8s.io/kubernetes/pkg/apis/core" - "k8s.io/kubernetes/pkg/features" -) - -// DropDisabledFields removes disabled fields from the pv spec. -// This should be called from PrepareForCreate/PrepareForUpdate for all resources containing a pv spec. -func DropDisabledFields(pvSpec *api.PersistentVolumeSpec, oldPVSpec *api.PersistentVolumeSpec) { - if !utilfeature.DefaultFeatureGate.Enabled(features.ExpandCSIVolumes) && !hasExpansionSecrets(oldPVSpec) { - if pvSpec.CSI != nil { - pvSpec.CSI.ControllerExpandSecretRef = nil - } - } -} - -func hasExpansionSecrets(oldPVSpec *api.PersistentVolumeSpec) bool { - if oldPVSpec == nil || oldPVSpec.CSI == nil { - return false - } - - if oldPVSpec.CSI.ControllerExpandSecretRef != nil { - return true - } - return false -} diff --git a/pkg/api/persistentvolume/util_test.go b/pkg/api/persistentvolume/util_test.go deleted file mode 100644 index d23d047a77e..00000000000 --- a/pkg/api/persistentvolume/util_test.go +++ /dev/null @@ -1,117 +0,0 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package persistentvolume - -import ( - "reflect" - "testing" - - "github.com/google/go-cmp/cmp" - - utilfeature "k8s.io/apiserver/pkg/util/feature" - featuregatetesting "k8s.io/component-base/featuregate/testing" - api "k8s.io/kubernetes/pkg/apis/core" - "k8s.io/kubernetes/pkg/features" -) - -func TestDropDisabledFields(t *testing.T) { - secretRef := &api.SecretReference{ - Name: "expansion-secret", - Namespace: "default", - } - - tests := map[string]struct { - oldSpec *api.PersistentVolumeSpec - newSpec *api.PersistentVolumeSpec - expectOldSpec *api.PersistentVolumeSpec - expectNewSpec *api.PersistentVolumeSpec - csiExpansionEnabled bool - }{ - "disabled csi expansion clears secrets": { - csiExpansionEnabled: false, - newSpec: specWithCSISecrets(secretRef), - expectNewSpec: specWithCSISecrets(nil), - oldSpec: nil, - expectOldSpec: nil, - }, - "enabled csi expansion preserve secrets": { - csiExpansionEnabled: true, - newSpec: specWithCSISecrets(secretRef), - expectNewSpec: specWithCSISecrets(secretRef), - oldSpec: nil, - expectOldSpec: nil, - }, - "enabled csi expansion preserve secrets when both old and new have it": { - csiExpansionEnabled: true, - newSpec: specWithCSISecrets(secretRef), - expectNewSpec: specWithCSISecrets(secretRef), - oldSpec: specWithCSISecrets(secretRef), - expectOldSpec: specWithCSISecrets(secretRef), - }, - "disabled csi expansion old pv had secrets": { - csiExpansionEnabled: false, - newSpec: specWithCSISecrets(secretRef), - expectNewSpec: specWithCSISecrets(secretRef), - oldSpec: specWithCSISecrets(secretRef), - expectOldSpec: specWithCSISecrets(secretRef), - }, - "enabled csi expansion preserves secrets when old pv did not had secrets": { - csiExpansionEnabled: true, - newSpec: specWithCSISecrets(secretRef), - expectNewSpec: specWithCSISecrets(secretRef), - oldSpec: specWithCSISecrets(nil), - expectOldSpec: specWithCSISecrets(nil), - }, - "disabled csi expansion clears secrets when old pv did not had secrets": { - csiExpansionEnabled: false, - newSpec: specWithCSISecrets(secretRef), - expectNewSpec: specWithCSISecrets(nil), - oldSpec: specWithCSISecrets(nil), - expectOldSpec: specWithCSISecrets(nil), - }, - } - - for name, tc := range tests { - t.Run(name, func(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ExpandCSIVolumes, tc.csiExpansionEnabled)() - - DropDisabledFields(tc.newSpec, tc.oldSpec) - if !reflect.DeepEqual(tc.newSpec, tc.expectNewSpec) { - t.Error(cmp.Diff(tc.newSpec, tc.expectNewSpec)) - } - if !reflect.DeepEqual(tc.oldSpec, tc.expectOldSpec) { - t.Error(cmp.Diff(tc.oldSpec, tc.expectOldSpec)) - } - }) - } -} - -func specWithCSISecrets(secret *api.SecretReference) *api.PersistentVolumeSpec { - pvSpec := &api.PersistentVolumeSpec{ - PersistentVolumeSource: api.PersistentVolumeSource{ - CSI: &api.CSIPersistentVolumeSource{ - Driver: "com.google.gcepd", - VolumeHandle: "foobar", - }, - }, - } - - if secret != nil { - pvSpec.CSI.ControllerExpandSecretRef = secret - } - return pvSpec -} diff --git a/pkg/api/persistentvolumeclaim/util.go b/pkg/api/persistentvolumeclaim/util.go index f0ff70546de..e8b56e74b7d 100644 --- a/pkg/api/persistentvolumeclaim/util.go +++ b/pkg/api/persistentvolumeclaim/util.go @@ -75,10 +75,6 @@ func EnforceDataSourceBackwardsCompatibility(pvcSpec, oldPVCSpec *core.Persisten } func DropDisabledFieldsFromStatus(pvc, oldPVC *core.PersistentVolumeClaim) { - if !utilfeature.DefaultFeatureGate.Enabled(features.ExpandPersistentVolumes) && oldPVC.Status.Conditions == nil { - pvc.Status.Conditions = nil - } - if !utilfeature.DefaultFeatureGate.Enabled(features.RecoverVolumeExpansionFailure) { if !allocatedResourcesInUse(oldPVC) { pvc.Status.AllocatedResources = nil diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index df67fccc5a9..6357a631f94 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -2019,8 +2019,6 @@ func ValidatePersistentVolumeStatusUpdate(newPv, oldPv *core.PersistentVolume) f type PersistentVolumeClaimSpecValidationOptions struct { // Allow spec to contain the "ReadWiteOncePod" access mode AllowReadWriteOncePod bool - // Allow pvc expansion after PVC is created and bound to a PV - EnableExpansion bool // Allow users to recover from previously failing expansion operation EnableRecoverFromExpansionFailure bool } @@ -2028,7 +2026,6 @@ type PersistentVolumeClaimSpecValidationOptions struct { func ValidationOptionsForPersistentVolumeClaim(pvc, oldPvc *core.PersistentVolumeClaim) PersistentVolumeClaimSpecValidationOptions { opts := PersistentVolumeClaimSpecValidationOptions{ AllowReadWriteOncePod: utilfeature.DefaultFeatureGate.Enabled(features.ReadWriteOncePod), - EnableExpansion: utilfeature.DefaultFeatureGate.Enabled(features.ExpandPersistentVolumes), EnableRecoverFromExpansionFailure: utilfeature.DefaultFeatureGate.Enabled(features.RecoverVolumeExpansionFailure), } if oldPvc == nil { @@ -2175,40 +2172,30 @@ func ValidatePersistentVolumeClaimUpdate(newPvc, oldPvc *core.PersistentVolumeCl allErrs = append(allErrs, ValidateImmutableAnnotation(newPvc.ObjectMeta.Annotations[v1.BetaStorageClassAnnotation], oldPvc.ObjectMeta.Annotations[v1.BetaStorageClassAnnotation], v1.BetaStorageClassAnnotation, field.NewPath("metadata"))...) } - if opts.EnableExpansion { - // lets make sure storage values are same. - if newPvc.Status.Phase == core.ClaimBound && newPvcClone.Spec.Resources.Requests != nil { - newPvcClone.Spec.Resources.Requests["storage"] = oldPvc.Spec.Resources.Requests["storage"] // +k8s:verify-mutation:reason=clone - } + // lets make sure storage values are same. + if newPvc.Status.Phase == core.ClaimBound && newPvcClone.Spec.Resources.Requests != nil { + newPvcClone.Spec.Resources.Requests["storage"] = oldPvc.Spec.Resources.Requests["storage"] // +k8s:verify-mutation:reason=clone + } - oldSize := oldPvc.Spec.Resources.Requests["storage"] - newSize := newPvc.Spec.Resources.Requests["storage"] - statusSize := oldPvc.Status.Capacity["storage"] + oldSize := oldPvc.Spec.Resources.Requests["storage"] + newSize := newPvc.Spec.Resources.Requests["storage"] + statusSize := oldPvc.Status.Capacity["storage"] - if !apiequality.Semantic.DeepEqual(newPvcClone.Spec, oldPvcClone.Spec) { - specDiff := cmp.Diff(oldPvcClone.Spec, newPvcClone.Spec) - allErrs = append(allErrs, field.Forbidden(field.NewPath("spec"), fmt.Sprintf("spec is immutable after creation except resources.requests for bound claims\n%v", specDiff))) - } - if newSize.Cmp(oldSize) < 0 { - if !opts.EnableRecoverFromExpansionFailure { - allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "resources", "requests", "storage"), "field can not be less than previous value")) - } else { - // This validation permits reducing pvc requested size up to capacity recorded in pvc.status - // so that users can recover from volume expansion failure, but Kubernetes does not actually - // support volume shrinking - if newSize.Cmp(statusSize) <= 0 { - allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "resources", "requests", "storage"), "field can not be less than status.capacity")) - } + if !apiequality.Semantic.DeepEqual(newPvcClone.Spec, oldPvcClone.Spec) { + specDiff := cmp.Diff(oldPvcClone.Spec, newPvcClone.Spec) + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec"), fmt.Sprintf("spec is immutable after creation except resources.requests for bound claims\n%v", specDiff))) + } + if newSize.Cmp(oldSize) < 0 { + if !opts.EnableRecoverFromExpansionFailure { + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "resources", "requests", "storage"), "field can not be less than previous value")) + } else { + // This validation permits reducing pvc requested size up to capacity recorded in pvc.status + // so that users can recover from volume expansion failure, but Kubernetes does not actually + // support volume shrinking + if newSize.Cmp(statusSize) <= 0 { + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "resources", "requests", "storage"), "field can not be less than status.capacity")) } } - - } else { - // changes to Spec are not allowed, but updates to label/and some annotations are OK. - // no-op updates pass validation. - if !apiequality.Semantic.DeepEqual(newPvcClone.Spec, oldPvcClone.Spec) { - specDiff := cmp.Diff(oldPvcClone.Spec, newPvcClone.Spec) - allErrs = append(allErrs, field.Forbidden(field.NewPath("spec"), fmt.Sprintf("field is immutable after creation\n%v", specDiff))) - } } allErrs = append(allErrs, ValidateImmutableField(newPvc.Spec.VolumeMode, oldPvc.Spec.VolumeMode, field.NewPath("volumeMode"))...) diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 0ccde3c1e17..72c77a31805 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -2108,215 +2108,173 @@ func TestValidatePersistentVolumeClaimUpdate(t *testing.T) { isExpectedFailure bool oldClaim *core.PersistentVolumeClaim newClaim *core.PersistentVolumeClaim - enableResize bool enableRecoverFromExpansion bool }{ "valid-update-volumeName-only": { isExpectedFailure: false, oldClaim: validClaim, newClaim: validUpdateClaim, - enableResize: false, }, "valid-no-op-update": { isExpectedFailure: false, oldClaim: validUpdateClaim, newClaim: validUpdateClaim, - enableResize: false, }, "invalid-update-change-resources-on-bound-claim": { isExpectedFailure: true, oldClaim: validUpdateClaim, newClaim: invalidUpdateClaimResources, - enableResize: false, }, "invalid-update-change-access-modes-on-bound-claim": { isExpectedFailure: true, oldClaim: validUpdateClaim, newClaim: invalidUpdateClaimAccessModes, - enableResize: false, }, "valid-update-volume-mode-block-to-block": { isExpectedFailure: false, oldClaim: validClaimVolumeModeBlock, newClaim: validClaimVolumeModeBlock, - enableResize: false, }, "valid-update-volume-mode-file-to-file": { isExpectedFailure: false, oldClaim: validClaimVolumeModeFile, newClaim: validClaimVolumeModeFile, - enableResize: false, }, "invalid-update-volume-mode-to-block": { isExpectedFailure: true, oldClaim: validClaimVolumeModeFile, newClaim: validClaimVolumeModeBlock, - enableResize: false, }, "invalid-update-volume-mode-to-file": { isExpectedFailure: true, oldClaim: validClaimVolumeModeBlock, newClaim: validClaimVolumeModeFile, - enableResize: false, }, "invalid-update-volume-mode-nil-to-file": { isExpectedFailure: true, oldClaim: invalidClaimVolumeModeNil, newClaim: validClaimVolumeModeFile, - enableResize: false, }, "invalid-update-volume-mode-nil-to-block": { isExpectedFailure: true, oldClaim: invalidClaimVolumeModeNil, newClaim: validClaimVolumeModeBlock, - enableResize: false, }, "invalid-update-volume-mode-block-to-nil": { isExpectedFailure: true, oldClaim: validClaimVolumeModeBlock, newClaim: invalidClaimVolumeModeNil, - enableResize: false, }, "invalid-update-volume-mode-file-to-nil": { isExpectedFailure: true, oldClaim: validClaimVolumeModeFile, newClaim: invalidClaimVolumeModeNil, - enableResize: false, }, "invalid-update-volume-mode-empty-to-mode": { isExpectedFailure: true, oldClaim: validClaim, newClaim: validClaimVolumeModeBlock, - enableResize: false, }, "invalid-update-volume-mode-mode-to-empty": { isExpectedFailure: true, oldClaim: validClaimVolumeModeBlock, newClaim: validClaim, - enableResize: false, }, "invalid-update-change-storage-class-annotation-after-creation": { isExpectedFailure: true, oldClaim: validClaimStorageClass, newClaim: invalidUpdateClaimStorageClass, - enableResize: false, }, "valid-update-mutable-annotation": { isExpectedFailure: false, oldClaim: validClaimAnnotation, newClaim: validUpdateClaimMutableAnnotation, - enableResize: false, }, "valid-update-add-annotation": { isExpectedFailure: false, oldClaim: validClaim, newClaim: validAddClaimAnnotation, - enableResize: false, }, "valid-size-update-resize-disabled": { - isExpectedFailure: true, - oldClaim: validClaim, - newClaim: validSizeUpdate, - enableResize: false, + oldClaim: validClaim, + newClaim: validSizeUpdate, }, "valid-size-update-resize-enabled": { isExpectedFailure: false, oldClaim: validClaim, newClaim: validSizeUpdate, - enableResize: true, }, "invalid-size-update-resize-enabled": { isExpectedFailure: true, oldClaim: validClaim, newClaim: invalidSizeUpdate, - enableResize: true, }, "unbound-size-update-resize-enabled": { isExpectedFailure: true, oldClaim: validClaim, newClaim: unboundSizeUpdate, - enableResize: true, }, "valid-upgrade-storage-class-annotation-to-spec": { isExpectedFailure: false, oldClaim: validClaimStorageClass, newClaim: validClaimStorageClassInSpec, - enableResize: false, }, "invalid-upgrade-storage-class-annotation-to-spec": { isExpectedFailure: true, oldClaim: validClaimStorageClass, newClaim: invalidClaimStorageClassInSpec, - enableResize: false, }, "valid-upgrade-storage-class-annotation-to-annotation-and-spec": { isExpectedFailure: false, oldClaim: validClaimStorageClass, newClaim: validClaimStorageClassInAnnotationAndSpec, - enableResize: false, }, "invalid-upgrade-storage-class-annotation-to-annotation-and-spec": { isExpectedFailure: true, oldClaim: validClaimStorageClass, newClaim: invalidClaimStorageClassInAnnotationAndSpec, - enableResize: false, }, "invalid-upgrade-storage-class-in-spec": { isExpectedFailure: true, oldClaim: validClaimStorageClassInSpec, newClaim: invalidClaimStorageClassInSpec, - enableResize: false, }, "invalid-downgrade-storage-class-spec-to-annotation": { isExpectedFailure: true, oldClaim: validClaimStorageClassInSpec, newClaim: validClaimStorageClass, - enableResize: false, }, "valid-update-rwop-used-and-rwop-feature-disabled": { isExpectedFailure: false, oldClaim: validClaimRWOPAccessMode, newClaim: validClaimRWOPAccessModeAddAnnotation, - enableResize: false, }, "valid-expand-shrink-resize-enabled": { oldClaim: validClaimShrinkInitial, newClaim: validClaimShrink, - enableResize: true, enableRecoverFromExpansion: true, }, "invalid-expand-shrink-resize-enabled": { oldClaim: validClaimShrinkInitial, newClaim: invalidClaimShrink, - enableResize: true, enableRecoverFromExpansion: true, isExpectedFailure: true, }, "invalid-expand-shrink-to-status-resize-enabled": { oldClaim: validClaimShrinkInitial, newClaim: invalidShrinkToStatus, - enableResize: true, enableRecoverFromExpansion: true, isExpectedFailure: true, }, "invalid-expand-shrink-recover-disabled": { oldClaim: validClaimShrinkInitial, newClaim: validClaimShrink, - enableResize: true, enableRecoverFromExpansion: false, isExpectedFailure: true, }, - "invalid-expand-shrink-resize-disabled": { - oldClaim: validClaimShrinkInitial, - newClaim: validClaimShrink, - enableResize: false, - enableRecoverFromExpansion: true, - isExpectedFailure: true, - }, "unbound-size-shrink-resize-enabled": { oldClaim: validClaimShrinkInitial, newClaim: unboundShrink, - enableResize: true, enableRecoverFromExpansion: true, isExpectedFailure: true, }, @@ -2324,7 +2282,6 @@ func TestValidatePersistentVolumeClaimUpdate(t *testing.T) { for name, scenario := range scenarios { t.Run(name, func(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ExpandPersistentVolumes, scenario.enableResize)() defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RecoverVolumeExpansionFailure, scenario.enableRecoverFromExpansion)() scenario.oldClaim.ResourceVersion = "1" scenario.newClaim.ResourceVersion = "1" @@ -2351,7 +2308,6 @@ func TestValidationOptionsForPersistentVolumeClaim(t *testing.T) { enableReadWriteOncePod: true, expectValidationOpts: PersistentVolumeClaimSpecValidationOptions{ AllowReadWriteOncePod: true, - EnableExpansion: true, EnableRecoverFromExpansionFailure: false, }, }, @@ -2360,7 +2316,6 @@ func TestValidationOptionsForPersistentVolumeClaim(t *testing.T) { enableReadWriteOncePod: true, expectValidationOpts: PersistentVolumeClaimSpecValidationOptions{ AllowReadWriteOncePod: true, - EnableExpansion: true, EnableRecoverFromExpansionFailure: false, }, }, @@ -2369,7 +2324,6 @@ func TestValidationOptionsForPersistentVolumeClaim(t *testing.T) { enableReadWriteOncePod: false, expectValidationOpts: PersistentVolumeClaimSpecValidationOptions{ AllowReadWriteOncePod: false, - EnableExpansion: true, EnableRecoverFromExpansionFailure: false, }, }, @@ -2378,7 +2332,6 @@ func TestValidationOptionsForPersistentVolumeClaim(t *testing.T) { enableReadWriteOncePod: true, expectValidationOpts: PersistentVolumeClaimSpecValidationOptions{ AllowReadWriteOncePod: true, - EnableExpansion: true, EnableRecoverFromExpansionFailure: false, }, }, @@ -2387,7 +2340,6 @@ func TestValidationOptionsForPersistentVolumeClaim(t *testing.T) { enableReadWriteOncePod: false, expectValidationOpts: PersistentVolumeClaimSpecValidationOptions{ AllowReadWriteOncePod: true, - EnableExpansion: true, EnableRecoverFromExpansionFailure: false, }, }, diff --git a/pkg/apis/storage/util/util.go b/pkg/apis/storage/util/util.go deleted file mode 100644 index 135c548125d..00000000000 --- a/pkg/apis/storage/util/util.go +++ /dev/null @@ -1,40 +0,0 @@ -/* -Copyright 2017 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package util - -import ( - utilfeature "k8s.io/apiserver/pkg/util/feature" - "k8s.io/kubernetes/pkg/apis/storage" - "k8s.io/kubernetes/pkg/features" -) - -// DropDisabledFields removes disabled fields from the StorageClass object. -func DropDisabledFields(class, oldClass *storage.StorageClass) { - if !utilfeature.DefaultFeatureGate.Enabled(features.ExpandPersistentVolumes) && !allowVolumeExpansionInUse(oldClass) { - class.AllowVolumeExpansion = nil - } -} - -func allowVolumeExpansionInUse(oldClass *storage.StorageClass) bool { - if oldClass == nil { - return false - } - if oldClass.AllowVolumeExpansion != nil { - return true - } - return false -} diff --git a/pkg/apis/storage/util/util_test.go b/pkg/apis/storage/util/util_test.go deleted file mode 100644 index aa296e0068b..00000000000 --- a/pkg/apis/storage/util/util_test.go +++ /dev/null @@ -1,108 +0,0 @@ -/* -Copyright 2017 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package util - -import ( - "fmt" - "reflect" - "testing" - - "k8s.io/apimachinery/pkg/util/diff" - utilfeature "k8s.io/apiserver/pkg/util/feature" - featuregatetesting "k8s.io/component-base/featuregate/testing" - "k8s.io/kubernetes/pkg/apis/storage" - "k8s.io/kubernetes/pkg/features" -) - -func TestDropAllowVolumeExpansion(t *testing.T) { - allowVolumeExpansion := false - scWithoutAllowVolumeExpansion := func() *storage.StorageClass { - return &storage.StorageClass{} - } - scWithAllowVolumeExpansion := func() *storage.StorageClass { - return &storage.StorageClass{ - AllowVolumeExpansion: &allowVolumeExpansion, - } - } - - scInfo := []struct { - description string - hasAllowVolumeExpansion bool - sc func() *storage.StorageClass - }{ - { - description: "StorageClass Without AllowVolumeExpansion", - hasAllowVolumeExpansion: false, - sc: scWithoutAllowVolumeExpansion, - }, - { - description: "StorageClass With AllowVolumeExpansion", - hasAllowVolumeExpansion: true, - sc: scWithAllowVolumeExpansion, - }, - { - description: "is nil", - hasAllowVolumeExpansion: false, - sc: func() *storage.StorageClass { return nil }, - }, - } - - for _, enabled := range []bool{true, false} { - for _, oldStorageClassInfo := range scInfo { - for _, newStorageClassInfo := range scInfo { - oldStorageClassHasAllowVolumeExpansion, oldStorageClass := oldStorageClassInfo.hasAllowVolumeExpansion, oldStorageClassInfo.sc() - newStorageClassHasAllowVolumeExpansion, newStorageClass := newStorageClassInfo.hasAllowVolumeExpansion, newStorageClassInfo.sc() - if newStorageClass == nil { - continue - } - - t.Run(fmt.Sprintf("feature enabled=%v, old StorageClass %v, new StorageClass %v", enabled, oldStorageClassInfo.description, newStorageClassInfo.description), func(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ExpandPersistentVolumes, enabled)() - - DropDisabledFields(newStorageClass, oldStorageClass) - - // old StorageClass should never be changed - if !reflect.DeepEqual(oldStorageClass, oldStorageClassInfo.sc()) { - t.Errorf("old StorageClass changed: %v", diff.ObjectReflectDiff(oldStorageClass, oldStorageClassInfo.sc())) - } - - switch { - case enabled || oldStorageClassHasAllowVolumeExpansion: - // new StorageClass should not be changed if the feature is enabled, or if the old StorageClass had AllowVolumeExpansion - if !reflect.DeepEqual(newStorageClass, newStorageClassInfo.sc()) { - t.Errorf("new StorageClass changed: %v", diff.ObjectReflectDiff(newStorageClass, newStorageClassInfo.sc())) - } - case newStorageClassHasAllowVolumeExpansion: - // new StorageClass should be changed - if reflect.DeepEqual(newStorageClass, newStorageClassInfo.sc()) { - t.Errorf("new StorageClass was not changed") - } - // new StorageClass should not have AllowVolumeExpansion - if !reflect.DeepEqual(newStorageClass, scWithoutAllowVolumeExpansion()) { - t.Errorf("new StorageClass had StorageClassAllowVolumeExpansion: %v", diff.ObjectReflectDiff(newStorageClass, scWithoutAllowVolumeExpansion())) - } - default: - // new StorageClass should not need to be changed - if !reflect.DeepEqual(newStorageClass, newStorageClassInfo.sc()) { - t.Errorf("new StorageClass changed: %v", diff.ObjectReflectDiff(newStorageClass, newStorageClassInfo.sc())) - } - } - }) - } - } - } -} diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index fb2588b80be..90d296734c1 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -78,17 +78,20 @@ const ( // owner: @gnufied // beta: v1.11 + // GA: 1.24 // Ability to Expand persistent volumes ExpandPersistentVolumes featuregate.Feature = "ExpandPersistentVolumes" - // owner: @mlmhl + // owner: @mlmhl @gnufied // beta: v1.15 + // GA: 1.24 // Ability to expand persistent volumes' file system without unmounting volumes. ExpandInUsePersistentVolumes featuregate.Feature = "ExpandInUsePersistentVolumes" // owner: @gnufied // alpha: v1.14 // beta: v1.16 + // GA: 1.24 // Ability to expand CSI volumes ExpandCSIVolumes featuregate.Feature = "ExpandCSIVolumes" @@ -865,9 +868,9 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS LocalStorageCapacityIsolation: {Default: true, PreRelease: featuregate.Beta}, EphemeralContainers: {Default: true, PreRelease: featuregate.Beta}, QOSReserved: {Default: false, PreRelease: featuregate.Alpha}, - ExpandPersistentVolumes: {Default: true, PreRelease: featuregate.Beta}, - ExpandInUsePersistentVolumes: {Default: true, PreRelease: featuregate.Beta}, - ExpandCSIVolumes: {Default: true, PreRelease: featuregate.Beta}, + ExpandPersistentVolumes: {Default: true, PreRelease: featuregate.GA}, // remove in 1.25 + ExpandInUsePersistentVolumes: {Default: true, PreRelease: featuregate.GA}, // remove in 1.25 + ExpandCSIVolumes: {Default: true, PreRelease: featuregate.GA}, // remove in 1.25 CPUManager: {Default: true, PreRelease: featuregate.Beta}, MemoryManager: {Default: true, PreRelease: featuregate.Beta}, CPUCFSQuotaPeriod: {Default: false, PreRelease: featuregate.Alpha}, diff --git a/pkg/kubelet/volumemanager/cache/actual_state_of_world.go b/pkg/kubelet/volumemanager/cache/actual_state_of_world.go index b048cf0ecd9..4ccd922569d 100644 --- a/pkg/kubelet/volumemanager/cache/actual_state_of_world.go +++ b/pkg/kubelet/volumemanager/cache/actual_state_of_world.go @@ -26,9 +26,7 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/klog/v2" - "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/volume" "k8s.io/kubernetes/pkg/volume/util" "k8s.io/kubernetes/pkg/volume/util/operationexecutor" @@ -714,8 +712,7 @@ func (asw *actualStateOfWorld) PodExistsInVolume( return true, volumeObj.devicePath, newRemountRequiredError(volumeObj.volumeName, podObj.podName) } if podObj.fsResizeRequired && - !volumeObj.volumeInUseErrorForExpansion && - utilfeature.DefaultFeatureGate.Enabled(features.ExpandInUsePersistentVolumes) { + !volumeObj.volumeInUseErrorForExpansion { return true, volumeObj.devicePath, newFsResizeRequiredError(volumeObj.volumeName, podObj.podName) } } diff --git a/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go b/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go index fff3c058605..e83325a60e9 100644 --- a/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go +++ b/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go @@ -34,10 +34,8 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/wait" - utilfeature "k8s.io/apiserver/pkg/util/feature" clientset "k8s.io/client-go/kubernetes" "k8s.io/component-helpers/storage/ephemeral" - "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/kubelet/config" kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" "k8s.io/kubernetes/pkg/kubelet/pod" @@ -188,15 +186,13 @@ func (dswp *desiredStateOfWorldPopulator) populatorLoop() { func (dswp *desiredStateOfWorldPopulator) findAndAddNewPods() { // Map unique pod name to outer volume name to MountedVolume. mountedVolumesForPod := make(map[volumetypes.UniquePodName]map[string]cache.MountedVolume) - if utilfeature.DefaultFeatureGate.Enabled(features.ExpandInUsePersistentVolumes) { - for _, mountedVolume := range dswp.actualStateOfWorld.GetMountedVolumes() { - mountedVolumes, exist := mountedVolumesForPod[mountedVolume.PodName] - if !exist { - mountedVolumes = make(map[string]cache.MountedVolume) - mountedVolumesForPod[mountedVolume.PodName] = mountedVolumes - } - mountedVolumes[mountedVolume.OuterVolumeSpecName] = mountedVolume + for _, mountedVolume := range dswp.actualStateOfWorld.GetMountedVolumes() { + mountedVolumes, exist := mountedVolumesForPod[mountedVolume.PodName] + if !exist { + mountedVolumes = make(map[string]cache.MountedVolume) + mountedVolumesForPod[mountedVolume.PodName] = mountedVolumes } + mountedVolumes[mountedVolume.OuterVolumeSpecName] = mountedVolume } processedVolumesForFSResize := sets.NewString() @@ -288,7 +284,6 @@ func (dswp *desiredStateOfWorldPopulator) processPodVolumes( allVolumesAdded := true mounts, devices := util.GetPodVolumeNames(pod) - expandInUsePV := utilfeature.DefaultFeatureGate.Enabled(features.ExpandInUsePersistentVolumes) // Process volume spec for each volume defined in pod for _, podVolume := range pod.Spec.Volumes { if !mounts.Has(podVolume.Name) && !devices.Has(podVolume.Name) { @@ -319,10 +314,9 @@ func (dswp *desiredStateOfWorldPopulator) processPodVolumes( // sync reconstructed volume dswp.actualStateOfWorld.SyncReconstructedVolume(uniqueVolumeName, uniquePodName, podVolume.Name) - if expandInUsePV { - dswp.checkVolumeFSResize(pod, podVolume, pvc, volumeSpec, - uniquePodName, mountedVolumesForPod, processedVolumesForFSResize) - } + dswp.checkVolumeFSResize(pod, podVolume, pvc, volumeSpec, + uniquePodName, mountedVolumesForPod, processedVolumesForFSResize) + } // some of the volume additions may have failed, should not mark this pod as fully processed diff --git a/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator_test.go b/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator_test.go index ec184ec5d18..e32dcd6a9f2 100644 --- a/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator_test.go +++ b/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator_test.go @@ -31,9 +31,7 @@ import ( utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/kubernetes/fake" core "k8s.io/client-go/testing" - featuregatetesting "k8s.io/component-base/featuregate/testing" csitrans "k8s.io/csi-translation-lib" - "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/kubelet/configmap" containertest "k8s.io/kubernetes/pkg/kubelet/container/testing" kubepod "k8s.io/kubernetes/pkg/kubelet/pod" @@ -911,11 +909,10 @@ func TestCheckVolumeFSResize(t *testing.T) { } testcases := []struct { - resize func(*testing.T, *v1.PersistentVolume, *v1.PersistentVolumeClaim, *desiredStateOfWorldPopulator) - verify func(*testing.T, []v1.UniqueVolumeName, v1.UniqueVolumeName) - enableResize bool - readOnlyVol bool - volumeMode v1.PersistentVolumeMode + resize func(*testing.T, *v1.PersistentVolume, *v1.PersistentVolumeClaim, *desiredStateOfWorldPopulator) + verify func(*testing.T, []v1.UniqueVolumeName, v1.UniqueVolumeName) + readOnlyVol bool + volumeMode v1.PersistentVolumeMode }{ { // No resize request for volume, volumes in ASW shouldn't be marked as fsResizeRequired @@ -926,22 +923,9 @@ func TestCheckVolumeFSResize(t *testing.T) { t.Errorf("No resize request for any volumes, but found resize required volumes in ASW: %v", vols) } }, - enableResize: true, - volumeMode: v1.PersistentVolumeFilesystem, - }, - { - // Disable the feature gate, so volume shouldn't be marked as fsResizeRequired - resize: func(_ *testing.T, pv *v1.PersistentVolume, pvc *v1.PersistentVolumeClaim, _ *desiredStateOfWorldPopulator) { - setCapacity(pv, pvc, 2) - }, - verify: func(t *testing.T, vols []v1.UniqueVolumeName, _ v1.UniqueVolumeName) { - if len(vols) > 0 { - t.Errorf("Feature gate disabled, but found resize required volumes in ASW: %v", vols) - } - }, - enableResize: false, - volumeMode: v1.PersistentVolumeFilesystem, + volumeMode: v1.PersistentVolumeFilesystem, }, + { // Make volume used as ReadOnly, so volume shouldn't be marked as fsResizeRequired resize: func(_ *testing.T, pv *v1.PersistentVolume, pvc *v1.PersistentVolumeClaim, _ *desiredStateOfWorldPopulator) { @@ -952,9 +936,8 @@ func TestCheckVolumeFSResize(t *testing.T) { t.Errorf("volume mounted as ReadOnly, but found resize required volumes in ASW: %v", vols) } }, - readOnlyVol: true, - enableResize: true, - volumeMode: v1.PersistentVolumeFilesystem, + readOnlyVol: true, + volumeMode: v1.PersistentVolumeFilesystem, }, { // Clear ASW, so volume shouldn't be marked as fsResizeRequired because they are not mounted @@ -967,8 +950,7 @@ func TestCheckVolumeFSResize(t *testing.T) { t.Errorf("volume hasn't been mounted, but found resize required volumes in ASW: %v", vols) } }, - enableResize: true, - volumeMode: v1.PersistentVolumeFilesystem, + volumeMode: v1.PersistentVolumeFilesystem, }, { // volume in ASW should be marked as fsResizeRequired @@ -986,8 +968,7 @@ func TestCheckVolumeFSResize(t *testing.T) { t.Fatalf("Mark wrong volume as fsResizeRequired: %s", vols[0]) } }, - enableResize: true, - volumeMode: v1.PersistentVolumeFilesystem, + volumeMode: v1.PersistentVolumeFilesystem, }, { // volume in ASW should be marked as fsResizeRequired @@ -1005,8 +986,7 @@ func TestCheckVolumeFSResize(t *testing.T) { t.Fatalf("Mark wrong volume as fsResizeRequired: %s", vols[0]) } }, - enableResize: true, - volumeMode: v1.PersistentVolumeBlock, + volumeMode: v1.PersistentVolumeBlock, }, } @@ -1071,8 +1051,6 @@ func TestCheckVolumeFSResize(t *testing.T) { reconcileASW(fakeASW, fakeDSW, t) func() { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ExpandInUsePersistentVolumes, tc.enableResize)() - tc.resize(t, pv, pvc, dswp) resizeRequiredVolumes := reprocess(dswp, uniquePodName, fakeDSW, fakeASW) diff --git a/pkg/kubelet/volumemanager/reconciler/reconciler.go b/pkg/kubelet/volumemanager/reconciler/reconciler.go index bbf943deef8..1188d2ef72f 100644 --- a/pkg/kubelet/volumemanager/reconciler/reconciler.go +++ b/pkg/kubelet/volumemanager/reconciler/reconciler.go @@ -37,9 +37,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/wait" - utilfeature "k8s.io/apiserver/pkg/util/feature" clientset "k8s.io/client-go/kubernetes" - "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/kubelet/config" "k8s.io/kubernetes/pkg/kubelet/volumemanager/cache" "k8s.io/kubernetes/pkg/util/goroutinemap/exponentialbackoff" @@ -260,8 +258,7 @@ func (rc *reconciler) mountAttachVolumes() { klog.V(5).InfoS(volumeToMount.GenerateMsgDetailed("operationExecutor.MountVolume started", remountingLogStr), "pod", klog.KObj(volumeToMount.Pod)) } } - } else if cache.IsFSResizeRequiredError(err) && - utilfeature.DefaultFeatureGate.Enabled(features.ExpandInUsePersistentVolumes) { + } else if cache.IsFSResizeRequiredError(err) { klog.V(4).InfoS(volumeToMount.GenerateMsgDetailed("Starting operationExecutor.ExpandInUseVolume", ""), "pod", klog.KObj(volumeToMount.Pod)) err := rc.operationExecutor.ExpandInUseVolume( volumeToMount.VolumeToMount, diff --git a/pkg/kubelet/volumemanager/reconciler/reconciler_test.go b/pkg/kubelet/volumemanager/reconciler/reconciler_test.go index 89e0319e992..a43110c5341 100644 --- a/pkg/kubelet/volumemanager/reconciler/reconciler_test.go +++ b/pkg/kubelet/volumemanager/reconciler/reconciler_test.go @@ -31,13 +31,10 @@ import ( "k8s.io/apimachinery/pkg/runtime" k8stypes "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/wait" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/kubernetes/fake" core "k8s.io/client-go/testing" "k8s.io/client-go/tools/record" - featuregatetesting "k8s.io/component-base/featuregate/testing" "k8s.io/klog/v2" - "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/kubelet/volumemanager/cache" "k8s.io/kubernetes/pkg/volume" volumetesting "k8s.io/kubernetes/pkg/volume/testing" @@ -1118,7 +1115,6 @@ func Test_GenerateUnmapDeviceFunc_Plugin_Not_Found(t *testing.T) { // Mark volume as fsResizeRequired in ASW. // Verifies volume's fsResizeRequired flag is cleared later. func Test_Run_Positive_VolumeFSResizeControllerAttachEnabled(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ExpandInUsePersistentVolumes, true)() blockMode := v1.PersistentVolumeBlock fsMode := v1.PersistentVolumeFilesystem diff --git a/pkg/quota/v1/evaluator/core/persistent_volume_claims.go b/pkg/quota/v1/evaluator/core/persistent_volume_claims.go index cbeb373fd02..d6f339bb727 100644 --- a/pkg/quota/v1/evaluator/core/persistent_volume_claims.go +++ b/pkg/quota/v1/evaluator/core/persistent_volume_claims.go @@ -94,7 +94,7 @@ func (p *pvcEvaluator) Handles(a admission.Attributes) bool { if op == admission.Create { return true } - if op == admission.Update && utilfeature.DefaultFeatureGate.Enabled(k8sfeatures.ExpandPersistentVolumes) { + if op == admission.Update { return true } return false diff --git a/pkg/registry/core/persistentvolume/strategy.go b/pkg/registry/core/persistentvolume/strategy.go index 89b5a0332fd..fdccda2a6f3 100644 --- a/pkg/registry/core/persistentvolume/strategy.go +++ b/pkg/registry/core/persistentvolume/strategy.go @@ -28,7 +28,6 @@ import ( "k8s.io/apiserver/pkg/storage" "k8s.io/apiserver/pkg/storage/names" "k8s.io/kubernetes/pkg/api/legacyscheme" - pvutil "k8s.io/kubernetes/pkg/api/persistentvolume" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/core/validation" volumevalidation "k8s.io/kubernetes/pkg/volume/validation" @@ -63,10 +62,6 @@ func (persistentvolumeStrategy) GetResetFields() map[fieldpath.APIVersion]*field // ResetBeforeCreate clears the Status field which is not allowed to be set by end users on creation. func (persistentvolumeStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) { - pv := obj.(*api.PersistentVolume) - pv.Status = api.PersistentVolumeStatus{} - - pvutil.DropDisabledFields(&pv.Spec, nil) } func (persistentvolumeStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList { @@ -94,8 +89,6 @@ func (persistentvolumeStrategy) PrepareForUpdate(ctx context.Context, obj, old r newPv := obj.(*api.PersistentVolume) oldPv := old.(*api.PersistentVolume) newPv.Status = oldPv.Status - - pvutil.DropDisabledFields(&newPv.Spec, &oldPv.Spec) } func (persistentvolumeStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { diff --git a/pkg/registry/core/persistentvolumeclaim/strategy_test.go b/pkg/registry/core/persistentvolumeclaim/strategy_test.go index 44d3091038e..1f6a332a9b9 100644 --- a/pkg/registry/core/persistentvolumeclaim/strategy_test.go +++ b/pkg/registry/core/persistentvolumeclaim/strategy_test.go @@ -76,47 +76,35 @@ func TestDropConditions(t *testing.T) { }, } - for _, enabled := range []bool{true, false} { - for _, oldPvcInfo := range pvcInfo { - for _, newPvcInfo := range pvcInfo { - oldPvcHasConditins, oldPvc := oldPvcInfo.hasConditions, oldPvcInfo.pvc() - newPvcHasConditions, newPvc := newPvcInfo.hasConditions, newPvcInfo.pvc() + for _, oldPvcInfo := range pvcInfo { + for _, newPvcInfo := range pvcInfo { + oldPvcHasConditins, oldPvc := oldPvcInfo.hasConditions, oldPvcInfo.pvc() + newPvcHasConditions, newPvc := newPvcInfo.hasConditions, newPvcInfo.pvc() - t.Run(fmt.Sprintf("feature enabled=%v, old pvc %v, new pvc %v", enabled, oldPvcInfo.description, newPvcInfo.description), func(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ExpandPersistentVolumes, enabled)() + t.Run(fmt.Sprintf("old pvc %s, new pvc %s", oldPvcInfo.description, newPvcInfo.description), func(t *testing.T) { + StatusStrategy.PrepareForUpdate(ctx, newPvc, oldPvc) - StatusStrategy.PrepareForUpdate(ctx, newPvc, oldPvc) + // old pvc should never be changed + if !reflect.DeepEqual(oldPvc, oldPvcInfo.pvc()) { + t.Errorf("old pvc changed: %v", diff.ObjectReflectDiff(oldPvc, oldPvcInfo.pvc())) + } - // old pvc should never be changed - if !reflect.DeepEqual(oldPvc, oldPvcInfo.pvc()) { - t.Errorf("old pvc changed: %v", diff.ObjectReflectDiff(oldPvc, oldPvcInfo.pvc())) + switch { + case oldPvcHasConditins || newPvcHasConditions: + // new pvc should not be changed if the feature is enabled, or if the old pvc had Conditions + if !reflect.DeepEqual(newPvc, newPvcInfo.pvc()) { + t.Errorf("new pvc changed: %v", diff.ObjectReflectDiff(newPvc, newPvcInfo.pvc())) } - - switch { - case enabled || oldPvcHasConditins: - // new pvc should not be changed if the feature is enabled, or if the old pvc had Conditions - if !reflect.DeepEqual(newPvc, newPvcInfo.pvc()) { - t.Errorf("new pvc changed: %v", diff.ObjectReflectDiff(newPvc, newPvcInfo.pvc())) - } - case newPvcHasConditions: - // new pvc should be changed - if reflect.DeepEqual(newPvc, newPvcInfo.pvc()) { - t.Errorf("new pvc was not changed") - } - // new pvc should not have Conditions - if !reflect.DeepEqual(newPvc, pvcWithoutConditions()) { - t.Errorf("new pvc had Conditions: %v", diff.ObjectReflectDiff(newPvc, pvcWithoutConditions())) - } - default: - // new pvc should not need to be changed - if !reflect.DeepEqual(newPvc, newPvcInfo.pvc()) { - t.Errorf("new pvc changed: %v", diff.ObjectReflectDiff(newPvc, newPvcInfo.pvc())) - } + default: + // new pvc should not need to be changed + if !reflect.DeepEqual(newPvc, newPvcInfo.pvc()) { + t.Errorf("new pvc changed: %v", diff.ObjectReflectDiff(newPvc, newPvcInfo.pvc())) } - }) - } + } + }) } } + } func TestPrepareForCreate(t *testing.T) { diff --git a/pkg/registry/storage/storageclass/strategy.go b/pkg/registry/storage/storageclass/strategy.go index 7572cec4fa7..a73f9067925 100644 --- a/pkg/registry/storage/storageclass/strategy.go +++ b/pkg/registry/storage/storageclass/strategy.go @@ -24,7 +24,6 @@ import ( "k8s.io/apiserver/pkg/storage/names" "k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/apis/storage" - storageutil "k8s.io/kubernetes/pkg/apis/storage/util" "k8s.io/kubernetes/pkg/apis/storage/validation" ) @@ -44,9 +43,6 @@ func (storageClassStrategy) NamespaceScoped() bool { // ResetBeforeCreate clears the Status field which is not allowed to be set by end users on creation. func (storageClassStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) { - class := obj.(*storage.StorageClass) - - storageutil.DropDisabledFields(class, nil) } func (storageClassStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList { @@ -69,10 +65,6 @@ func (storageClassStrategy) AllowCreateOnUpdate() bool { // PrepareForUpdate sets the Status fields which is not allowed to be set by an end user updating a PV func (storageClassStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) { - newClass := obj.(*storage.StorageClass) - oldClass := old.(*storage.StorageClass) - - storageutil.DropDisabledFields(oldClass, newClass) } func (storageClassStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { diff --git a/pkg/volume/csi/expander.go b/pkg/volume/csi/expander.go index 58594a5bfdc..22692f8e688 100644 --- a/pkg/volume/csi/expander.go +++ b/pkg/volume/csi/expander.go @@ -23,9 +23,7 @@ import ( "google.golang.org/grpc/codes" "google.golang.org/grpc/status" api "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" "k8s.io/kubernetes/pkg/volume/util" volumetypes "k8s.io/kubernetes/pkg/volume/util/types" @@ -34,13 +32,6 @@ import ( var _ volume.NodeExpandableVolumePlugin = &csiPlugin{} func (c *csiPlugin) RequiresFSResize() bool { - // We could check plugin's node capability but we instead are going to rely on - // NodeExpand to do the right thing and return early if plugin does not have - // node expansion capability. - if !utilfeature.DefaultFeatureGate.Enabled(features.ExpandCSIVolumes) { - klog.V(4).Infof("Resizing is not enabled for CSI volume") - return false - } return true } diff --git a/pkg/volume/util/operationexecutor/operation_generator.go b/pkg/volume/util/operationexecutor/operation_generator.go index 51f74c3acdd..50fcac3cbf0 100644 --- a/pkg/volume/util/operationexecutor/operation_generator.go +++ b/pkg/volume/util/operationexecutor/operation_generator.go @@ -2026,10 +2026,6 @@ func (og *operationGenerator) nodeExpandVolume( volumeToMount VolumeToMount, actualStateOfWorld ActualStateOfWorldMounterUpdater, rsOpts volume.NodeResizeOptions) (bool, error) { - if !utilfeature.DefaultFeatureGate.Enabled(features.ExpandPersistentVolumes) { - klog.V(4).Infof("Resizing is not enabled for this volume %s", volumeToMount.VolumeName) - return true, nil - } if volumeToMount.VolumeSpec != nil && volumeToMount.VolumeSpec.InlineVolumeSpecForCSIMigration { diff --git a/plugin/pkg/admission/noderestriction/admission.go b/plugin/pkg/admission/noderestriction/admission.go index 0a0d48d131f..ee12bce0c26 100644 --- a/plugin/pkg/admission/noderestriction/admission.go +++ b/plugin/pkg/admission/noderestriction/admission.go @@ -71,8 +71,7 @@ type Plugin struct { podsGetter corev1lister.PodLister nodesGetter corev1lister.NodeLister - expandPersistentVolumesEnabled bool - expansionRecoveryEnabled bool + expansionRecoveryEnabled bool } var ( @@ -83,7 +82,6 @@ var ( // InspectFeatureGates allows setting bools without taking a dep on a global variable func (p *Plugin) InspectFeatureGates(featureGates featuregate.FeatureGate) { - p.expandPersistentVolumesEnabled = featureGates.Enabled(features.ExpandPersistentVolumes) p.expansionRecoveryEnabled = featureGates.Enabled(features.RecoverVolumeExpansionFailure) } @@ -336,10 +334,6 @@ func (p *Plugin) admitPodEviction(nodeName string, a admission.Attributes) error func (p *Plugin) admitPVCStatus(nodeName string, a admission.Attributes) error { switch a.GetOperation() { case admission.Update: - if !p.expandPersistentVolumesEnabled { - return admission.NewForbidden(a, fmt.Errorf("node %q is not allowed to update persistentvolumeclaim metadata", nodeName)) - } - oldPVC, ok := a.GetOldObject().(*api.PersistentVolumeClaim) if !ok { return admission.NewForbidden(a, fmt.Errorf("unexpected type %T", a.GetOldObject())) diff --git a/plugin/pkg/admission/noderestriction/admission_test.go b/plugin/pkg/admission/noderestriction/admission_test.go index 6a4ccf2d948..f5450ad5f87 100644 --- a/plugin/pkg/admission/noderestriction/admission_test.go +++ b/plugin/pkg/admission/noderestriction/admission_test.go @@ -18,14 +18,15 @@ package noderestriction import ( "context" - "k8s.io/apiserver/pkg/util/feature" - featuregatetesting "k8s.io/component-base/featuregate/testing" - "k8s.io/kubernetes/pkg/features" "reflect" "strings" "testing" "time" + "k8s.io/apiserver/pkg/util/feature" + featuregatetesting "k8s.io/component-base/featuregate/testing" + "k8s.io/kubernetes/pkg/features" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -1527,8 +1528,6 @@ func TestAdmitPVCStatus(t *testing.T) { attributes := admission.NewAttributesRecord( test.newObj, test.oldObj, schema.GroupVersionKind{}, metav1.NamespaceDefault, "foo", apiResource, test.subresource, operation, &metav1.CreateOptions{}, false, mynode) - - defer featuregatetesting.SetFeatureGateDuringTest(t, feature.DefaultFeatureGate, features.ExpandPersistentVolumes, test.expansionFeatureEnabled)() defer featuregatetesting.SetFeatureGateDuringTest(t, feature.DefaultFeatureGate, features.RecoverVolumeExpansionFailure, test.recoveryFeatureEnabled)() a := &admitTestCase{ name: test.name, diff --git a/plugin/pkg/admission/resourcequota/admission_test.go b/plugin/pkg/admission/resourcequota/admission_test.go index d084db8e9df..1313b955641 100644 --- a/plugin/pkg/admission/resourcequota/admission_test.go +++ b/plugin/pkg/admission/resourcequota/admission_test.go @@ -30,15 +30,12 @@ import ( "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/admission/plugin/resourcequota" resourcequotaapi "k8s.io/apiserver/pkg/admission/plugin/resourcequota/apis/resourcequota" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/fake" testcore "k8s.io/client-go/testing" "k8s.io/client-go/tools/cache" - featuregatetesting "k8s.io/component-base/featuregate/testing" api "k8s.io/kubernetes/pkg/apis/core" - "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/quota/v1/install" ) @@ -408,8 +405,6 @@ func TestAdmitHandlesNegativePVCUpdates(t *testing.T) { stopCh := make(chan struct{}) defer close(stopCh) - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ExpandPersistentVolumes, true)() - kubeClient := fake.NewSimpleClientset(resourceQuota) informerFactory := informers.NewSharedInformerFactory(kubeClient, 0) @@ -458,8 +453,6 @@ func TestAdmitHandlesPVCUpdates(t *testing.T) { }, } - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ExpandPersistentVolumes, true)() - // start up quota system stopCh := make(chan struct{}) defer close(stopCh) diff --git a/plugin/pkg/auth/authorizer/node/node_authorizer.go b/plugin/pkg/auth/authorizer/node/node_authorizer.go index 661ab96fbac..2203b549e3a 100644 --- a/plugin/pkg/auth/authorizer/node/node_authorizer.go +++ b/plugin/pkg/auth/authorizer/node/node_authorizer.go @@ -32,7 +32,6 @@ import ( api "k8s.io/kubernetes/pkg/apis/core" storageapi "k8s.io/kubernetes/pkg/apis/storage" "k8s.io/kubernetes/pkg/auth/nodeidentifier" - "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac" "k8s.io/kubernetes/third_party/forked/gonum/graph" "k8s.io/kubernetes/third_party/forked/gonum/graph/traverse" @@ -112,10 +111,8 @@ func (r *NodeAuthorizer) Authorize(ctx context.Context, attrs authorizer.Attribu case configMapResource: return r.authorizeReadNamespacedObject(nodeName, configMapVertexType, attrs) case pvcResource: - if r.features.Enabled(features.ExpandPersistentVolumes) { - if attrs.GetSubresource() == "status" { - return r.authorizeStatusUpdate(nodeName, pvcVertexType, attrs) - } + if attrs.GetSubresource() == "status" { + return r.authorizeStatusUpdate(nodeName, pvcVertexType, attrs) } return r.authorizeGet(nodeName, pvcVertexType, attrs) case pvResource: diff --git a/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/controller_policy.go b/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/controller_policy.go index 0429ed45c1c..6002e6cd6a0 100644 --- a/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/controller_policy.go +++ b/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/controller_policy.go @@ -177,21 +177,19 @@ func buildControllerRoles() ([]rbacv1.ClusterRole, []rbacv1.ClusterRoleBinding) }, }) - if utilfeature.DefaultFeatureGate.Enabled(features.ExpandPersistentVolumes) { - addControllerRole(&controllerRoles, &controllerRoleBindings, rbacv1.ClusterRole{ - ObjectMeta: metav1.ObjectMeta{Name: saRolePrefix + "expand-controller"}, - Rules: []rbacv1.PolicyRule{ - rbacv1helpers.NewRule("get", "list", "watch", "update", "patch").Groups(legacyGroup).Resources("persistentvolumes").RuleOrDie(), - rbacv1helpers.NewRule("update", "patch").Groups(legacyGroup).Resources("persistentvolumeclaims/status").RuleOrDie(), - rbacv1helpers.NewRule("get", "list", "watch").Groups(legacyGroup).Resources("persistentvolumeclaims").RuleOrDie(), - // glusterfs - rbacv1helpers.NewRule("get", "list", "watch").Groups(storageGroup).Resources("storageclasses").RuleOrDie(), - rbacv1helpers.NewRule("get").Groups(legacyGroup).Resources("services", "endpoints").RuleOrDie(), - rbacv1helpers.NewRule("get").Groups(legacyGroup).Resources("secrets").RuleOrDie(), - eventsRule(), - }, - }) - } + addControllerRole(&controllerRoles, &controllerRoleBindings, rbacv1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{Name: saRolePrefix + "expand-controller"}, + Rules: []rbacv1.PolicyRule{ + rbacv1helpers.NewRule("get", "list", "watch", "update", "patch").Groups(legacyGroup).Resources("persistentvolumes").RuleOrDie(), + rbacv1helpers.NewRule("update", "patch").Groups(legacyGroup).Resources("persistentvolumeclaims/status").RuleOrDie(), + rbacv1helpers.NewRule("get", "list", "watch").Groups(legacyGroup).Resources("persistentvolumeclaims").RuleOrDie(), + // glusterfs + rbacv1helpers.NewRule("get", "list", "watch").Groups(storageGroup).Resources("storageclasses").RuleOrDie(), + rbacv1helpers.NewRule("get").Groups(legacyGroup).Resources("services", "endpoints").RuleOrDie(), + rbacv1helpers.NewRule("get").Groups(legacyGroup).Resources("secrets").RuleOrDie(), + eventsRule(), + }, + }) addControllerRole(&controllerRoles, &controllerRoleBindings, rbacv1.ClusterRole{ ObjectMeta: metav1.ObjectMeta{Name: saRolePrefix + "ephemeral-volume-controller"}, diff --git a/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go b/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go index f12c9e7ec98..a5b92f037c2 100644 --- a/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go +++ b/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go @@ -24,9 +24,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apiserver/pkg/authentication/serviceaccount" "k8s.io/apiserver/pkg/authentication/user" - utilfeature "k8s.io/apiserver/pkg/util/feature" rbacv1helpers "k8s.io/kubernetes/pkg/apis/rbac/v1" - "k8s.io/kubernetes/pkg/features" ) // Write and other vars are slices of the allowed verbs. @@ -161,12 +159,10 @@ func NodeRules() []rbacv1.PolicyRule { rbacv1helpers.NewRule("create").Groups(legacyGroup).Resources("serviceaccounts/token").RuleOrDie(), } - if utilfeature.DefaultFeatureGate.Enabled(features.ExpandPersistentVolumes) { - // Use the Node authorization mode to limit a node to update status of pvc objects referenced by pods bound to itself. - // Use the NodeRestriction admission plugin to limit a node to just update the status stanza. - pvcStatusPolicyRule := rbacv1helpers.NewRule("get", "update", "patch").Groups(legacyGroup).Resources("persistentvolumeclaims/status").RuleOrDie() - nodePolicyRules = append(nodePolicyRules, pvcStatusPolicyRule) - } + // Use the Node authorization mode to limit a node to update status of pvc objects referenced by pods bound to itself. + // Use the NodeRestriction admission plugin to limit a node to just update the status stanza. + pvcStatusPolicyRule := rbacv1helpers.NewRule("get", "update", "patch").Groups(legacyGroup).Resources("persistentvolumeclaims/status").RuleOrDie() + nodePolicyRules = append(nodePolicyRules, pvcStatusPolicyRule) // CSI csiDriverRule := rbacv1helpers.NewRule("get", "watch", "list").Groups("storage.k8s.io").Resources("csidrivers").RuleOrDie() diff --git a/test/e2e/storage/flexvolume_online_resize.go b/test/e2e/storage/flexvolume_online_resize.go index 07e3952cc4a..4b2721e0b8e 100644 --- a/test/e2e/storage/flexvolume_online_resize.go +++ b/test/e2e/storage/flexvolume_online_resize.go @@ -39,7 +39,7 @@ import ( imageutils "k8s.io/kubernetes/test/utils/image" ) -var _ = utils.SIGDescribe("Mounted flexvolume volume expand [Slow] [Feature:ExpandInUsePersistentVolumes]", func() { +var _ = utils.SIGDescribe("[Feature:Flexvolumes] Mounted flexvolume volume expand [Slow]", func() { var ( c clientset.Interface ns string @@ -51,7 +51,7 @@ var _ = utils.SIGDescribe("Mounted flexvolume volume expand [Slow] [Feature:Expa nodeKeyValueLabel map[string]string nodeLabelValue string nodeKey string - nodeList *v1.NodeList + node *v1.Node ) f := framework.NewDefaultFramework("mounted-flexvolume-expand") @@ -63,8 +63,9 @@ var _ = utils.SIGDescribe("Mounted flexvolume volume expand [Slow] [Feature:Expa c = f.ClientSet ns = f.Namespace.Name framework.ExpectNoError(framework.WaitForAllNodesSchedulable(c, framework.TestContext.NodeSchedulableTimeout)) + var err error - node, err := e2enode.GetRandomReadySchedulableNode(f.ClientSet) + node, err = e2enode.GetRandomReadySchedulableNode(f.ClientSet) framework.ExpectNoError(err) nodeName = node.Name @@ -125,9 +126,8 @@ var _ = utils.SIGDescribe("Mounted flexvolume volume expand [Slow] [Feature:Expa driver := "dummy-attachable" - node := nodeList.Items[0] ginkgo.By(fmt.Sprintf("installing flexvolume %s on node %s as %s", path.Join(driverDir, driver), node.Name, driver)) - installFlex(c, &node, "k8s", driver, path.Join(driverDir, driver)) + installFlex(c, node, "k8s", driver, path.Join(driverDir, driver)) ginkgo.By(fmt.Sprintf("installing flexvolume %s on (master) node %s as %s", path.Join(driverDir, driver), node.Name, driver)) installFlex(c, nil, "k8s", driver, path.Join(driverDir, driver)) diff --git a/test/integration/auth/node_test.go b/test/integration/auth/node_test.go index 23c2d431bad..60e4b029483 100644 --- a/test/integration/auth/node_test.go +++ b/test/integration/auth/node_test.go @@ -567,13 +567,7 @@ func TestNodeAuthorizer(t *testing.T) { // re-create a pod as an admin to add object references expectAllowed(t, createNode2NormalPod(superuserClient)) - // ExpandPersistentVolumes feature disabled - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ExpandPersistentVolumes, false)() - expectForbidden(t, updatePVCCapacity(node1Client)) - expectForbidden(t, updatePVCCapacity(node2Client)) - // ExpandPersistentVolumes feature enabled - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ExpandPersistentVolumes, true)() expectForbidden(t, updatePVCCapacity(node1Client)) expectAllowed(t, updatePVCCapacity(node2Client)) expectForbidden(t, updatePVCPhase(node2Client))