From 932d8c30ce226e912cd401bde1dee956c54957b8 Mon Sep 17 00:00:00 2001 From: Akshay Saini <109056238+AkshaySainiDell@users.noreply.github.com> Date: Tue, 14 May 2024 17:54:02 +0530 Subject: [PATCH] Add support for Authorization upgrade (#549) * Add support for authorization upgrade * Add upgrade-path.yaml for authorization proxy server * Add min upgrade path support for authorization proxy server * Fix status loggers * Fix linting issues * Fix linting issues * Fix authorization pre-checks * Update checkUpgrade for authorization * Update checkUpgrade for authorization * Fix controller UT * Add test scenarios for authorization sidecar upgrade to e2e * Fix module UT failure * Increase code coverage of controller pkg * Increase code coverage of controller pkg * Address review comments --------- Co-authored-by: Jooseppi Luna --- api/v1/csm_types.go | 20 +++ api/v1/types.go | 5 + controllers/csm_controller.go | 69 +++----- controllers/csm_controller_test.go | 31 +++- .../authorization/v1.10.0/upgrade-path.yaml | 2 + .../authorization/v1.8.0/upgrade-path.yaml | 1 + .../authorization/v1.9.0/upgrade-path.yaml | 2 + .../authorization/v1.9.1/upgrade-path.yaml | 2 + pkg/modules/authorization.go | 2 +- pkg/utils/status.go | 39 ++--- pkg/utils/utils.go | 74 ++++++++- tests/e2e/steps/steps_def.go | 92 +++++++---- tests/e2e/steps/steps_runner.go | 6 +- .../storage_csm_powerflex_auth_n_minus_1.yaml | 151 ++++++++++++++++++ tests/e2e/testfiles/values.yaml | 42 +++++ tests/shared/common.go | 1 + 16 files changed, 430 insertions(+), 109 deletions(-) create mode 100644 operatorconfig/moduleconfig/authorization/v1.10.0/upgrade-path.yaml create mode 100644 operatorconfig/moduleconfig/authorization/v1.8.0/upgrade-path.yaml create mode 100644 operatorconfig/moduleconfig/authorization/v1.9.0/upgrade-path.yaml create mode 100644 operatorconfig/moduleconfig/authorization/v1.9.1/upgrade-path.yaml create mode 100644 tests/e2e/testfiles/storage_csm_powerflex_auth_n_minus_1.yaml diff --git a/api/v1/csm_types.go b/api/v1/csm_types.go index 421bfc058..9afcfc4bc 100644 --- a/api/v1/csm_types.go +++ b/api/v1/csm_types.go @@ -104,6 +104,26 @@ func (cr *ContainerStorageModule) GetDriverType() DriverType { return cr.Spec.Driver.CSIDriverType } +// GetModule - Returns the module of type moduleType +func (cr *ContainerStorageModule) GetModule(moduleType ModuleType) Module { + for _, m := range cr.Spec.Modules { + if m.Name == moduleType { + return m + } + } + return Module{} +} + +// HasModule - Returns true if the cr has a module of type moduleType +func (cr *ContainerStorageModule) HasModule(moduleType ModuleType) bool { + for _, m := range cr.Spec.Modules { + if m.Name == moduleType { + return true + } + } + return false +} + // IsBeingDeleted - Returns true if a deletion timestamp is set func (cr *ContainerStorageModule) IsBeingDeleted() bool { return !cr.ObjectMeta.DeletionTimestamp.IsZero() diff --git a/api/v1/types.go b/api/v1/types.go index 0e98f43b8..e32e921e7 100644 --- a/api/v1/types.go +++ b/api/v1/types.go @@ -31,6 +31,11 @@ type DriverType string // ModuleType - type representing the type of the modules. e.g. - authorization, podmon type ModuleType string +// CSMComponentType - type constraint for DriverType and ModuleType +type CSMComponentType interface { + ModuleType | DriverType +} + // ObservabilityComponentType - type representing the type of components inside observability module. e.g. - topology type ObservabilityComponentType string diff --git a/controllers/csm_controller.go b/controllers/csm_controller.go index acd1b6c82..855515cf4 100644 --- a/controllers/csm_controller.go +++ b/controllers/csm_controller.go @@ -328,10 +328,8 @@ func (r *ContainerStorageModuleReconciler) Reconcile(ctx context.Context, req ct } newStatus := csm.GetCSMStatus() - requeue, err := utils.HandleSuccess(ctx, csm, r, newStatus, oldStatus) - if err != nil { - log.Error(err, "Failed to update CR status") - } + requeue := utils.HandleSuccess(ctx, csm, r, newStatus, oldStatus) + // Update the driver syncErr := r.SyncCSM(ctx, *csm, *operatorConfig, r.Client) if syncErr == nil && !requeue.Requeue { @@ -1248,7 +1246,6 @@ func (r *ContainerStorageModuleReconciler) removeModule(ctx context.Context, ins // PreChecks - validate input values func (r *ContainerStorageModuleReconciler) PreChecks(ctx context.Context, cr *csmv1.ContainerStorageModule, operatorConfig utils.OperatorConfig) error { log := logger.GetLogger(ctx) - var am bool // Check drivers switch cr.Spec.Driver.CSIDriverType { case csmv1.PowerScale: @@ -1278,17 +1275,11 @@ func (r *ContainerStorageModuleReconciler) PreChecks(ctx context.Context, cr *cs return fmt.Errorf("failed powermax validation: %v", err) } default: - for _, m := range cr.Spec.Modules { - if m.Name == csmv1.AuthorizationServer { - return nil - } - if m.Name == csmv1.ApplicationMobility { - am = true - } - } - if am { + // Go to checkUpgrade if it is standalone module i.e. app mobility or authorizatio proxy server + if cr.HasModule(csmv1.ApplicationMobility) || cr.HasModule(csmv1.AuthorizationServer) { break } + return fmt.Errorf("unsupported driver type %s", cr.Spec.Driver.CSIDriverType) } @@ -1368,40 +1359,25 @@ func (r *ContainerStorageModuleReconciler) PreChecks(ctx context.Context, cr *cs // Check for upgrade/if upgrade is appropriate func checkUpgrade(ctx context.Context, cr *csmv1.ContainerStorageModule, operatorConfig utils.OperatorConfig) (bool, error) { log := logger.GetLogger(ctx) - driverType := cr.Spec.Driver.CSIDriverType - if driverType == csmv1.PowerScale { - // use powerscale instead of isilon as the folder name is powerscale - driverType = csmv1.PowerScaleName - } // If it is an upgrade/downgrade, check to see if we meet the minimum version using GetUpgradeInfo, which returns the minimum version required // for the desired upgrade. If the upgrade path is not valid fail // Existing version annotations := cr.GetAnnotations() oldVersion, configVersionExists := annotations[configVersionKey] - // If annotation exists, we are doing an upgrade or modify if configVersionExists { - // if versions are equal, it is a modification - if oldVersion == cr.Spec.Driver.ConfigVersion { - log.Infow("proceeding with modification of driver install") - return true, nil - } - // if not equal, it is an upgrade/downgrade - // get minimum required version for upgrade - minUpgradePath, err := drivers.GetUpgradeInfo(ctx, operatorConfig, driverType, oldVersion) - if err != nil { - log.Infow("GetUpgradeInfo not successful") - return false, err + if cr.HasModule(csmv1.AuthorizationServer) { + newVersion := cr.GetModule(csmv1.AuthorizationServer).ConfigVersion + return utils.IsValidUpgrade(ctx, oldVersion, newVersion, csmv1.Authorization, operatorConfig) } - // - installValid, _ := utils.MinVersionCheck(minUpgradePath, cr.Spec.Driver.ConfigVersion) - if installValid { - log.Infow("proceeding with valid driver upgrade from version %s to version %s", oldVersion, cr.Spec.Driver.ConfigVersion) - return installValid, nil + driverType := cr.Spec.Driver.CSIDriverType + if driverType == csmv1.PowerScale { + // use powerscale instead of isilon as the folder name is powerscale + driverType = csmv1.PowerScaleName } - log.Infow("not proceeding with invalid driver upgrade") - return installValid, fmt.Errorf("failed upgrade check: upgrade from version %s to %s not valid", oldVersion, cr.Spec.Driver.ConfigVersion) + newVersion := cr.Spec.Driver.ConfigVersion + return utils.IsValidUpgrade(ctx, oldVersion, newVersion, driverType, operatorConfig) } log.Infow("proceeding with fresh driver install") return true, nil @@ -1411,7 +1387,7 @@ func checkUpgrade(ctx context.Context, cr *csmv1.ContainerStorageModule, operato func applyConfigVersionAnnotations(ctx context.Context, instance *csmv1.ContainerStorageModule) bool { log := logger.GetLogger(ctx) - // If driver has not been initialized yet, we first annotate the driver with the config version annotation + // If driver/module has not been initialized yet, we first annotate the component with the config version annotation annotations := instance.GetAnnotations() isUpdated := false @@ -1419,15 +1395,20 @@ func applyConfigVersionAnnotations(ctx context.Context, instance *csmv1.Containe annotations = make(map[string]string) } annotations[CSMVersionKey] = CSMVersion - instance.SetAnnotations(annotations) if _, ok := annotations[configVersionKey]; !ok { - annotations[configVersionKey] = instance.Spec.Driver.ConfigVersion + configVersion := "" + if instance.HasModule(csmv1.AuthorizationServer) { + configVersion = instance.GetModule(csmv1.AuthorizationServer).ConfigVersion + } else { + configVersion = instance.Spec.Driver.ConfigVersion + } + annotations[configVersionKey] = configVersion isUpdated = true - instance.SetAnnotations(annotations) - log.Infof("Installing storage component %s with config Version %s. Updating Annotations with Config Version", - instance.GetName(), instance.Spec.Driver.ConfigVersion) + log.Infof("Installing csm component %s with config Version %s. Updating Annotations with Config Version", + instance.GetName(), configVersion) } + instance.SetAnnotations(annotations) return isUpdated } diff --git a/controllers/csm_controller_test.go b/controllers/csm_controller_test.go index 42b944a22..a545a6f7d 100644 --- a/controllers/csm_controller_test.go +++ b/controllers/csm_controller_test.go @@ -192,6 +192,13 @@ func (suite *CSMControllerTestSuite) TestAuthorizationServerReconcile() { suite.runFakeAuthCSMManager("", true) } +func (suite *CSMControllerTestSuite) TestAuthorizationServerPreCheck() { + suite.makeFakeAuthServerCSMWithoutPreRequisite(csmName, suite.namespace) + suite.runFakeAuthCSMManager("failed authorization proxy server validation", false) + suite.deleteCSM(csmName) + suite.runFakeAuthCSMManager("", true) +} + func (suite *CSMControllerTestSuite) TestAppMobReconcile() { suite.makeFakeAppMobCSM(csmName, suite.namespace, getAppMob()) suite.runFakeAuthCSMManager("", false) @@ -416,10 +423,12 @@ func (suite *CSMControllerTestSuite) TestCsmUpgradeSkipVersion() { annotations = make(map[string]string) } if _, ok := annotations[configVersionKey]; !ok { - annotations[configVersionKey] = jumpUpgradeConfigVersion + annotations[configVersionKey] = configVersion csm.SetAnnotations(annotations) } + csm.Spec.Driver.ConfigVersion = jumpUpgradeConfigVersion + reconciler := suite.createReconciler() _, err := reconciler.Reconcile(ctx, req) assert.Error(suite.T(), err) @@ -1693,30 +1702,40 @@ func (suite *CSMControllerTestSuite) makeFakeAppMobCSM(name, ns string, _ []csmv func (suite *CSMControllerTestSuite) makeFakeAuthServerCSM(name, ns string, _ []csmv1.Module) { // this secret required by authorization module - sec := shared.MakeSecret("karavi-config-secret", ns, configVersion) + sec := shared.MakeSecret("karavi-config-secret", ns, shared.AuthServerConfigVersion) err := suite.fakeClient.Create(ctx, sec) assert.Nil(suite.T(), err) // this secret required by authorization module - sec = shared.MakeSecret("proxy-storage-secret", ns, configVersion) + sec = shared.MakeSecret("karavi-storage-secret", ns, shared.AuthServerConfigVersion) err = suite.fakeClient.Create(ctx, sec) assert.Nil(suite.T(), err) // this secret required by authorization module - sec = shared.MakeSecret("karavi-auth-tls", ns, configVersion) + sec = shared.MakeSecret("karavi-auth-tls", ns, shared.AuthServerConfigVersion) err = suite.fakeClient.Create(ctx, sec) assert.Nil(suite.T(), err) - csm := shared.MakeModuleCSM(name, ns, configVersion) + csm := shared.MakeModuleCSM(name, ns, shared.AuthServerConfigVersion) csm.Spec.Modules = getAuthProxyServer() csm.Spec.Modules[0].ForceRemoveModule = true - csm.Annotations[configVersionKey] = configVersion err = suite.fakeClient.Create(ctx, &csm) assert.Nil(suite.T(), err) } +func (suite *CSMControllerTestSuite) makeFakeAuthServerCSMWithoutPreRequisite(name, ns string) { + csm := shared.MakeModuleCSM(name, ns, shared.AuthServerConfigVersion) + + csm.Spec.Modules = getAuthProxyServer() + csm.Spec.Modules[0].ForceRemoveModule = true + csm.Annotations[configVersionKey] = shared.AuthServerConfigVersion + + err := suite.fakeClient.Create(ctx, &csm) + assert.Nil(suite.T(), err) +} + func (suite *CSMControllerTestSuite) makeFakePod(name, ns string) { pod := shared.MakePod(name, ns) pod.Labels["csm"] = csmName diff --git a/operatorconfig/moduleconfig/authorization/v1.10.0/upgrade-path.yaml b/operatorconfig/moduleconfig/authorization/v1.10.0/upgrade-path.yaml new file mode 100644 index 000000000..b1e8ff46a --- /dev/null +++ b/operatorconfig/moduleconfig/authorization/v1.10.0/upgrade-path.yaml @@ -0,0 +1,2 @@ + +minUpgradePath: v1.9.0 diff --git a/operatorconfig/moduleconfig/authorization/v1.8.0/upgrade-path.yaml b/operatorconfig/moduleconfig/authorization/v1.8.0/upgrade-path.yaml new file mode 100644 index 000000000..be786dc83 --- /dev/null +++ b/operatorconfig/moduleconfig/authorization/v1.8.0/upgrade-path.yaml @@ -0,0 +1 @@ +minUpgradePath: v1.7.0 \ No newline at end of file diff --git a/operatorconfig/moduleconfig/authorization/v1.9.0/upgrade-path.yaml b/operatorconfig/moduleconfig/authorization/v1.9.0/upgrade-path.yaml new file mode 100644 index 000000000..c4ea00961 --- /dev/null +++ b/operatorconfig/moduleconfig/authorization/v1.9.0/upgrade-path.yaml @@ -0,0 +1,2 @@ + +minUpgradePath: v1.8.0 diff --git a/operatorconfig/moduleconfig/authorization/v1.9.1/upgrade-path.yaml b/operatorconfig/moduleconfig/authorization/v1.9.1/upgrade-path.yaml new file mode 100644 index 000000000..c4ea00961 --- /dev/null +++ b/operatorconfig/moduleconfig/authorization/v1.9.1/upgrade-path.yaml @@ -0,0 +1,2 @@ + +minUpgradePath: v1.8.0 diff --git a/pkg/modules/authorization.go b/pkg/modules/authorization.go index 6be04d62f..b826b2a32 100644 --- a/pkg/modules/authorization.go +++ b/pkg/modules/authorization.go @@ -447,7 +447,7 @@ func AuthorizationServerPrecheck(ctx context.Context, op utils.OperatorConfig, a } // Check for secrets - proxyServerSecrets := []string{"karavi-config-secret", "karavi-storage-secret", "karavi-auth-tls"} + proxyServerSecrets := []string{"karavi-config-secret", "karavi-storage-secret"} for _, name := range proxyServerSecrets { found := &corev1.Secret{} err := r.GetClient().Get(ctx, types.NamespacedName{Name: name, Namespace: cr.GetNamespace()}, found) diff --git a/pkg/utils/status.go b/pkg/utils/status.go index ac35ac932..d8b05ee15 100644 --- a/pkg/utils/status.go +++ b/pkg/utils/status.go @@ -522,7 +522,7 @@ func HandleAccValidationError(ctx context.Context, instance *csmv1.ApexConnectiv } // HandleSuccess for csm -func HandleSuccess(ctx context.Context, instance *csmv1.ContainerStorageModule, r ReconcileCSM, newStatus, oldStatus *csmv1.ContainerStorageModuleStatus) (reconcile.Result, error) { +func HandleSuccess(ctx context.Context, instance *csmv1.ContainerStorageModule, r ReconcileCSM, newStatus, oldStatus *csmv1.ContainerStorageModuleStatus) reconcile.Result { dMutex.Lock() defer dMutex.Unlock() @@ -556,9 +556,10 @@ func HandleSuccess(ctx context.Context, instance *csmv1.ContainerStorageModule, } else { log.Info("HandleSuccess Driver state changed to Succeeded") } - return requeue, nil + return requeue } - return LogBannerAndReturn(requeue, nil) + requeue, _ = LogBannerAndReturn(requeue, nil) + return requeue } // HandleAccSuccess for csm @@ -807,21 +808,21 @@ func observabilityStatusCheck(ctx context.Context, instance *csmv1.ContainerStor case "otel-collector": if otelEnabled { if !checkFn(&deployment) { - log.Info("%s component not running in observability deployment", deployment.Name) + log.Infof("%s component not running in observability deployment", deployment.Name) return false, nil } } case fmt.Sprintf("%s-metrics-%s", ObservabilityNamespace, driverName): if metricsEnabled { if !checkFn(&deployment) { - log.Info("%s component not running in observability deployment", deployment.Name) + log.Infof("%s component not running in observability deployment", deployment.Name) return false, nil } } case fmt.Sprintf("%s-topology", ObservabilityNamespace): if topologyEnabled { if !checkFn(&deployment) { - log.Info("%s component not running in observability deployment", deployment.Name) + log.Infof("%s component not running in observability deployment", deployment.Name) return false, nil } } @@ -845,21 +846,21 @@ func observabilityStatusCheck(ctx context.Context, instance *csmv1.ContainerStor case "cert-manager": if certEnabled { if !checkFn(&deployment) { - log.Info("%s component not running in observability deployment", deployment.Name) + log.Infof("%s component not running in observability deployment", deployment.Name) return false, nil } } case "cert-manager-cainjector": if certEnabled { if !checkFn(&deployment) { - log.Info("%s component not running in observability deployment", deployment.Name) + log.Infof("%s component not running in observability deployment", deployment.Name) return false, nil } } case "cert-manager-webhook": if certEnabled { if !checkFn(&deployment) { - log.Info("%s component not running in observability deployment", deployment.Name) + log.Infof("%s component not running in observability deployment", deployment.Name) return false, nil } } @@ -909,59 +910,59 @@ func authProxyStatusCheck(ctx context.Context, instance *csmv1.ContainerStorageM case fmt.Sprintf("%s-ingress-nginx-controller", authNamespace): if nginxEnabled { if !checkFn(&deployment) { - log.Info("%s component not running in auth proxy deployment", deployment.Name) + log.Infof("%s component not running in auth proxy deployment", deployment.Name) return false, nil } } case "cert-manager": if certEnabled { if !checkFn(&deployment) { - log.Info("%s component not running in auth proxy deployment", deployment.Name) + log.Infof("%s component not running in auth proxy deployment", deployment.Name) return false, nil } } case "cert-manager-cainjector": if certEnabled { if !checkFn(&deployment) { - log.Info("%s component not running in auth proxy deployment", deployment.Name) + log.Infof("%s component not running in auth proxy deployment", deployment.Name) return false, nil } } case "cert-manager-webhook": if certEnabled { if !checkFn(&deployment) { - log.Info("%s component not running in auth proxy deployment", deployment.Name) + log.Infof("%s component not running in auth proxy deployment", deployment.Name) return false, nil } } case "proxy-server": if !checkFn(&deployment) { - log.Info("%s component not running in auth proxy deployment", deployment.Name) + log.Infof("%s component not running in auth proxy deployment", deployment.Name) return false, nil } case "redis-commander": if !checkFn(&deployment) { - log.Info("%s component not running in auth proxy deployment", deployment.Name) + log.Infof("%s component not running in auth proxy deployment", deployment.Name) return false, nil } case "redis-primary": if !checkFn(&deployment) { - log.Info("%s component not running in auth proxy deployment", deployment.Name) + log.Infof("%s component not running in auth proxy deployment", deployment.Name) return false, nil } case "role-service": if !checkFn(&deployment) { - log.Info("%s component not running in auth proxy deployment", deployment.Name) + log.Infof("%s component not running in auth proxy deployment", deployment.Name) return false, nil } case "storage-service": if !checkFn(&deployment) { - log.Info("%s component not running in auth proxy deployment", deployment.Name) + log.Infof("%s component not running in auth proxy deployment", deployment.Name) return false, nil } case "tenant-service": if !checkFn(&deployment) { - log.Info("%s component not running in auth proxy deployment", deployment.Name) + log.Infof("%s component not running in auth proxy deployment", deployment.Name) return false, nil } } diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index d06e40cff..ccbc62a3f 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -15,6 +15,7 @@ package utils import ( "bytes" "context" + "encoding/json" "fmt" "io" "os" @@ -744,7 +745,8 @@ func ApplyObject(ctx context.Context, obj crclient.Object, ctrlClient crclient.C kind := obj.GetObjectKind().GroupVersionKind().Kind name := obj.GetName() - err := ctrlClient.Get(ctx, t1.NamespacedName{Name: name, Namespace: obj.GetNamespace()}, obj) + k8sObj := obj.DeepCopyObject().(crclient.Object) + err := ctrlClient.Get(ctx, t1.NamespacedName{Name: name, Namespace: obj.GetNamespace()}, k8sObj) if err != nil && k8serror.IsNotFound(err) { log.Infow("Creating a new Object", "Name:", name, "Kind:", kind) @@ -758,8 +760,16 @@ func ApplyObject(ctx context.Context, obj crclient.Object, ctrlClient crclient.C return err } else { log.Infow("Updating a new Object", "Name:", name, "Kind:", kind) + // Copy data/changes from obj to k8s object that already exists on the cluster + if jsonBytes, err := json.Marshal(obj); err == nil { + if err := json.Unmarshal(jsonBytes, &k8sObj); err == nil { + obj = k8sObj + } + } err = ctrlClient.Update(ctx, obj) - if err != nil { + if err != nil && k8serror.IsForbidden(err) || k8serror.IsInvalid(err) { + log.Warnw("Object update failed", "Warning", err.Error()) + } else if err != nil { return err } } @@ -1105,3 +1115,63 @@ func DetermineUnitTestRun(ctx context.Context) bool { } return unitTestRun } + +// IsValidUpgrade will check if upgrade of module/driver is allowed +func IsValidUpgrade[T csmv1.CSMComponentType](ctx context.Context, oldVersion, newVersion string, csmComponentType T, operatorConfig OperatorConfig) (bool, error) { + log := logger.GetLogger(ctx) + + // if versions are equal, it is a modification + if oldVersion == newVersion { + log.Infow("proceeding with modification of driver/module install") + return true, nil + } + + // if not equal, it is an upgrade/downgrade + // get minimum required version for upgrade + minUpgradePath, err := getUpgradeInfo(ctx, operatorConfig, csmComponentType, newVersion) + if err != nil { + log.Infow("getUpgradeInfo not successful") + return false, err + } + + isUpgradeValid, _ := MinVersionCheck(minUpgradePath, oldVersion) + if isUpgradeValid { + log.Infof("proceeding with valid upgrade of %s from version %s to version %s", csmComponentType, oldVersion, newVersion) + return isUpgradeValid, nil + } + + log.Infof("not proceeding with invalid driver/module upgrade") + return isUpgradeValid, fmt.Errorf("upgrade of %s from version %s to %s not valid", csmComponentType, oldVersion, newVersion) +} + +func getUpgradeInfo[T csmv1.CSMComponentType](ctx context.Context, operatorConfig OperatorConfig, csmCompType T, oldVersion string) (string, error) { + log := logger.GetLogger(ctx) + + csmCompConfigDir := "" + switch any(csmCompType).(type) { + case csmv1.DriverType: + csmCompConfigDir = "driverconfig" + case csmv1.ModuleType: + csmCompConfigDir = "moduleconfig" + } + + upgradeInfoPath := fmt.Sprintf("%s/%s/%s/%s/upgrade-path.yaml", operatorConfig.ConfigDirectory, csmCompConfigDir, csmCompType, oldVersion) + log.Debugw("getUpgradeInfo", "upgradeInfoPath", upgradeInfoPath) + + buf, err := os.ReadFile(filepath.Clean(upgradeInfoPath)) + if err != nil { + log.Errorw("getUpgradeInfo failed", "Error", err.Error()) + return "", err + } + YamlString := string(buf) + + var upgradePath UpgradePaths + err = yaml.Unmarshal([]byte(YamlString), &upgradePath) + if err != nil { + log.Errorw("getUpgradeInfo yaml marshall failed", "Error", err.Error()) + return "", err + } + + // Example return value: "v2.2.0" + return upgradePath.MinUpgradePath, nil +} diff --git a/tests/e2e/steps/steps_def.go b/tests/e2e/steps/steps_def.go index 3b5356cb5..cd67855e4 100644 --- a/tests/e2e/steps/steps_def.go +++ b/tests/e2e/steps/steps_def.go @@ -128,6 +128,26 @@ func (step *Step) applyCustomResource(res Resource, crNumStr string) error { return nil } +func (step *Step) upgradeCustomResource(res Resource, oldCrNumStr, newCrNumStr string) error { + oldCrNum, _ := strconv.Atoi(oldCrNumStr) + oldCr := res.CustomResource[oldCrNum-1] + + newCrNum, _ := strconv.Atoi(newCrNumStr) + newCr := res.CustomResource[newCrNum-1] + + found := new(csmv1.ContainerStorageModule) + if err := step.ctrlClient.Get(context.TODO(), client.ObjectKey{ + Namespace: oldCr.Namespace, + Name: oldCr.Name, + }, found); err != nil { + return err + } + + // Update old CR with the spec of new CR + found.Spec = newCr.Spec + return step.ctrlClient.Update(context.TODO(), found) +} + func (step *Step) installThirdPartyModule(res Resource, thirdPartyModule string) error { if thirdPartyModule == "cert-manager" { cmd := exec.Command("kubectl", "apply", "-f", "testfiles/cert-manager-crds.yaml") @@ -166,7 +186,7 @@ func (step *Step) installThirdPartyModule(res Resource, thirdPartyModule string) return err } - //give wp time to setup before we create backup/restores + // give wp time to setup before we create backup/restores fmt.Println("Sleeping 60 seconds to allow WP time to create") time.Sleep(60 * time.Second) @@ -205,7 +225,8 @@ func (step *Step) deleteCustomResource(res Resource, crNumStr string) error { found := new(csmv1.ContainerStorageModule) err := step.ctrlClient.Get(context.TODO(), client.ObjectKey{ Namespace: cr.Namespace, - Name: cr.Name}, found, + Name: cr.Name, + }, found, ) if err != nil { if errors.IsNotFound(err) { @@ -222,7 +243,8 @@ func (step *Step) validateCustomResourceStatus(res Resource, crNumStr string) er found := new(csmv1.ContainerStorageModule) err := step.ctrlClient.Get(context.TODO(), client.ObjectKey{ Namespace: cr.Namespace, - Name: cr.Name}, found) + Name: cr.Name, + }, found) if err != nil { return err } @@ -269,7 +291,8 @@ func (step *Step) validateModuleInstalled(res Resource, module string, crNumStr found := new(csmv1.ContainerStorageModule) if err := step.ctrlClient.Get(context.TODO(), client.ObjectKey{ Namespace: cr.Namespace, - Name: cr.Name}, found, + Name: cr.Name, + }, found, ); err != nil { return err } @@ -312,7 +335,8 @@ func (step *Step) validateModuleNotInstalled(res Resource, module string, crNumS found := new(csmv1.ContainerStorageModule) if err := step.ctrlClient.Get(context.TODO(), client.ObjectKey{ Namespace: cr.Namespace, - Name: cr.Name}, found, + Name: cr.Name, + }, found, ); err != nil { return err } @@ -350,7 +374,8 @@ func (step *Step) validateObservabilityInstalled(cr csmv1.ContainerStorageModule instance := new(csmv1.ContainerStorageModule) if err := step.ctrlClient.Get(context.TODO(), client.ObjectKey{ Namespace: cr.Namespace, - Name: cr.Name}, instance, + Name: cr.Name, + }, instance, ); err != nil { return err } @@ -426,7 +451,8 @@ func (step *Step) validateReplicationInstalled(cr csmv1.ContainerStorageModule) // cluster role clusterRole := &rbacv1.ClusterRole{} err = step.ctrlClient.Get(context.TODO(), types.NamespacedName{ - Name: fmt.Sprintf("%s-controller", cr.Name)}, clusterRole) + Name: fmt.Sprintf("%s-controller", cr.Name), + }, clusterRole) if err != nil { return err } @@ -478,7 +504,6 @@ func (step *Step) validateReplicationNotInstalled(cr csmv1.ContainerStorageModul // check no driver is not installed in target clusters if cluster.ClusterID != utils.DefaultSourceClusterID { - if err := checkNoRunningPods(context.TODO(), cr.Namespace, cluster.ClusterK8sClient); err != nil { return fmt.Errorf("failed replica installation check %s: %v", cluster.ClusterID, err) } @@ -523,14 +548,12 @@ func (step *Step) validateAuthorizationNotInstalled(cr csmv1.ContainerStorageMod if *cnt.Name == authString { return fmt.Errorf("found authorization in deployment: %v", err) } - } for _, cnt := range dsApply.Spec.Template.Spec.Containers { if *cnt.Name == authString { return fmt.Errorf("found authorization in daemonset: %v", err) } - } return nil @@ -581,7 +604,6 @@ func (step *Step) setupSecretFromFile(res Resource, file, namespace string) erro } func (step *Step) setUpSecret(res Resource, templateFile, name, namespace, crType string) error { - // find which map to use for secret values mapValues, err := determineMap(crType) if err != nil { @@ -595,7 +617,7 @@ func (step *Step) setUpSecret(res Resource, templateFile, name, namespace, crTyp } } - //if secret exists- delete it + // if secret exists- delete it if secretExists(namespace, name) { cmd := exec.Command("kubectl", "delete", "secret", "-n", namespace, name) err := cmd.Run() @@ -672,7 +694,6 @@ func replaceInFile(old, new, templateFile string) error { return fmt.Errorf("failed to substitute %s with %s in file %s: %s", old, new, templateFile, err.Error()) } return nil - } func (step *Step) runCustomTest(res Resource) error { @@ -704,7 +725,8 @@ func (step *Step) enableModule(res Resource, module string, crNumStr string) err found := new(csmv1.ContainerStorageModule) if err := step.ctrlClient.Get(context.TODO(), client.ObjectKey{ Namespace: cr.Namespace, - Name: cr.Name}, found, + Name: cr.Name, + }, found, ); err != nil { return err } @@ -712,7 +734,7 @@ func (step *Step) enableModule(res Resource, module string, crNumStr string) err for i, m := range found.Spec.Modules { if !m.Enabled && m.Name == csmv1.ModuleType(module) { found.Spec.Modules[i].Enabled = true - //for observability, enable all components + // for observability, enable all components if m.Name == csmv1.Observability { for j := range m.Components { found.Spec.Modules[i].Components[j].Enabled = pointer.Bool(true) @@ -730,7 +752,8 @@ func (step *Step) setDriverSecret(res Resource, crNumStr string, driverSecretNam found := new(csmv1.ContainerStorageModule) if err := step.ctrlClient.Get(context.TODO(), client.ObjectKey{ Namespace: cr.Namespace, - Name: cr.Name}, found, + Name: cr.Name, + }, found, ); err != nil { return err } @@ -744,7 +767,8 @@ func (step *Step) disableModule(res Resource, module string, crNumStr string) er found := new(csmv1.ContainerStorageModule) if err := step.ctrlClient.Get(context.TODO(), client.ObjectKey{ Namespace: cr.Namespace, - Name: cr.Name}, found, + Name: cr.Name, + }, found, ); err != nil { return err } @@ -770,7 +794,8 @@ func (step *Step) enableForceRemoveDriver(res Resource, crNumStr string) error { found := new(csmv1.ContainerStorageModule) if err := step.ctrlClient.Get(context.TODO(), client.ObjectKey{ Namespace: cr.Namespace, - Name: cr.Name}, found, + Name: cr.Name, + }, found, ); err != nil { return err } @@ -785,7 +810,8 @@ func (step *Step) enableForceRemoveModule(res Resource, crNumStr string) error { found := new(csmv1.ContainerStorageModule) if err := step.ctrlClient.Get(context.TODO(), client.ObjectKey{ Namespace: cr.Namespace, - Name: cr.Name}, found, + Name: cr.Name, + }, found, ); err != nil { return err } @@ -847,7 +873,8 @@ func (step *Step) validateAuthorizationProxyServerInstalled(cr csmv1.ContainerSt instance := new(csmv1.ContainerStorageModule) if err := step.ctrlClient.Get(context.TODO(), client.ObjectKey{ Namespace: cr.Namespace, - Name: cr.Name}, instance, + Name: cr.Name, + }, instance, ); err != nil { return err } @@ -896,12 +923,13 @@ func (step *Step) validateAuthorizationProxyServerNotInstalled(cr csmv1.Containe } func (step *Step) validateAppMobInstalled(cr csmv1.ContainerStorageModule) error { - //providing additional time to get appmob pods up to running + // providing additional time to get appmob pods up to running time.Sleep(10 * time.Second) instance := new(csmv1.ContainerStorageModule) if err := step.ctrlClient.Get(context.TODO(), client.ObjectKey{ Namespace: cr.Namespace, - Name: cr.Name}, instance, + Name: cr.Name, + }, instance, ); err != nil { return err } @@ -1025,7 +1053,7 @@ func (step *Step) configureAuthorizationProxyServer(res Resource, driver string, proxyHost = "" ) - //by default, use set defined in env file + // by default, use set defined in env file endpointvar := "END_POINT" systemIdvar := "SYSTEM_ID" uservar := "STORAGE_USER" @@ -1092,7 +1120,7 @@ func (step *Step) configureAuthorizationProxyServer(res Resource, driver string, } fmt.Println("=== Writing Admin Token to Tmp File ===\n ") - err = os.WriteFile("/tmp/adminToken.yaml", b, 0644) + err = os.WriteFile("/tmp/adminToken.yaml", b, 0o644) if err != nil { return fmt.Errorf("failed to write admin token: %v\nErrMessage:\n%s", err, string(b)) } @@ -1111,7 +1139,6 @@ func (step *Step) configureAuthorizationProxyServer(res Resource, driver string, ) fmt.Println("=== Storage === \n", cmd.String()) b, err = cmd.CombinedOutput() - if err != nil { return fmt.Errorf("failed to create storage %s: %v\nErrMessage:\n%s", storageType, err, string(b)) } @@ -1162,7 +1189,6 @@ func (step *Step) configureAuthorizationProxyServer(res Resource, driver string, ) fmt.Println("=== Binding Role ===\n", cmd.String()) b, err = cmd.CombinedOutput() - if err != nil { return fmt.Errorf("failed to create rolebinding %s: %v\nErrMessage:\n%s", roleName, err, string(b)) } @@ -1178,7 +1204,6 @@ func (step *Step) configureAuthorizationProxyServer(res Resource, driver string, ) fmt.Println("=== Token ===\n", cmd.String()) b, err = cmd.CombinedOutput() - if err != nil { return fmt.Errorf("failed to generate token for %s: %v\nErrMessage:\n%s", tenantName, err, string(b)) } @@ -1186,7 +1211,7 @@ func (step *Step) configureAuthorizationProxyServer(res Resource, driver string, // Apply token to CSI driver host fmt.Println("=== Applying token ===\n ") - err = os.WriteFile("/tmp/token.yaml", b, 0644) + err = os.WriteFile("/tmp/token.yaml", b, 0o644) if err != nil { return fmt.Errorf("failed to write tenant token: %v\nErrMessage:\n%s", err, string(b)) } @@ -1247,7 +1272,6 @@ func (step *Step) validateResiliencyInstalled(cr csmv1.ContainerStorageModule) e } func (step *Step) validateResiliencyNotInstalled(cr csmv1.ContainerStorageModule) error { - // check that resiliency sidecar(podmon) is not in cluster: for controller dp, err := getDriverDeployment(cr, step.ctrlClient) if err != nil { @@ -1301,7 +1325,8 @@ func (step *Step) validateConnectivityClientInstalled(res Resource, crNumStr str if err := step.ctrlClient.Get(context.TODO(), client.ObjectKey{ Namespace: cr.Namespace, - Name: cr.Name}, found); err != nil { + Name: cr.Name, + }, found); err != nil { return err } @@ -1314,7 +1339,8 @@ func (step *Step) validateConnectivityClientNotInstalled(res Resource, crNumStr found := new(csmv1.ApexConnectivityClient) if err := step.ctrlClient.Get(context.TODO(), client.ObjectKey{ Namespace: cr.Namespace, - Name: cr.Name}, found); err == nil { + Name: cr.Name, + }, found); err == nil { return fmt.Errorf("Found traces of client installation in namespace %s: %v", cr.Namespace, found) } @@ -1329,7 +1355,8 @@ func (step *Step) uninstallConnectivityClient(res Resource, crNumStr string) err found := new(csmv1.ApexConnectivityClient) err := step.ctrlClient.Get(context.TODO(), client.ObjectKey{ Namespace: cr.Namespace, - Name: cr.Name}, found, + Name: cr.Name, + }, found, ) if err != nil { if errors.IsNotFound(err) { @@ -1351,7 +1378,6 @@ func (step *Step) uninstallConnectivityClient(res Resource, crNumStr string) err } func (step *Step) validateApplicationMobilityNotInstalled(cr csmv1.ContainerStorageModule) error { - fakeReconcile := utils.FakeReconcileCSM{ Client: step.ctrlClient, K8sClient: step.clientSet, diff --git a/tests/e2e/steps/steps_runner.go b/tests/e2e/steps/steps_runner.go index a54e1cfb3..86fb8a916 100644 --- a/tests/e2e/steps/steps_runner.go +++ b/tests/e2e/steps/steps_runner.go @@ -32,9 +32,7 @@ type Runner struct { Definitions []StepDefinition } -var ( - errorInterface = reflect.TypeOf((*error)(nil)).Elem() -) +var errorInterface = reflect.TypeOf((*error)(nil)).Elem() // StepRunnerInit - func StepRunnerInit(runner *Runner, ctrlClient client.Client, clientSet *kubernetes.Clientset) { @@ -46,6 +44,7 @@ func StepRunnerInit(runner *Runner, ctrlClient client.Client, clientSet *kuberne runner.addStep(`^Install \[([^"]*)\]$`, step.installThirdPartyModule) runner.addStep(`^Uninstall \[([^"]*)\]$`, step.uninstallThirdPartyModule) runner.addStep(`^Apply custom resource \[(\d+)\]$`, step.applyCustomResource) + runner.addStep(`^Upgrade from custom resource \[(\d+)\] to \[(\d+)\]$`, step.upgradeCustomResource) runner.addStep(`^Validate custom resource \[(\d+)\]$`, step.validateCustomResourceStatus) runner.addStep(`^Validate \[([^"]*)\] driver from CR \[(\d+)\] is installed$`, step.validateDriverInstalled) runner.addStep(`^Validate \[([^"]*)\] driver from CR \[(\d+)\] is not installed$`, step.validateDriverNotInstalled) @@ -110,7 +109,6 @@ func (runner *Runner) addStep(expr string, stepFunc interface{}) { Handler: v, Expr: re, }) - } // RunStep - runs a step diff --git a/tests/e2e/testfiles/storage_csm_powerflex_auth_n_minus_1.yaml b/tests/e2e/testfiles/storage_csm_powerflex_auth_n_minus_1.yaml new file mode 100644 index 000000000..0fb3bff64 --- /dev/null +++ b/tests/e2e/testfiles/storage_csm_powerflex_auth_n_minus_1.yaml @@ -0,0 +1,151 @@ +apiVersion: storage.dell.com/v1 +kind: ContainerStorageModule +metadata: + name: test-vxflexos + namespace: test-vxflexos +spec: + driver: + csiDriverType: "powerflex" + csiDriverSpec: + # fsGroupPolicy: Defines if the underlying volume supports changing ownership and permission of the volume before being mounted. + # Allowed values: ReadWriteOnceWithFSType, File , None + # Default value: ReadWriteOnceWithFSType + fSGroupPolicy: "File" + configVersion: v2.9.2 + replicas: 1 + dnsPolicy: ClusterFirstWithHostNet + forceUpdate: false + forceRemoveDriver: true + common: + image: "dellemc/csi-vxflexos:v2.9.2" + imagePullPolicy: Always + envs: + - name: X_CSI_VXFLEXOS_ENABLELISTVOLUMESNAPSHOT + value: "false" + - name: X_CSI_VXFLEXOS_ENABLESNAPSHOTCGDELETE + value: "false" + - name: X_CSI_DEBUG + value: "true" + - name: X_CSI_ALLOW_RWO_MULTI_POD_ACCESS + value: "false" + # Specify kubelet config dir path. + # Ensure that the config.yaml file is present at this path. + # Default value: None + - name: KUBELET_CONFIG_DIR + value: "/var/lib/kubelet" + - name: "CERT_SECRET_COUNT" + value: "0" + + + sideCars: + # sdc-monitor is disabled by default, due to high CPU usage + - name: sdc-monitor + enabled: false + image: dellemc/sdc:4.5 + envs: + - name: HOST_PID + value: "1" + - name: MDM + value: "10.x.x.x,10.x.x.x" #provide MDM value + + # health monitor is disabled by default, refer to driver documentation before enabling it + # Also set the env variable controller.envs.X_CSI_HEALTH_MONITOR_ENABLED to "true". + - name: csi-external-health-monitor-controller + enabled: false + args: ["--monitor-interval=60s"] + + controller: + envs: + # X_CSI_HEALTH_MONITOR_ENABLED: Enable/Disable health monitor of CSI volumes from Controller plugin - volume condition. + # Install the 'external-health-monitor' sidecar accordingly. + # Allowed values: + # true: enable checking of health condition of CSI volumes + # false: disable checking of health condition of CSI volumes + # Default value: false + - name: X_CSI_HEALTH_MONITOR_ENABLED + value: "false" + + #"controller.nodeSelector" defines what nodes would be selected for pods of controller deployment + # Leave as blank to use all nodes + # Allowed values: map of key-value pairs + # Default value: None + # Examples: + # node-role.kubernetes.io/control-plane: "" + nodeSelector: + # Uncomment if nodes you wish to use have the node-role.kubernetes.io/master taint + # node-role.kubernetes.io/master: "" + # Uncomment if nodes you wish to use have the node-role.kubernetes.io/control-plane taint + # node-role.kubernetes.io/control-plane: "" + + # "controller.tolerations" defines tolerations that would be applied to controller deployment + # Leave as blank to install controller on worker nodes + # Default value: None + tolerations: + # Uncomment if nodes you wish to use have the node-role.kubernetes.io/master taint + # - key: "node-role.kubernetes.io/master" + # operator: "Exists" + # effect: "NoSchedule" + # Uncomment if nodes you wish to use have the node-role.kubernetes.io/control-plane taint + # - key: "node-role.kubernetes.io/control-plane" + # operator: "Exists" + # effect: "NoSchedule" + + node: + envs: + # X_CSI_HEALTH_MONITOR_ENABLED: Enable/Disable health monitor of CSI volumes from node plugin - volume usage + # Allowed values: + # true: enable checking of health condition of CSI volumes + # false: disable checking of health condition of CSI volumes + # Default value: false + - name: X_CSI_HEALTH_MONITOR_ENABLED + value: "false" + + # "node.nodeSelector" defines what nodes would be selected for pods of node daemonset + # Leave as blank to use all nodes + # Allowed values: map of key-value pairs + # Default value: None + # Examples: + # node-role.kubernetes.io/control-plane: "" + nodeSelector: + # Uncomment if nodes you wish to use have the node-role.kubernetes.io/master taint + # node-role.kubernetes.io/master: "" + # Uncomment if nodes you wish to use have the node-role.kubernetes.io/control-plane taint + # node-role.kubernetes.io/control-plane: "" + + # "node.tolerations" defines tolerations that would be applied to node daemonset + # Leave as blank to install node driver only on worker nodes + # Default value: None + tolerations: + # Uncomment if nodes you wish to use have the node-role.kubernetes.io/master taint + # - key: "node-role.kubernetes.io/master" + # operator: "Exists" + # effect: "NoSchedule" + # Uncomment if nodes you wish to use have the node-role.kubernetes.io/control-plane taint + # - key: "node-role.kubernetes.io/control-plane" + # operator: "Exists" + # effect: "NoSchedule" + + initContainers: + - image: dellemc/sdc:4.5 + imagePullPolicy: IfNotPresent + name: sdc + envs: + - name: MDM + value: "10.225.109.64,10.225.109.65" #provide MDM value + modules: + # Authorization: enable csm-authorization for RBAC + - name: authorization + # enable: Enable/Disable csm-authorization + enabled: true + configVersion: v1.9.1 + components: + - name: karavi-authorization-proxy + image: dellemc/csm-authorization-sidecar:v1.9.1 + envs: + # proxyHost: hostname of the csm-authorization server + - name: "PROXY_HOST" + value: "authorization-ingress-nginx-controller.authorization.svc.cluster.local" + + # skipCertificateValidation: Enable/Disable certificate validation of the csm-authorization server + - name: "SKIP_CERTIFICATE_VALIDATION" + value: "true" diff --git a/tests/e2e/testfiles/values.yaml b/tests/e2e/testfiles/values.yaml index c2dfb299c..d88b024af 100644 --- a/tests/e2e/testfiles/values.yaml +++ b/tests/e2e/testfiles/values.yaml @@ -857,6 +857,48 @@ run: - ./cert-csi test vio --sc op-e2e-vxflexos --chainNumber 2 --chainLength 2 +- scenario: "Install PowerFlex Driver (With Authorization), Upgrade Authorization module" + paths: + - "testfiles/authorization-templates/csm_authorization_proxy_server.yaml" + - "testfiles/storage_csm_powerflex_auth_n_minus_1.yaml" + - "testfiles/storage_csm_powerflex_auth.yaml" + modules: + - "authorization" + - "authorizationproxyserver" + steps: + - "Given an environment with k8s or openshift, and CSM operator installed" + - "Create [authorization-proxy-server] prerequisites from CR [1]" + - "Apply custom resource [1]" + - "Validate [authorization-proxy-server] module from CR [1] is installed" + - "Configure authorization-proxy-server for [powerflex] for CR [1]" + - "Create storageclass with name [op-e2e-vxflexos] and template [testfiles/powerflex-templates/powerflex-storageclass-template.yaml] for [pflex]" + - "Set up secret with template [testfiles/powerflex-templates/csm-authorization-config.json] name [karavi-authorization-config] in namespace [test-vxflexos] for [pflexAuthSidecar]" + - "Set up secret with template [testfiles/powerflex-templates/powerflex-secret-template.yaml] name [test-vxflexos-config] in namespace [test-vxflexos] for [pflex]" + - "Restore template [testfiles/powerflex-templates/powerflex-secret-template.yaml] for [pflex]" + - "Set up secret with template [testfiles/powerflex-templates/powerflex-secret-template.yaml] name [test-vxflexos-config] in namespace [test-vxflexos] for [pflexAuth]" + - "Apply custom resource [2]" + - "Validate custom resource [2]" + - "Validate [powerflex] driver from CR [2] is installed" + - "Validate [authorization] module from CR [2] is installed" + - "Run custom test" + # upgrade + - "Upgrade from custom resource [2] to [3]" + - "Validate custom resource [3]" + - "Validate [powerflex] driver from CR [3] is installed" + - "Validate [authorization] module from CR [3] is installed" + - "Run custom test" + # cleanup + - "Enable forceRemoveDriver on CR [3]" + - "Delete custom resource [3]" + - "Delete custom resource [1]" + - "Restore template [testfiles/powerflex-templates/csm-authorization-config.json] for [pflexAuthSidecar]" + - "Restore template [testfiles/powerflex-templates/powerflex-secret-template.yaml] for [pflexAuth]" + - "Restore template [testfiles/powerflex-templates/powerflex-storageclass-template.yaml] for [pflex]" + customTest: + name: Cert CSI + run: + - ./cert-csi test vio --sc op-e2e-vxflexos --chainNumber 2 --chainLength 2 + - scenario: "Install PowerFlex Driver(With Observability)" paths: - "testfiles/storage_csm_powerflex_observability.yaml" diff --git a/tests/shared/common.go b/tests/shared/common.go index b72b2347a..1e3621cab 100644 --- a/tests/shared/common.go +++ b/tests/shared/common.go @@ -37,6 +37,7 @@ const ( UnityConfigVersion string = "v2.10.0" PScaleConfigVersion string = "v2.10.0" PmaxConfigVersion string = "v2.10.0" + AuthServerConfigVersion string = "v1.10.0" AccConfigVersion string = "v1.0.0" )