From f7109ed281a6a8538cd98c7369593c2737af76d3 Mon Sep 17 00:00:00 2001 From: Drew Sirenko <68304519+AndrewSirenko@users.noreply.github.com> Date: Wed, 23 Jul 2025 10:59:18 -0400 Subject: [PATCH] [KEP-3751] Allow PVC VACName to update to nil or empty when status.currentVAC is nil --- api/openapi-spec/swagger.json | 2 +- api/openapi-spec/v3/api__v1_openapi.json | 2 +- .../v3/apis__apps__v1_openapi.json | 2 +- .../v3/apis__batch__v1_openapi.json | 2 +- pkg/apis/core/types.go | 4 +- pkg/apis/core/validation/validation.go | 9 ++-- pkg/apis/core/validation/validation_test.go | 42 +++++++++++++++---- pkg/generated/openapi/zz_generated.openapi.go | 2 +- .../src/k8s.io/api/core/v1/generated.proto | 4 +- staging/src/k8s.io/api/core/v1/types.go | 4 +- .../core/v1/types_swagger_doc_generated.go | 2 +- 11 files changed, 50 insertions(+), 25 deletions(-) diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json index b25cc6be50e..cd8f2f35c5c 100644 --- a/api/openapi-spec/swagger.json +++ b/api/openapi-spec/swagger.json @@ -8963,7 +8963,7 @@ "type": "string" }, "volumeAttributesClassName": { - "description": "volumeAttributesClassName may be used to set the VolumeAttributesClass used by this claim. If specified, the CSI driver will create or update the volume with the attributes defined in the corresponding VolumeAttributesClass. This has a different purpose than storageClassName, it can be changed after the claim is created. An empty string or nil value indicates that no VolumeAttributesClass will be applied to the claim. This field can be reset to an empty string or nil to perform a rollback if the claim enters an Infeasible error state. If the resource referred to by volumeAttributesClass does not exist, this PersistentVolumeClaim will be set to a Pending state, as reflected by the modifyVolumeStatus field, until such as a resource exists. More info: https://kubernetes.io/docs/concepts/storage/volume-attributes-classes/ (Beta) Using this field requires the VolumeAttributesClass feature gate to be enabled (off by default).", + "description": "volumeAttributesClassName may be used to set the VolumeAttributesClass used by this claim. If specified, the CSI driver will create or update the volume with the attributes defined in the corresponding VolumeAttributesClass. This has a different purpose than storageClassName, it can be changed after the claim is created. An empty string or nil value indicates that no VolumeAttributesClass will be applied to the claim. If the claim enters an Infeasible error state, this field can be reset to its previous value (including nil) to cancel the modification. If the resource referred to by volumeAttributesClass does not exist, this PersistentVolumeClaim will be set to a Pending state, as reflected by the modifyVolumeStatus field, until such as a resource exists. More info: https://kubernetes.io/docs/concepts/storage/volume-attributes-classes/ (Beta) Using this field requires the VolumeAttributesClass feature gate to be enabled (off by default).", "type": "string" }, "volumeMode": { diff --git a/api/openapi-spec/v3/api__v1_openapi.json b/api/openapi-spec/v3/api__v1_openapi.json index a62ed072e1f..4e6385f2c1d 100644 --- a/api/openapi-spec/v3/api__v1_openapi.json +++ b/api/openapi-spec/v3/api__v1_openapi.json @@ -4534,7 +4534,7 @@ "type": "string" }, "volumeAttributesClassName": { - "description": "volumeAttributesClassName may be used to set the VolumeAttributesClass used by this claim. If specified, the CSI driver will create or update the volume with the attributes defined in the corresponding VolumeAttributesClass. This has a different purpose than storageClassName, it can be changed after the claim is created. An empty string or nil value indicates that no VolumeAttributesClass will be applied to the claim. This field can be reset to an empty string or nil to perform a rollback if the claim enters an Infeasible error state. If the resource referred to by volumeAttributesClass does not exist, this PersistentVolumeClaim will be set to a Pending state, as reflected by the modifyVolumeStatus field, until such as a resource exists. More info: https://kubernetes.io/docs/concepts/storage/volume-attributes-classes/ (Beta) Using this field requires the VolumeAttributesClass feature gate to be enabled (off by default).", + "description": "volumeAttributesClassName may be used to set the VolumeAttributesClass used by this claim. If specified, the CSI driver will create or update the volume with the attributes defined in the corresponding VolumeAttributesClass. This has a different purpose than storageClassName, it can be changed after the claim is created. An empty string or nil value indicates that no VolumeAttributesClass will be applied to the claim. If the claim enters an Infeasible error state, this field can be reset to its previous value (including nil) to cancel the modification. If the resource referred to by volumeAttributesClass does not exist, this PersistentVolumeClaim will be set to a Pending state, as reflected by the modifyVolumeStatus field, until such as a resource exists. More info: https://kubernetes.io/docs/concepts/storage/volume-attributes-classes/ (Beta) Using this field requires the VolumeAttributesClass feature gate to be enabled (off by default).", "type": "string" }, "volumeMode": { diff --git a/api/openapi-spec/v3/apis__apps__v1_openapi.json b/api/openapi-spec/v3/apis__apps__v1_openapi.json index a9a3156c4dd..dc997bd569a 100644 --- a/api/openapi-spec/v3/apis__apps__v1_openapi.json +++ b/api/openapi-spec/v3/apis__apps__v1_openapi.json @@ -3247,7 +3247,7 @@ "type": "string" }, "volumeAttributesClassName": { - "description": "volumeAttributesClassName may be used to set the VolumeAttributesClass used by this claim. If specified, the CSI driver will create or update the volume with the attributes defined in the corresponding VolumeAttributesClass. This has a different purpose than storageClassName, it can be changed after the claim is created. An empty string or nil value indicates that no VolumeAttributesClass will be applied to the claim. This field can be reset to an empty string or nil to perform a rollback if the claim enters an Infeasible error state. If the resource referred to by volumeAttributesClass does not exist, this PersistentVolumeClaim will be set to a Pending state, as reflected by the modifyVolumeStatus field, until such as a resource exists. More info: https://kubernetes.io/docs/concepts/storage/volume-attributes-classes/ (Beta) Using this field requires the VolumeAttributesClass feature gate to be enabled (off by default).", + "description": "volumeAttributesClassName may be used to set the VolumeAttributesClass used by this claim. If specified, the CSI driver will create or update the volume with the attributes defined in the corresponding VolumeAttributesClass. This has a different purpose than storageClassName, it can be changed after the claim is created. An empty string or nil value indicates that no VolumeAttributesClass will be applied to the claim. If the claim enters an Infeasible error state, this field can be reset to its previous value (including nil) to cancel the modification. If the resource referred to by volumeAttributesClass does not exist, this PersistentVolumeClaim will be set to a Pending state, as reflected by the modifyVolumeStatus field, until such as a resource exists. More info: https://kubernetes.io/docs/concepts/storage/volume-attributes-classes/ (Beta) Using this field requires the VolumeAttributesClass feature gate to be enabled (off by default).", "type": "string" }, "volumeMode": { diff --git a/api/openapi-spec/v3/apis__batch__v1_openapi.json b/api/openapi-spec/v3/apis__batch__v1_openapi.json index 7d4537ddeaf..1eba8c08e2d 100644 --- a/api/openapi-spec/v3/apis__batch__v1_openapi.json +++ b/api/openapi-spec/v3/apis__batch__v1_openapi.json @@ -2511,7 +2511,7 @@ "type": "string" }, "volumeAttributesClassName": { - "description": "volumeAttributesClassName may be used to set the VolumeAttributesClass used by this claim. If specified, the CSI driver will create or update the volume with the attributes defined in the corresponding VolumeAttributesClass. This has a different purpose than storageClassName, it can be changed after the claim is created. An empty string or nil value indicates that no VolumeAttributesClass will be applied to the claim. This field can be reset to an empty string or nil to perform a rollback if the claim enters an Infeasible error state. If the resource referred to by volumeAttributesClass does not exist, this PersistentVolumeClaim will be set to a Pending state, as reflected by the modifyVolumeStatus field, until such as a resource exists. More info: https://kubernetes.io/docs/concepts/storage/volume-attributes-classes/ (Beta) Using this field requires the VolumeAttributesClass feature gate to be enabled (off by default).", + "description": "volumeAttributesClassName may be used to set the VolumeAttributesClass used by this claim. If specified, the CSI driver will create or update the volume with the attributes defined in the corresponding VolumeAttributesClass. This has a different purpose than storageClassName, it can be changed after the claim is created. An empty string or nil value indicates that no VolumeAttributesClass will be applied to the claim. If the claim enters an Infeasible error state, this field can be reset to its previous value (including nil) to cancel the modification. If the resource referred to by volumeAttributesClass does not exist, this PersistentVolumeClaim will be set to a Pending state, as reflected by the modifyVolumeStatus field, until such as a resource exists. More info: https://kubernetes.io/docs/concepts/storage/volume-attributes-classes/ (Beta) Using this field requires the VolumeAttributesClass feature gate to be enabled (off by default).", "type": "string" }, "volumeMode": { diff --git a/pkg/apis/core/types.go b/pkg/apis/core/types.go index d8eba4bfb04..0f575286fb2 100644 --- a/pkg/apis/core/types.go +++ b/pkg/apis/core/types.go @@ -563,8 +563,8 @@ type PersistentVolumeClaimSpec struct { // If specified, the CSI driver will create or update the volume with the attributes defined // in the corresponding VolumeAttributesClass. This has a different purpose than storageClassName, // it can be changed after the claim is created. An empty string or nil value indicates that no - // VolumeAttributesClass will be applied to the claim. This field can be reset to an empty string - // or nil to perform a rollback if the claim enters an Infeasible error state. + // VolumeAttributesClass will be applied to the claim. If the claim enters an Infeasible error state, + // this field can be reset to its previous value (including nil) to cancel the modification. // If the resource referred to by volumeAttributesClass does not exist, this PersistentVolumeClaim will be // set to a Pending state, as reflected by the modifyVolumeStatus field, until such as a resource // exists. diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 75f0d634737..5e1723125a2 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -2431,7 +2431,7 @@ func ValidatePersistentVolumeClaimUpdate(newPvc, oldPvc *core.PersistentVolumeCl newPvcClone.Spec.Resources.Requests["storage"] = oldPvc.Spec.Resources.Requests["storage"] // +k8s:verify-mutation:reason=clone } // lets make sure volume attributes class name is same. - if newPvc.Status.Phase == core.ClaimBound && newPvcClone.Spec.VolumeAttributesClassName != nil { + if newPvc.Status.Phase == core.ClaimBound { newPvcClone.Spec.VolumeAttributesClassName = oldPvcClone.Spec.VolumeAttributesClassName // +k8s:verify-mutation:reason=clone } @@ -2463,11 +2463,12 @@ func ValidatePersistentVolumeClaimUpdate(newPvc, oldPvc *core.PersistentVolumeCl allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "volumeAttributesClassName"), "update is forbidden when the VolumeAttributesClass feature gate is disabled")) } if opts.EnableVolumeAttributesClass { - if oldPvc.Spec.VolumeAttributesClassName != nil { + // Forbid removing VAC once one is successfully applied. + if oldPvc.Status.CurrentVolumeAttributesClassName != nil { if newPvc.Spec.VolumeAttributesClassName == nil { - allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "volumeAttributesClassName"), "update from non-nil value to nil is forbidden")) + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "volumeAttributesClassName"), "update to nil is forbidden when status.currentVolumeAttributesClassName is not nil")) } else if len(*newPvc.Spec.VolumeAttributesClassName) == 0 { - allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "volumeAttributesClassName"), "update from non-nil value to an empty string is forbidden")) + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "volumeAttributesClassName"), "update to empty string is forbidden when status.currentVolumeAttributesClassName is not nil")) } } } diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 61e2e7c4bab..091fb358c25 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -3032,22 +3032,46 @@ func TestValidatePersistentVolumeClaimUpdate(t *testing.T) { enableVolumeAttributesClass: true, isExpectedFailure: false, }, - "invalid-update-volume-attributes-class": { + "valid-update-volume-attributes-class-to-nil": { oldClaim: validClaimVolumeAttributesClass1, newClaim: validClaimNilVolumeAttributesClass, enableVolumeAttributesClass: true, - isExpectedFailure: true, + isExpectedFailure: false, }, - "invalid-update-volume-attributes-class-to-nil": { - oldClaim: validClaimVolumeAttributesClass1, - newClaim: validClaimNilVolumeAttributesClass, - enableVolumeAttributesClass: true, - isExpectedFailure: true, - }, - "invalid-update-volume-attributes-class-to-empty": { + "valid-update-volume-attributes-class-to-empty": { oldClaim: validClaimVolumeAttributesClass1, newClaim: validClaimEmptyVolumeAttributesClass, enableVolumeAttributesClass: true, + isExpectedFailure: false, + }, + "invalid-update-volume-attributes-class-to-nil-when-current-vac-set": { + oldClaim: func() *core.PersistentVolumeClaim { + clone := validClaimVolumeAttributesClass1.DeepCopy() + clone.Status.CurrentVolumeAttributesClassName = ptr.To("vac1") + return clone + }(), + newClaim: func() *core.PersistentVolumeClaim { + clone := validClaimNilVolumeAttributesClass.DeepCopy() + clone.Status.CurrentVolumeAttributesClassName = ptr.To("vac1") + clone.Spec.VolumeAttributesClassName = nil + return clone + }(), + enableVolumeAttributesClass: true, + isExpectedFailure: true, + }, + "invalid-update-volume-attributes-class-to-empty-when-current-vac-set": { + oldClaim: func() *core.PersistentVolumeClaim { + clone := validClaimVolumeAttributesClass1.DeepCopy() + clone.Status.CurrentVolumeAttributesClassName = ptr.To("vac1") + return clone + }(), + newClaim: func() *core.PersistentVolumeClaim { + clone := validClaimNilVolumeAttributesClass.DeepCopy() + clone.Status.CurrentVolumeAttributesClassName = ptr.To("vac1") + clone.Spec.VolumeAttributesClassName = ptr.To("") + return clone + }(), + enableVolumeAttributesClass: true, isExpectedFailure: true, }, "invalid-update-volume-attributes-class-when-claim-not-bound": { diff --git a/pkg/generated/openapi/zz_generated.openapi.go b/pkg/generated/openapi/zz_generated.openapi.go index a30cb9b1979..10db941356a 100644 --- a/pkg/generated/openapi/zz_generated.openapi.go +++ b/pkg/generated/openapi/zz_generated.openapi.go @@ -27119,7 +27119,7 @@ func schema_k8sio_api_core_v1_PersistentVolumeClaimSpec(ref common.ReferenceCall }, "volumeAttributesClassName": { SchemaProps: spec.SchemaProps{ - Description: "volumeAttributesClassName may be used to set the VolumeAttributesClass used by this claim. If specified, the CSI driver will create or update the volume with the attributes defined in the corresponding VolumeAttributesClass. This has a different purpose than storageClassName, it can be changed after the claim is created. An empty string or nil value indicates that no VolumeAttributesClass will be applied to the claim. This field can be reset to an empty string or nil to perform a rollback if the claim enters an Infeasible error state. If the resource referred to by volumeAttributesClass does not exist, this PersistentVolumeClaim will be set to a Pending state, as reflected by the modifyVolumeStatus field, until such as a resource exists. More info: https://kubernetes.io/docs/concepts/storage/volume-attributes-classes/ (Beta) Using this field requires the VolumeAttributesClass feature gate to be enabled (off by default).", + Description: "volumeAttributesClassName may be used to set the VolumeAttributesClass used by this claim. If specified, the CSI driver will create or update the volume with the attributes defined in the corresponding VolumeAttributesClass. This has a different purpose than storageClassName, it can be changed after the claim is created. An empty string or nil value indicates that no VolumeAttributesClass will be applied to the claim. If the claim enters an Infeasible error state, this field can be reset to its previous value (including nil) to cancel the modification. If the resource referred to by volumeAttributesClass does not exist, this PersistentVolumeClaim will be set to a Pending state, as reflected by the modifyVolumeStatus field, until such as a resource exists. More info: https://kubernetes.io/docs/concepts/storage/volume-attributes-classes/ (Beta) Using this field requires the VolumeAttributesClass feature gate to be enabled (off by default).", Type: []string{"string"}, Format: "", }, diff --git a/staging/src/k8s.io/api/core/v1/generated.proto b/staging/src/k8s.io/api/core/v1/generated.proto index 8bd4969a1eb..575bcf0c580 100644 --- a/staging/src/k8s.io/api/core/v1/generated.proto +++ b/staging/src/k8s.io/api/core/v1/generated.proto @@ -3162,8 +3162,8 @@ message PersistentVolumeClaimSpec { // If specified, the CSI driver will create or update the volume with the attributes defined // in the corresponding VolumeAttributesClass. This has a different purpose than storageClassName, // it can be changed after the claim is created. An empty string or nil value indicates that no - // VolumeAttributesClass will be applied to the claim. This field can be reset to an empty string - // or nil to perform a rollback if the claim enters an Infeasible error state. + // VolumeAttributesClass will be applied to the claim. If the claim enters an Infeasible error state, + // this field can be reset to its previous value (including nil) to cancel the modification. // If the resource referred to by volumeAttributesClass does not exist, this PersistentVolumeClaim will be // set to a Pending state, as reflected by the modifyVolumeStatus field, until such as a resource // exists. diff --git a/staging/src/k8s.io/api/core/v1/types.go b/staging/src/k8s.io/api/core/v1/types.go index d10bf129747..1958ac770e9 100644 --- a/staging/src/k8s.io/api/core/v1/types.go +++ b/staging/src/k8s.io/api/core/v1/types.go @@ -615,8 +615,8 @@ type PersistentVolumeClaimSpec struct { // If specified, the CSI driver will create or update the volume with the attributes defined // in the corresponding VolumeAttributesClass. This has a different purpose than storageClassName, // it can be changed after the claim is created. An empty string or nil value indicates that no - // VolumeAttributesClass will be applied to the claim. This field can be reset to an empty string - // or nil to perform a rollback if the claim enters an Infeasible error state. + // VolumeAttributesClass will be applied to the claim. If the claim enters an Infeasible error state, + // this field can be reset to its previous value (including nil) to cancel the modification. // If the resource referred to by volumeAttributesClass does not exist, this PersistentVolumeClaim will be // set to a Pending state, as reflected by the modifyVolumeStatus field, until such as a resource // exists. diff --git a/staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go b/staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go index 3cc25910e51..50e7e5105dc 100644 --- a/staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go +++ b/staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go @@ -1446,7 +1446,7 @@ var map_PersistentVolumeClaimSpec = map[string]string{ "volumeMode": "volumeMode defines what type of volume is required by the claim. Value of Filesystem is implied when not included in claim spec.", "dataSource": "dataSource field can be used to specify either: * An existing VolumeSnapshot object (snapshot.storage.k8s.io/VolumeSnapshot) * An existing PVC (PersistentVolumeClaim) If the provisioner or an external controller can support the specified data source, it will create a new volume based on the contents of the specified data source. When the AnyVolumeDataSource feature gate is enabled, dataSource contents will be copied to dataSourceRef, and dataSourceRef contents will be copied to dataSource when dataSourceRef.namespace is not specified. If the namespace is specified, then dataSourceRef will not be copied to dataSource.", "dataSourceRef": "dataSourceRef specifies the object from which to populate the volume with data, if a non-empty volume is desired. This may be any object from a non-empty API group (non core object) or a PersistentVolumeClaim object. When this field is specified, volume binding will only succeed if the type of the specified object matches some installed volume populator or dynamic provisioner. This field will replace the functionality of the dataSource field and as such if both fields are non-empty, they must have the same value. For backwards compatibility, when namespace isn't specified in dataSourceRef, both fields (dataSource and dataSourceRef) will be set to the same value automatically if one of them is empty and the other is non-empty. When namespace is specified in dataSourceRef, dataSource isn't set to the same value and must be empty. There are three important differences between dataSource and dataSourceRef: * While dataSource only allows two specific types of objects, dataSourceRef\n allows any non-core object, as well as PersistentVolumeClaim objects.\n* While dataSource ignores disallowed values (dropping them), dataSourceRef\n preserves all values, and generates an error if a disallowed value is\n specified.\n* While dataSource only allows local objects, dataSourceRef allows objects\n in any namespaces.\n(Beta) Using this field requires the AnyVolumeDataSource feature gate to be enabled. (Alpha) Using the namespace field of dataSourceRef requires the CrossNamespaceVolumeDataSource feature gate to be enabled.", - "volumeAttributesClassName": "volumeAttributesClassName may be used to set the VolumeAttributesClass used by this claim. If specified, the CSI driver will create or update the volume with the attributes defined in the corresponding VolumeAttributesClass. This has a different purpose than storageClassName, it can be changed after the claim is created. An empty string or nil value indicates that no VolumeAttributesClass will be applied to the claim. This field can be reset to an empty string or nil to perform a rollback if the claim enters an Infeasible error state. If the resource referred to by volumeAttributesClass does not exist, this PersistentVolumeClaim will be set to a Pending state, as reflected by the modifyVolumeStatus field, until such as a resource exists. More info: https://kubernetes.io/docs/concepts/storage/volume-attributes-classes/ (Beta) Using this field requires the VolumeAttributesClass feature gate to be enabled (off by default).", + "volumeAttributesClassName": "volumeAttributesClassName may be used to set the VolumeAttributesClass used by this claim. If specified, the CSI driver will create or update the volume with the attributes defined in the corresponding VolumeAttributesClass. This has a different purpose than storageClassName, it can be changed after the claim is created. An empty string or nil value indicates that no VolumeAttributesClass will be applied to the claim. If the claim enters an Infeasible error state, this field can be reset to its previous value (including nil) to cancel the modification. If the resource referred to by volumeAttributesClass does not exist, this PersistentVolumeClaim will be set to a Pending state, as reflected by the modifyVolumeStatus field, until such as a resource exists. More info: https://kubernetes.io/docs/concepts/storage/volume-attributes-classes/ (Beta) Using this field requires the VolumeAttributesClass feature gate to be enabled (off by default).", } func (PersistentVolumeClaimSpec) SwaggerDoc() map[string]string {