From dfaeacb51f9e68f7730d9e400c7f19ddb08c0087 Mon Sep 17 00:00:00 2001
From: Patrick Ohly <patrick.ohly@intel.com>
Date: Fri, 7 May 2021 15:13:09 +0200
Subject: [PATCH] CSIDriver: allow "StorageCapacity" to be modified

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.
---
 pkg/apis/storage/types.go                      |  2 +-
 pkg/apis/storage/validation/validation.go      |  2 +-
 pkg/apis/storage/validation/validation_test.go | 18 ++++++++++++------
 staging/src/k8s.io/api/storage/v1/types.go     |  2 +-
 .../src/k8s.io/api/storage/v1beta1/types.go    |  2 +-
 5 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/pkg/apis/storage/types.go b/pkg/apis/storage/types.go
index 045df66c85b..db88831902d 100644
--- a/pkg/apis/storage/types.go
+++ b/pkg/apis/storage/types.go
@@ -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.
diff --git a/pkg/apis/storage/validation/validation.go b/pkg/apis/storage/validation/validation.go
index 7c6ec5daac6..9cd48cd7176 100644
--- a/pkg/apis/storage/validation/validation.go
+++ b/pkg/apis/storage/validation/validation.go
@@ -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
diff --git a/pkg/apis/storage/validation/validation_test.go b/pkg/apis/storage/validation/validation_test.go
index ccc7fe76651..7b483d48cae 100644
--- a/pkg/apis/storage/validation/validation_test.go
+++ b/pkg/apis/storage/validation/validation_test.go
@@ -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) {
@@ -2061,18 +2067,18 @@ func TestCSIDriverValidationUpdate(t *testing.T) {
 				new.Spec.FSGroupPolicy = &fileFSGroupPolicy
 			},
 		},
-		{
-			name: "StorageCapacity changed",
-			modify: func(new *storage.CSIDriver) {
-				new.Spec.StorageCapacity = &notStorageCapacity
-			},
-		},
 		{
 			name: "TokenRequests invalidated",
 			modify: func(new *storage.CSIDriver) {
 				new.Spec.TokenRequests = []storage.TokenRequest{{Audience: gcp}, {Audience: gcp}}
 			},
 		},
+		{
+			name: "invalid nil StorageCapacity",
+			modify: func(new *storage.CSIDriver) {
+				new.Spec.StorageCapacity = nil
+			},
+		},
 	}
 
 	for _, test := range errorCases {
diff --git a/staging/src/k8s.io/api/storage/v1/types.go b/staging/src/k8s.io/api/storage/v1/types.go
index d805e1539b0..f69b40b9ffe 100644
--- a/staging/src/k8s.io/api/storage/v1/types.go
+++ b/staging/src/k8s.io/api/storage/v1/types.go
@@ -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.
diff --git a/staging/src/k8s.io/api/storage/v1beta1/types.go b/staging/src/k8s.io/api/storage/v1beta1/types.go
index 9fe5646adc3..3b74ce5eeac 100644
--- a/staging/src/k8s.io/api/storage/v1beta1/types.go
+++ b/staging/src/k8s.io/api/storage/v1beta1/types.go
@@ -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.