Skip to content

Commit

Permalink
[BUG]: CSM Operator not deleting the deployment and daemon sets after…
Browse files Browse the repository at this point in the history
… deleting the CSM (#815)

* Set default value for forceRemoveDriver to true if not specified by the user

* Update commonconfig.go

* Added e2e scenarios and test files

* Fixed linting issues

* Update steps_def.go
  • Loading branch information
riya-kaushal7997 authored Dec 13, 2024
1 parent b734947 commit 4f9b0fd
Show file tree
Hide file tree
Showing 18 changed files with 428 additions and 7 deletions.
2 changes: 1 addition & 1 deletion api/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ type Driver struct {

// ForceRemoveDriver is the boolean flag used to remove driver deployment when CR is deleted
// +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Force Remove Driver"
ForceRemoveDriver bool `json:"forceRemoveDriver,omitempty" yaml:"forceRemoveDriver"`
ForceRemoveDriver *bool `json:"forceRemoveDriver,omitempty" yaml:"forceRemoveDriver"`
}

// ContainerTemplate template
Expand Down
5 changes: 5 additions & 0 deletions api/v1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 7 additions & 1 deletion controllers/csm_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,12 @@ func (r *ContainerStorageModuleReconciler) Reconcile(_ context.Context, req ctrl
ConfigDirectory: r.Config.ConfigDirectory,
}

// Set default value for forceRemoveDriver to true if not specified by the user
if csm.Spec.Driver.ForceRemoveDriver == nil {
truebool := true
csm.Spec.Driver.ForceRemoveDriver = &truebool
}

// Set default components if using miminal manifest (without components)
err = utils.LoadDefaultComponents(ctx, csm, *operatorConfig)
if err != nil {
Expand All @@ -289,7 +295,7 @@ func (r *ContainerStorageModuleReconciler) Reconcile(_ context.Context, req ctrl
log.Infow("Delete request", "csm", req.Namespace, "Name", req.Name)

// check for force cleanup
if csm.Spec.Driver.ForceRemoveDriver {
if *csm.Spec.Driver.ForceRemoveDriver {
// remove all resources deployed from CR by operator
if err := r.removeDriver(ctx, *csm, *operatorConfig); err != nil {
r.EventRecorder.Event(csm, corev1.EventTypeWarning, csmv1.EventDeleted, fmt.Sprintf("Failed to remove driver: %s", err))
Expand Down
6 changes: 3 additions & 3 deletions controllers/csm_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2068,7 +2068,7 @@ func (suite *CSMControllerTestSuite) makeFakeCSM(name, ns string, withFinalizer
csm.ObjectMeta.Finalizers = []string{CSMFinalizerName}
}
// remove driver when deleting csm
csm.Spec.Driver.ForceRemoveDriver = true
csm.Spec.Driver.ForceRemoveDriver = &truebool
csm.Annotations[configVersionKey] = configVersion

csm.Spec.Modules = modules
Expand Down Expand Up @@ -2102,7 +2102,7 @@ func (suite *CSMControllerTestSuite) makeFakeResiliencyCSM(name, ns string, with
csm.ObjectMeta.Finalizers = []string{CSMFinalizerName}
}
// remove driver when deleting csm
csm.Spec.Driver.ForceRemoveDriver = true
csm.Spec.Driver.ForceRemoveDriver = &truebool
csm.Annotations[configVersionKey] = configVersion

csm.Spec.Modules = modules
Expand Down Expand Up @@ -2374,7 +2374,7 @@ func (suite *CSMControllerTestSuite) makeFakeRevProxyCSM(name string, ns string,
csm.ObjectMeta.Finalizers = []string{CSMFinalizerName}
}
// remove driver when deleting csm
csm.Spec.Driver.ForceRemoveDriver = true
csm.Spec.Driver.ForceRemoveDriver = &trueBool
csm.Spec.Modules = modules
out, _ := json.Marshal(&csm)
csm.Annotations[previouslyAppliedCustomResource] = string(out)
Expand Down
2 changes: 1 addition & 1 deletion pkg/drivers/commonconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ func GetController(ctx context.Context, cr csmv1.ContainerStorageModule, operato

crUID := cr.GetUID()
bController := true
bOwnerDeletion := !cr.Spec.Driver.ForceRemoveDriver
bOwnerDeletion := cr.Spec.Driver.ForceRemoveDriver != nil && !*cr.Spec.Driver.ForceRemoveDriver
kind := cr.Kind
v1 := "apps/v1"
controllerYAML.Deployment.OwnerReferences = []metacv1.OwnerReferenceApplyConfiguration{
Expand Down
41 changes: 40 additions & 1 deletion tests/e2e/steps/steps_def.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ import (
"encoding/json"
"path/filepath"

"path/filepath"

"github.com/dell/csm-operator/pkg/constants"
"github.com/dell/csm-operator/pkg/modules"
"github.com/dell/csm-operator/pkg/utils"
Expand Down Expand Up @@ -1073,10 +1075,47 @@ func (step *Step) enableForceRemoveDriver(res Resource, crNumStr string) error {
return err
}

found.Spec.Driver.ForceRemoveDriver = true
truebool := true
found.Spec.Driver.ForceRemoveDriver = &truebool
return step.ctrlClient.Update(context.TODO(), found)
}

func (step *Step) validateForceRemoveDriverEnabled(res Resource, crNumStr string) error {
crNum, _ := strconv.Atoi(crNumStr)
cr := res.CustomResource[crNum-1].(csmv1.ContainerStorageModule)
found := new(csmv1.ContainerStorageModule)
if err := step.ctrlClient.Get(context.TODO(), client.ObjectKey{
Namespace: cr.Namespace,
Name: cr.Name,
}, found,
); err != nil {
return err
}

if found.Spec.Driver.ForceRemoveDriver != nil && *found.Spec.Driver.ForceRemoveDriver {
return nil
}
return fmt.Errorf("forceRemoveDriver is not set to true")
}

func (step *Step) validateForceRemoveDriverDisabled(res Resource, crNumStr string) error {
crNum, _ := strconv.Atoi(crNumStr)
cr := res.CustomResource[crNum-1].(csmv1.ContainerStorageModule)
found := new(csmv1.ContainerStorageModule)
if err := step.ctrlClient.Get(context.TODO(), client.ObjectKey{
Namespace: cr.Namespace,
Name: cr.Name,
}, found,
); err != nil {
return err
}

if found.Spec.Driver.ForceRemoveDriver != nil && !*found.Spec.Driver.ForceRemoveDriver {
return nil
}
return fmt.Errorf("forceRemoveDriver is not set to false")
}

func (step *Step) enableForceRemoveModule(res Resource, crNumStr string) error {
crNum, _ := strconv.Atoi(crNumStr)
cr := res.CustomResource[crNum-1].(csmv1.ContainerStorageModule)
Expand Down
2 changes: 2 additions & 0 deletions tests/e2e/steps/steps_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ 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(`^Validate \[(\d+)\] CSM has forceRemoveDriver set to true$`, step.validateForceRemoveDriverEnabled)
runner.addStep(`^Validate \[(\d+)\] CSM has forceRemoveDriver set to false$`, step.validateForceRemoveDriverDisabled)
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)
Expand Down
Loading

0 comments on commit 4f9b0fd

Please sign in to comment.