diff --git a/internal/controller/drplacementcontrol_controller.go b/internal/controller/drplacementcontrol_controller.go index 87d7a990e4..ab3f1aed4a 100644 --- a/internal/controller/drplacementcontrol_controller.go +++ b/internal/controller/drplacementcontrol_controller.go @@ -564,41 +564,37 @@ func GetDRClusters(ctx context.Context, client client.Client, drPolicy *rmn.DRPo return drClusters, nil } -// updateObjectMetadata updates drpc labels, annotations and finalizer, and also updates placementObj finalizer -func (r DRPlacementControlReconciler) updateObjectMetadata(ctx context.Context, - drpc *rmn.DRPlacementControl, placementObj client.Object, log logr.Logger, -) error { - var update bool +// updateDRPCMetadata updates DRPC labels, annotations and finalizer only +func (r DRPlacementControlReconciler) updateDRPCMetadata( + drpc *rmn.DRPlacementControl, placementObj client.Object, +) (modified bool, err error) { + modified = false + + if rmnutil.AddLabel(drpc, rmnutil.OCMBackupLabelKey, rmnutil.OCMBackupLabelValue) { + modified = true + } - update = rmnutil.AddLabel(drpc, rmnutil.OCMBackupLabelKey, rmnutil.OCMBackupLabelValue) - update = rmnutil.AddFinalizer(drpc, DRPCFinalizer) || update + if rmnutil.AddFinalizer(drpc, DRPCFinalizer) { + modified = true + } vrgNamespace, err := selectVRGNamespace(r.Client, r.Log, drpc, placementObj) if err != nil { - return err + return modified, err } - update = rmnutil.AddAnnotation(drpc, DRPCAppNamespace, vrgNamespace) || update - - if update { - if err := r.Update(ctx, drpc); err != nil { - log.Error(err, "Failed to add annotations, labels, or finalizer to drpc") - - return fmt.Errorf("%w", err) - } + if rmnutil.AddAnnotation(drpc, DRPCAppNamespace, vrgNamespace) { + modified = true } - // add finalizer to User PlacementRule/Placement - finalizerAdded := rmnutil.AddFinalizer(placementObj, DRPCFinalizer) - if finalizerAdded { - if err := r.Update(ctx, placementObj); err != nil { - log.Error(err, "Failed to add finalizer to user placement rule") - - return fmt.Errorf("%w", err) - } - } + return modified, nil +} - return nil +// updatePlacementMetadata updates placement object finalizer only +func (r DRPlacementControlReconciler) updatePlacementMetadata( + placementObj client.Object, +) bool { + return rmnutil.AddFinalizer(placementObj, DRPCFinalizer) } func (r *DRPlacementControlReconciler) processDeletion(ctx context.Context, @@ -854,15 +850,74 @@ func (r *DRPlacementControlReconciler) updateAndSetOwner( usrPlacement client.Object, log logr.Logger, ) (bool, error) { - if err := r.annotateObject(ctx, drpc, usrPlacement, log); err != nil { + var ( + err error + ownerReferenceDRPCModified bool + placementModified bool + placementAnnotateModified bool + placementMetadataModified bool + ) + + // Annotate the placement object + if placementAnnotateModified, err = r.annotateObject(drpc, usrPlacement, log); err != nil { + return false, err + } + + // Update object metadata + if _, err = r.updateDRPCMetadata(drpc, usrPlacement); err != nil { + return false, err + } + + placementMetadataModified = r.updatePlacementMetadata(usrPlacement) + + // Merge both modifications + placementModified = placementAnnotateModified || placementMetadataModified + + // Set DRPC Owner + ownerReferenceDRPCModified, err = r.setDRPCOwner(drpc, usrPlacement, log) + if err != nil { return false, err } - if err := r.updateObjectMetadata(ctx, drpc, usrPlacement, log); err != nil { + if placementModified { + if err = r.updatePlacement(ctx, usrPlacement, log); err != nil { + return false, err + } + } + + if err = r.updateDRPC(ctx, drpc, log); err != nil { return false, err } - return r.setDRPCOwner(ctx, drpc, usrPlacement, log) + return ownerReferenceDRPCModified, nil +} + +func (r *DRPlacementControlReconciler) updatePlacement( + ctx context.Context, + usrPlacement client.Object, + log logr.Logger, +) error { + if err := r.Update(ctx, usrPlacement); err != nil { + return fmt.Errorf("failed to update placement %s/%s: %w", usrPlacement.GetNamespace(), usrPlacement.GetName(), err) + } + + log.Info("Updated placement", "placement", usrPlacement.GetName()) + + return nil +} + +func (r *DRPlacementControlReconciler) updateDRPC( + ctx context.Context, + drpc *rmn.DRPlacementControl, + log logr.Logger, +) error { + if err := r.Update(ctx, drpc); err != nil { + return fmt.Errorf("failed to update DRPC %s: %w", drpc.GetName(), err) + } + + log.Info("Successfully updated DRPC resource", "drpc", drpc.GetName()) + + return nil } func getPlacementOrPlacementRule( @@ -984,11 +1039,13 @@ func getPlacement(ctx context.Context, k8sclient client.Client, return usrPlmnt, nil } -func (r *DRPlacementControlReconciler) annotateObject(ctx context.Context, - drpc *rmn.DRPlacementControl, obj client.Object, log logr.Logger, -) error { +func (r *DRPlacementControlReconciler) annotateObject( + drpc *rmn.DRPlacementControl, + obj client.Object, + log logr.Logger, +) (bool, error) { if rmnutil.ResourceIsDeleted(obj) { - return nil + return false, nil } if obj.GetAnnotations() == nil { @@ -1002,51 +1059,38 @@ func (r *DRPlacementControlReconciler) annotateObject(ctx context.Context, obj.GetAnnotations()[DRPCNameAnnotation] = drpc.Name obj.GetAnnotations()[DRPCNamespaceAnnotation] = drpc.Namespace - err := r.Update(ctx, obj) - if err != nil { - log.Error(err, "Failed to update Object annotation", "objName", obj.GetName()) - - return fmt.Errorf("failed to update Object %s annotation '%s/%s' (%w)", - obj.GetName(), DRPCNameAnnotation, drpc.Name, err) - } - - return nil + return true, nil } if ownerName != drpc.Name || ownerNamespace != drpc.Namespace { log.Info("Object not owned by this DRPC", "objName", obj.GetName()) - return fmt.Errorf("object %s not owned by this DRPC '%s/%s'", + return false, fmt.Errorf("object %s not owned by this DRPC '%s/%s'", obj.GetName(), drpc.Name, drpc.Namespace) } - return nil + return false, nil } func (r *DRPlacementControlReconciler) setDRPCOwner( - ctx context.Context, drpc *rmn.DRPlacementControl, owner client.Object, log logr.Logger, + drpc *rmn.DRPlacementControl, owner client.Object, log logr.Logger, ) (bool, error) { - const updated = true + const needUpdate = true for _, ownerReference := range drpc.GetOwnerReferences() { if ownerReference.Name == owner.GetName() { - return !updated, nil // ownerreference already set + return !needUpdate, nil // ownerreference already set } } err := ctrl.SetControllerReference(owner, drpc, r.Client.Scheme()) if err != nil { - return !updated, fmt.Errorf("failed to set DRPC owner %w", err) - } - - err = r.Update(ctx, drpc) - if err != nil { - return !updated, fmt.Errorf("failed to update drpc %s (%w)", drpc.GetName(), err) + return !needUpdate, fmt.Errorf("failed to set DRPC owner %w", err) } log.Info(fmt.Sprintf("Object %s owns DRPC %s", owner.GetName(), drpc.GetName())) - return updated, nil + return needUpdate, nil } func (r *DRPlacementControlReconciler) getOrClonePlacementRule(ctx context.Context,