Commit 9e8f00a4 authored by Pavel Pospisil's avatar Pavel Pospisil

Always Start pvc-protection-controller and pv-protection-controller

StorageObjectInUseProtection feature is enabled by default in K8s 1.10+. Assume K8s cluster is used with this feature enabled, i.e. finalizers are added to all PVs and PVCs. In case the K8s cluster admin disables the StorageObjectInUseProtection feature and a user deletes a PVC that is not in active use by a pod then the PVC is not removed from the system because of the finalizer. Therefore, the user will have to remove the finalizer manually in order to have the PVC removed from the system. Note: deleted PVs won't be removed from the system also because of finalizers. That's why pvc-protection-controller is always started because the pvc-protection-controller removes finalizers from PVCs automatically when a PVC is not in active use by a pod. Also the pv-protection-controller is always started to remove finalizers from PVs automatically when a PV is not Bound to a PVC. Related issue: https://github.com/kubernetes/kubernetes/issues/60764 Related PRs: https://github.com/kubernetes/kubernetes/pull/61370 https://github.com/kubernetes/kubernetes/pull/61324
parent 7195f798
...@@ -395,24 +395,20 @@ func startGarbageCollectorController(ctx ControllerContext) (bool, error) { ...@@ -395,24 +395,20 @@ func startGarbageCollectorController(ctx ControllerContext) (bool, error) {
} }
func startPVCProtectionController(ctx ControllerContext) (bool, error) { func startPVCProtectionController(ctx ControllerContext) (bool, error) {
if utilfeature.DefaultFeatureGate.Enabled(features.StorageObjectInUseProtection) { go pvcprotection.NewPVCProtectionController(
go pvcprotection.NewPVCProtectionController( ctx.InformerFactory.Core().V1().PersistentVolumeClaims(),
ctx.InformerFactory.Core().V1().PersistentVolumeClaims(), ctx.InformerFactory.Core().V1().Pods(),
ctx.InformerFactory.Core().V1().Pods(), ctx.ClientBuilder.ClientOrDie("pvc-protection-controller"),
ctx.ClientBuilder.ClientOrDie("pvc-protection-controller"), utilfeature.DefaultFeatureGate.Enabled(features.StorageObjectInUseProtection),
).Run(1, ctx.Stop) ).Run(1, ctx.Stop)
return true, nil return true, nil
}
return false, nil
} }
func startPVProtectionController(ctx ControllerContext) (bool, error) { func startPVProtectionController(ctx ControllerContext) (bool, error) {
if utilfeature.DefaultFeatureGate.Enabled(features.StorageObjectInUseProtection) { go pvprotection.NewPVProtectionController(
go pvprotection.NewPVProtectionController( ctx.InformerFactory.Core().V1().PersistentVolumes(),
ctx.InformerFactory.Core().V1().PersistentVolumes(), ctx.ClientBuilder.ClientOrDie("pv-protection-controller"),
ctx.ClientBuilder.ClientOrDie("pv-protection-controller"), utilfeature.DefaultFeatureGate.Enabled(features.StorageObjectInUseProtection),
).Run(1, ctx.Stop) ).Run(1, ctx.Stop)
return true, nil return true, nil
}
return false, nil
} }
...@@ -49,13 +49,17 @@ type Controller struct { ...@@ -49,13 +49,17 @@ type Controller struct {
podListerSynced cache.InformerSynced podListerSynced cache.InformerSynced
queue workqueue.RateLimitingInterface queue workqueue.RateLimitingInterface
// allows overriding of StorageObjectInUseProtection feature Enabled/Disabled for testing
storageObjectInUseProtectionEnabled bool
} }
// NewPVCProtectionController returns a new *{VCProtectionController. // 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{ e := &Controller{
client: cl, client: cl,
queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "pvcprotection"), queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "pvcprotection"),
storageObjectInUseProtectionEnabled: storageObjectInUseProtectionFeatureEnabled,
} }
if cl != nil && cl.CoreV1().RESTClient().GetRateLimiter() != nil { if cl != nil && cl.CoreV1().RESTClient().GetRateLimiter() != nil {
metrics.RegisterMetricAndTrackRateLimiterUsage("persistentvolumeclaim_protection_controller", cl.CoreV1().RESTClient().GetRateLimiter()) metrics.RegisterMetricAndTrackRateLimiterUsage("persistentvolumeclaim_protection_controller", cl.CoreV1().RESTClient().GetRateLimiter())
...@@ -176,6 +180,10 @@ func (c *Controller) processPVC(pvcNamespace, pvcName string) error { ...@@ -176,6 +180,10 @@ func (c *Controller) processPVC(pvcNamespace, pvcName string) error {
} }
func (c *Controller) addFinalizer(pvc *v1.PersistentVolumeClaim) 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 := pvc.DeepCopy()
claimClone.ObjectMeta.Finalizers = append(claimClone.ObjectMeta.Finalizers, volumeutil.PVCProtectionFinalizer) claimClone.ObjectMeta.Finalizers = append(claimClone.ObjectMeta.Finalizers, volumeutil.PVCProtectionFinalizer)
_, err := c.client.CoreV1().PersistentVolumeClaims(claimClone.Namespace).Update(claimClone) _, err := c.client.CoreV1().PersistentVolumeClaims(claimClone.Namespace).Update(claimClone)
......
...@@ -162,22 +162,31 @@ func TestPVCProtectionController(t *testing.T) { ...@@ -162,22 +162,31 @@ func TestPVCProtectionController(t *testing.T) {
deletedPod *v1.Pod deletedPod *v1.Pod
// List of expected kubeclient actions that should happen during the // List of expected kubeclient actions that should happen during the
// test. // test.
expectedActions []clienttesting.Action expectedActions []clienttesting.Action
storageObjectInUseProtectionEnabled bool
}{ }{
// //
// PVC events // PVC events
// //
{ {
name: "PVC without finalizer -> finalizer is added", name: "StorageObjectInUseProtection Enabled, PVC without finalizer -> finalizer is added",
updatedPVC: pvc(), updatedPVC: pvc(),
expectedActions: []clienttesting.Action{ expectedActions: []clienttesting.Action{
clienttesting.NewUpdateAction(pvcVer, defaultNS, withProtectionFinalizer(pvc())), clienttesting.NewUpdateAction(pvcVer, defaultNS, withProtectionFinalizer(pvc())),
}, },
storageObjectInUseProtectionEnabled: true,
}, },
{ {
name: "PVC with finalizer -> no action", name: "StorageObjectInUseProtection Disabled, PVC without finalizer -> finalizer is added",
updatedPVC: withProtectionFinalizer(pvc()), updatedPVC: pvc(),
expectedActions: []clienttesting.Action{}, 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", name: "saving PVC finalizer fails -> controller retries",
...@@ -197,13 +206,23 @@ func TestPVCProtectionController(t *testing.T) { ...@@ -197,13 +206,23 @@ func TestPVCProtectionController(t *testing.T) {
// This succeeds // This succeeds
clienttesting.NewUpdateAction(pvcVer, defaultNS, withProtectionFinalizer(pvc())), 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())), updatedPVC: deleted(withProtectionFinalizer(pvc())),
expectedActions: []clienttesting.Action{ expectedActions: []clienttesting.Action{
clienttesting.NewUpdateAction(pvcVer, defaultNS, deleted(pvc())), clienttesting.NewUpdateAction(pvcVer, defaultNS, deleted(pvc())),
}, },
storageObjectInUseProtectionEnabled: false,
}, },
{ {
name: "finalizer removal fails -> controller retries", name: "finalizer removal fails -> controller retries",
...@@ -223,6 +242,7 @@ func TestPVCProtectionController(t *testing.T) { ...@@ -223,6 +242,7 @@ func TestPVCProtectionController(t *testing.T) {
// Succeeds // Succeeds
clienttesting.NewUpdateAction(pvcVer, defaultNS, deleted(pvc())), clienttesting.NewUpdateAction(pvcVer, defaultNS, deleted(pvc())),
}, },
storageObjectInUseProtectionEnabled: true,
}, },
{ {
name: "deleted PVC with finalizer + pods with the PVC exists -> finalizer is not removed", name: "deleted PVC with finalizer + pods with the PVC exists -> finalizer is not removed",
...@@ -241,9 +261,10 @@ func TestPVCProtectionController(t *testing.T) { ...@@ -241,9 +261,10 @@ func TestPVCProtectionController(t *testing.T) {
expectedActions: []clienttesting.Action{ expectedActions: []clienttesting.Action{
clienttesting.NewUpdateAction(pvcVer, defaultNS, deleted(pvc())), 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{ initialObjects: []runtime.Object{
withStatus(v1.PodFailed, withPVC(defaultPVCName, pod())), withStatus(v1.PodFailed, withPVC(defaultPVCName, pod())),
}, },
...@@ -251,6 +272,7 @@ func TestPVCProtectionController(t *testing.T) { ...@@ -251,6 +272,7 @@ func TestPVCProtectionController(t *testing.T) {
expectedActions: []clienttesting.Action{ expectedActions: []clienttesting.Action{
clienttesting.NewUpdateAction(pvcVer, defaultNS, deleted(pvc())), clienttesting.NewUpdateAction(pvcVer, defaultNS, deleted(pvc())),
}, },
storageObjectInUseProtectionEnabled: true,
}, },
// //
// Pod events // Pod events
...@@ -260,8 +282,9 @@ func TestPVCProtectionController(t *testing.T) { ...@@ -260,8 +282,9 @@ func TestPVCProtectionController(t *testing.T) {
initialObjects: []runtime.Object{ initialObjects: []runtime.Object{
deleted(withProtectionFinalizer(pvc())), deleted(withProtectionFinalizer(pvc())),
}, },
updatedPod: withStatus(v1.PodRunning, withPVC(defaultPVCName, pod())), updatedPod: withStatus(v1.PodRunning, withPVC(defaultPVCName, pod())),
expectedActions: []clienttesting.Action{}, expectedActions: []clienttesting.Action{},
storageObjectInUseProtectionEnabled: true,
}, },
{ {
name: "updated finished Pod -> finalizer is removed", name: "updated finished Pod -> finalizer is removed",
...@@ -272,6 +295,7 @@ func TestPVCProtectionController(t *testing.T) { ...@@ -272,6 +295,7 @@ func TestPVCProtectionController(t *testing.T) {
expectedActions: []clienttesting.Action{ expectedActions: []clienttesting.Action{
clienttesting.NewUpdateAction(pvcVer, defaultNS, deleted(pvc())), clienttesting.NewUpdateAction(pvcVer, defaultNS, deleted(pvc())),
}, },
storageObjectInUseProtectionEnabled: true,
}, },
{ {
name: "updated unscheduled Pod -> finalizer is removed", name: "updated unscheduled Pod -> finalizer is removed",
...@@ -282,6 +306,7 @@ func TestPVCProtectionController(t *testing.T) { ...@@ -282,6 +306,7 @@ func TestPVCProtectionController(t *testing.T) {
expectedActions: []clienttesting.Action{ expectedActions: []clienttesting.Action{
clienttesting.NewUpdateAction(pvcVer, defaultNS, deleted(pvc())), clienttesting.NewUpdateAction(pvcVer, defaultNS, deleted(pvc())),
}, },
storageObjectInUseProtectionEnabled: true,
}, },
{ {
name: "deleted running Pod -> finalizer is removed", name: "deleted running Pod -> finalizer is removed",
...@@ -292,6 +317,7 @@ func TestPVCProtectionController(t *testing.T) { ...@@ -292,6 +317,7 @@ func TestPVCProtectionController(t *testing.T) {
expectedActions: []clienttesting.Action{ expectedActions: []clienttesting.Action{
clienttesting.NewUpdateAction(pvcVer, defaultNS, deleted(pvc())), clienttesting.NewUpdateAction(pvcVer, defaultNS, deleted(pvc())),
}, },
storageObjectInUseProtectionEnabled: true,
}, },
} }
...@@ -330,7 +356,7 @@ func TestPVCProtectionController(t *testing.T) { ...@@ -330,7 +356,7 @@ func TestPVCProtectionController(t *testing.T) {
} }
// Create the controller // Create the controller
ctrl := NewPVCProtectionController(pvcInformer, podInformer, client) ctrl := NewPVCProtectionController(pvcInformer, podInformer, client, test.storageObjectInUseProtectionEnabled)
// Start the test by simulating an event // Start the test by simulating an event
if test.updatedPVC != nil { if test.updatedPVC != nil {
......
...@@ -45,13 +45,17 @@ type Controller struct { ...@@ -45,13 +45,17 @@ type Controller struct {
pvListerSynced cache.InformerSynced pvListerSynced cache.InformerSynced
queue workqueue.RateLimitingInterface queue workqueue.RateLimitingInterface
// allows overriding of StorageObjectInUseProtection feature Enabled/Disabled for testing
storageObjectInUseProtectionEnabled bool
} }
// NewPVProtectionController returns a new *Controller. // 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{ e := &Controller{
client: cl, client: cl,
queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "pvprotection"), queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "pvprotection"),
storageObjectInUseProtectionEnabled: storageObjectInUseProtectionFeatureEnabled,
} }
if cl != nil && cl.CoreV1().RESTClient().GetRateLimiter() != nil { if cl != nil && cl.CoreV1().RESTClient().GetRateLimiter() != nil {
metrics.RegisterMetricAndTrackRateLimiterUsage("persistentvolume_protection_controller", cl.CoreV1().RESTClient().GetRateLimiter()) metrics.RegisterMetricAndTrackRateLimiterUsage("persistentvolume_protection_controller", cl.CoreV1().RESTClient().GetRateLimiter())
...@@ -151,6 +155,10 @@ func (c *Controller) processPV(pvName string) error { ...@@ -151,6 +155,10 @@ func (c *Controller) processPV(pvName string) error {
} }
func (c *Controller) addFinalizer(pv *v1.PersistentVolume) 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 := pv.DeepCopy()
pvClone.ObjectMeta.Finalizers = append(pvClone.ObjectMeta.Finalizers, volumeutil.PVProtectionFinalizer) pvClone.ObjectMeta.Finalizers = append(pvClone.ObjectMeta.Finalizers, volumeutil.PVProtectionFinalizer)
_, err := c.client.CoreV1().PersistentVolumes().Update(pvClone) _, err := c.client.CoreV1().PersistentVolumes().Update(pvClone)
......
...@@ -111,21 +111,30 @@ func TestPVProtectionController(t *testing.T) { ...@@ -111,21 +111,30 @@ func TestPVProtectionController(t *testing.T) {
updatedPV *v1.PersistentVolume updatedPV *v1.PersistentVolume
// List of expected kubeclient actions that should happen during the // List of expected kubeclient actions that should happen during the
// test. // test.
expectedActions []clienttesting.Action expectedActions []clienttesting.Action
storageObjectInUseProtectionEnabled bool
}{ }{
// PV events // PV events
// //
{ {
name: "PV without finalizer -> finalizer is added", name: "StorageObjectInUseProtection Enabled, PV without finalizer -> finalizer is added",
updatedPV: pv(), updatedPV: pv(),
expectedActions: []clienttesting.Action{ expectedActions: []clienttesting.Action{
clienttesting.NewUpdateAction(pvVer, "", withProtectionFinalizer(pv())), clienttesting.NewUpdateAction(pvVer, "", withProtectionFinalizer(pv())),
}, },
storageObjectInUseProtectionEnabled: true,
}, },
{ {
name: "PVC with finalizer -> no action", name: "StorageObjectInUseProtection Disabled, PV without finalizer -> finalizer is added",
updatedPV: withProtectionFinalizer(pv()), updatedPV: pv(),
expectedActions: []clienttesting.Action{}, 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", name: "saving PVC finalizer fails -> controller retries",
...@@ -145,13 +154,23 @@ func TestPVProtectionController(t *testing.T) { ...@@ -145,13 +154,23 @@ func TestPVProtectionController(t *testing.T) {
// This succeeds // This succeeds
clienttesting.NewUpdateAction(pvVer, "", withProtectionFinalizer(pv())), 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())), updatedPV: deleted(withProtectionFinalizer(pv())),
expectedActions: []clienttesting.Action{ expectedActions: []clienttesting.Action{
clienttesting.NewUpdateAction(pvVer, "", deleted(pv())), clienttesting.NewUpdateAction(pvVer, "", deleted(pv())),
}, },
storageObjectInUseProtectionEnabled: false,
}, },
{ {
name: "finalizer removal fails -> controller retries", name: "finalizer removal fails -> controller retries",
...@@ -171,11 +190,13 @@ func TestPVProtectionController(t *testing.T) { ...@@ -171,11 +190,13 @@ func TestPVProtectionController(t *testing.T) {
// Succeeds // Succeeds
clienttesting.NewUpdateAction(pvVer, "", deleted(pv())), clienttesting.NewUpdateAction(pvVer, "", deleted(pv())),
}, },
storageObjectInUseProtectionEnabled: true,
}, },
{ {
name: "deleted PVC with finalizer + PV is bound -> finalizer is not removed", name: "deleted PVC with finalizer + PV is bound -> finalizer is not removed",
updatedPV: deleted(withProtectionFinalizer(boundPV())), updatedPV: deleted(withProtectionFinalizer(boundPV())),
expectedActions: []clienttesting.Action{}, expectedActions: []clienttesting.Action{},
storageObjectInUseProtectionEnabled: true,
}, },
} }
...@@ -209,7 +230,7 @@ func TestPVProtectionController(t *testing.T) { ...@@ -209,7 +230,7 @@ func TestPVProtectionController(t *testing.T) {
} }
// Create the controller // Create the controller
ctrl := NewPVProtectionController(pvInformer, client) ctrl := NewPVProtectionController(pvInformer, client, test.storageObjectInUseProtectionEnabled)
// Start the test by simulating an event // Start the test by simulating an event
if test.updatedPV != nil { if test.updatedPV != nil {
......
...@@ -324,25 +324,21 @@ func buildControllerRoles() ([]rbac.ClusterRole, []rbac.ClusterRoleBinding) { ...@@ -324,25 +324,21 @@ func buildControllerRoles() ([]rbac.ClusterRole, []rbac.ClusterRoleBinding) {
eventsRule(), eventsRule(),
}, },
}) })
if utilfeature.DefaultFeatureGate.Enabled(features.StorageObjectInUseProtection) { addControllerRole(&controllerRoles, &controllerRoleBindings, rbac.ClusterRole{
addControllerRole(&controllerRoles, &controllerRoleBindings, rbac.ClusterRole{ ObjectMeta: metav1.ObjectMeta{Name: saRolePrefix + "pvc-protection-controller"},
ObjectMeta: metav1.ObjectMeta{Name: saRolePrefix + "pvc-protection-controller"}, Rules: []rbac.PolicyRule{
Rules: []rbac.PolicyRule{ rbac.NewRule("get", "list", "watch", "update").Groups(legacyGroup).Resources("persistentvolumeclaims").RuleOrDie(),
rbac.NewRule("get", "list", "watch", "update").Groups(legacyGroup).Resources("persistentvolumeclaims").RuleOrDie(), rbac.NewRule("list", "watch", "get").Groups(legacyGroup).Resources("pods").RuleOrDie(),
rbac.NewRule("list", "watch", "get").Groups(legacyGroup).Resources("pods").RuleOrDie(), eventsRule(),
eventsRule(), },
}, })
}) addControllerRole(&controllerRoles, &controllerRoleBindings, rbac.ClusterRole{
} ObjectMeta: metav1.ObjectMeta{Name: saRolePrefix + "pv-protection-controller"},
if utilfeature.DefaultFeatureGate.Enabled(features.StorageObjectInUseProtection) { Rules: []rbac.PolicyRule{
addControllerRole(&controllerRoles, &controllerRoleBindings, rbac.ClusterRole{ rbac.NewRule("get", "list", "watch", "update").Groups(legacyGroup).Resources("persistentvolumes").RuleOrDie(),
ObjectMeta: metav1.ObjectMeta{Name: saRolePrefix + "pv-protection-controller"}, eventsRule(),
Rules: []rbac.PolicyRule{ },
rbac.NewRule("get", "list", "watch", "update").Groups(legacyGroup).Resources("persistentvolumes").RuleOrDie(), })
eventsRule(),
},
})
}
return controllerRoles, controllerRoleBindings return controllerRoles, controllerRoleBindings
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment