diff --git a/cmd/kube-controller-manager/app/core.go b/cmd/kube-controller-manager/app/core.go index 010c2942554..acaea316867 100644 --- a/cmd/kube-controller-manager/app/core.go +++ b/cmd/kube-controller-manager/app/core.go @@ -552,7 +552,6 @@ func startPVCProtectionController(ctx context.Context, controllerContext Control controllerContext.InformerFactory.Core().V1().PersistentVolumeClaims(), controllerContext.InformerFactory.Core().V1().Pods(), controllerContext.ClientBuilder.ClientOrDie("pvc-protection-controller"), - utilfeature.DefaultFeatureGate.Enabled(features.StorageObjectInUseProtection), ) if err != nil { return nil, true, fmt.Errorf("failed to start the pvc protection controller: %v", err) @@ -565,7 +564,6 @@ func startPVProtectionController(ctx context.Context, controllerContext Controll go pvprotection.NewPVProtectionController( controllerContext.InformerFactory.Core().V1().PersistentVolumes(), controllerContext.ClientBuilder.ClientOrDie("pv-protection-controller"), - utilfeature.DefaultFeatureGate.Enabled(features.StorageObjectInUseProtection), ).Run(ctx, 1) return nil, true, nil } diff --git a/pkg/controller/volume/persistentvolume/index_test.go b/pkg/controller/volume/persistentvolume/index_test.go index 3098c6905a6..cc0af4cf61d 100644 --- a/pkg/controller/volume/persistentvolume/index_test.go +++ b/pkg/controller/volume/persistentvolume/index_test.go @@ -23,12 +23,9 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/kubernetes/scheme" ref "k8s.io/client-go/tools/reference" - featuregatetesting "k8s.io/component-base/featuregate/testing" pvutil "k8s.io/kubernetes/pkg/controller/volume/persistentvolume/util" - "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/volume/util" ) @@ -1226,29 +1223,24 @@ func TestStorageObjectInUseProtectionFiltering(t *testing.T) { } satisfyingTestCases := map[string]struct { - isExpectedMatch bool - vol *v1.PersistentVolume - pvc *v1.PersistentVolumeClaim - enableStorageObjectInUseProtection bool + isExpectedMatch bool + vol *v1.PersistentVolume + pvc *v1.PersistentVolumeClaim }{ "pv deletionTimeStamp not set": { - isExpectedMatch: true, - vol: pv, - pvc: pvc, - enableStorageObjectInUseProtection: true, + isExpectedMatch: true, + vol: pv, + pvc: pvc, }, "pv deletionTimeStamp set": { - isExpectedMatch: false, - vol: pvToDelete, - pvc: pvc, - enableStorageObjectInUseProtection: true, + isExpectedMatch: false, + vol: pvToDelete, + pvc: pvc, }, } for name, testCase := range satisfyingTestCases { t.Run(name, func(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StorageObjectInUseProtection, testCase.enableStorageObjectInUseProtection)() - err := checkVolumeSatisfyClaim(testCase.vol, testCase.pvc) // expected to match but got an error if err != nil && testCase.isExpectedMatch { @@ -1262,28 +1254,23 @@ func TestStorageObjectInUseProtectionFiltering(t *testing.T) { } filteringTestCases := map[string]struct { - isExpectedMatch bool - vol persistentVolumeOrderedIndex - pvc *v1.PersistentVolumeClaim - enableStorageObjectInUseProtection bool + isExpectedMatch bool + vol persistentVolumeOrderedIndex + pvc *v1.PersistentVolumeClaim }{ "pv deletionTimeStamp not set": { - isExpectedMatch: true, - vol: createTestVolOrderedIndex(pv), - pvc: pvc, - enableStorageObjectInUseProtection: true, + isExpectedMatch: true, + vol: createTestVolOrderedIndex(pv), + pvc: pvc, }, "pv deletionTimeStamp set": { - isExpectedMatch: false, - vol: createTestVolOrderedIndex(pvToDelete), - pvc: pvc, - enableStorageObjectInUseProtection: true, + isExpectedMatch: false, + vol: createTestVolOrderedIndex(pvToDelete), + pvc: pvc, }, } for name, testCase := range filteringTestCases { t.Run(name, func(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StorageObjectInUseProtection, testCase.enableStorageObjectInUseProtection)() - pvmatch, err := testCase.vol.findBestMatchForClaim(testCase.pvc, false) // expected to match but either got an error or no returned pvmatch if pvmatch == nil && testCase.isExpectedMatch { diff --git a/pkg/controller/volume/persistentvolume/pv_controller.go b/pkg/controller/volume/persistentvolume/pv_controller.go index 2bdb9c1d485..caaea704e43 100644 --- a/pkg/controller/volume/persistentvolume/pv_controller.go +++ b/pkg/controller/volume/persistentvolume/pv_controller.go @@ -29,7 +29,6 @@ import ( "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" - utilfeature "k8s.io/apiserver/pkg/util/feature" clientset "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/scheme" corelisters "k8s.io/client-go/listers/core/v1" @@ -45,7 +44,6 @@ import ( "k8s.io/kubernetes/pkg/controller/volume/events" "k8s.io/kubernetes/pkg/controller/volume/persistentvolume/metrics" pvutil "k8s.io/kubernetes/pkg/controller/volume/persistentvolume/util" - "k8s.io/kubernetes/pkg/features" proxyutil "k8s.io/kubernetes/pkg/proxy/util" "k8s.io/kubernetes/pkg/util/goroutinemap" "k8s.io/kubernetes/pkg/util/goroutinemap/exponentialbackoff" @@ -275,10 +273,8 @@ func checkVolumeSatisfyClaim(volume *v1.PersistentVolume, claim *v1.PersistentVo requestedSize := requestedQty.Value() // check if PV's DeletionTimeStamp is set, if so, return error. - if utilfeature.DefaultFeatureGate.Enabled(features.StorageObjectInUseProtection) { - if volume.ObjectMeta.DeletionTimestamp != nil { - return fmt.Errorf("the volume is marked for deletion %q", volume.Name) - } + if volume.ObjectMeta.DeletionTimestamp != nil { + return fmt.Errorf("the volume is marked for deletion %q", volume.Name) } volumeQty := volume.Spec.Capacity[v1.ResourceStorage] diff --git a/pkg/controller/volume/persistentvolume/util/util.go b/pkg/controller/volume/persistentvolume/util/util.go index bb314134974..df532d550c4 100644 --- a/pkg/controller/volume/persistentvolume/util/util.go +++ b/pkg/controller/volume/persistentvolume/util/util.go @@ -25,12 +25,10 @@ import ( "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/kubernetes/scheme" storagelisters "k8s.io/client-go/listers/storage/v1" "k8s.io/client-go/tools/reference" storagehelpers "k8s.io/component-helpers/storage/volume" - "k8s.io/kubernetes/pkg/features" volumeutil "k8s.io/kubernetes/pkg/volume/util" ) @@ -227,10 +225,8 @@ func FindMatchingVolume( } // check if PV's DeletionTimeStamp is set, if so, skip this volume. - if utilfeature.DefaultFeatureGate.Enabled(features.StorageObjectInUseProtection) { - if volume.ObjectMeta.DeletionTimestamp != nil { - continue - } + if volume.ObjectMeta.DeletionTimestamp != nil { + continue } nodeAffinityValid := true diff --git a/pkg/controller/volume/pvcprotection/pvc_protection_controller.go b/pkg/controller/volume/pvcprotection/pvc_protection_controller.go index aa6f4431165..1434f169364 100644 --- a/pkg/controller/volume/pvcprotection/pvc_protection_controller.go +++ b/pkg/controller/volume/pvcprotection/pvc_protection_controller.go @@ -53,17 +53,13 @@ type Controller struct { podIndexer cache.Indexer queue workqueue.RateLimitingInterface - - // allows overriding of StorageObjectInUseProtection feature Enabled/Disabled for testing - storageObjectInUseProtectionEnabled bool } // NewPVCProtectionController returns a new instance of PVCProtectionController. -func NewPVCProtectionController(pvcInformer coreinformers.PersistentVolumeClaimInformer, podInformer coreinformers.PodInformer, cl clientset.Interface, storageObjectInUseProtectionFeatureEnabled bool) (*Controller, error) { +func NewPVCProtectionController(pvcInformer coreinformers.PersistentVolumeClaimInformer, podInformer coreinformers.PodInformer, cl clientset.Interface) (*Controller, error) { e := &Controller{ - client: cl, - queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "pvcprotection"), - storageObjectInUseProtectionEnabled: storageObjectInUseProtectionFeatureEnabled, + client: cl, + queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "pvcprotection"), } if cl != nil && cl.CoreV1().RESTClient().GetRateLimiter() != nil { ratelimiter.RegisterMetricAndTrackRateLimiterUsage("persistentvolumeclaim_protection_controller", cl.CoreV1().RESTClient().GetRateLimiter()) @@ -189,10 +185,6 @@ func (c *Controller) processPVC(ctx context.Context, pvcNamespace, pvcName strin } func (c *Controller) addFinalizer(ctx context.Context, pvc *v1.PersistentVolumeClaim) error { - // Skip adding Finalizer in case the StorageObjectInUseProtection feature is not enabled - if !c.storageObjectInUseProtectionEnabled { - return nil - } claimClone := pvc.DeepCopy() claimClone.ObjectMeta.Finalizers = append(claimClone.ObjectMeta.Finalizers, volumeutil.PVCProtectionFinalizer) _, err := c.client.CoreV1().PersistentVolumeClaims(claimClone.Namespace).Update(ctx, claimClone, metav1.UpdateOptions{}) diff --git a/pkg/controller/volume/pvcprotection/pvc_protection_controller_test.go b/pkg/controller/volume/pvcprotection/pvc_protection_controller_test.go index 0246e384991..a0b25061f73 100644 --- a/pkg/controller/volume/pvcprotection/pvc_protection_controller_test.go +++ b/pkg/controller/volume/pvcprotection/pvc_protection_controller_test.go @@ -185,31 +185,22 @@ func TestPVCProtectionController(t *testing.T) { deletedPod *v1.Pod // List of expected kubeclient actions that should happen during the // test. - expectedActions []clienttesting.Action - storageObjectInUseProtectionEnabled bool + expectedActions []clienttesting.Action }{ // // PVC events // { - name: "StorageObjectInUseProtection Enabled, PVC without finalizer -> finalizer is added", + name: "PVC without finalizer -> finalizer is added", updatedPVC: pvc(), expectedActions: []clienttesting.Action{ clienttesting.NewUpdateAction(pvcGVR, defaultNS, withProtectionFinalizer(pvc())), }, - storageObjectInUseProtectionEnabled: true, }, { - name: "StorageObjectInUseProtection Disabled, PVC without finalizer -> finalizer is not added", - updatedPVC: pvc(), - expectedActions: []clienttesting.Action{}, - storageObjectInUseProtectionEnabled: false, - }, - { - name: "PVC with finalizer -> no action", - updatedPVC: withProtectionFinalizer(pvc()), - expectedActions: []clienttesting.Action{}, - storageObjectInUseProtectionEnabled: true, + name: "PVC with finalizer -> no action", + updatedPVC: withProtectionFinalizer(pvc()), + expectedActions: []clienttesting.Action{}, }, { name: "saving PVC finalizer fails -> controller retries", @@ -229,25 +220,14 @@ func TestPVCProtectionController(t *testing.T) { // This succeeds clienttesting.NewUpdateAction(pvcGVR, defaultNS, withProtectionFinalizer(pvc())), }, - storageObjectInUseProtectionEnabled: true, }, { - name: "StorageObjectInUseProtection Enabled, deleted PVC with finalizer -> finalizer is removed", + name: "deleted PVC with finalizer -> finalizer is removed", updatedPVC: deleted(withProtectionFinalizer(pvc())), expectedActions: []clienttesting.Action{ clienttesting.NewListAction(podGVR, podGVK, defaultNS, metav1.ListOptions{}), clienttesting.NewUpdateAction(pvcGVR, defaultNS, deleted(pvc())), }, - storageObjectInUseProtectionEnabled: true, - }, - { - name: "StorageObjectInUseProtection Disabled, deleted PVC with finalizer -> finalizer is removed", - updatedPVC: deleted(withProtectionFinalizer(pvc())), - expectedActions: []clienttesting.Action{ - clienttesting.NewListAction(podGVR, podGVK, defaultNS, metav1.ListOptions{}), - clienttesting.NewUpdateAction(pvcGVR, defaultNS, deleted(pvc())), - }, - storageObjectInUseProtectionEnabled: false, }, { name: "finalizer removal fails -> controller retries", @@ -270,7 +250,6 @@ func TestPVCProtectionController(t *testing.T) { // Succeeds clienttesting.NewUpdateAction(pvcGVR, defaultNS, deleted(pvc())), }, - storageObjectInUseProtectionEnabled: true, }, { name: "deleted PVC with finalizer + pod with the PVC exists -> finalizer is not removed", @@ -290,16 +269,14 @@ func TestPVCProtectionController(t *testing.T) { clienttesting.NewListAction(podGVR, podGVK, defaultNS, metav1.ListOptions{}), clienttesting.NewUpdateAction(pvcGVR, defaultNS, deleted(pvc())), }, - storageObjectInUseProtectionEnabled: true, }, { name: "deleted PVC with finalizer + pod with the PVC finished but is not deleted -> finalizer is not removed", initialObjects: []runtime.Object{ withStatus(v1.PodFailed, withPVC(defaultPVCName, pod())), }, - updatedPVC: deleted(withProtectionFinalizer(pvc())), - expectedActions: []clienttesting.Action{}, - storageObjectInUseProtectionEnabled: true, + updatedPVC: deleted(withProtectionFinalizer(pvc())), + expectedActions: []clienttesting.Action{}, }, { name: "deleted PVC with finalizer + pod with the PVC exists but is not in the Informer's cache yet -> finalizer is not removed", @@ -311,7 +288,6 @@ func TestPVCProtectionController(t *testing.T) { expectedActions: []clienttesting.Action{ clienttesting.NewListAction(podGVR, podGVK, defaultNS, metav1.ListOptions{}), }, - storageObjectInUseProtectionEnabled: true, }, // // Pod events @@ -321,18 +297,16 @@ func TestPVCProtectionController(t *testing.T) { initialObjects: []runtime.Object{ deleted(withProtectionFinalizer(pvc())), }, - updatedPod: withStatus(v1.PodRunning, withPVC(defaultPVCName, pod())), - expectedActions: []clienttesting.Action{}, - storageObjectInUseProtectionEnabled: true, + updatedPod: withStatus(v1.PodRunning, withPVC(defaultPVCName, pod())), + expectedActions: []clienttesting.Action{}, }, { name: "updated finished Pod -> finalizer is not removed", initialObjects: []runtime.Object{ deleted(withProtectionFinalizer(pvc())), }, - updatedPod: withStatus(v1.PodSucceeded, withPVC(defaultPVCName, pod())), - expectedActions: []clienttesting.Action{}, - storageObjectInUseProtectionEnabled: true, + updatedPod: withStatus(v1.PodSucceeded, withPVC(defaultPVCName, pod())), + expectedActions: []clienttesting.Action{}, }, { name: "updated unscheduled Pod -> finalizer is removed", @@ -344,7 +318,6 @@ func TestPVCProtectionController(t *testing.T) { clienttesting.NewListAction(podGVR, podGVK, defaultNS, metav1.ListOptions{}), clienttesting.NewUpdateAction(pvcGVR, defaultNS, deleted(pvc())), }, - storageObjectInUseProtectionEnabled: true, }, { name: "deleted running Pod -> finalizer is removed", @@ -356,7 +329,6 @@ func TestPVCProtectionController(t *testing.T) { clienttesting.NewListAction(podGVR, podGVK, defaultNS, metav1.ListOptions{}), clienttesting.NewUpdateAction(pvcGVR, defaultNS, deleted(pvc())), }, - storageObjectInUseProtectionEnabled: true, }, { name: "pod delete and create with same namespaced name seen as an update, old pod used deleted PVC -> finalizer is removed", @@ -369,37 +341,33 @@ func TestPVCProtectionController(t *testing.T) { clienttesting.NewListAction(podGVR, podGVK, defaultNS, metav1.ListOptions{}), clienttesting.NewUpdateAction(pvcGVR, defaultNS, deleted(pvc())), }, - storageObjectInUseProtectionEnabled: true, }, { name: "pod delete and create with same namespaced name seen as an update, old pod used non-deleted PVC -> finalizer is not removed", initialObjects: []runtime.Object{ withProtectionFinalizer(pvc()), }, - deletedPod: withPVC(defaultPVCName, pod()), - updatedPod: withUID("uid2", pod()), - expectedActions: []clienttesting.Action{}, - storageObjectInUseProtectionEnabled: true, + deletedPod: withPVC(defaultPVCName, pod()), + updatedPod: withUID("uid2", pod()), + expectedActions: []clienttesting.Action{}, }, { name: "pod delete and create with same namespaced name seen as an update, both pods reference deleted PVC -> finalizer is not removed", initialObjects: []runtime.Object{ deleted(withProtectionFinalizer(pvc())), }, - deletedPod: withPVC(defaultPVCName, pod()), - updatedPod: withUID("uid2", withPVC(defaultPVCName, pod())), - expectedActions: []clienttesting.Action{}, - storageObjectInUseProtectionEnabled: true, + deletedPod: withPVC(defaultPVCName, pod()), + updatedPod: withUID("uid2", withPVC(defaultPVCName, pod())), + expectedActions: []clienttesting.Action{}, }, { name: "pod update from unscheduled to scheduled, deleted PVC is referenced -> finalizer is not removed", initialObjects: []runtime.Object{ deleted(withProtectionFinalizer(pvc())), }, - deletedPod: unscheduled(withPVC(defaultPVCName, pod())), - updatedPod: withPVC(defaultPVCName, pod()), - expectedActions: []clienttesting.Action{}, - storageObjectInUseProtectionEnabled: true, + deletedPod: unscheduled(withPVC(defaultPVCName, pod())), + updatedPod: withPVC(defaultPVCName, pod()), + expectedActions: []clienttesting.Action{}, }, } @@ -431,7 +399,7 @@ func TestPVCProtectionController(t *testing.T) { podInformer := informers.Core().V1().Pods() // Create the controller - ctrl, err := NewPVCProtectionController(pvcInformer, podInformer, client, test.storageObjectInUseProtectionEnabled) + ctrl, err := NewPVCProtectionController(pvcInformer, podInformer, client) if err != nil { t.Fatalf("unexpected error: %v", err) } diff --git a/pkg/controller/volume/pvprotection/pv_protection_controller.go b/pkg/controller/volume/pvprotection/pv_protection_controller.go index 0bf3650f56c..86c73cd5c3d 100644 --- a/pkg/controller/volume/pvprotection/pv_protection_controller.go +++ b/pkg/controller/volume/pvprotection/pv_protection_controller.go @@ -21,7 +21,7 @@ import ( "fmt" "time" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" utilruntime "k8s.io/apimachinery/pkg/util/runtime" @@ -47,17 +47,13 @@ type Controller struct { pvListerSynced cache.InformerSynced queue workqueue.RateLimitingInterface - - // allows overriding of StorageObjectInUseProtection feature Enabled/Disabled for testing - storageObjectInUseProtectionEnabled bool } // NewPVProtectionController returns a new *Controller. -func NewPVProtectionController(pvInformer coreinformers.PersistentVolumeInformer, cl clientset.Interface, storageObjectInUseProtectionFeatureEnabled bool) *Controller { +func NewPVProtectionController(pvInformer coreinformers.PersistentVolumeInformer, cl clientset.Interface) *Controller { e := &Controller{ - client: cl, - queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "pvprotection"), - storageObjectInUseProtectionEnabled: storageObjectInUseProtectionFeatureEnabled, + client: cl, + queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "pvprotection"), } if cl != nil && cl.CoreV1().RESTClient().GetRateLimiter() != nil { ratelimiter.RegisterMetricAndTrackRateLimiterUsage("persistentvolume_protection_controller", cl.CoreV1().RESTClient().GetRateLimiter()) @@ -158,10 +154,6 @@ func (c *Controller) processPV(ctx context.Context, pvName string) error { } func (c *Controller) addFinalizer(ctx context.Context, pv *v1.PersistentVolume) error { - // Skip adding Finalizer in case the StorageObjectInUseProtection feature is not enabled - if !c.storageObjectInUseProtectionEnabled { - return nil - } pvClone := pv.DeepCopy() pvClone.ObjectMeta.Finalizers = append(pvClone.ObjectMeta.Finalizers, volumeutil.PVProtectionFinalizer) _, err := c.client.CoreV1().PersistentVolumes().Update(ctx, pvClone, metav1.UpdateOptions{}) diff --git a/pkg/controller/volume/pvprotection/pv_protection_controller_test.go b/pkg/controller/volume/pvprotection/pv_protection_controller_test.go index ea522ffee61..b0645781820 100644 --- a/pkg/controller/volume/pvprotection/pv_protection_controller_test.go +++ b/pkg/controller/volume/pvprotection/pv_protection_controller_test.go @@ -25,7 +25,7 @@ import ( "github.com/davecgh/go-spew/spew" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -112,30 +112,21 @@ func TestPVProtectionController(t *testing.T) { updatedPV *v1.PersistentVolume // List of expected kubeclient actions that should happen during the // test. - expectedActions []clienttesting.Action - storageObjectInUseProtectionEnabled bool + expectedActions []clienttesting.Action }{ // PV events // { - name: "StorageObjectInUseProtection Enabled, PV without finalizer -> finalizer is added", + name: "PV without finalizer -> finalizer is added", updatedPV: pv(), expectedActions: []clienttesting.Action{ clienttesting.NewUpdateAction(pvVer, "", withProtectionFinalizer(pv())), }, - storageObjectInUseProtectionEnabled: true, }, { - name: "StorageObjectInUseProtection Disabled, PV without finalizer -> finalizer is added", - updatedPV: pv(), - expectedActions: []clienttesting.Action{}, - storageObjectInUseProtectionEnabled: false, - }, - { - name: "PVC with finalizer -> no action", - updatedPV: withProtectionFinalizer(pv()), - expectedActions: []clienttesting.Action{}, - storageObjectInUseProtectionEnabled: true, + name: "PVC with finalizer -> no action", + updatedPV: withProtectionFinalizer(pv()), + expectedActions: []clienttesting.Action{}, }, { name: "saving PVC finalizer fails -> controller retries", @@ -155,23 +146,13 @@ func TestPVProtectionController(t *testing.T) { // This succeeds clienttesting.NewUpdateAction(pvVer, "", withProtectionFinalizer(pv())), }, - storageObjectInUseProtectionEnabled: true, }, { - name: "StorageObjectInUseProtection Enabled, deleted PV with finalizer -> finalizer is removed", + name: "deleted PV with finalizer -> finalizer is removed", updatedPV: deleted(withProtectionFinalizer(pv())), expectedActions: []clienttesting.Action{ clienttesting.NewUpdateAction(pvVer, "", deleted(pv())), }, - storageObjectInUseProtectionEnabled: true, - }, - { - name: "StorageObjectInUseProtection Disabled, deleted PV with finalizer -> finalizer is removed", - updatedPV: deleted(withProtectionFinalizer(pv())), - expectedActions: []clienttesting.Action{ - clienttesting.NewUpdateAction(pvVer, "", deleted(pv())), - }, - storageObjectInUseProtectionEnabled: false, }, { name: "finalizer removal fails -> controller retries", @@ -191,13 +172,11 @@ func TestPVProtectionController(t *testing.T) { // Succeeds clienttesting.NewUpdateAction(pvVer, "", deleted(pv())), }, - storageObjectInUseProtectionEnabled: true, }, { - name: "deleted PVC with finalizer + PV is bound -> finalizer is not removed", - updatedPV: deleted(withProtectionFinalizer(boundPV())), - expectedActions: []clienttesting.Action{}, - storageObjectInUseProtectionEnabled: true, + name: "deleted PVC with finalizer + PV is bound -> finalizer is not removed", + updatedPV: deleted(withProtectionFinalizer(boundPV())), + expectedActions: []clienttesting.Action{}, }, } @@ -231,7 +210,7 @@ func TestPVProtectionController(t *testing.T) { } // Create the controller - ctrl := NewPVProtectionController(pvInformer, client, test.storageObjectInUseProtectionEnabled) + ctrl := NewPVProtectionController(pvInformer, client) // Start the test by simulating an event if test.updatedPV != nil { 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 27ee7ebcd03..fff3c058605 100644 --- a/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go +++ b/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go @@ -570,18 +570,16 @@ func (dswp *desiredStateOfWorldPopulator) getPVCExtractPV( return nil, fmt.Errorf("failed to fetch PVC from API server: %v", err) } - if utilfeature.DefaultFeatureGate.Enabled(features.StorageObjectInUseProtection) { - // Pods that uses a PVC that is being deleted must not be started. - // - // In case an old kubelet is running without this check or some kubelets - // have this feature disabled, the worst that can happen is that such - // pod is scheduled. This was the default behavior in 1.8 and earlier - // and users should not be that surprised. - // It should happen only in very rare case when scheduler schedules - // a pod and user deletes a PVC that's used by it at the same time. - if pvc.ObjectMeta.DeletionTimestamp != nil { - return nil, errors.New("PVC is being deleted") - } + // Pods that uses a PVC that is being deleted must not be started. + // + // In case an old kubelet is running without this check or some kubelets + // have this feature disabled, the worst that can happen is that such + // pod is scheduled. This was the default behavior in 1.8 and earlier + // and users should not be that surprised. + // It should happen only in very rare case when scheduler schedules + // a pod and user deletes a PVC that's used by it at the same time. + if pvc.ObjectMeta.DeletionTimestamp != nil { + return nil, errors.New("PVC is being deleted") } if pvc.Status.Phase != v1.ClaimBound { diff --git a/plugin/pkg/admission/storage/storageobjectinuseprotection/admission.go b/plugin/pkg/admission/storage/storageobjectinuseprotection/admission.go index a3c6a1e5c4b..82ed1ff9cc4 100644 --- a/plugin/pkg/admission/storage/storageobjectinuseprotection/admission.go +++ b/plugin/pkg/admission/storage/storageobjectinuseprotection/admission.go @@ -21,11 +21,8 @@ import ( "io" "k8s.io/apiserver/pkg/admission" - "k8s.io/apiserver/pkg/admission/initializer" - "k8s.io/component-base/featuregate" "k8s.io/klog/v2" api "k8s.io/kubernetes/pkg/apis/core" - "k8s.io/kubernetes/pkg/features" volumeutil "k8s.io/kubernetes/pkg/volume/util" ) @@ -45,12 +42,9 @@ func Register(plugins *admission.Plugins) { // storageProtectionPlugin holds state for and implements the admission plugin. type storageProtectionPlugin struct { *admission.Handler - - storageObjectInUseProtection bool } var _ admission.Interface = &storageProtectionPlugin{} -var _ initializer.WantsFeatures = &storageProtectionPlugin{} // newPlugin creates a new admission plugin. func newPlugin() *storageProtectionPlugin { @@ -70,10 +64,6 @@ var ( // This prevents users from deleting a PVC that's used by a running pod. // This also prevents admin from deleting a PV that's bound by a PVC func (c *storageProtectionPlugin) Admit(ctx context.Context, a admission.Attributes, o admission.ObjectInterfaces) error { - if !c.storageObjectInUseProtection { - return nil - } - switch a.GetResource().GroupResource() { case pvResource: return c.admitPV(a) @@ -129,11 +119,3 @@ func (c *storageProtectionPlugin) admitPVC(a admission.Attributes) error { pvc.Finalizers = append(pvc.Finalizers, volumeutil.PVCProtectionFinalizer) return nil } - -func (c *storageProtectionPlugin) InspectFeatureGates(featureGates featuregate.FeatureGate) { - c.storageObjectInUseProtection = featureGates.Enabled(features.StorageObjectInUseProtection) -} - -func (c *storageProtectionPlugin) ValidateInitialization() error { - return nil -} diff --git a/plugin/pkg/admission/storage/storageobjectinuseprotection/admission_test.go b/plugin/pkg/admission/storage/storageobjectinuseprotection/admission_test.go index b48ea070d57..95f6db9a645 100644 --- a/plugin/pkg/admission/storage/storageobjectinuseprotection/admission_test.go +++ b/plugin/pkg/admission/storage/storageobjectinuseprotection/admission_test.go @@ -61,7 +61,6 @@ func TestAdmit(t *testing.T) { resource schema.GroupVersionResource object runtime.Object expectedObject runtime.Object - featureEnabled bool namespace string }{ { @@ -69,7 +68,6 @@ func TestAdmit(t *testing.T) { api.SchemeGroupVersion.WithResource("persistentvolumeclaims"), claim, claimWithFinalizer, - true, claim.Namespace, }, { @@ -77,23 +75,13 @@ func TestAdmit(t *testing.T) { api.SchemeGroupVersion.WithResource("persistentvolumeclaims"), claimWithFinalizer, claimWithFinalizer, - true, claimWithFinalizer.Namespace, }, - { - "disabled feature -> no finalizer", - api.SchemeGroupVersion.WithResource("persistentvolumeclaims"), - claim, - claim, - false, - claim.Namespace, - }, { "create -> add finalizer", api.SchemeGroupVersion.WithResource("persistentvolumes"), pv, pvWithFinalizer, - true, pv.Namespace, }, { @@ -101,23 +89,13 @@ func TestAdmit(t *testing.T) { api.SchemeGroupVersion.WithResource("persistentvolumes"), pvWithFinalizer, pvWithFinalizer, - true, pvWithFinalizer.Namespace, }, - { - "disabled feature -> no finalizer", - api.SchemeGroupVersion.WithResource("persistentvolumes"), - pv, - pv, - false, - pv.Namespace, - }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { ctrl := newPlugin() - ctrl.storageObjectInUseProtection = test.featureEnabled obj := test.object.DeepCopyObject() attrs := admission.NewAttributesRecord(