Remove StorageObjectInUseProtection feature gate logic

This feature has graduated to GA in v1.11 and will always be
enabled. So no longe need to check if enabled.

Signed-off-by: Konstantin Misyutin <konstantin.misyutin@huawei.com>
This commit is contained in:
Konstantin Misyutin 2021-09-10 18:15:10 +08:00
parent ec8e6e8778
commit 808c8f42d5
11 changed files with 72 additions and 206 deletions

View File

@ -552,7 +552,6 @@ func startPVCProtectionController(ctx context.Context, controllerContext Control
controllerContext.InformerFactory.Core().V1().PersistentVolumeClaims(), controllerContext.InformerFactory.Core().V1().PersistentVolumeClaims(),
controllerContext.InformerFactory.Core().V1().Pods(), controllerContext.InformerFactory.Core().V1().Pods(),
controllerContext.ClientBuilder.ClientOrDie("pvc-protection-controller"), controllerContext.ClientBuilder.ClientOrDie("pvc-protection-controller"),
utilfeature.DefaultFeatureGate.Enabled(features.StorageObjectInUseProtection),
) )
if err != nil { if err != nil {
return nil, true, fmt.Errorf("failed to start the pvc protection controller: %v", err) 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( go pvprotection.NewPVProtectionController(
controllerContext.InformerFactory.Core().V1().PersistentVolumes(), controllerContext.InformerFactory.Core().V1().PersistentVolumes(),
controllerContext.ClientBuilder.ClientOrDie("pv-protection-controller"), controllerContext.ClientBuilder.ClientOrDie("pv-protection-controller"),
utilfeature.DefaultFeatureGate.Enabled(features.StorageObjectInUseProtection),
).Run(ctx, 1) ).Run(ctx, 1)
return nil, true, nil return nil, true, nil
} }

View File

@ -23,12 +23,9 @@ import (
v1 "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/kubernetes/scheme"
ref "k8s.io/client-go/tools/reference" ref "k8s.io/client-go/tools/reference"
featuregatetesting "k8s.io/component-base/featuregate/testing"
pvutil "k8s.io/kubernetes/pkg/controller/volume/persistentvolume/util" pvutil "k8s.io/kubernetes/pkg/controller/volume/persistentvolume/util"
"k8s.io/kubernetes/pkg/features"
"k8s.io/kubernetes/pkg/volume/util" "k8s.io/kubernetes/pkg/volume/util"
) )
@ -1226,29 +1223,24 @@ func TestStorageObjectInUseProtectionFiltering(t *testing.T) {
} }
satisfyingTestCases := map[string]struct { satisfyingTestCases := map[string]struct {
isExpectedMatch bool isExpectedMatch bool
vol *v1.PersistentVolume vol *v1.PersistentVolume
pvc *v1.PersistentVolumeClaim pvc *v1.PersistentVolumeClaim
enableStorageObjectInUseProtection bool
}{ }{
"pv deletionTimeStamp not set": { "pv deletionTimeStamp not set": {
isExpectedMatch: true, isExpectedMatch: true,
vol: pv, vol: pv,
pvc: pvc, pvc: pvc,
enableStorageObjectInUseProtection: true,
}, },
"pv deletionTimeStamp set": { "pv deletionTimeStamp set": {
isExpectedMatch: false, isExpectedMatch: false,
vol: pvToDelete, vol: pvToDelete,
pvc: pvc, pvc: pvc,
enableStorageObjectInUseProtection: true,
}, },
} }
for name, testCase := range satisfyingTestCases { for name, testCase := range satisfyingTestCases {
t.Run(name, func(t *testing.T) { t.Run(name, func(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StorageObjectInUseProtection, testCase.enableStorageObjectInUseProtection)()
err := checkVolumeSatisfyClaim(testCase.vol, testCase.pvc) err := checkVolumeSatisfyClaim(testCase.vol, testCase.pvc)
// expected to match but got an error // expected to match but got an error
if err != nil && testCase.isExpectedMatch { if err != nil && testCase.isExpectedMatch {
@ -1262,28 +1254,23 @@ func TestStorageObjectInUseProtectionFiltering(t *testing.T) {
} }
filteringTestCases := map[string]struct { filteringTestCases := map[string]struct {
isExpectedMatch bool isExpectedMatch bool
vol persistentVolumeOrderedIndex vol persistentVolumeOrderedIndex
pvc *v1.PersistentVolumeClaim pvc *v1.PersistentVolumeClaim
enableStorageObjectInUseProtection bool
}{ }{
"pv deletionTimeStamp not set": { "pv deletionTimeStamp not set": {
isExpectedMatch: true, isExpectedMatch: true,
vol: createTestVolOrderedIndex(pv), vol: createTestVolOrderedIndex(pv),
pvc: pvc, pvc: pvc,
enableStorageObjectInUseProtection: true,
}, },
"pv deletionTimeStamp set": { "pv deletionTimeStamp set": {
isExpectedMatch: false, isExpectedMatch: false,
vol: createTestVolOrderedIndex(pvToDelete), vol: createTestVolOrderedIndex(pvToDelete),
pvc: pvc, pvc: pvc,
enableStorageObjectInUseProtection: true,
}, },
} }
for name, testCase := range filteringTestCases { for name, testCase := range filteringTestCases {
t.Run(name, func(t *testing.T) { 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) pvmatch, err := testCase.vol.findBestMatchForClaim(testCase.pvc, false)
// expected to match but either got an error or no returned pvmatch // expected to match but either got an error or no returned pvmatch
if pvmatch == nil && testCase.isExpectedMatch { if pvmatch == nil && testCase.isExpectedMatch {

View File

@ -29,7 +29,6 @@ import (
"k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/sets"
utilfeature "k8s.io/apiserver/pkg/util/feature"
clientset "k8s.io/client-go/kubernetes" clientset "k8s.io/client-go/kubernetes"
"k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/kubernetes/scheme"
corelisters "k8s.io/client-go/listers/core/v1" 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/events"
"k8s.io/kubernetes/pkg/controller/volume/persistentvolume/metrics" "k8s.io/kubernetes/pkg/controller/volume/persistentvolume/metrics"
pvutil "k8s.io/kubernetes/pkg/controller/volume/persistentvolume/util" pvutil "k8s.io/kubernetes/pkg/controller/volume/persistentvolume/util"
"k8s.io/kubernetes/pkg/features"
proxyutil "k8s.io/kubernetes/pkg/proxy/util" proxyutil "k8s.io/kubernetes/pkg/proxy/util"
"k8s.io/kubernetes/pkg/util/goroutinemap" "k8s.io/kubernetes/pkg/util/goroutinemap"
"k8s.io/kubernetes/pkg/util/goroutinemap/exponentialbackoff" "k8s.io/kubernetes/pkg/util/goroutinemap/exponentialbackoff"
@ -275,10 +273,8 @@ func checkVolumeSatisfyClaim(volume *v1.PersistentVolume, claim *v1.PersistentVo
requestedSize := requestedQty.Value() requestedSize := requestedQty.Value()
// check if PV's DeletionTimeStamp is set, if so, return error. // check if PV's DeletionTimeStamp is set, if so, return error.
if utilfeature.DefaultFeatureGate.Enabled(features.StorageObjectInUseProtection) { if volume.ObjectMeta.DeletionTimestamp != nil {
if volume.ObjectMeta.DeletionTimestamp != nil { return fmt.Errorf("the volume is marked for deletion %q", volume.Name)
return fmt.Errorf("the volume is marked for deletion %q", volume.Name)
}
} }
volumeQty := volume.Spec.Capacity[v1.ResourceStorage] volumeQty := volume.Spec.Capacity[v1.ResourceStorage]

View File

@ -25,12 +25,10 @@ import (
"k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/labels"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/kubernetes/scheme"
storagelisters "k8s.io/client-go/listers/storage/v1" storagelisters "k8s.io/client-go/listers/storage/v1"
"k8s.io/client-go/tools/reference" "k8s.io/client-go/tools/reference"
storagehelpers "k8s.io/component-helpers/storage/volume" storagehelpers "k8s.io/component-helpers/storage/volume"
"k8s.io/kubernetes/pkg/features"
volumeutil "k8s.io/kubernetes/pkg/volume/util" 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. // check if PV's DeletionTimeStamp is set, if so, skip this volume.
if utilfeature.DefaultFeatureGate.Enabled(features.StorageObjectInUseProtection) { if volume.ObjectMeta.DeletionTimestamp != nil {
if volume.ObjectMeta.DeletionTimestamp != nil { continue
continue
}
} }
nodeAffinityValid := true nodeAffinityValid := true

View File

@ -53,17 +53,13 @@ type Controller struct {
podIndexer cache.Indexer podIndexer cache.Indexer
queue workqueue.RateLimitingInterface queue workqueue.RateLimitingInterface
// allows overriding of StorageObjectInUseProtection feature Enabled/Disabled for testing
storageObjectInUseProtectionEnabled bool
} }
// NewPVCProtectionController returns a new instance of PVCProtectionController. // 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{ 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 {
ratelimiter.RegisterMetricAndTrackRateLimiterUsage("persistentvolumeclaim_protection_controller", cl.CoreV1().RESTClient().GetRateLimiter()) 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 { 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 := 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(ctx, claimClone, metav1.UpdateOptions{}) _, err := c.client.CoreV1().PersistentVolumeClaims(claimClone.Namespace).Update(ctx, claimClone, metav1.UpdateOptions{})

View File

@ -185,31 +185,22 @@ 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: "StorageObjectInUseProtection Enabled, PVC without finalizer -> finalizer is added", name: "PVC without finalizer -> finalizer is added",
updatedPVC: pvc(), updatedPVC: pvc(),
expectedActions: []clienttesting.Action{ expectedActions: []clienttesting.Action{
clienttesting.NewUpdateAction(pvcGVR, defaultNS, withProtectionFinalizer(pvc())), clienttesting.NewUpdateAction(pvcGVR, defaultNS, withProtectionFinalizer(pvc())),
}, },
storageObjectInUseProtectionEnabled: true,
}, },
{ {
name: "StorageObjectInUseProtection Disabled, PVC without finalizer -> finalizer is not added", name: "PVC with finalizer -> no action",
updatedPVC: pvc(), updatedPVC: withProtectionFinalizer(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",
@ -229,25 +220,14 @@ func TestPVCProtectionController(t *testing.T) {
// This succeeds // This succeeds
clienttesting.NewUpdateAction(pvcGVR, defaultNS, withProtectionFinalizer(pvc())), 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())), updatedPVC: deleted(withProtectionFinalizer(pvc())),
expectedActions: []clienttesting.Action{ expectedActions: []clienttesting.Action{
clienttesting.NewListAction(podGVR, podGVK, defaultNS, metav1.ListOptions{}), clienttesting.NewListAction(podGVR, podGVK, defaultNS, metav1.ListOptions{}),
clienttesting.NewUpdateAction(pvcGVR, defaultNS, deleted(pvc())), 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", name: "finalizer removal fails -> controller retries",
@ -270,7 +250,6 @@ func TestPVCProtectionController(t *testing.T) {
// Succeeds // Succeeds
clienttesting.NewUpdateAction(pvcGVR, defaultNS, deleted(pvc())), clienttesting.NewUpdateAction(pvcGVR, defaultNS, deleted(pvc())),
}, },
storageObjectInUseProtectionEnabled: true,
}, },
{ {
name: "deleted PVC with finalizer + pod with the PVC exists -> finalizer is not removed", 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.NewListAction(podGVR, podGVK, defaultNS, metav1.ListOptions{}),
clienttesting.NewUpdateAction(pvcGVR, defaultNS, deleted(pvc())), 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", name: "deleted PVC with finalizer + pod with the PVC finished but is not deleted -> finalizer is not removed",
initialObjects: []runtime.Object{ initialObjects: []runtime.Object{
withStatus(v1.PodFailed, withPVC(defaultPVCName, pod())), withStatus(v1.PodFailed, withPVC(defaultPVCName, pod())),
}, },
updatedPVC: deleted(withProtectionFinalizer(pvc())), updatedPVC: deleted(withProtectionFinalizer(pvc())),
expectedActions: []clienttesting.Action{}, expectedActions: []clienttesting.Action{},
storageObjectInUseProtectionEnabled: true,
}, },
{ {
name: "deleted PVC with finalizer + pod with the PVC exists but is not in the Informer's cache yet -> finalizer is not removed", 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{ expectedActions: []clienttesting.Action{
clienttesting.NewListAction(podGVR, podGVK, defaultNS, metav1.ListOptions{}), clienttesting.NewListAction(podGVR, podGVK, defaultNS, metav1.ListOptions{}),
}, },
storageObjectInUseProtectionEnabled: true,
}, },
// //
// Pod events // Pod events
@ -321,18 +297,16 @@ 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 not removed", name: "updated finished Pod -> finalizer is not removed",
initialObjects: []runtime.Object{ initialObjects: []runtime.Object{
deleted(withProtectionFinalizer(pvc())), deleted(withProtectionFinalizer(pvc())),
}, },
updatedPod: withStatus(v1.PodSucceeded, withPVC(defaultPVCName, pod())), updatedPod: withStatus(v1.PodSucceeded, withPVC(defaultPVCName, pod())),
expectedActions: []clienttesting.Action{}, expectedActions: []clienttesting.Action{},
storageObjectInUseProtectionEnabled: true,
}, },
{ {
name: "updated unscheduled Pod -> finalizer is removed", name: "updated unscheduled Pod -> finalizer is removed",
@ -344,7 +318,6 @@ func TestPVCProtectionController(t *testing.T) {
clienttesting.NewListAction(podGVR, podGVK, defaultNS, metav1.ListOptions{}), clienttesting.NewListAction(podGVR, podGVK, defaultNS, metav1.ListOptions{}),
clienttesting.NewUpdateAction(pvcGVR, defaultNS, deleted(pvc())), clienttesting.NewUpdateAction(pvcGVR, defaultNS, deleted(pvc())),
}, },
storageObjectInUseProtectionEnabled: true,
}, },
{ {
name: "deleted running Pod -> finalizer is removed", name: "deleted running Pod -> finalizer is removed",
@ -356,7 +329,6 @@ func TestPVCProtectionController(t *testing.T) {
clienttesting.NewListAction(podGVR, podGVK, defaultNS, metav1.ListOptions{}), clienttesting.NewListAction(podGVR, podGVK, defaultNS, metav1.ListOptions{}),
clienttesting.NewUpdateAction(pvcGVR, defaultNS, deleted(pvc())), 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", 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.NewListAction(podGVR, podGVK, defaultNS, metav1.ListOptions{}),
clienttesting.NewUpdateAction(pvcGVR, defaultNS, deleted(pvc())), 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", 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{ initialObjects: []runtime.Object{
withProtectionFinalizer(pvc()), withProtectionFinalizer(pvc()),
}, },
deletedPod: withPVC(defaultPVCName, pod()), deletedPod: withPVC(defaultPVCName, pod()),
updatedPod: withUID("uid2", pod()), updatedPod: withUID("uid2", pod()),
expectedActions: []clienttesting.Action{}, expectedActions: []clienttesting.Action{},
storageObjectInUseProtectionEnabled: true,
}, },
{ {
name: "pod delete and create with same namespaced name seen as an update, both pods reference deleted PVC -> finalizer is not removed", 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{ initialObjects: []runtime.Object{
deleted(withProtectionFinalizer(pvc())), deleted(withProtectionFinalizer(pvc())),
}, },
deletedPod: withPVC(defaultPVCName, pod()), deletedPod: withPVC(defaultPVCName, pod()),
updatedPod: withUID("uid2", withPVC(defaultPVCName, pod())), updatedPod: withUID("uid2", withPVC(defaultPVCName, pod())),
expectedActions: []clienttesting.Action{}, expectedActions: []clienttesting.Action{},
storageObjectInUseProtectionEnabled: true,
}, },
{ {
name: "pod update from unscheduled to scheduled, deleted PVC is referenced -> finalizer is not removed", name: "pod update from unscheduled to scheduled, deleted PVC is referenced -> finalizer is not removed",
initialObjects: []runtime.Object{ initialObjects: []runtime.Object{
deleted(withProtectionFinalizer(pvc())), deleted(withProtectionFinalizer(pvc())),
}, },
deletedPod: unscheduled(withPVC(defaultPVCName, pod())), deletedPod: unscheduled(withPVC(defaultPVCName, pod())),
updatedPod: withPVC(defaultPVCName, pod()), updatedPod: withPVC(defaultPVCName, pod()),
expectedActions: []clienttesting.Action{}, expectedActions: []clienttesting.Action{},
storageObjectInUseProtectionEnabled: true,
}, },
} }
@ -431,7 +399,7 @@ func TestPVCProtectionController(t *testing.T) {
podInformer := informers.Core().V1().Pods() podInformer := informers.Core().V1().Pods()
// Create the controller // Create the controller
ctrl, err := NewPVCProtectionController(pvcInformer, podInformer, client, test.storageObjectInUseProtectionEnabled) ctrl, err := NewPVCProtectionController(pvcInformer, podInformer, client)
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }

View File

@ -21,7 +21,7 @@ import (
"fmt" "fmt"
"time" "time"
"k8s.io/api/core/v1" v1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors" apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
utilruntime "k8s.io/apimachinery/pkg/util/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime"
@ -47,17 +47,13 @@ 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, storageObjectInUseProtectionFeatureEnabled bool) *Controller { func NewPVProtectionController(pvInformer coreinformers.PersistentVolumeInformer, cl clientset.Interface) *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 {
ratelimiter.RegisterMetricAndTrackRateLimiterUsage("persistentvolume_protection_controller", cl.CoreV1().RESTClient().GetRateLimiter()) 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 { 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 := 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(ctx, pvClone, metav1.UpdateOptions{}) _, err := c.client.CoreV1().PersistentVolumes().Update(ctx, pvClone, metav1.UpdateOptions{})

View File

@ -25,7 +25,7 @@ import (
"github.com/davecgh/go-spew/spew" "github.com/davecgh/go-spew/spew"
"k8s.io/api/core/v1" v1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors" apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@ -112,30 +112,21 @@ 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: "StorageObjectInUseProtection Enabled, PV without finalizer -> finalizer is added", name: "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: "StorageObjectInUseProtection Disabled, PV without finalizer -> finalizer is added", name: "PVC with finalizer -> no action",
updatedPV: pv(), updatedPV: withProtectionFinalizer(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",
@ -155,23 +146,13 @@ 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", name: "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: 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", name: "finalizer removal fails -> controller retries",
@ -191,13 +172,11 @@ 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,
}, },
} }
@ -231,7 +210,7 @@ func TestPVProtectionController(t *testing.T) {
} }
// Create the controller // Create the controller
ctrl := NewPVProtectionController(pvInformer, client, test.storageObjectInUseProtectionEnabled) ctrl := NewPVProtectionController(pvInformer, client)
// Start the test by simulating an event // Start the test by simulating an event
if test.updatedPV != nil { if test.updatedPV != nil {

View File

@ -570,18 +570,16 @@ func (dswp *desiredStateOfWorldPopulator) getPVCExtractPV(
return nil, fmt.Errorf("failed to fetch PVC from API server: %v", err) 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.
// 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
// 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
// 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
// pod is scheduled. This was the default behavior in 1.8 and earlier // and users should not be that surprised.
// and users should not be that surprised. // It should happen only in very rare case when scheduler schedules
// 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.
// a pod and user deletes a PVC that's used by it at the same time. if pvc.ObjectMeta.DeletionTimestamp != nil {
if pvc.ObjectMeta.DeletionTimestamp != nil { return nil, errors.New("PVC is being deleted")
return nil, errors.New("PVC is being deleted")
}
} }
if pvc.Status.Phase != v1.ClaimBound { if pvc.Status.Phase != v1.ClaimBound {

View File

@ -21,11 +21,8 @@ import (
"io" "io"
"k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/admission"
"k8s.io/apiserver/pkg/admission/initializer"
"k8s.io/component-base/featuregate"
"k8s.io/klog/v2" "k8s.io/klog/v2"
api "k8s.io/kubernetes/pkg/apis/core" api "k8s.io/kubernetes/pkg/apis/core"
"k8s.io/kubernetes/pkg/features"
volumeutil "k8s.io/kubernetes/pkg/volume/util" 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. // storageProtectionPlugin holds state for and implements the admission plugin.
type storageProtectionPlugin struct { type storageProtectionPlugin struct {
*admission.Handler *admission.Handler
storageObjectInUseProtection bool
} }
var _ admission.Interface = &storageProtectionPlugin{} var _ admission.Interface = &storageProtectionPlugin{}
var _ initializer.WantsFeatures = &storageProtectionPlugin{}
// newPlugin creates a new admission plugin. // newPlugin creates a new admission plugin.
func newPlugin() *storageProtectionPlugin { func newPlugin() *storageProtectionPlugin {
@ -70,10 +64,6 @@ var (
// This prevents users from deleting a PVC that's used by a running pod. // 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 // 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 { func (c *storageProtectionPlugin) Admit(ctx context.Context, a admission.Attributes, o admission.ObjectInterfaces) error {
if !c.storageObjectInUseProtection {
return nil
}
switch a.GetResource().GroupResource() { switch a.GetResource().GroupResource() {
case pvResource: case pvResource:
return c.admitPV(a) return c.admitPV(a)
@ -129,11 +119,3 @@ func (c *storageProtectionPlugin) admitPVC(a admission.Attributes) error {
pvc.Finalizers = append(pvc.Finalizers, volumeutil.PVCProtectionFinalizer) pvc.Finalizers = append(pvc.Finalizers, volumeutil.PVCProtectionFinalizer)
return nil return nil
} }
func (c *storageProtectionPlugin) InspectFeatureGates(featureGates featuregate.FeatureGate) {
c.storageObjectInUseProtection = featureGates.Enabled(features.StorageObjectInUseProtection)
}
func (c *storageProtectionPlugin) ValidateInitialization() error {
return nil
}

View File

@ -61,7 +61,6 @@ func TestAdmit(t *testing.T) {
resource schema.GroupVersionResource resource schema.GroupVersionResource
object runtime.Object object runtime.Object
expectedObject runtime.Object expectedObject runtime.Object
featureEnabled bool
namespace string namespace string
}{ }{
{ {
@ -69,7 +68,6 @@ func TestAdmit(t *testing.T) {
api.SchemeGroupVersion.WithResource("persistentvolumeclaims"), api.SchemeGroupVersion.WithResource("persistentvolumeclaims"),
claim, claim,
claimWithFinalizer, claimWithFinalizer,
true,
claim.Namespace, claim.Namespace,
}, },
{ {
@ -77,23 +75,13 @@ func TestAdmit(t *testing.T) {
api.SchemeGroupVersion.WithResource("persistentvolumeclaims"), api.SchemeGroupVersion.WithResource("persistentvolumeclaims"),
claimWithFinalizer, claimWithFinalizer,
claimWithFinalizer, claimWithFinalizer,
true,
claimWithFinalizer.Namespace, claimWithFinalizer.Namespace,
}, },
{
"disabled feature -> no finalizer",
api.SchemeGroupVersion.WithResource("persistentvolumeclaims"),
claim,
claim,
false,
claim.Namespace,
},
{ {
"create -> add finalizer", "create -> add finalizer",
api.SchemeGroupVersion.WithResource("persistentvolumes"), api.SchemeGroupVersion.WithResource("persistentvolumes"),
pv, pv,
pvWithFinalizer, pvWithFinalizer,
true,
pv.Namespace, pv.Namespace,
}, },
{ {
@ -101,23 +89,13 @@ func TestAdmit(t *testing.T) {
api.SchemeGroupVersion.WithResource("persistentvolumes"), api.SchemeGroupVersion.WithResource("persistentvolumes"),
pvWithFinalizer, pvWithFinalizer,
pvWithFinalizer, pvWithFinalizer,
true,
pvWithFinalizer.Namespace, pvWithFinalizer.Namespace,
}, },
{
"disabled feature -> no finalizer",
api.SchemeGroupVersion.WithResource("persistentvolumes"),
pv,
pv,
false,
pv.Namespace,
},
} }
for _, test := range tests { for _, test := range tests {
t.Run(test.name, func(t *testing.T) { t.Run(test.name, func(t *testing.T) {
ctrl := newPlugin() ctrl := newPlugin()
ctrl.storageObjectInUseProtection = test.featureEnabled
obj := test.object.DeepCopyObject() obj := test.object.DeepCopyObject()
attrs := admission.NewAttributesRecord( attrs := admission.NewAttributesRecord(