From 504f105a9e8164bc55ee8b6e65364f0556dba7aa Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Wed, 3 Mar 2021 21:01:05 +0100 Subject: [PATCH] CSIStorageCapacity: prepare tests for enabling the feature by default Defaults and validation are such that the field has to be set when the feature is enabled, just as for the other boolean fields. This was missing in some tests, which was okay as long as they ran with the feature disabled. Once it gets enabled, validation will flag the missing field as error. Other tests didn't run at all. --- .../storage/validation/validation_test.go | 111 +++++++++++++++--- .../storage/csidriver/storage/storage_test.go | 4 + test/integration/etcd/data.go | 15 +++ 3 files changed, 116 insertions(+), 14 deletions(-) diff --git a/pkg/apis/storage/validation/validation_test.go b/pkg/apis/storage/validation/validation_test.go index c0e80641422..b26b29d294f 100644 --- a/pkg/apis/storage/validation/validation_test.go +++ b/pkg/apis/storage/validation/validation_test.go @@ -1666,6 +1666,8 @@ func TestCSIDriverValidation(t *testing.T) { podInfoOnMount := true notPodInfoOnMount := false notRequiresRepublish := false + storageCapacity := true + notStorageCapacity := false supportedFSGroupPolicy := storage.FileFSGroupPolicy invalidFSGroupPolicy := storage.ReadWriteOnceWithFSTypeFSGroupPolicy invalidFSGroupPolicy = "invalid-mode" @@ -1676,6 +1678,7 @@ func TestCSIDriverValidation(t *testing.T) { AttachRequired: &attachRequired, PodInfoOnMount: &podInfoOnMount, RequiresRepublish: ¬RequiresRepublish, + StorageCapacity: &storageCapacity, }, }, { @@ -1685,6 +1688,7 @@ func TestCSIDriverValidation(t *testing.T) { AttachRequired: &attachRequired, PodInfoOnMount: &podInfoOnMount, RequiresRepublish: ¬RequiresRepublish, + StorageCapacity: ¬StorageCapacity, }, }, { @@ -1694,6 +1698,7 @@ func TestCSIDriverValidation(t *testing.T) { AttachRequired: &attachRequired, PodInfoOnMount: ¬PodInfoOnMount, RequiresRepublish: ¬RequiresRepublish, + StorageCapacity: &storageCapacity, }, }, { @@ -1703,6 +1708,7 @@ func TestCSIDriverValidation(t *testing.T) { AttachRequired: &attachRequired, PodInfoOnMount: &podInfoOnMount, RequiresRepublish: ¬RequiresRepublish, + StorageCapacity: &storageCapacity, }, }, { @@ -1712,6 +1718,7 @@ func TestCSIDriverValidation(t *testing.T) { AttachRequired: &attachRequired, PodInfoOnMount: &podInfoOnMount, RequiresRepublish: ¬RequiresRepublish, + StorageCapacity: &storageCapacity, }, }, { @@ -1720,6 +1727,7 @@ func TestCSIDriverValidation(t *testing.T) { AttachRequired: &attachRequired, PodInfoOnMount: ¬PodInfoOnMount, RequiresRepublish: ¬RequiresRepublish, + StorageCapacity: &storageCapacity, }, }, { @@ -1728,6 +1736,7 @@ func TestCSIDriverValidation(t *testing.T) { AttachRequired: &attachRequired, PodInfoOnMount: &podInfoOnMount, RequiresRepublish: ¬RequiresRepublish, + StorageCapacity: &storageCapacity, }, }, { @@ -1736,6 +1745,7 @@ func TestCSIDriverValidation(t *testing.T) { AttachRequired: &attachNotRequired, PodInfoOnMount: ¬PodInfoOnMount, RequiresRepublish: ¬RequiresRepublish, + StorageCapacity: &storageCapacity, }, }, { @@ -1744,6 +1754,7 @@ func TestCSIDriverValidation(t *testing.T) { AttachRequired: &attachNotRequired, PodInfoOnMount: ¬PodInfoOnMount, RequiresRepublish: ¬RequiresRepublish, + StorageCapacity: &storageCapacity, VolumeLifecycleModes: []storage.VolumeLifecycleMode{ storage.VolumeLifecyclePersistent, }, @@ -1755,6 +1766,7 @@ func TestCSIDriverValidation(t *testing.T) { AttachRequired: &attachNotRequired, PodInfoOnMount: ¬PodInfoOnMount, RequiresRepublish: ¬RequiresRepublish, + StorageCapacity: &storageCapacity, VolumeLifecycleModes: []storage.VolumeLifecycleMode{ storage.VolumeLifecycleEphemeral, }, @@ -1766,6 +1778,7 @@ func TestCSIDriverValidation(t *testing.T) { AttachRequired: &attachNotRequired, PodInfoOnMount: ¬PodInfoOnMount, RequiresRepublish: ¬RequiresRepublish, + StorageCapacity: &storageCapacity, VolumeLifecycleModes: []storage.VolumeLifecycleMode{ storage.VolumeLifecycleEphemeral, storage.VolumeLifecyclePersistent, @@ -1778,6 +1791,7 @@ func TestCSIDriverValidation(t *testing.T) { AttachRequired: &attachNotRequired, PodInfoOnMount: ¬PodInfoOnMount, RequiresRepublish: ¬RequiresRepublish, + StorageCapacity: &storageCapacity, VolumeLifecycleModes: []storage.VolumeLifecycleMode{ storage.VolumeLifecycleEphemeral, storage.VolumeLifecyclePersistent, @@ -1791,6 +1805,7 @@ func TestCSIDriverValidation(t *testing.T) { AttachRequired: &attachNotRequired, PodInfoOnMount: ¬PodInfoOnMount, RequiresRepublish: ¬RequiresRepublish, + StorageCapacity: &storageCapacity, FSGroupPolicy: &supportedFSGroupPolicy, }, }, @@ -1805,39 +1820,53 @@ func TestCSIDriverValidation(t *testing.T) { { ObjectMeta: metav1.ObjectMeta{Name: invalidName}, Spec: storage.CSIDriverSpec{ - AttachRequired: &attachRequired, - PodInfoOnMount: &podInfoOnMount, + AttachRequired: &attachRequired, + PodInfoOnMount: &podInfoOnMount, + StorageCapacity: &storageCapacity, }, }, { ObjectMeta: metav1.ObjectMeta{Name: longName}, Spec: storage.CSIDriverSpec{ - AttachRequired: &attachNotRequired, - PodInfoOnMount: ¬PodInfoOnMount, + AttachRequired: &attachNotRequired, + PodInfoOnMount: ¬PodInfoOnMount, + StorageCapacity: &storageCapacity, }, }, { // AttachRequired not set ObjectMeta: metav1.ObjectMeta{Name: driverName}, Spec: storage.CSIDriverSpec{ - AttachRequired: nil, - PodInfoOnMount: &podInfoOnMount, + AttachRequired: nil, + PodInfoOnMount: &podInfoOnMount, + StorageCapacity: &storageCapacity, }, }, { // PodInfoOnMount not set ObjectMeta: metav1.ObjectMeta{Name: driverName}, Spec: storage.CSIDriverSpec{ - AttachRequired: &attachNotRequired, - PodInfoOnMount: nil, + AttachRequired: &attachNotRequired, + PodInfoOnMount: nil, + StorageCapacity: &storageCapacity, + }, + }, + { + // StorageCapacity not set + ObjectMeta: metav1.ObjectMeta{Name: driverName}, + Spec: storage.CSIDriverSpec{ + AttachRequired: &attachNotRequired, + PodInfoOnMount: &podInfoOnMount, + StorageCapacity: nil, }, }, { // invalid mode ObjectMeta: metav1.ObjectMeta{Name: driverName}, Spec: storage.CSIDriverSpec{ - AttachRequired: &attachNotRequired, - PodInfoOnMount: ¬PodInfoOnMount, + AttachRequired: &attachNotRequired, + PodInfoOnMount: ¬PodInfoOnMount, + StorageCapacity: &storageCapacity, VolumeLifecycleModes: []storage.VolumeLifecycleMode{ "no-such-mode", }, @@ -1847,9 +1876,10 @@ func TestCSIDriverValidation(t *testing.T) { // invalid fsGroupPolicy ObjectMeta: metav1.ObjectMeta{Name: driverName}, Spec: storage.CSIDriverSpec{ - AttachRequired: &attachNotRequired, - PodInfoOnMount: ¬PodInfoOnMount, - FSGroupPolicy: &invalidFSGroupPolicy, + AttachRequired: &attachNotRequired, + PodInfoOnMount: ¬PodInfoOnMount, + FSGroupPolicy: &invalidFSGroupPolicy, + StorageCapacity: &storageCapacity, }, }, } @@ -1868,8 +1898,10 @@ func TestCSIDriverValidationUpdate(t *testing.T) { attachRequired := true attachNotRequired := false podInfoOnMount := true + storageCapacity := true notPodInfoOnMount := false notRequiresRepublish := false + notStorageCapacity := false resourceVersion := "1" invalidFSGroupPolicy := storage.ReadWriteOnceWithFSTypeFSGroupPolicy invalidFSGroupPolicy = "invalid-mode" @@ -1883,6 +1915,7 @@ func TestCSIDriverValidationUpdate(t *testing.T) { storage.VolumeLifecycleEphemeral, storage.VolumeLifecyclePersistent, }, + StorageCapacity: &storageCapacity, }, } @@ -1900,7 +1933,8 @@ func TestCSIDriverValidationUpdate(t *testing.T) { storage.VolumeLifecycleEphemeral, storage.VolumeLifecyclePersistent, }, - FSGroupPolicy: &invalidFSGroupPolicy, + FSGroupPolicy: &invalidFSGroupPolicy, + StorageCapacity: &storageCapacity, }, }, } @@ -1997,6 +2031,12 @@ func TestCSIDriverValidationUpdate(t *testing.T) { new.Spec.FSGroupPolicy = &fileFSGroupPolicy }, }, + { + name: "StorageCapacity changed", + modify: func(new *storage.CSIDriver) { + new.Spec.StorageCapacity = ¬StorageCapacity + }, + }, } for _, test := range errorCases { @@ -2010,6 +2050,48 @@ func TestCSIDriverValidationUpdate(t *testing.T) { } } +func TestCSIDriverStorageCapacityEnablement(t *testing.T) { + run := func(t *testing.T, enabled, withField bool) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIStorageCapacity, enabled)() + + driverName := "test-driver" + attachRequired := true + podInfoOnMount := true + requiresRepublish := true + storageCapacity := true + csiDriver := storage.CSIDriver{ + ObjectMeta: metav1.ObjectMeta{Name: driverName}, + Spec: storage.CSIDriverSpec{ + AttachRequired: &attachRequired, + PodInfoOnMount: &podInfoOnMount, + RequiresRepublish: &requiresRepublish, + }, + } + if withField { + csiDriver.Spec.StorageCapacity = &storageCapacity + } + errs := ValidateCSIDriver(&csiDriver) + success := !enabled || withField + if success && len(errs) != 0 { + t.Errorf("expected success, got: %v", errs) + } + if !success && len(errs) == 0 { + t.Errorf("expected error, got success") + } + } + + yesNo := []bool{true, false} + for _, enabled := range yesNo { + t.Run(fmt.Sprintf("CSIStorageCapacity=%v", enabled), func(t *testing.T) { + for _, withField := range yesNo { + t.Run(fmt.Sprintf("with-field=%v", withField), func(t *testing.T) { + run(t, enabled, withField) + }) + } + }) + } +} + func TestValidateCSIStorageCapacity(t *testing.T) { storageClassName := "test-sc" invalidName := "-invalid-@#$%^&*()-" @@ -2179,6 +2261,7 @@ func TestCSIServiceAccountToken(t *testing.T) { for _, test := range tests { test.csiDriver.Spec.AttachRequired = new(bool) test.csiDriver.Spec.PodInfoOnMount = new(bool) + test.csiDriver.Spec.StorageCapacity = new(bool) if errs := ValidateCSIDriver(test.csiDriver); test.wantErr != (len(errs) != 0) { t.Errorf("ValidateCSIDriver = %v, want err: %v", errs, test.wantErr) } diff --git a/pkg/registry/storage/csidriver/storage/storage_test.go b/pkg/registry/storage/csidriver/storage/storage_test.go index f57c8a9bf65..383f2aba7a3 100644 --- a/pkg/registry/storage/csidriver/storage/storage_test.go +++ b/pkg/registry/storage/csidriver/storage/storage_test.go @@ -49,6 +49,7 @@ func validNewCSIDriver(name string) *storageapi.CSIDriver { attachRequired := true podInfoOnMount := true requiresRepublish := true + storageCapacity := true return &storageapi.CSIDriver{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -57,6 +58,7 @@ func validNewCSIDriver(name string) *storageapi.CSIDriver { AttachRequired: &attachRequired, PodInfoOnMount: &podInfoOnMount, RequiresRepublish: &requiresRepublish, + StorageCapacity: &storageCapacity, }, } } @@ -71,6 +73,7 @@ func TestCreate(t *testing.T) { attachNotRequired := false notPodInfoOnMount := false notRequiresRepublish := false + notStorageCapacity := false test.TestCreate( // valid csiDriver, @@ -81,6 +84,7 @@ func TestCreate(t *testing.T) { AttachRequired: &attachNotRequired, PodInfoOnMount: ¬PodInfoOnMount, RequiresRepublish: ¬RequiresRepublish, + StorageCapacity: ¬StorageCapacity, }, }, ) diff --git a/test/integration/etcd/data.go b/test/integration/etcd/data.go index bdec9bc00f5..befff1e2ee5 100644 --- a/test/integration/etcd/data.go +++ b/test/integration/etcd/data.go @@ -284,6 +284,14 @@ func GetEtcdStorageDataForNamespace(namespace string) map[schema.GroupVersionRes }, // -- + // k8s.io/kubernetes/pkg/apis/storage/v1alpha1 + gvr("storage.k8s.io", "v1alpha1", "csistoragecapacities"): { + Stub: `{"metadata": {"name": "csc-12345-1"}, "storageClassName": "sc1"}`, + ExpectedEtcdPath: "/registry/csistoragecapacities/" + namespace + "/csc-12345-1", + ExpectedGVK: gvkP("storage.k8s.io", "v1beta1", "CSIStorageCapacity"), + }, + // -- + // k8s.io/kubernetes/pkg/apis/flowcontrol/v1alpha1 gvr("flowcontrol.apiserver.k8s.io", "v1alpha1", "flowschemas"): { Stub: `{"metadata": {"name": "va1"}, "spec": {"priorityLevelConfiguration": {"name": "name1"}}}`, @@ -337,6 +345,13 @@ func GetEtcdStorageDataForNamespace(namespace string) map[schema.GroupVersionRes }, // -- + // k8s.io/kubernetes/pkg/apis/storage/v1beta1 + gvr("storage.k8s.io", "v1beta1", "csistoragecapacities"): { + Stub: `{"metadata": {"name": "csc-12345-2"}, "storageClassName": "sc1"}`, + ExpectedEtcdPath: "/registry/csistoragecapacities/" + namespace + "/csc-12345-2", + }, + // -- + // k8s.io/kubernetes/pkg/apis/storage/v1 gvr("storage.k8s.io", "v1", "storageclasses"): { Stub: `{"metadata": {"name": "sc2"}, "provisioner": "aws"}`,