diff --git a/plugin/pkg/admission/noderestriction/admission.go b/plugin/pkg/admission/noderestriction/admission.go index 1e01f32f69e..0a0d48d131f 100644 --- a/plugin/pkg/admission/noderestriction/admission.go +++ b/plugin/pkg/admission/noderestriction/admission.go @@ -72,6 +72,7 @@ type Plugin struct { nodesGetter corev1lister.NodeLister expandPersistentVolumesEnabled bool + expansionRecoveryEnabled bool } var ( @@ -83,6 +84,7 @@ var ( // InspectFeatureGates allows setting bools without taking a dep on a global variable func (p *Plugin) InspectFeatureGates(featureGates featuregate.FeatureGate) { p.expandPersistentVolumesEnabled = featureGates.Enabled(features.ExpandPersistentVolumes) + p.expansionRecoveryEnabled = featureGates.Enabled(features.RecoverVolumeExpansionFailure) } // SetExternalKubeInformerFactory registers an informer factory into Plugin @@ -363,6 +365,14 @@ func (p *Plugin) admitPVCStatus(nodeName string, a admission.Attributes) error { oldPVC.Status.Conditions = nil newPVC.Status.Conditions = nil + if p.expansionRecoveryEnabled { + oldPVC.Status.ResizeStatus = nil + newPVC.Status.ResizeStatus = nil + + oldPVC.Status.AllocatedResources = nil + newPVC.Status.AllocatedResources = nil + } + // TODO(apelisse): We don't have a good mechanism to // verify that only the things that should have changed // have changed. Ignore it for now. diff --git a/plugin/pkg/admission/noderestriction/admission_test.go b/plugin/pkg/admission/noderestriction/admission_test.go index 0ac8809d50a..6a4ccf2d948 100644 --- a/plugin/pkg/admission/noderestriction/admission_test.go +++ b/plugin/pkg/admission/noderestriction/admission_test.go @@ -18,13 +18,18 @@ package noderestriction import ( "context" + "k8s.io/apiserver/pkg/util/feature" + featuregatetesting "k8s.io/component-base/featuregate/testing" + "k8s.io/kubernetes/pkg/features" "reflect" "strings" "testing" "time" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" @@ -1424,6 +1429,157 @@ func Test_getModifiedLabels(t *testing.T) { } } +func TestAdmitPVCStatus(t *testing.T) { + expectedNodeIndex := cache.NewIndexer(cache.MetaNamespaceKeyFunc, nil) + expectedNodeIndex.Add(&corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: "mynode", UID: "mynode-uid"}}) + expectedNode := corev1lister.NewNodeLister(expectedNodeIndex) + noExistingPodsIndex := cache.NewIndexer(cache.MetaNamespaceKeyFunc, nil) + noExistingPods := corev1lister.NewPodLister(noExistingPodsIndex) + mynode := &user.DefaultInfo{Name: "system:node:mynode", Groups: []string{"system:nodes"}} + + tests := []struct { + name string + resource schema.GroupVersionResource + subresource string + newObj runtime.Object + oldObj runtime.Object + expansionFeatureEnabled bool + recoveryFeatureEnabled bool + expectError string + }{ + { + name: "should not allow full pvc update from nodes", + oldObj: makeTestPVC( + api.PersistentVolumeClaimResizing, + api.PersistentVolumeClaimNoExpansionInProgress, "10G", + ), + subresource: "", + newObj: makeTestPVC( + "", api.PersistentVolumeClaimNoExpansionInProgress, "10G", + ), + expectError: "is forbidden: may only update PVC status", + }, + { + name: "should allow capacity and condition updates, if expansion is enabled", + oldObj: makeTestPVC( + api.PersistentVolumeClaimResizing, + api.PersistentVolumeClaimNoExpansionInProgress, "10G", + ), + expansionFeatureEnabled: true, + subresource: "status", + newObj: makeTestPVC( + api.PersistentVolumeClaimFileSystemResizePending, + api.PersistentVolumeClaimNoExpansionInProgress, "10G", + ), + expectError: "", + }, + { + name: "should not allow updates to allocatedResources with just expansion enabled", + oldObj: makeTestPVC( + api.PersistentVolumeClaimResizing, + api.PersistentVolumeClaimNoExpansionInProgress, "10G", + ), + subresource: "status", + expansionFeatureEnabled: true, + newObj: makeTestPVC( + api.PersistentVolumeClaimFileSystemResizePending, + api.PersistentVolumeClaimNoExpansionInProgress, "15G", + ), + expectError: "is not allowed to update fields other than", + }, + { + name: "should allow updates to allocatedResources with expansion and recovery enabled", + oldObj: makeTestPVC( + api.PersistentVolumeClaimResizing, + api.PersistentVolumeClaimNoExpansionInProgress, "10G", + ), + subresource: "status", + expansionFeatureEnabled: true, + recoveryFeatureEnabled: true, + newObj: makeTestPVC( + api.PersistentVolumeClaimFileSystemResizePending, + api.PersistentVolumeClaimNoExpansionInProgress, "15G", + ), + expectError: "", + }, + { + name: "should allow updates to resizeStatus with expansion and recovery enabled", + oldObj: makeTestPVC( + api.PersistentVolumeClaimResizing, + api.PersistentVolumeClaimNoExpansionInProgress, "10G", + ), + subresource: "status", + expansionFeatureEnabled: true, + recoveryFeatureEnabled: true, + newObj: makeTestPVC( + api.PersistentVolumeClaimResizing, + api.PersistentVolumeClaimNodeExpansionFailed, "10G", + ), + expectError: "", + }, + } + + for i := range tests { + test := tests[i] + t.Run(test.name, func(t *testing.T) { + operation := admission.Update + apiResource := api.SchemeGroupVersion.WithResource("persistentvolumeclaims") + attributes := admission.NewAttributesRecord( + test.newObj, test.oldObj, schema.GroupVersionKind{}, + metav1.NamespaceDefault, "foo", apiResource, test.subresource, operation, &metav1.CreateOptions{}, false, mynode) + + defer featuregatetesting.SetFeatureGateDuringTest(t, feature.DefaultFeatureGate, features.ExpandPersistentVolumes, test.expansionFeatureEnabled)() + defer featuregatetesting.SetFeatureGateDuringTest(t, feature.DefaultFeatureGate, features.RecoverVolumeExpansionFailure, test.recoveryFeatureEnabled)() + a := &admitTestCase{ + name: test.name, + podsGetter: noExistingPods, + nodesGetter: expectedNode, + attributes: attributes, + features: feature.DefaultFeatureGate, + err: test.expectError, + } + a.run(t) + }) + + } +} + +func makeTestPVC( + condition api.PersistentVolumeClaimConditionType, + resizeStatus api.PersistentVolumeClaimResizeStatus, + allocatedResources string) *api.PersistentVolumeClaim { + pvc := &api.PersistentVolumeClaim{ + Spec: api.PersistentVolumeClaimSpec{ + VolumeName: "volume1", + Resources: api.ResourceRequirements{ + Requests: api.ResourceList{ + api.ResourceStorage: resource.MustParse("10G"), + }, + }, + }, + Status: api.PersistentVolumeClaimStatus{ + Capacity: api.ResourceList{ + api.ResourceStorage: resource.MustParse(allocatedResources), + }, + Phase: api.ClaimBound, + ResizeStatus: &resizeStatus, + AllocatedResources: api.ResourceList{ + api.ResourceStorage: resource.MustParse(allocatedResources), + }, + }, + } + if len(condition) > 0 { + pvc.Status.Conditions = []api.PersistentVolumeClaimCondition{ + { + Type: condition, + Status: api.ConditionTrue, + }, + } + } + + return pvc +} + func createPodAttributes(pod *api.Pod, user user.Info) admission.Attributes { podResource := api.Resource("pods").WithVersion("v1") podKind := api.Kind("Pod").WithVersion("v1")