Skip to content

Commit

Permalink
CSIDriver: allow "StorageCapacity" to be modified
Browse files Browse the repository at this point in the history
When originally introduced, the field was made immutable to be
consistent with the other fields. But in practice allowing it to be
toggled makes more sense, in particular when considering the rollout
of a CSI driver (let it run without using the published
CSIStorageCapacity object, then flip the field, or upgrading from a
driver without support to one which supports it).

The only consumer of this field, the kube-scheduler, can handle
mutation without problems because it always consults the informer
cache to get the current value.
  • Loading branch information
pohly committed Aug 11, 2021
1 parent 7dd4f17 commit dfaeacb
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 8 deletions.
2 changes: 1 addition & 1 deletion pkg/apis/storage/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ type CSIDriverSpec struct {
// unset or false and it can be flipped later when storage
// capacity information has been published.
//
// This field is immutable.
// This field was immutable in Kubernetes <= 1.22 and now is mutable.
//
// This is a beta field and only available when the CSIStorageCapacity
// feature is enabled. The default is false.
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/storage/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -420,13 +420,13 @@ func ValidateCSIDriver(csiDriver *storage.CSIDriver) field.ErrorList {
// ValidateCSIDriverUpdate validates a CSIDriver.
func ValidateCSIDriverUpdate(new, old *storage.CSIDriver) field.ErrorList {
allErrs := apivalidation.ValidateObjectMetaUpdate(&new.ObjectMeta, &old.ObjectMeta, field.NewPath("metadata"))
allErrs = append(allErrs, validateCSIDriverSpec(&new.Spec, field.NewPath("spec"))...)

// immutable fields should not be mutated.
allErrs = append(allErrs, apimachineryvalidation.ValidateImmutableField(new.Spec.AttachRequired, old.Spec.AttachRequired, field.NewPath("spec", "attachedRequired"))...)
allErrs = append(allErrs, apimachineryvalidation.ValidateImmutableField(new.Spec.FSGroupPolicy, old.Spec.FSGroupPolicy, field.NewPath("spec", "fsGroupPolicy"))...)
allErrs = append(allErrs, apimachineryvalidation.ValidateImmutableField(new.Spec.PodInfoOnMount, old.Spec.PodInfoOnMount, field.NewPath("spec", "podInfoOnMount"))...)
allErrs = append(allErrs, apimachineryvalidation.ValidateImmutableField(new.Spec.VolumeLifecycleModes, old.Spec.VolumeLifecycleModes, field.NewPath("spec", "volumeLifecycleModes"))...)
allErrs = append(allErrs, apimachineryvalidation.ValidateImmutableField(new.Spec.StorageCapacity, old.Spec.StorageCapacity, field.NewPath("spec", "storageCapacity"))...)

allErrs = append(allErrs, validateTokenRequests(new.Spec.TokenRequests, field.NewPath("spec", "tokenRequests"))...)
return allErrs
Expand Down
14 changes: 10 additions & 4 deletions pkg/apis/storage/validation/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1965,6 +1965,12 @@ func TestCSIDriverValidationUpdate(t *testing.T) {
new.Spec.RequiresRepublish = &requiresRepublish
},
},
{
name: "StorageCapacity changed",
modify: func(new *storage.CSIDriver) {
new.Spec.StorageCapacity = &notStorageCapacity
},
},
}
for _, test := range successCases {
t.Run(test.name, func(t *testing.T) {
Expand Down Expand Up @@ -2062,15 +2068,15 @@ func TestCSIDriverValidationUpdate(t *testing.T) {
},
},
{
name: "StorageCapacity changed",
name: "TokenRequests invalidated",
modify: func(new *storage.CSIDriver) {
new.Spec.StorageCapacity = &notStorageCapacity
new.Spec.TokenRequests = []storage.TokenRequest{{Audience: gcp}, {Audience: gcp}}
},
},
{
name: "TokenRequests invalidated",
name: "invalid nil StorageCapacity",
modify: func(new *storage.CSIDriver) {
new.Spec.TokenRequests = []storage.TokenRequest{{Audience: gcp}, {Audience: gcp}}
new.Spec.StorageCapacity = nil
},
},
}
Expand Down
2 changes: 1 addition & 1 deletion staging/src/k8s.io/api/storage/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ type CSIDriverSpec struct {
// unset or false and it can be flipped later when storage
// capacity information has been published.
//
// This field is immutable.
// This field was immutable in Kubernetes <= 1.22 and now is mutable.
//
// This is a beta field and only available when the CSIStorageCapacity
// feature is enabled. The default is false.
Expand Down
2 changes: 1 addition & 1 deletion staging/src/k8s.io/api/storage/v1beta1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ type CSIDriverSpec struct {
// unset or false and it can be flipped later when storage
// capacity information has been published.
//
// This field is immutable.
// This field was immutable in Kubernetes <= 1.22 and now is mutable.
//
// This is a beta field and only available when the CSIStorageCapacity
// feature is enabled. The default is false.
Expand Down

0 comments on commit dfaeacb

Please sign in to comment.