Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for Authorization upgrade #549

Merged
merged 16 commits into from
May 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions api/v1/csm_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
5 changes: 5 additions & 0 deletions api/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
69 changes: 25 additions & 44 deletions controllers/csm_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are we getting rid of this error message? I'm good with making it a warning instead of an error, but I don't think we should get rid of it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 {
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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
Expand All @@ -1411,23 +1387,28 @@ 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
if annotations == nil {
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
}

Expand Down
31 changes: 25 additions & 6 deletions controllers/csm_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@

minUpgradePath: v1.9.0
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
minUpgradePath: v1.7.0
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@

minUpgradePath: v1.8.0
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@

minUpgradePath: v1.8.0
2 changes: 1 addition & 1 deletion pkg/modules/authorization.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading
Loading