diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 1e2cec0ffd0..96bba99f465 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -1930,10 +1930,6 @@ func ValidatePersistentVolumeClaimStatusUpdate(newPvc, oldPvc *core.PersistentVo if len(newPvc.Spec.AccessModes) == 0 { allErrs = append(allErrs, field.Required(field.NewPath("Spec", "accessModes"), "")) } - if !utilfeature.DefaultFeatureGate.Enabled(features.ExpandPersistentVolumes) && len(newPvc.Status.Conditions) > 0 { - conditionPath := field.NewPath("status", "conditions") - allErrs = append(allErrs, field.Forbidden(conditionPath, "invalid field")) - } capPath := field.NewPath("status", "capacity") for r, qty := range newPvc.Status.Capacity { allErrs = append(allErrs, validateBasicResource(qty, capPath.Key(string(r)))...) diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 8247d9f591e..443289f769d 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -11274,12 +11274,6 @@ func TestValidatePersistentVolumeClaimStatusUpdate(t *testing.T) { newClaim *core.PersistentVolumeClaim enableResize bool }{ - "condition-update-with-disabled-feature-gate": { - isExpectedFailure: true, - oldClaim: validClaim, - newClaim: validConditionUpdate, - enableResize: false, - }, "condition-update-with-enabled-feature-gate": { isExpectedFailure: false, oldClaim: validClaim, @@ -11290,8 +11284,6 @@ func TestValidatePersistentVolumeClaimStatusUpdate(t *testing.T) { for name, scenario := range scenarios { t.Run(name, func(t *testing.T) { // ensure we have a resource version specified for updates - defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ExpandPersistentVolumes, scenario.enableResize)() - scenario.oldClaim.ResourceVersion = "1" scenario.newClaim.ResourceVersion = "1" errs := ValidatePersistentVolumeClaimStatusUpdate(scenario.newClaim, scenario.oldClaim) diff --git a/pkg/registry/core/persistentvolumeclaim/BUILD b/pkg/registry/core/persistentvolumeclaim/BUILD index 1fafa124d18..7c5f5a76288 100644 --- a/pkg/registry/core/persistentvolumeclaim/BUILD +++ b/pkg/registry/core/persistentvolumeclaim/BUILD @@ -18,6 +18,7 @@ go_library( "//pkg/api/persistentvolumeclaim:go_default_library", "//pkg/apis/core:go_default_library", "//pkg/apis/core/validation:go_default_library", + "//pkg/features:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/fields:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", @@ -25,6 +26,7 @@ go_library( "//staging/src/k8s.io/apiserver/pkg/registry/generic:go_default_library", "//staging/src/k8s.io/apiserver/pkg/storage:go_default_library", "//staging/src/k8s.io/apiserver/pkg/storage/names:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", ], ) @@ -36,6 +38,11 @@ go_test( "//pkg/api/testapi:go_default_library", "//pkg/api/testing:go_default_library", "//pkg/apis/core:go_default_library", + "//pkg/features:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/endpoints/request:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/util/feature/testing:go_default_library", ], ) diff --git a/pkg/registry/core/persistentvolumeclaim/strategy.go b/pkg/registry/core/persistentvolumeclaim/strategy.go index 16ce8481104..8f32c162a83 100644 --- a/pkg/registry/core/persistentvolumeclaim/strategy.go +++ b/pkg/registry/core/persistentvolumeclaim/strategy.go @@ -27,10 +27,12 @@ import ( "k8s.io/apiserver/pkg/registry/generic" "k8s.io/apiserver/pkg/storage" "k8s.io/apiserver/pkg/storage/names" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/kubernetes/pkg/api/legacyscheme" pvcutil "k8s.io/kubernetes/pkg/api/persistentvolumeclaim" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/core/validation" + "k8s.io/kubernetes/pkg/features" ) // persistentvolumeclaimStrategy implements behavior for PersistentVolumeClaim objects @@ -97,6 +99,9 @@ func (persistentvolumeclaimStatusStrategy) PrepareForUpdate(ctx context.Context, newPv := obj.(*api.PersistentVolumeClaim) oldPv := old.(*api.PersistentVolumeClaim) newPv.Spec = oldPv.Spec + if !utilfeature.DefaultFeatureGate.Enabled(features.ExpandPersistentVolumes) && oldPv.Status.Conditions == nil { + newPv.Status.Conditions = nil + } } func (persistentvolumeclaimStatusStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { diff --git a/pkg/registry/core/persistentvolumeclaim/strategy_test.go b/pkg/registry/core/persistentvolumeclaim/strategy_test.go index b34eb72b9f3..40dee7eed87 100644 --- a/pkg/registry/core/persistentvolumeclaim/strategy_test.go +++ b/pkg/registry/core/persistentvolumeclaim/strategy_test.go @@ -17,10 +17,17 @@ limitations under the License. package persistentvolumeclaim import ( + "fmt" + "reflect" "testing" + "k8s.io/apimachinery/pkg/util/diff" + genericapirequest "k8s.io/apiserver/pkg/endpoints/request" + utilfeature "k8s.io/apiserver/pkg/util/feature" + utilfeaturetesting "k8s.io/apiserver/pkg/util/feature/testing" apitesting "k8s.io/kubernetes/pkg/api/testing" api "k8s.io/kubernetes/pkg/apis/core" + "k8s.io/kubernetes/pkg/features" // install all api groups for testing _ "k8s.io/kubernetes/pkg/api/testapi" @@ -34,3 +41,80 @@ func TestSelectableFieldLabelConversions(t *testing.T) { map[string]string{"name": "metadata.name"}, ) } + +func TestDropConditions(t *testing.T) { + ctx := genericapirequest.NewDefaultContext() + pvcWithConditions := func() *api.PersistentVolumeClaim { + return &api.PersistentVolumeClaim{ + Status: api.PersistentVolumeClaimStatus{ + Conditions: []api.PersistentVolumeClaimCondition{ + {Type: api.PersistentVolumeClaimResizing, Status: api.ConditionTrue}, + }, + }, + } + } + pvcWithoutConditions := func() *api.PersistentVolumeClaim { + return &api.PersistentVolumeClaim{ + Status: api.PersistentVolumeClaimStatus{}, + } + } + + pvcInfo := []struct { + description string + hasConditions bool + pvc func() *api.PersistentVolumeClaim + }{ + { + description: "has Conditions", + hasConditions: true, + pvc: pvcWithConditions, + }, + { + description: "does not have Conditions", + hasConditions: false, + pvc: pvcWithoutConditions, + }, + } + + for _, enabled := range []bool{true, false} { + for _, oldPvcInfo := range pvcInfo { + for _, newPvcInfo := range pvcInfo { + oldPvcHasConditins, oldPvc := oldPvcInfo.hasConditions, oldPvcInfo.pvc() + newPvcHasConditions, newPvc := newPvcInfo.hasConditions, newPvcInfo.pvc() + + t.Run(fmt.Sprintf("feature enabled=%v, old pvc %v, new pvc %v", enabled, oldPvcInfo.description, newPvcInfo.description), func(t *testing.T) { + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ExpandPersistentVolumes, enabled)() + + StatusStrategy.PrepareForUpdate(ctx, newPvc, oldPvc) + + // old pvc should never be changed + if !reflect.DeepEqual(oldPvc, oldPvcInfo.pvc()) { + t.Errorf("old pvc changed: %v", diff.ObjectReflectDiff(oldPvc, oldPvcInfo.pvc())) + } + + switch { + case enabled || oldPvcHasConditins: + // new pvc should not be changed if the feature is enabled, or if the old pvc had Conditions + if !reflect.DeepEqual(newPvc, newPvcInfo.pvc()) { + t.Errorf("new pvc changed: %v", diff.ObjectReflectDiff(newPvc, newPvcInfo.pvc())) + } + case newPvcHasConditions: + // new pvc should be changed + if reflect.DeepEqual(newPvc, newPvcInfo.pvc()) { + t.Errorf("new pvc was not changed") + } + // new pvc should not have Conditions + if !reflect.DeepEqual(newPvc, pvcWithoutConditions()) { + t.Errorf("new pvc had Conditions: %v", diff.ObjectReflectDiff(newPvc, pvcWithoutConditions())) + } + default: + // new pvc should not need to be changed + if !reflect.DeepEqual(newPvc, newPvcInfo.pvc()) { + t.Errorf("new pvc changed: %v", diff.ObjectReflectDiff(newPvc, newPvcInfo.pvc())) + } + } + }) + } + } + } +}