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.

Sign up for GitHub

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

handle error improvement #59

Merged
merged 3 commits into from
Mar 2, 2022
Merged

handle error improvement #59

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0b80310
handle error improvement
yifeijin Mar 2, 2022
9271800
fix logging but
yifeijin Mar 2, 2022
4abbd63
pass ctx to finalizeradd/move
yifeijin Mar 2, 2022
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
11 changes: 6 additions & 5 deletions controllers/csm_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,6 @@ func (r *ContainerStorageModuleReconciler) Reconcile(ctx context.Context, req ct
return ctrl.Result{}, fmt.Errorf("error when adding finalizer: %v", err)
}
r.EventRecorder.Event(csm, corev1.EventTypeNormal, v1alpha1.EventUpdated, "Object finalizer is added")
return ctrl.Result{}, nil
}

oldStatus := csm.GetCSMStatus()
Expand Down Expand Up @@ -604,8 +603,8 @@ func (r *ContainerStorageModuleReconciler) removeDriver(ctx context.Context, ins
return err
} else {
log.Infow("Deleting object", "Name:", name, "Kind:", kind)
err = r.GetClient().Delete(ctx, obj)
if err != nil {
err = r.Delete(ctx, obj)
if err != nil && !k8serror.IsNotFound(err) {
return err
}
}
Expand Down Expand Up @@ -667,8 +666,9 @@ func (r *ContainerStorageModuleReconciler) removeDriver(ctx context.Context, ins
daemonsetObj := &appsv1.DaemonSet{}
err = r.Get(ctx, daemonsetKey, daemonsetObj)
if err == nil {
if err = r.Delete(ctx, daemonsetObj); err != nil {
if err = r.Delete(ctx, daemonsetObj); err != nil && !k8serror.IsNotFound(err) {
log.Errorw("error delete daemonset", "Error", err.Error())
return err
}
} else {
log.Infow("error getting daemonset", "daemonsetKey", daemonsetKey)
Expand All @@ -681,8 +681,9 @@ func (r *ContainerStorageModuleReconciler) removeDriver(ctx context.Context, ins

deploymentObj := &appsv1.Deployment{}
if err = r.Get(ctx, deploymentKey, deploymentObj); err == nil {
if err = r.Delete(ctx, deploymentObj); err != nil {
if err = r.Delete(ctx, deploymentObj); err != nil && !k8serror.IsNotFound(err) {
log.Errorw("error delete deployment", "Error", err.Error())
return err
}
} else {
log.Infow("error getting deployment", "deploymentKey", deploymentKey)
Expand Down
8 changes: 3 additions & 5 deletions controllers/csm_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,6 @@ func (suite *CSMControllerTestSuite) TestErrorInjection() {
suite.runFakeCSMManager("", true)
// make a csm without finalizer
suite.makeFakeCSM(csmName, suite.namespace, false)
// reconcile adds finalizer
suite.runFakeCSMManager("", true)
suite.reconcileWithErrorInjection(csmName, "")
}

Expand Down Expand Up @@ -219,9 +217,8 @@ func (suite *CSMControllerTestSuite) TestRemoveDriver() {
{"get CM error", csm, &getCMError, getCMErrorStr},
{"get Driver error", csm, &getCSIError, getCSIErrorStr},
{"delete SA error", csm, &deleteSAError, deleteSAErrorStr},
// code only logs the error when delete fails. No error returned
{"delete Daemonset error", csm, &deleteDSError, ""},
{"delete Deployment error", csm, &deleteDeploymentError, ""},
{"delete Daemonset error", csm, &deleteDSError, deleteDSErrorStr},
{"delete Deployment error", csm, &deleteDeploymentError, deleteDeploymentErrorStr},
}

for _, tt := range removeDriverTests {
Expand All @@ -238,6 +235,7 @@ func (suite *CSMControllerTestSuite) TestRemoveDriver() {
if tt.expectedErr == "" {
assert.Nil(t, err)
} else {
assert.Error(t, err)
assert.Containsf(t, err.Error(), tt.expectedErr, "expected error containing %q, got %s", tt.expectedErr, err)
}

Expand Down
6 changes: 3 additions & 3 deletions pkg/drivers/commonconfig.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,13 @@ func GetController(ctx context.Context, cr csmv1.ContainerStorageModule, operato
for _, s := range cr.Spec.Driver.SideCars {
if s.Name == *c.Name {
if s.Enabled == nil {
log.Info("Container to be enabled : %s\n", *c.Name)
log.Infow("Container to be enabled", "name", *c.Name)
break
} else if !*s.Enabled {
removeContainer = true
log.Info("Container to be removed : %s\n", *c.Name)
log.Infow("Container to be removed", "name", *c.Name)
} else {
log.Info("Container to be enabled : %s\n", *c.Name)
log.Infow("Container to be enabled", "name", *c.Name)
}
break
}
Expand Down