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

[BUG]: CSM Operator not deleting the deployment and daemon sets after deleting the CSM #815

Merged
merged 5 commits into from
Dec 13, 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
2 changes: 1 addition & 1 deletion api/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,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
42 changes: 40 additions & 2 deletions tests/e2e/steps/steps_def.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ import (

"encoding/json"

"path/filepath"

"github.com/dell/csm-operator/pkg/constants"
"github.com/dell/csm-operator/pkg/modules"
"github.com/dell/csm-operator/pkg/utils"
Expand All @@ -39,7 +41,6 @@ import (
"k8s.io/kubernetes/test/e2e/framework/kubectl"
fpod "k8s.io/kubernetes/test/e2e/framework/pod"
"k8s.io/utils/pointer"
"path/filepath"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/yaml"
)
Expand Down Expand Up @@ -987,10 +988,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
Loading