From 76de7c8db954ebe7a0fd23e3a85ded154106532d Mon Sep 17 00:00:00 2001 From: Jonathan Dobson Date: Thu, 30 Mar 2023 09:31:55 -0600 Subject: [PATCH 1/7] STOR-1267: Bump openshift/api --- go.mod | 2 +- go.sum | 4 +- ...fig-operator_01_apiserver-Default.crd.yaml | 1 + .../config/v1/stable.apiserver.testsuite.yaml | 11 ++- .../api/config/v1/types_apiserver.go | 3 +- .../openshift/api/config/v1/types_feature.go | 2 +- ...00_50_cluster_storage_operator_01_crd.yaml | 13 +++ .../operator/v1/stable.storage.testsuite.yaml | 88 +++++++++++++++++++ .../api/operator/v1/types_storage.go | 19 ++++ .../v1/zz_generated.swagger_doc_generated.go | 3 +- vendor/modules.txt | 2 +- 11 files changed, 138 insertions(+), 10 deletions(-) diff --git a/go.mod b/go.mod index 8263678d7..2261bba88 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( github.com/go-logr/logr v1.2.3 // indirect github.com/google/go-cmp v0.5.9 github.com/google/gofuzz v1.2.0 // indirect - github.com/openshift/api v0.0.0-20230228142948-d170fcdc0fa6 + github.com/openshift/api v0.0.0-20230330150608-05635858d40f github.com/openshift/build-machinery-go v0.0.0-20220913142420-e25cf57ea46d github.com/openshift/client-go v0.0.0-20230120202327-72f107311084 github.com/openshift/library-go v0.0.0-20230130121854-13dc1e57ef79 diff --git a/go.sum b/go.sum index 37400861e..44a0662cd 100644 --- a/go.sum +++ b/go.sum @@ -937,8 +937,8 @@ github.com/op/go-logging v0.0.0-20160315200505-970db520ece7/go.mod h1:HzydrMdWEr github.com/opencontainers/go-digest v1.0.0-rc1/go.mod h1:cMLVZDEM3+U2I4VmLI6N8jQYUd2OVphdqWwCJHrFt2s= github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM= github.com/opencontainers/image-spec v1.0.1/go.mod h1:BtxoFyWECRxE4U/7sNtV5W15zMzWCbyJoFRP3s7yZA0= -github.com/openshift/api v0.0.0-20230228142948-d170fcdc0fa6 h1:SKsW89hdqceWIg1BT+vhLia8CltOV/zpY63hO67ckbw= -github.com/openshift/api v0.0.0-20230228142948-d170fcdc0fa6/go.mod h1:ctXNyWanKEjGj8sss1KjjHQ3ENKFm33FFnS5BKaIPh4= +github.com/openshift/api v0.0.0-20230330150608-05635858d40f h1:mGpCtfoehMcvmg/sSYLiv6nCbTl04cmtkUfYzP7H1AQ= +github.com/openshift/api v0.0.0-20230330150608-05635858d40f/go.mod h1:ctXNyWanKEjGj8sss1KjjHQ3ENKFm33FFnS5BKaIPh4= github.com/openshift/build-machinery-go v0.0.0-20220913142420-e25cf57ea46d h1:RR4ah7FfaPR1WePizm0jlrsbmPu91xQZnAsVVreQV1k= github.com/openshift/build-machinery-go v0.0.0-20220913142420-e25cf57ea46d/go.mod h1:b1BuldmJlbA/xYtdZvKi+7j5YGB44qJUJDZ9zwiNCfE= github.com/openshift/client-go v0.0.0-20230120202327-72f107311084 h1:66uaqNwA+qYyQDwsMWUfjjau8ezmg1dzCqub13KZOcE= diff --git a/vendor/github.com/openshift/api/config/v1/0000_10_config-operator_01_apiserver-Default.crd.yaml b/vendor/github.com/openshift/api/config/v1/0000_10_config-operator_01_apiserver-Default.crd.yaml index 808fae375..7edc7f23a 100644 --- a/vendor/github.com/openshift/api/config/v1/0000_10_config-operator_01_apiserver-Default.crd.yaml +++ b/vendor/github.com/openshift/api/config/v1/0000_10_config-operator_01_apiserver-Default.crd.yaml @@ -102,6 +102,7 @@ spec: - "" - identity - aescbc + - aesgcm servingCerts: description: servingCert is the TLS cert info for serving secure traffic. If not specified, operator managed certificates will be used for serving secure traffic. type: object diff --git a/vendor/github.com/openshift/api/config/v1/stable.apiserver.testsuite.yaml b/vendor/github.com/openshift/api/config/v1/stable.apiserver.testsuite.yaml index 632525236..75f846a3d 100644 --- a/vendor/github.com/openshift/api/config/v1/stable.apiserver.testsuite.yaml +++ b/vendor/github.com/openshift/api/config/v1/stable.apiserver.testsuite.yaml @@ -18,12 +18,19 @@ tests: profile: Default encryption: type: aescbc - - name: Should not be able to create encrypt with aesgcm. Yet. + - name: Should be able to create encrypt with aesgcm initial: | apiVersion: config.openshift.io/v1 kind: APIServer spec: encryption: type: aesgcm - expectedError: "spec.encryption.type: Unsupported value: \"aesgcm\": supported values: \"\", \"identity\", \"aescbc\"" + expected: | + apiVersion: config.openshift.io/v1 + kind: APIServer + spec: + audit: + profile: Default + encryption: + type: aesgcm diff --git a/vendor/github.com/openshift/api/config/v1/types_apiserver.go b/vendor/github.com/openshift/api/config/v1/types_apiserver.go index 9940755c3..f4b52a227 100644 --- a/vendor/github.com/openshift/api/config/v1/types_apiserver.go +++ b/vendor/github.com/openshift/api/config/v1/types_apiserver.go @@ -184,8 +184,7 @@ type APIServerEncryption struct { Type EncryptionType `json:"type,omitempty"` } -// +openshift:validation:FeatureSetAwareEnum:featureSet=Default,enum="";identity;aescbc -// +openshift:validation:FeatureSetAwareEnum:featureSet=TechPreviewNoUpgrade,enum="";identity;aescbc;aesgcm +// +kubebuilder:validation:Enum="";identity;aescbc;aesgcm type EncryptionType string const ( diff --git a/vendor/github.com/openshift/api/config/v1/types_feature.go b/vendor/github.com/openshift/api/config/v1/types_feature.go index 26846cc36..a254cac4f 100644 --- a/vendor/github.com/openshift/api/config/v1/types_feature.go +++ b/vendor/github.com/openshift/api/config/v1/types_feature.go @@ -119,6 +119,7 @@ var FeatureSets = map[FeatureSet]*FeatureGateEnabledDisabled{ with("RetroactiveDefaultStorageClass"). // sig-storage, RomanBednar, Kubernetes feature gate with("PDBUnhealthyPodEvictionPolicy"). // sig-apps, atiratree (#forum-workloads), Kubernetes feature gate with("DynamicResourceAllocation"). // sig-scheduling, jchaloup (#forum-workloads), Kubernetes feature gate + with("OpenShiftPodSecurityAdmission"). // bz-auth, stlaz, OCP specific toFeatures(), LatencySensitive: newDefaultFeatures(). with( @@ -132,7 +133,6 @@ var defaultFeatures = &FeatureGateEnabledDisabled{ "APIPriorityAndFairness", // sig-apimachinery, deads2k "RotateKubeletServerCertificate", // sig-pod, sjenning "DownwardAPIHugePages", // sig-node, rphillips - "OpenShiftPodSecurityAdmission", // bz-auth, stlaz, OCP specific }, Disabled: []string{ "RetroactiveDefaultStorageClass", // sig-storage, RomanBednar, Kubernetes feature gate diff --git a/vendor/github.com/openshift/api/operator/v1/0000_50_cluster_storage_operator_01_crd.yaml b/vendor/github.com/openshift/api/operator/v1/0000_50_cluster_storage_operator_01_crd.yaml index 2bf181862..484576c1a 100644 --- a/vendor/github.com/openshift/api/operator/v1/0000_50_cluster_storage_operator_01_crd.yaml +++ b/vendor/github.com/openshift/api/operator/v1/0000_50_cluster_storage_operator_01_crd.yaml @@ -69,6 +69,19 @@ spec: type: object nullable: true x-kubernetes-preserve-unknown-fields: true + vsphereStorageDriver: + description: 'VSphereStorageDriver indicates the storage driver to use on VSphere clusters. Once this field is set to CSIWithMigrationDriver, it can not be changed. If this is empty, the platform will choose a good default, which may change over time without notice. DEPRECATED: This field will be removed in a future release.' + type: string + enum: + - "" + - LegacyDeprecatedInTreeDriver + - CSIWithMigrationDriver + x-kubernetes-validations: + - rule: self == oldSelf || oldSelf == "" || self == "CSIWithMigrationDriver" + message: VSphereStorageDriver can not be changed once it is set to CSIWithMigrationDriver + x-kubernetes-validations: + - rule: '!has(oldSelf.vsphereStorageDriver) || has(self.vsphereStorageDriver)' + message: VSphereStorageDriver is required once set status: description: status holds observed values from the cluster. They may not be overridden. type: object diff --git a/vendor/github.com/openshift/api/operator/v1/stable.storage.testsuite.yaml b/vendor/github.com/openshift/api/operator/v1/stable.storage.testsuite.yaml index 42903f22d..63a7ca454 100644 --- a/vendor/github.com/openshift/api/operator/v1/stable.storage.testsuite.yaml +++ b/vendor/github.com/openshift/api/operator/v1/stable.storage.testsuite.yaml @@ -14,3 +14,91 @@ tests: spec: logLevel: Normal operatorLogLevel: Normal + onUpdate: + - name: Should allow enabling CSI migration for vSphere + initial: | + apiVersion: operator.openshift.io/v1 + kind: Storage + spec: {} # No spec is required + updated: | + apiVersion: operator.openshift.io/v1 + kind: Storage + spec: + vsphereStorageDriver: CSIWithMigrationDriver + expected: | + apiVersion: operator.openshift.io/v1 + kind: Storage + spec: + vsphereStorageDriver: CSIWithMigrationDriver + logLevel: Normal + operatorLogLevel: Normal + - name: Should allow disabling CSI migration for vSphere + initial: | + apiVersion: operator.openshift.io/v1 + kind: Storage + spec: {} # No spec is required + updated: | + apiVersion: operator.openshift.io/v1 + kind: Storage + spec: + vsphereStorageDriver: LegacyDeprecatedInTreeDriver + expected: | + apiVersion: operator.openshift.io/v1 + kind: Storage + spec: + vsphereStorageDriver: LegacyDeprecatedInTreeDriver + logLevel: Normal + operatorLogLevel: Normal + - name: Should allow changing LegacyDeprecatedInTreeDriver to CSIWithMigrationDriver + initial: | + apiVersion: operator.openshift.io/v1 + kind: Storage + spec: + vsphereStorageDriver: LegacyDeprecatedInTreeDriver + updated: | + apiVersion: operator.openshift.io/v1 + kind: Storage + spec: + vsphereStorageDriver: CSIWithMigrationDriver + expected: | + apiVersion: operator.openshift.io/v1 + kind: Storage + spec: + vsphereStorageDriver: CSIWithMigrationDriver + logLevel: Normal + operatorLogLevel: Normal + - name: Should not allow changing CSIWithMigrationDriver to LegacyDeprecatedInTreeDriver + initial: | + apiVersion: operator.openshift.io/v1 + kind: Storage + spec: + vsphereStorageDriver: CSIWithMigrationDriver + updated: | + apiVersion: operator.openshift.io/v1 + kind: Storage + spec: + vsphereStorageDriver: LegacyDeprecatedInTreeDriver + expectedError: "VSphereStorageDriver can not be changed once it is set to CSIWithMigrationDriver" + - name: Should not allow changing CSIWithMigrationDriver to empty string + initial: | + apiVersion: operator.openshift.io/v1 + kind: Storage + spec: + vsphereStorageDriver: CSIWithMigrationDriver + updated: | + apiVersion: operator.openshift.io/v1 + kind: Storage + spec: + vsphereStorageDriver: "" + expectedError: "VSphereStorageDriver can not be changed once it is set to CSIWithMigrationDriver" + - name: Should not allow unsetting VSphereStorageDriver once it is set + initial: | + apiVersion: operator.openshift.io/v1 + kind: Storage + spec: + vsphereStorageDriver: CSIWithMigrationDriver + updated: | + apiVersion: operator.openshift.io/v1 + kind: Storage + spec: {} + expectedError: "VSphereStorageDriver is required once set" diff --git a/vendor/github.com/openshift/api/operator/v1/types_storage.go b/vendor/github.com/openshift/api/operator/v1/types_storage.go index 38ffe26d5..044c9c32a 100644 --- a/vendor/github.com/openshift/api/operator/v1/types_storage.go +++ b/vendor/github.com/openshift/api/operator/v1/types_storage.go @@ -26,9 +26,28 @@ type Storage struct { Status StorageStatus `json:"status"` } +// StorageDriverType indicates whether CSI migration should be enabled for drivers where it is optional. +// +kubebuilder:validation:Enum="";LegacyDeprecatedInTreeDriver;CSIWithMigrationDriver +type StorageDriverType string + +const ( + LegacyDeprecatedInTreeDriver StorageDriverType = "LegacyDeprecatedInTreeDriver" + CSIWithMigrationDriver StorageDriverType = "CSIWithMigrationDriver" +) + // StorageSpec is the specification of the desired behavior of the cluster storage operator. +// +kubebuilder:validation:XValidation:rule="!has(oldSelf.vsphereStorageDriver) || has(self.vsphereStorageDriver)", message="VSphereStorageDriver is required once set" type StorageSpec struct { OperatorSpec `json:",inline"` + + // VSphereStorageDriver indicates the storage driver to use on VSphere clusters. + // Once this field is set to CSIWithMigrationDriver, it can not be changed. + // If this is empty, the platform will choose a good default, + // which may change over time without notice. + // DEPRECATED: This field will be removed in a future release. + // +kubebuilder:validation:XValidation:rule="self == oldSelf || oldSelf == \"\" || self == \"CSIWithMigrationDriver\"",message="VSphereStorageDriver can not be changed once it is set to CSIWithMigrationDriver" + // +optional + VSphereStorageDriver StorageDriverType `json:"vsphereStorageDriver"` } // StorageStatus defines the observed status of the cluster storage operator. diff --git a/vendor/github.com/openshift/api/operator/v1/zz_generated.swagger_doc_generated.go b/vendor/github.com/openshift/api/operator/v1/zz_generated.swagger_doc_generated.go index bab435e0c..1248ffb1b 100644 --- a/vendor/github.com/openshift/api/operator/v1/zz_generated.swagger_doc_generated.go +++ b/vendor/github.com/openshift/api/operator/v1/zz_generated.swagger_doc_generated.go @@ -1610,7 +1610,8 @@ func (StorageList) SwaggerDoc() map[string]string { } var map_StorageSpec = map[string]string{ - "": "StorageSpec is the specification of the desired behavior of the cluster storage operator.", + "": "StorageSpec is the specification of the desired behavior of the cluster storage operator.", + "vsphereStorageDriver": "VSphereStorageDriver indicates the storage driver to use on VSphere clusters. Once this field is set to CSIWithMigrationDriver, it can not be changed. If this is empty, the platform will choose a good default, which may change over time without notice. DEPRECATED: This field will be removed in a future release.", } func (StorageSpec) SwaggerDoc() map[string]string { diff --git a/vendor/modules.txt b/vendor/modules.txt index 59132b38e..5bbe77c10 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -154,7 +154,7 @@ github.com/modern-go/reflect2 # github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 ## explicit github.com/munnerz/goautoneg -# github.com/openshift/api v0.0.0-20230228142948-d170fcdc0fa6 +# github.com/openshift/api v0.0.0-20230330150608-05635858d40f ## explicit; go 1.19 github.com/openshift/api github.com/openshift/api/apiserver From 8e9ed83929785a297c1d456a5013beec1973adf2 Mon Sep 17 00:00:00 2001 From: Jonathan Dobson Date: Wed, 29 Mar 2023 15:26:54 -0600 Subject: [PATCH 2/7] STOR-1283: Enable CSI migration in new clusters --- manifests/06_operator_cr-hypershift.yaml | 1 + manifests/06_operator_cr.yaml | 1 + 2 files changed, 2 insertions(+) diff --git a/manifests/06_operator_cr-hypershift.yaml b/manifests/06_operator_cr-hypershift.yaml index 3dd1096e2..d6b796b88 100644 --- a/manifests/06_operator_cr-hypershift.yaml +++ b/manifests/06_operator_cr-hypershift.yaml @@ -7,3 +7,4 @@ spec: logLevel: Normal managementState: Managed operatorLogLevel: Normal + vsphereStorageDriver: CSIWithMigrationDriver diff --git a/manifests/06_operator_cr.yaml b/manifests/06_operator_cr.yaml index a28d1fa01..4d6d531d0 100644 --- a/manifests/06_operator_cr.yaml +++ b/manifests/06_operator_cr.yaml @@ -12,3 +12,4 @@ spec: managementState: Managed logLevel: Normal operatorLogLevel: Normal + vsphereStorageDriver: CSIWithMigrationDriver From dad70df08b3458df036d29cefae32c932f784802 Mon Sep 17 00:00:00 2001 From: Jonathan Dobson Date: Wed, 29 Mar 2023 15:38:27 -0600 Subject: [PATCH 3/7] STOR-1272: cluster-storage-operator must re-create in-tree StorageClass --- assets/storageclasses/vsphere.yaml | 11 ++ .../defaultstorageclass/controller.go | 32 ++++- .../defaultstorageclass/controller_test.go | 132 ++++++++++++++++++ 3 files changed, 171 insertions(+), 4 deletions(-) create mode 100644 assets/storageclasses/vsphere.yaml diff --git a/assets/storageclasses/vsphere.yaml b/assets/storageclasses/vsphere.yaml new file mode 100644 index 000000000..214f3fe4b --- /dev/null +++ b/assets/storageclasses/vsphere.yaml @@ -0,0 +1,11 @@ +apiVersion: storage.k8s.io/v1 +kind: StorageClass +metadata: + name: thin + annotations: + storageclass.kubernetes.io/is-default-class: "true" +provisioner: kubernetes.io/vsphere-volume +parameters: + diskformat: thin +reclaimPolicy: Delete +volumeBindingMode: Immediate diff --git a/pkg/operator/defaultstorageclass/controller.go b/pkg/operator/defaultstorageclass/controller.go index 3bac7d2fb..c46297f79 100644 --- a/pkg/operator/defaultstorageclass/controller.go +++ b/pkg/operator/defaultstorageclass/controller.go @@ -7,10 +7,13 @@ import ( configv1 "github.com/openshift/api/config/v1" operatorapi "github.com/openshift/api/operator/v1" openshiftv1 "github.com/openshift/client-go/config/listers/config/v1" + oplisters "github.com/openshift/client-go/operator/listers/operator/v1" + "github.com/openshift/cluster-storage-operator/assets" "github.com/openshift/cluster-storage-operator/pkg/csoclients" "github.com/openshift/library-go/pkg/controller/factory" "github.com/openshift/library-go/pkg/operator/events" "github.com/openshift/library-go/pkg/operator/resource/resourceapply" + "github.com/openshift/library-go/pkg/operator/resource/resourceread" "github.com/openshift/library-go/pkg/operator/v1helpers" storagev1 "k8s.io/api/storage/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -23,6 +26,7 @@ import ( const ( conditionsPrefix = "DefaultStorageClassController" infraConfigName = "cluster" + storageOperatorName = "cluster" disabledConditionType = "Disabled" ) @@ -41,6 +45,7 @@ type Controller struct { operatorClient v1helpers.OperatorClient kubeClient kubernetes.Interface infraLister openshiftv1.InfrastructureLister + storageLister oplisters.StorageLister storageClassLister v1.StorageClassLister eventRecorder events.Recorder } @@ -52,12 +57,14 @@ func NewController( operatorClient: clients.OperatorClient, kubeClient: clients.KubeClient, infraLister: clients.ConfigInformers.Config().V1().Infrastructures().Lister(), + storageLister: clients.OperatorInformers.Operator().V1().Storages().Lister(), storageClassLister: clients.KubeInformers.InformersFor("").Storage().V1().StorageClasses().Lister(), eventRecorder: eventRecorder, } return factory.New().WithSync(c.sync).WithSyncDegradedOnError(clients.OperatorClient).WithInformers( clients.OperatorClient.Informer(), clients.ConfigInformers.Config().V1().Infrastructures().Informer(), + clients.OperatorInformers.Operator().V1().Storages().Informer(), clients.KubeInformers.InformersFor("").Storage().V1().StorageClasses().Informer(), ).ToController("DefaultStorageClassController", eventRecorder) } @@ -153,13 +160,17 @@ func (c *Controller) syncStorageClass(ctx context.Context) error { if err != nil { return err } + storageOperator, err := c.storageLister.Get(storageOperatorName) + if err != nil { + return err + } // Check to see if the PlatformStatus is nil. This has been seen on some // UPI installs on baremetal platforms if infrastructure.Status.PlatformStatus == nil { return unsupportedPlatformError } - expectedSC, err := newStorageClassForCluster(infrastructure) + expectedSC, err := newStorageClassForCluster(infrastructure, storageOperator) if err != nil { return err } @@ -183,15 +194,22 @@ func (c *Controller) syncStorageClass(ctx context.Context) error { return err } -// Returns an error indicating whether the StorageClass is provided by a CSI driver or an unsupported platform. -func newStorageClassForCluster(infrastructure *configv1.Infrastructure) (*storagev1.StorageClass, error) { +// Returns either the StorageClass, if the PlatformType is supported, or an error +// indicating whether the StorageClass is provided by a CSI driver or an unsupported platform +func newStorageClassForCluster(infrastructure *configv1.Infrastructure, storageOperator *operatorapi.Storage) (*storagev1.StorageClass, error) { + var storageClassFile string switch infrastructure.Status.PlatformStatus.Type { case configv1.AWSPlatformType: return nil, supportedByCSIError case configv1.GCPPlatformType: return nil, supportedByCSIError case configv1.VSpherePlatformType: - return nil, supportedByCSIError + // Only create the in-tree SC if CSI migration is disabled + if storageOperator.Spec.VSphereStorageDriver != operatorapi.CSIWithMigrationDriver { + storageClassFile = "storageclasses/vsphere.yaml" + } else { + return nil, supportedByCSIError + } case configv1.AlibabaCloudPlatformType: return nil, supportedByCSIError case configv1.AzurePlatformType: @@ -205,6 +223,12 @@ func newStorageClassForCluster(infrastructure *configv1.Infrastructure) (*storag default: return nil, unsupportedPlatformError } + + scBytes, err := assets.ReadFile(storageClassFile) + if err != nil { + return nil, err + } + return resourceread.ReadStorageClassV1OrDie(scBytes), nil } // UpdateConditionFunc returns a func to update a condition. diff --git a/pkg/operator/defaultstorageclass/controller_test.go b/pkg/operator/defaultstorageclass/controller_test.go index 0c0d3a541..18ce9a941 100644 --- a/pkg/operator/defaultstorageclass/controller_test.go +++ b/pkg/operator/defaultstorageclass/controller_test.go @@ -158,8 +158,38 @@ func withFalseConditions(conditions ...string) crModifier { } } +func withVSphereMigrationEnabled() crModifier { + return func(i *opv1.Storage) *opv1.Storage { + i.Spec.VSphereStorageDriver = opv1.CSIWithMigrationDriver + return i + } +} + +func withVSphereMigrationDisabled() crModifier { + return func(i *opv1.Storage) *opv1.Storage { + i.Spec.VSphereStorageDriver = opv1.LegacyDeprecatedInTreeDriver + return i + } +} + func TestSync(t *testing.T) { tests := []operatorTest{ + { + // Default storage class is deployed if it does not exist + name: "initial VSphere deployment", + initialObjects: testObjects{ + storage: getCR(), + infrastructure: getInfrastructure(cfgv1.VSpherePlatformType), + }, + expectedObjects: testObjects{ + storage: getCR( + withTrueConditions(conditionsPrefix+opv1.OperatorStatusTypeAvailable), + withFalseConditions(conditionsPrefix+opv1.OperatorStatusTypeProgressing), + ), + storageClasses: []*storagev1.StorageClass{getPlatformStorageClass("storageclasses/vsphere.yaml")}, + }, + expectErr: false, + }, { // The controller reports Disable on unsupported platforms name: "initial unsupported platform deployment", @@ -175,6 +205,43 @@ func TestSync(t *testing.T) { }, expectErr: false, }, + { + // Everything is deployed and the controller does nothing + name: "everything deployed", + initialObjects: testObjects{ + storage: getCR( + withTrueConditions(conditionsPrefix+opv1.OperatorStatusTypeAvailable), + withFalseConditions(conditionsPrefix+opv1.OperatorStatusTypeProgressing), + ), + storageClasses: []*storagev1.StorageClass{getPlatformStorageClass("storageclasses/vsphere.yaml")}, + infrastructure: getInfrastructure(cfgv1.VSpherePlatformType), + }, + expectedObjects: testObjects{ + storage: getCR( + withTrueConditions(conditionsPrefix+opv1.OperatorStatusTypeAvailable), + withFalseConditions(conditionsPrefix+opv1.OperatorStatusTypeProgressing), + ), + storageClasses: []*storagev1.StorageClass{getPlatformStorageClass("storageclasses/vsphere.yaml")}, + }, + expectErr: false, + }, + { + // The controller does not set default storage class when removed by user + name: "default storage class removed by user", + initialObjects: testObjects{ + storage: getCR(), + storageClasses: []*storagev1.StorageClass{getPlatformStorageClass("storageclasses/vsphere.yaml", withNoDefault)}, + infrastructure: getInfrastructure(cfgv1.VSpherePlatformType), + }, + expectedObjects: testObjects{ + storage: getCR( + withTrueConditions(conditionsPrefix+opv1.OperatorStatusTypeAvailable), + withFalseConditions(conditionsPrefix+opv1.OperatorStatusTypeProgressing), + ), + storageClasses: []*storagev1.StorageClass{getPlatformStorageClass("storageclasses/vsphere.yaml", withNoDefault)}, + }, + expectErr: false, + }, { // The controller returns error - missing Available is added name: "infrastructure not found", @@ -189,6 +256,21 @@ func TestSync(t *testing.T) { }, expectErr: true, }, + { + // The controller syncs fine when azurestackhub is used and does not create a storage class + name: "azurestackhub", + initialObjects: testObjects{ + storage: getCR(), + infrastructure: getAzureStackHubInfrastructure(), + }, + expectedObjects: testObjects{ + storage: getCR( + withFalseConditions(conditionsPrefix+opv1.OperatorStatusTypeProgressing), + withTrueConditions(conditionsPrefix+opv1.OperatorStatusTypeAvailable), + ), + }, + expectErr: false, + }, { // The controller returns error + Available is True -> not flipped to False name: "available not false after error", @@ -206,6 +288,56 @@ func TestSync(t *testing.T) { }, expectErr: true, }, + { + // The controller flips Avialable=False to True after a successful sync + name: "available=False set to True after OK", + initialObjects: testObjects{ + infrastructure: getInfrastructure(cfgv1.VSpherePlatformType), + storage: getCR( + withFalseConditions(conditionsPrefix+opv1.OperatorStatusTypeAvailable), + withTrueConditions(conditionsPrefix+opv1.OperatorStatusTypeProgressing), + ), + }, + expectedObjects: testObjects{ + storageClasses: []*storagev1.StorageClass{getPlatformStorageClass("storageclasses/vsphere.yaml")}, + storage: getCR( + withTrueConditions(conditionsPrefix+opv1.OperatorStatusTypeAvailable), + withFalseConditions(conditionsPrefix+opv1.OperatorStatusTypeProgressing), + ), + }, + expectErr: false, + }, + { + name: "VSphere StorageClass created if migration disabled", + initialObjects: testObjects{ + storage: getCR(withVSphereMigrationDisabled()), + infrastructure: getInfrastructure(cfgv1.VSpherePlatformType), + }, + expectedObjects: testObjects{ + storage: getCR( + withVSphereMigrationDisabled(), + withTrueConditions(conditionsPrefix+opv1.OperatorStatusTypeAvailable), + withFalseConditions(conditionsPrefix+opv1.OperatorStatusTypeProgressing), + ), + storageClasses: []*storagev1.StorageClass{getPlatformStorageClass("storageclasses/vsphere.yaml")}, + }, + expectErr: false, + }, + { + name: "VSphere StorageClass not created if migration enabled", + initialObjects: testObjects{ + storage: getCR(withVSphereMigrationEnabled()), + infrastructure: getInfrastructure(cfgv1.VSpherePlatformType), + }, + expectedObjects: testObjects{ + storage: getCR( + withVSphereMigrationEnabled(), + withTrueConditions(conditionsPrefix+opv1.OperatorStatusTypeAvailable), + withFalseConditions(conditionsPrefix+opv1.OperatorStatusTypeProgressing), + ), + }, + expectErr: false, + }, } for _, test := range tests { From 658e038556f32958177464c2cca22c2978b2144c Mon Sep 17 00:00:00 2001 From: Jonathan Dobson Date: Thu, 30 Mar 2023 09:54:41 -0600 Subject: [PATCH 4/7] STOR-1269: allow vmware-vsphere-csi-driver-operator to list storage CRD --- assets/csidriveroperators/vsphere/06_clusterrole.yaml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/assets/csidriveroperators/vsphere/06_clusterrole.yaml b/assets/csidriveroperators/vsphere/06_clusterrole.yaml index f140e97f4..af84cff3d 100644 --- a/assets/csidriveroperators/vsphere/06_clusterrole.yaml +++ b/assets/csidriveroperators/vsphere/06_clusterrole.yaml @@ -340,3 +340,11 @@ rules: - "tokenreviews" verbs: - "create" +- apiGroups: + - operator.openshift.io + resources: + - storages + verbs: + - get + - list + - watch From 46173a68c8112c2c5bb9d04645f8de262996a21c Mon Sep 17 00:00:00 2001 From: Jonathan Dobson Date: Thu, 6 Apr 2023 12:09:42 -0600 Subject: [PATCH 5/7] OCPBUGS-11498: vsphereStorageDriver validation is misleading --- go.mod | 2 +- go.sum | 4 ++-- ...000_50_cluster_storage_operator_01_crd.yaml | 2 +- .../operator/v1/stable.storage.testsuite.yaml | 18 ++++++++++++++++++ .../openshift/api/operator/v1/types_storage.go | 2 +- vendor/modules.txt | 2 +- 6 files changed, 24 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index 2261bba88..c1e1058b6 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( github.com/go-logr/logr v1.2.3 // indirect github.com/google/go-cmp v0.5.9 github.com/google/gofuzz v1.2.0 // indirect - github.com/openshift/api v0.0.0-20230330150608-05635858d40f + github.com/openshift/api v0.0.0-20230406180441-6bc7296cd4fa github.com/openshift/build-machinery-go v0.0.0-20220913142420-e25cf57ea46d github.com/openshift/client-go v0.0.0-20230120202327-72f107311084 github.com/openshift/library-go v0.0.0-20230130121854-13dc1e57ef79 diff --git a/go.sum b/go.sum index 44a0662cd..100b3d625 100644 --- a/go.sum +++ b/go.sum @@ -937,8 +937,8 @@ github.com/op/go-logging v0.0.0-20160315200505-970db520ece7/go.mod h1:HzydrMdWEr github.com/opencontainers/go-digest v1.0.0-rc1/go.mod h1:cMLVZDEM3+U2I4VmLI6N8jQYUd2OVphdqWwCJHrFt2s= github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM= github.com/opencontainers/image-spec v1.0.1/go.mod h1:BtxoFyWECRxE4U/7sNtV5W15zMzWCbyJoFRP3s7yZA0= -github.com/openshift/api v0.0.0-20230330150608-05635858d40f h1:mGpCtfoehMcvmg/sSYLiv6nCbTl04cmtkUfYzP7H1AQ= -github.com/openshift/api v0.0.0-20230330150608-05635858d40f/go.mod h1:ctXNyWanKEjGj8sss1KjjHQ3ENKFm33FFnS5BKaIPh4= +github.com/openshift/api v0.0.0-20230406180441-6bc7296cd4fa h1:2oUaQNfxhgX+3lq49YkshMWR4MbX9N8OEDevUCyEJ2I= +github.com/openshift/api v0.0.0-20230406180441-6bc7296cd4fa/go.mod h1:ctXNyWanKEjGj8sss1KjjHQ3ENKFm33FFnS5BKaIPh4= github.com/openshift/build-machinery-go v0.0.0-20220913142420-e25cf57ea46d h1:RR4ah7FfaPR1WePizm0jlrsbmPu91xQZnAsVVreQV1k= github.com/openshift/build-machinery-go v0.0.0-20220913142420-e25cf57ea46d/go.mod h1:b1BuldmJlbA/xYtdZvKi+7j5YGB44qJUJDZ9zwiNCfE= github.com/openshift/client-go v0.0.0-20230120202327-72f107311084 h1:66uaqNwA+qYyQDwsMWUfjjau8ezmg1dzCqub13KZOcE= diff --git a/vendor/github.com/openshift/api/operator/v1/0000_50_cluster_storage_operator_01_crd.yaml b/vendor/github.com/openshift/api/operator/v1/0000_50_cluster_storage_operator_01_crd.yaml index 484576c1a..08e011b11 100644 --- a/vendor/github.com/openshift/api/operator/v1/0000_50_cluster_storage_operator_01_crd.yaml +++ b/vendor/github.com/openshift/api/operator/v1/0000_50_cluster_storage_operator_01_crd.yaml @@ -77,7 +77,7 @@ spec: - LegacyDeprecatedInTreeDriver - CSIWithMigrationDriver x-kubernetes-validations: - - rule: self == oldSelf || oldSelf == "" || self == "CSIWithMigrationDriver" + - rule: oldSelf != "CSIWithMigrationDriver" || self == "CSIWithMigrationDriver" message: VSphereStorageDriver can not be changed once it is set to CSIWithMigrationDriver x-kubernetes-validations: - rule: '!has(oldSelf.vsphereStorageDriver) || has(self.vsphereStorageDriver)' diff --git a/vendor/github.com/openshift/api/operator/v1/stable.storage.testsuite.yaml b/vendor/github.com/openshift/api/operator/v1/stable.storage.testsuite.yaml index 63a7ca454..5fa36e335 100644 --- a/vendor/github.com/openshift/api/operator/v1/stable.storage.testsuite.yaml +++ b/vendor/github.com/openshift/api/operator/v1/stable.storage.testsuite.yaml @@ -102,3 +102,21 @@ tests: kind: Storage spec: {} expectedError: "VSphereStorageDriver is required once set" + - name: Should allow changing LegacyDeprecatedInTreeDriver to empty string + initial: | + apiVersion: operator.openshift.io/v1 + kind: Storage + spec: + vsphereStorageDriver: LegacyDeprecatedInTreeDriver + updated: | + apiVersion: operator.openshift.io/v1 + kind: Storage + spec: + vsphereStorageDriver: "" + expected: | + apiVersion: operator.openshift.io/v1 + kind: Storage + spec: + vsphereStorageDriver: "" + logLevel: Normal + operatorLogLevel: Normal diff --git a/vendor/github.com/openshift/api/operator/v1/types_storage.go b/vendor/github.com/openshift/api/operator/v1/types_storage.go index 044c9c32a..aaf011542 100644 --- a/vendor/github.com/openshift/api/operator/v1/types_storage.go +++ b/vendor/github.com/openshift/api/operator/v1/types_storage.go @@ -45,7 +45,7 @@ type StorageSpec struct { // If this is empty, the platform will choose a good default, // which may change over time without notice. // DEPRECATED: This field will be removed in a future release. - // +kubebuilder:validation:XValidation:rule="self == oldSelf || oldSelf == \"\" || self == \"CSIWithMigrationDriver\"",message="VSphereStorageDriver can not be changed once it is set to CSIWithMigrationDriver" + // +kubebuilder:validation:XValidation:rule="oldSelf != \"CSIWithMigrationDriver\" || self == \"CSIWithMigrationDriver\"",message="VSphereStorageDriver can not be changed once it is set to CSIWithMigrationDriver" // +optional VSphereStorageDriver StorageDriverType `json:"vsphereStorageDriver"` } diff --git a/vendor/modules.txt b/vendor/modules.txt index 5bbe77c10..b94aa2866 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -154,7 +154,7 @@ github.com/modern-go/reflect2 # github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 ## explicit github.com/munnerz/goautoneg -# github.com/openshift/api v0.0.0-20230330150608-05635858d40f +# github.com/openshift/api v0.0.0-20230406180441-6bc7296cd4fa ## explicit; go 1.19 github.com/openshift/api github.com/openshift/api/apiserver From 0f36052d1e273304800934dfdaea507c96e541bf Mon Sep 17 00:00:00 2001 From: Jonathan Dobson Date: Fri, 14 Apr 2023 08:40:11 -0600 Subject: [PATCH 6/7] OCPBUGS-11776: VSphereStorageDriver does not document the platform default --- go.mod | 2 +- go.sum | 4 ++-- .../operator/v1/0000_50_cluster_storage_operator_01_crd.yaml | 2 +- vendor/github.com/openshift/api/operator/v1/types_storage.go | 1 + .../api/operator/v1/zz_generated.swagger_doc_generated.go | 2 +- vendor/modules.txt | 2 +- 6 files changed, 7 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index c1e1058b6..20c005d07 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( github.com/go-logr/logr v1.2.3 // indirect github.com/google/go-cmp v0.5.9 github.com/google/gofuzz v1.2.0 // indirect - github.com/openshift/api v0.0.0-20230406180441-6bc7296cd4fa + github.com/openshift/api v0.0.0-20230414143018-3367bc7e6ac7 github.com/openshift/build-machinery-go v0.0.0-20220913142420-e25cf57ea46d github.com/openshift/client-go v0.0.0-20230120202327-72f107311084 github.com/openshift/library-go v0.0.0-20230130121854-13dc1e57ef79 diff --git a/go.sum b/go.sum index 100b3d625..2bbb0c603 100644 --- a/go.sum +++ b/go.sum @@ -937,8 +937,8 @@ github.com/op/go-logging v0.0.0-20160315200505-970db520ece7/go.mod h1:HzydrMdWEr github.com/opencontainers/go-digest v1.0.0-rc1/go.mod h1:cMLVZDEM3+U2I4VmLI6N8jQYUd2OVphdqWwCJHrFt2s= github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM= github.com/opencontainers/image-spec v1.0.1/go.mod h1:BtxoFyWECRxE4U/7sNtV5W15zMzWCbyJoFRP3s7yZA0= -github.com/openshift/api v0.0.0-20230406180441-6bc7296cd4fa h1:2oUaQNfxhgX+3lq49YkshMWR4MbX9N8OEDevUCyEJ2I= -github.com/openshift/api v0.0.0-20230406180441-6bc7296cd4fa/go.mod h1:ctXNyWanKEjGj8sss1KjjHQ3ENKFm33FFnS5BKaIPh4= +github.com/openshift/api v0.0.0-20230414143018-3367bc7e6ac7 h1:rncLxJBpFGqBztyxCMwNRnMjhhIDOWHJowi6q8G6koI= +github.com/openshift/api v0.0.0-20230414143018-3367bc7e6ac7/go.mod h1:ctXNyWanKEjGj8sss1KjjHQ3ENKFm33FFnS5BKaIPh4= github.com/openshift/build-machinery-go v0.0.0-20220913142420-e25cf57ea46d h1:RR4ah7FfaPR1WePizm0jlrsbmPu91xQZnAsVVreQV1k= github.com/openshift/build-machinery-go v0.0.0-20220913142420-e25cf57ea46d/go.mod h1:b1BuldmJlbA/xYtdZvKi+7j5YGB44qJUJDZ9zwiNCfE= github.com/openshift/client-go v0.0.0-20230120202327-72f107311084 h1:66uaqNwA+qYyQDwsMWUfjjau8ezmg1dzCqub13KZOcE= diff --git a/vendor/github.com/openshift/api/operator/v1/0000_50_cluster_storage_operator_01_crd.yaml b/vendor/github.com/openshift/api/operator/v1/0000_50_cluster_storage_operator_01_crd.yaml index 08e011b11..3a07026f2 100644 --- a/vendor/github.com/openshift/api/operator/v1/0000_50_cluster_storage_operator_01_crd.yaml +++ b/vendor/github.com/openshift/api/operator/v1/0000_50_cluster_storage_operator_01_crd.yaml @@ -70,7 +70,7 @@ spec: nullable: true x-kubernetes-preserve-unknown-fields: true vsphereStorageDriver: - description: 'VSphereStorageDriver indicates the storage driver to use on VSphere clusters. Once this field is set to CSIWithMigrationDriver, it can not be changed. If this is empty, the platform will choose a good default, which may change over time without notice. DEPRECATED: This field will be removed in a future release.' + description: 'VSphereStorageDriver indicates the storage driver to use on VSphere clusters. Once this field is set to CSIWithMigrationDriver, it can not be changed. If this is empty, the platform will choose a good default, which may change over time without notice. The current default is LegacyDeprecatedInTreeDriver. DEPRECATED: This field will be removed in a future release.' type: string enum: - "" diff --git a/vendor/github.com/openshift/api/operator/v1/types_storage.go b/vendor/github.com/openshift/api/operator/v1/types_storage.go index aaf011542..271d5c033 100644 --- a/vendor/github.com/openshift/api/operator/v1/types_storage.go +++ b/vendor/github.com/openshift/api/operator/v1/types_storage.go @@ -44,6 +44,7 @@ type StorageSpec struct { // Once this field is set to CSIWithMigrationDriver, it can not be changed. // If this is empty, the platform will choose a good default, // which may change over time without notice. + // The current default is LegacyDeprecatedInTreeDriver. // DEPRECATED: This field will be removed in a future release. // +kubebuilder:validation:XValidation:rule="oldSelf != \"CSIWithMigrationDriver\" || self == \"CSIWithMigrationDriver\"",message="VSphereStorageDriver can not be changed once it is set to CSIWithMigrationDriver" // +optional diff --git a/vendor/github.com/openshift/api/operator/v1/zz_generated.swagger_doc_generated.go b/vendor/github.com/openshift/api/operator/v1/zz_generated.swagger_doc_generated.go index 1248ffb1b..727344550 100644 --- a/vendor/github.com/openshift/api/operator/v1/zz_generated.swagger_doc_generated.go +++ b/vendor/github.com/openshift/api/operator/v1/zz_generated.swagger_doc_generated.go @@ -1611,7 +1611,7 @@ func (StorageList) SwaggerDoc() map[string]string { var map_StorageSpec = map[string]string{ "": "StorageSpec is the specification of the desired behavior of the cluster storage operator.", - "vsphereStorageDriver": "VSphereStorageDriver indicates the storage driver to use on VSphere clusters. Once this field is set to CSIWithMigrationDriver, it can not be changed. If this is empty, the platform will choose a good default, which may change over time without notice. DEPRECATED: This field will be removed in a future release.", + "vsphereStorageDriver": "VSphereStorageDriver indicates the storage driver to use on VSphere clusters. Once this field is set to CSIWithMigrationDriver, it can not be changed. If this is empty, the platform will choose a good default, which may change over time without notice. The current default is LegacyDeprecatedInTreeDriver. DEPRECATED: This field will be removed in a future release.", } func (StorageSpec) SwaggerDoc() map[string]string { diff --git a/vendor/modules.txt b/vendor/modules.txt index b94aa2866..50820dc99 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -154,7 +154,7 @@ github.com/modern-go/reflect2 # github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 ## explicit github.com/munnerz/goautoneg -# github.com/openshift/api v0.0.0-20230406180441-6bc7296cd4fa +# github.com/openshift/api v0.0.0-20230414143018-3367bc7e6ac7 ## explicit; go 1.19 github.com/openshift/api github.com/openshift/api/apiserver From 781b2c79eaae1040b3767c8d2887993761f8cde0 Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Wed, 12 Apr 2023 13:07:03 -0400 Subject: [PATCH 7/7] Add permission to read csinode objects --- assets/vsphere_problem_detector/04_clusterrole.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/assets/vsphere_problem_detector/04_clusterrole.yaml b/assets/vsphere_problem_detector/04_clusterrole.yaml index c58af7157..c0887d622 100644 --- a/assets/vsphere_problem_detector/04_clusterrole.yaml +++ b/assets/vsphere_problem_detector/04_clusterrole.yaml @@ -78,6 +78,7 @@ rules: - storage.k8s.io resources: - storageclasses + - csinodes verbs: - get - list