Skip to content

Commit 4ffd7ee

Browse files
committed
Avoid bogus errors during deletion
When deleting the DRPC we may need to adopt the VRG, delete the secondary VRG, wait until the secondary VRG is deleted, delete the primary VRG, and wait until the primary VRG is deleted. This takes 60-90 seconds and many reconciles (18 seen in e2e test), and creates huge amount of noise in the log. Suppress the noise by introducing the deleteInProgress error. When the reconcile is successful but it is still in progress, we return a deleteInProgress error describing the current progression. The top level error handle logs an INFO message and requeue the request. With this change we will see multiple logs for the secondary VRG: INFO Deleting DRPC in progress {"reason", "secondary VRG deletion in progress"} ... And finally more logs for the primary VRG: INFO Deleting DRPC in progress {"reason", "primary VRG deletion in progress"} ... Notes: - We logged errors during finalizeDRPC twice; once as INFO log, and once as ERROR with a stacktrace when we return error from the reconcile. Remove the duplicate INFO log. - The linter is not happy about the new nested if. We can avoid this by extracting a helper to handle finalize errors, but I want to keep the change minimal for easy backport. We can improve this later upstream. Signed-off-by: Nir Soffer <nsoffer@redhat.com>
1 parent 2d47141 commit 4ffd7ee

File tree

1 file changed

+26
-6
lines changed

1 file changed

+26
-6
lines changed

internal/controller/drplacementcontrol_controller.go

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,18 @@ var InitialWaitTimeForDRPCPlacementRule = errorswrapper.New("Waiting for DRPC Pl
6565
// ProgressCallback of function type
6666
type ProgressCallback func(string, string)
6767

68+
// deleteInProgress error is returned during DRPC deletion when we need to wait and try later.
69+
type deleteInProgress struct{ string }
70+
71+
func (e deleteInProgress) Error() string { return e.string }
72+
73+
// Is called by errors.Is() to match errors of same type.
74+
func (deleteInProgress) Is(target error) bool {
75+
_, ok := target.(deleteInProgress)
76+
77+
return ok
78+
}
79+
6880
// DRPlacementControlReconciler reconciles a DRPlacementControl object
6981
type DRPlacementControlReconciler struct {
7082
client.Client
@@ -119,7 +131,7 @@ func (r *DRPlacementControlReconciler) SetupWithManager(mgr ctrl.Manager) error
119131
// For more details, check Reconcile and its Result here:
120132
// - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.7.0/pkg/reconcile
121133
//
122-
//nolint:funlen,gocognit,gocyclo,cyclop
134+
//nolint:funlen,gocognit,gocyclo,cyclop,nestif
123135
func (r *DRPlacementControlReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
124136
logger := r.Log.WithValues("DRPC", req.NamespacedName, "rid", uuid.New())
125137

@@ -166,13 +178,21 @@ func (r *DRPlacementControlReconciler) Reconcile(ctx context.Context, req ctrl.R
166178
// then the DRPC should be deleted as well. The least we should do here is to clean up DPRC.
167179
err := r.processDeletion(ctx, drpc, placementObj, logger)
168180
if err != nil {
169-
logger.Info(fmt.Sprintf("Error in deleting DRPC: (%v)", err))
170-
171181
statusErr := r.setDeletionStatusAndUpdate(ctx, drpc)
172182
if statusErr != nil {
173183
err = fmt.Errorf("drpc deletion failed: %w and status update failed: %w", err, statusErr)
184+
185+
return ctrl.Result{}, err
186+
}
187+
188+
// Is this an expected condition?
189+
if errorswrapper.Is(err, deleteInProgress{}) {
190+
logger.Info("Deleting DRPC in progress", "reason", err)
191+
192+
return ctrl.Result{Requeue: true}, nil
174193
}
175194

195+
// Unexpected error.
176196
return ctrl.Result{}, err
177197
}
178198

@@ -736,7 +756,7 @@ func (r *DRPlacementControlReconciler) cleanupVRGs(
736756
}
737757

738758
if len(vrgs) != 0 {
739-
return fmt.Errorf("waiting for VRGs count to go to zero")
759+
return deleteInProgress{"waiting for VRGs count to go to zero"}
740760
}
741761

742762
// delete MCVs
@@ -761,7 +781,7 @@ func (r *DRPlacementControlReconciler) ensureVRGsDeleted(
761781
for cluster, vrg := range vrgs {
762782
if vrg.Spec.ReplicationState == replicationState {
763783
if !ensureVRGsManagedByDRPC(r.Log, mwu, vrgs, drpc, vrgNamespace) {
764-
return fmt.Errorf("%s VRG adoption in progress", replicationState)
784+
return deleteInProgress{fmt.Sprintf("%s VRG adoption in progress", replicationState)}
765785
}
766786

767787
if err := mwu.DeleteManifestWork(mwu.BuildManifestWorkName(rmnutil.MWTypeVRG), cluster); err != nil {
@@ -773,7 +793,7 @@ func (r *DRPlacementControlReconciler) ensureVRGsDeleted(
773793
}
774794

775795
if inProgress {
776-
return fmt.Errorf("%s VRG manifestwork deletion in progress", replicationState)
796+
return deleteInProgress{fmt.Sprintf("%s VRG manifestwork deletion in progress", replicationState)}
777797
}
778798

779799
return nil

0 commit comments

Comments
 (0)