Skip to content

K
k3s
Unverified Commit bbcb3aa5 authored by Kubernetes Submit Queue's avatar Kubernetes Submit Queue Committed by GitHub
Browse files
Options

Merge pull request #62938 from…

Merge pull request #62938 from pospispa/60764-StorageObjectInUseProtection-downgrade-issue-cherry-pick-into-K8s-1.10 Automatic merge from submit-queue. cherry-pick into K8s 1.10: Always Start pvc-protection-controller and pv-protection-controller **What this PR does / why we need it**: 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. This problem was fixed in [K8s 1.9.6](https://github.com/kubernetes/kubernetes/releases/tag/v1.9.6) in PR https://github.com/kubernetes/kubernetes/pull/61370 This problem is also fixed in K8s 1.11+ in PR https://github.com/kubernetes/kubernetes/pull/61324 However, this problem is not fixed in K8s 1.10, that's why I've cherry-picked the PR https://github.com/kubernetes/kubernetes/pull/61324 and proposing to merge it into K8s 1.10. **Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*: Fixes N/A Related issue: https://github.com/kubernetes/kubernetes/issues/60764 **Special notes for your reviewer**: **Release note**: ```release-note In case StorageObjectInUse feature is disabled and Persistent Volume (PV) or Persistent Volume Claim (PVC) contains a finalizer and the PV or PVC is deleted it is not automatically removed from the system. Now, it is automatically removed. ```
parents 8eeecb07 9e8f00a4
Show whitespace changes
Inline Side-by-side
Showing with 74 additions and 19 deletions
cmd/kube-controller-manager/app/core.go
View file @ bbcb3aa5
......@@ -397,24 +397,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"),
utilfeature.DefaultFeatureGate.Enabled(features.StorageObjectInUseProtection),
).Run(1, ctx.Stop)
return true, nil
}
return false, 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"),
utilfeature.DefaultFeatureGate.Enabled(features.StorageObjectInUseProtection),
).Run(1, ctx.Stop)
return true, nil
}
return false, nil
}
pkg/controller/volume/pvcprotection/pvc_protection_controller.go
View file @ bbcb3aa5
......@@ -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)
......
pkg/controller/volume/pvcprotection/pvc_protection_controller_test.go
View file @ bbcb3aa5
......@@ -163,21 +163,30 @@ func TestPVCProtectionController(t *testing.T) {
// List of expected kubeclient actions that should happen during the
// test.
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: "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
......@@ -262,6 +284,7 @@ func TestPVCProtectionController(t *testing.T) {
},
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 {
......
pkg/controller/volume/pvprotection/pv_protection_controller.go
View file @ bbcb3aa5
......@@ -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)
......
pkg/controller/volume/pvprotection/pv_protection_controller_test.go
View file @ bbcb3aa5
......@@ -112,20 +112,29 @@ func TestPVProtectionController(t *testing.T) {
// List of expected kubeclient actions that should happen during the
// test.
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: "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{},
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 {
......
plugin/pkg/auth/authorizer/rbac/bootstrappolicy/controller_policy.go
View file @ bbcb3aa5
......@@ -324,7 +324,6 @@ 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{
......@@ -333,8 +332,6 @@ func buildControllerRoles() ([]rbac.ClusterRole, []rbac.ClusterRoleBinding) {
eventsRule(),
},
})
}
if utilfeature.DefaultFeatureGate.Enabled(features.StorageObjectInUseProtection) {
addControllerRole(&controllerRoles, &controllerRoleBindings, rbac.ClusterRole{
ObjectMeta: metav1.ObjectMeta{Name: saRolePrefix + "pv-protection-controller"},
Rules: []rbac.PolicyRule{
......@@ -342,7 +339,6 @@ func buildControllerRoles() ([]rbac.ClusterRole, []rbac.ClusterRoleBinding) {
eventsRule(),
},
})
}
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