diff --git a/cmd/kube-controller-manager/app/core.go b/cmd/kube-controller-manager/app/core.go index 71b03ef86ecb5..61d1ce38e7297 100644 --- a/cmd/kube-controller-manager/app/core.go +++ b/cmd/kube-controller-manager/app/core.go @@ -395,24 +395,20 @@ func startGarbageCollectorController(ctx ControllerContext) (bool, error) { } func startPVCProtectionController(ctx ControllerContext) (bool, error) { - if utilfeature.DefaultFeatureGate.Enabled(features.StorageObjectInUseProtection) { - go pvcprotection.NewPVCProtectionController( - ctx.InformerFactory.Core().V1().PersistentVolumeClaims(), - ctx.InformerFactory.Core().V1().Pods(), - ctx.ClientBuilder.ClientOrDie("pvc-protection-controller"), - ).Run(1, ctx.Stop) - return true, nil - } - return false, nil + go pvcprotection.NewPVCProtectionController( + ctx.InformerFactory.Core().V1().PersistentVolumeClaims(), + ctx.InformerFactory.Core().V1().Pods(), + ctx.ClientBuilder.ClientOrDie("pvc-protection-controller"), + utilfeature.DefaultFeatureGate.Enabled(features.StorageObjectInUseProtection), + ).Run(1, ctx.Stop) + return true, nil } func startPVProtectionController(ctx ControllerContext) (bool, error) { - if utilfeature.DefaultFeatureGate.Enabled(features.StorageObjectInUseProtection) { - go pvprotection.NewPVProtectionController( - ctx.InformerFactory.Core().V1().PersistentVolumes(), - ctx.ClientBuilder.ClientOrDie("pv-protection-controller"), - ).Run(1, ctx.Stop) - return true, nil - } - return false, nil + go pvprotection.NewPVProtectionController( + ctx.InformerFactory.Core().V1().PersistentVolumes(), + ctx.ClientBuilder.ClientOrDie("pv-protection-controller"), + utilfeature.DefaultFeatureGate.Enabled(features.StorageObjectInUseProtection), + ).Run(1, ctx.Stop) + return true, nil } diff --git a/pkg/controller/volume/pvcprotection/pvc_protection_controller.go b/pkg/controller/volume/pvcprotection/pvc_protection_controller.go index 7c1ae1f562df2..90b178496492c 100644 --- a/pkg/controller/volume/pvcprotection/pvc_protection_controller.go +++ b/pkg/controller/volume/pvcprotection/pvc_protection_controller.go @@ -49,13 +49,17 @@ type Controller struct { podListerSynced cache.InformerSynced queue workqueue.RateLimitingInterface + + // allows overriding of StorageObjectInUseProtection feature Enabled/Disabled for testing + storageObjectInUseProtectionEnabled bool } // NewPVCProtectionController returns a new *{VCProtectionController. -func NewPVCProtectionController(pvcInformer coreinformers.PersistentVolumeClaimInformer, podInformer coreinformers.PodInformer, cl clientset.Interface) *Controller { +func NewPVCProtectionController(pvcInformer coreinformers.PersistentVolumeClaimInformer, podInformer coreinformers.PodInformer, cl clientset.Interface, storageObjectInUseProtectionFeatureEnabled bool) *Controller { e := &Controller{ client: cl, queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "pvcprotection"), + storageObjectInUseProtectionEnabled: storageObjectInUseProtectionFeatureEnabled, } if cl != nil && cl.CoreV1().RESTClient().GetRateLimiter() != nil { metrics.RegisterMetricAndTrackRateLimiterUsage("persistentvolumeclaim_protection_controller", cl.CoreV1().RESTClient().GetRateLimiter()) @@ -176,6 +180,10 @@ func (c *Controller) processPVC(pvcNamespace, pvcName string) error { } func (c *Controller) addFinalizer(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(claimClone) diff --git a/pkg/controller/volume/pvcprotection/pvc_protection_controller_test.go b/pkg/controller/volume/pvcprotection/pvc_protection_controller_test.go index 0d7a2f9302c4c..531b6fbea7474 100644 --- a/pkg/controller/volume/pvcprotection/pvc_protection_controller_test.go +++ b/pkg/controller/volume/pvcprotection/pvc_protection_controller_test.go @@ -162,22 +162,31 @@ func TestPVCProtectionController(t *testing.T) { deletedPod *v1.Pod // List of expected kubeclient actions that should happen during the // test. - expectedActions []clienttesting.Action + expectedActions []clienttesting.Action + storageObjectInUseProtectionEnabled bool }{ // // PVC events // { - name: "PVC without finalizer -> finalizer is added", + name: "StorageObjectInUseProtection Enabled, PVC without finalizer -> finalizer is added", updatedPVC: pvc(), expectedActions: []clienttesting.Action{ clienttesting.NewUpdateAction(pvcVer, defaultNS, withProtectionFinalizer(pvc())), }, + storageObjectInUseProtectionEnabled: true, }, { - name: "PVC with finalizer -> no action", - updatedPVC: withProtectionFinalizer(pvc()), - expectedActions: []clienttesting.Action{}, + name: "StorageObjectInUseProtection Disabled, PVC without finalizer -> finalizer is added", + updatedPVC: pvc(), + expectedActions: []clienttesting.Action{}, + storageObjectInUseProtectionEnabled: false, + }, + { + name: "PVC with finalizer -> no action", + updatedPVC: withProtectionFinalizer(pvc()), + expectedActions: []clienttesting.Action{}, + storageObjectInUseProtectionEnabled: true, }, { name: "saving PVC finalizer fails -> controller retries", @@ -197,13 +206,23 @@ func TestPVCProtectionController(t *testing.T) { // This succeeds clienttesting.NewUpdateAction(pvcVer, defaultNS, withProtectionFinalizer(pvc())), }, + storageObjectInUseProtectionEnabled: true, + }, + { + name: "StorageObjectInUseProtection Enabled, deleted PVC with finalizer -> finalizer is removed", + updatedPVC: deleted(withProtectionFinalizer(pvc())), + expectedActions: []clienttesting.Action{ + clienttesting.NewUpdateAction(pvcVer, defaultNS, deleted(pvc())), + }, + storageObjectInUseProtectionEnabled: true, }, { - name: "deleted PVC with finalizer -> finalizer is removed", + name: "StorageObjectInUseProtection Disabled, deleted PVC with finalizer -> finalizer is removed", updatedPVC: deleted(withProtectionFinalizer(pvc())), expectedActions: []clienttesting.Action{ clienttesting.NewUpdateAction(pvcVer, defaultNS, deleted(pvc())), }, + storageObjectInUseProtectionEnabled: false, }, { name: "finalizer removal fails -> controller retries", @@ -223,6 +242,7 @@ func TestPVCProtectionController(t *testing.T) { // Succeeds clienttesting.NewUpdateAction(pvcVer, defaultNS, deleted(pvc())), }, + storageObjectInUseProtectionEnabled: true, }, { name: "deleted PVC with finalizer + pods with the PVC exists -> finalizer is not removed", @@ -241,9 +261,10 @@ func TestPVCProtectionController(t *testing.T) { expectedActions: []clienttesting.Action{ clienttesting.NewUpdateAction(pvcVer, defaultNS, deleted(pvc())), }, + storageObjectInUseProtectionEnabled: true, }, { - name: "deleted PVC with finalizer + pods with the PVC andis finished -> finalizer is removed", + name: "deleted PVC with finalizer + pods with the PVC and is finished -> finalizer is removed", initialObjects: []runtime.Object{ withStatus(v1.PodFailed, withPVC(defaultPVCName, pod())), }, @@ -251,6 +272,7 @@ func TestPVCProtectionController(t *testing.T) { expectedActions: []clienttesting.Action{ clienttesting.NewUpdateAction(pvcVer, defaultNS, deleted(pvc())), }, + storageObjectInUseProtectionEnabled: true, }, // // Pod events @@ -260,8 +282,9 @@ func TestPVCProtectionController(t *testing.T) { initialObjects: []runtime.Object{ deleted(withProtectionFinalizer(pvc())), }, - updatedPod: withStatus(v1.PodRunning, withPVC(defaultPVCName, pod())), - expectedActions: []clienttesting.Action{}, + updatedPod: withStatus(v1.PodRunning, withPVC(defaultPVCName, pod())), + expectedActions: []clienttesting.Action{}, + storageObjectInUseProtectionEnabled: true, }, { name: "updated finished Pod -> finalizer is removed", @@ -272,6 +295,7 @@ func TestPVCProtectionController(t *testing.T) { expectedActions: []clienttesting.Action{ clienttesting.NewUpdateAction(pvcVer, defaultNS, deleted(pvc())), }, + storageObjectInUseProtectionEnabled: true, }, { name: "updated unscheduled Pod -> finalizer is removed", @@ -282,6 +306,7 @@ func TestPVCProtectionController(t *testing.T) { expectedActions: []clienttesting.Action{ clienttesting.NewUpdateAction(pvcVer, defaultNS, deleted(pvc())), }, + storageObjectInUseProtectionEnabled: true, }, { name: "deleted running Pod -> finalizer is removed", @@ -292,6 +317,7 @@ func TestPVCProtectionController(t *testing.T) { expectedActions: []clienttesting.Action{ clienttesting.NewUpdateAction(pvcVer, defaultNS, deleted(pvc())), }, + storageObjectInUseProtectionEnabled: true, }, } @@ -330,7 +356,7 @@ func TestPVCProtectionController(t *testing.T) { } // Create the controller - ctrl := NewPVCProtectionController(pvcInformer, podInformer, client) + ctrl := NewPVCProtectionController(pvcInformer, podInformer, client, test.storageObjectInUseProtectionEnabled) // Start the test by simulating an event if test.updatedPVC != nil { diff --git a/pkg/controller/volume/pvprotection/pv_protection_controller.go b/pkg/controller/volume/pvprotection/pv_protection_controller.go index cd4b384141de9..93ce14b1bacf6 100644 --- a/pkg/controller/volume/pvprotection/pv_protection_controller.go +++ b/pkg/controller/volume/pvprotection/pv_protection_controller.go @@ -45,13 +45,17 @@ 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) *Controller { +func NewPVProtectionController(pvInformer coreinformers.PersistentVolumeInformer, cl clientset.Interface, storageObjectInUseProtectionFeatureEnabled bool) *Controller { e := &Controller{ client: cl, queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "pvprotection"), + storageObjectInUseProtectionEnabled: storageObjectInUseProtectionFeatureEnabled, } if cl != nil && cl.CoreV1().RESTClient().GetRateLimiter() != nil { metrics.RegisterMetricAndTrackRateLimiterUsage("persistentvolume_protection_controller", cl.CoreV1().RESTClient().GetRateLimiter()) @@ -151,6 +155,10 @@ func (c *Controller) processPV(pvName string) error { } func (c *Controller) addFinalizer(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(pvClone) diff --git a/pkg/controller/volume/pvprotection/pv_protection_controller_test.go b/pkg/controller/volume/pvprotection/pv_protection_controller_test.go index 8f0969a09e9ba..ba7b342a3d837 100644 --- a/pkg/controller/volume/pvprotection/pv_protection_controller_test.go +++ b/pkg/controller/volume/pvprotection/pv_protection_controller_test.go @@ -111,21 +111,30 @@ func TestPVProtectionController(t *testing.T) { updatedPV *v1.PersistentVolume // List of expected kubeclient actions that should happen during the // test. - expectedActions []clienttesting.Action + expectedActions []clienttesting.Action + storageObjectInUseProtectionEnabled bool }{ // PV events // { - name: "PV without finalizer -> finalizer is added", + name: "StorageObjectInUseProtection Enabled, PV without finalizer -> finalizer is added", updatedPV: pv(), expectedActions: []clienttesting.Action{ clienttesting.NewUpdateAction(pvVer, "", withProtectionFinalizer(pv())), }, + storageObjectInUseProtectionEnabled: true, }, { - name: "PVC with finalizer -> no action", - updatedPV: withProtectionFinalizer(pv()), - expectedActions: []clienttesting.Action{}, + 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: "saving PVC finalizer fails -> controller retries", @@ -145,13 +154,23 @@ func TestPVProtectionController(t *testing.T) { // This succeeds clienttesting.NewUpdateAction(pvVer, "", withProtectionFinalizer(pv())), }, + storageObjectInUseProtectionEnabled: true, + }, + { + name: "StorageObjectInUseProtection Enabled, deleted PV with finalizer -> finalizer is removed", + updatedPV: deleted(withProtectionFinalizer(pv())), + expectedActions: []clienttesting.Action{ + clienttesting.NewUpdateAction(pvVer, "", deleted(pv())), + }, + storageObjectInUseProtectionEnabled: true, }, { - name: "deleted PV with finalizer -> finalizer is removed", + 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", @@ -171,11 +190,13 @@ 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{}, + name: "deleted PVC with finalizer + PV is bound -> finalizer is not removed", + updatedPV: deleted(withProtectionFinalizer(boundPV())), + expectedActions: []clienttesting.Action{}, + storageObjectInUseProtectionEnabled: true, }, } @@ -209,7 +230,7 @@ func TestPVProtectionController(t *testing.T) { } // Create the controller - ctrl := NewPVProtectionController(pvInformer, client) + ctrl := NewPVProtectionController(pvInformer, client, test.storageObjectInUseProtectionEnabled) // Start the test by simulating an event if test.updatedPV != nil { diff --git a/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/controller_policy.go b/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/controller_policy.go index fcb146385a237..af7e16edcc102 100644 --- a/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/controller_policy.go +++ b/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/controller_policy.go @@ -324,25 +324,21 @@ func buildControllerRoles() ([]rbac.ClusterRole, []rbac.ClusterRoleBinding) { eventsRule(), }, }) - if utilfeature.DefaultFeatureGate.Enabled(features.StorageObjectInUseProtection) { - addControllerRole(&controllerRoles, &controllerRoleBindings, rbac.ClusterRole{ - ObjectMeta: metav1.ObjectMeta{Name: saRolePrefix + "pvc-protection-controller"}, - Rules: []rbac.PolicyRule{ - rbac.NewRule("get", "list", "watch", "update").Groups(legacyGroup).Resources("persistentvolumeclaims").RuleOrDie(), - rbac.NewRule("list", "watch", "get").Groups(legacyGroup).Resources("pods").RuleOrDie(), - eventsRule(), - }, - }) - } - if utilfeature.DefaultFeatureGate.Enabled(features.StorageObjectInUseProtection) { - addControllerRole(&controllerRoles, &controllerRoleBindings, rbac.ClusterRole{ - ObjectMeta: metav1.ObjectMeta{Name: saRolePrefix + "pv-protection-controller"}, - Rules: []rbac.PolicyRule{ - rbac.NewRule("get", "list", "watch", "update").Groups(legacyGroup).Resources("persistentvolumes").RuleOrDie(), - eventsRule(), - }, - }) - } + addControllerRole(&controllerRoles, &controllerRoleBindings, rbac.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{Name: saRolePrefix + "pvc-protection-controller"}, + Rules: []rbac.PolicyRule{ + rbac.NewRule("get", "list", "watch", "update").Groups(legacyGroup).Resources("persistentvolumeclaims").RuleOrDie(), + rbac.NewRule("list", "watch", "get").Groups(legacyGroup).Resources("pods").RuleOrDie(), + eventsRule(), + }, + }) + addControllerRole(&controllerRoles, &controllerRoleBindings, rbac.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{Name: saRolePrefix + "pv-protection-controller"}, + Rules: []rbac.PolicyRule{ + rbac.NewRule("get", "list", "watch", "update").Groups(legacyGroup).Resources("persistentvolumes").RuleOrDie(), + eventsRule(), + }, + }) return controllerRoles, controllerRoleBindings }