Fix bug with node restriction blocking pvc.status.resizestatus change

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.

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")