Merge pull request #93 from mittwald/fix/reconciliation-bugs

Fix some reconciliation bugs
mittwald · Dec 14, 2021 · 022b627 · 022b627
2 parents af96446 + 96c2f18
commit 022b627
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 26 deletions.
diff --git a/controllers/registries/project_controller.go b/controllers/registries/project_controller.go
@@ -20,6 +20,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"reflect"
 	"time"
 
 	controllererrors "github.com/mittwald/harbor-operator/controllers/registries/errors"
@@ -84,7 +85,8 @@ func (r *ProjectReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 		return ctrl.Result{}, err
 	}
 
-	patch := client.MergeFrom(project.DeepCopy())
+	original := project.DeepCopy()
+	patch := client.MergeFrom(original)
 
 	if project.ObjectMeta.DeletionTimestamp != nil &&
 		project.Status.Phase != v1alpha2.ProjectStatusPhaseTerminating {
@@ -117,8 +119,10 @@ func (r *ProjectReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 	if err != nil {
 		return ctrl.Result{}, err
 	}
-	if err := r.Client.Status().Patch(ctx, project, patch); err != nil {
-		return ctrl.Result{}, err
+	if !reflect.DeepEqual(original.ObjectMeta.OwnerReferences, project.ObjectMeta.OwnerReferences) {
+		if err := r.Client.Patch(ctx, project, patch); err != nil {
+			return ctrl.Result{}, err
+		}
 	}
 
 	// Build a client to connect to the harbor API

diff --git a/controllers/registries/registry_controller.go b/controllers/registries/registry_controller.go
@@ -20,11 +20,12 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	controllererrors "github.com/mittwald/harbor-operator/controllers/registries/errors"
 	"net/url"
 	"reflect"
 	"time"
 
+	controllererrors "github.com/mittwald/harbor-operator/controllers/registries/errors"
+
 	"github.com/mittwald/goharbor-client/v5/apiv2/model"
 	clienterrors "github.com/mittwald/goharbor-client/v5/apiv2/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
@@ -75,7 +76,8 @@ func (r *RegistryReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 		return ctrl.Result{}, err
 	}
 
-	patch := client.MergeFrom(registry.DeepCopy())
+	original := registry.DeepCopy()
+	patch := client.MergeFrom(original)
 
 	if registry.ObjectMeta.DeletionTimestamp != nil &&
 		registry.Status.Phase != v1alpha2.RegistryStatusPhaseTerminating {
@@ -94,7 +96,7 @@ func (r *RegistryReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 		switch err.Error() {
 		case controllererrors.ErrInstanceNotInstalledMsg:
 			reqLogger.Info("waiting till harbor instance is installed")
-			return ctrl.Result{RequeueAfter: 30*time.Second}, nil
+			return ctrl.Result{RequeueAfter: 30 * time.Second}, nil
 		case controllererrors.ErrInstanceNotFoundMsg:
 			controllerutil.RemoveFinalizer(registry, internal.FinalizerName)
 			fallthrough
@@ -108,8 +110,10 @@ func (r *RegistryReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 	if err != nil {
 		return ctrl.Result{}, err
 	}
-	if err := r.Client.Status().Patch(ctx, registry, patch); err != nil {
-		return ctrl.Result{}, err
+	if !reflect.DeepEqual(original.ObjectMeta.OwnerReferences, registry.ObjectMeta.OwnerReferences) {
+		if err := r.Client.Patch(ctx, registry, patch); err != nil {
+			return ctrl.Result{}, err
+		}
 	}
 
 	// Build a client to connect to the harbor API

diff --git a/controllers/registries/replication_controller.go b/controllers/registries/replication_controller.go
@@ -20,10 +20,11 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	controllererrors "github.com/mittwald/harbor-operator/controllers/registries/errors"
 	"reflect"
 	"time"
 
+	controllererrors "github.com/mittwald/harbor-operator/controllers/registries/errors"
+
 	clienterrors "github.com/mittwald/goharbor-client/v5/apiv2/pkg/errors"
 
 	h "github.com/mittwald/goharbor-client/v5/apiv2"
@@ -73,7 +74,8 @@ func (r *ReplicationReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		return ctrl.Result{}, err
 	}
 
-	patch := client.MergeFrom(replication.DeepCopy())
+	original := replication.DeepCopy()
+	patch := client.MergeFrom(original)
 
 	if replication.ObjectMeta.DeletionTimestamp != nil && replication.Status.Phase != v1alpha2.ReplicationStatusPhaseTerminating {
 		replication.Status.Phase = v1alpha2.ReplicationStatusPhaseTerminating
@@ -90,7 +92,7 @@ func (r *ReplicationReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		switch err.Error() {
 		case controllererrors.ErrInstanceNotInstalledMsg:
 			reqLogger.Info("waiting till harbor instance is installed")
-			return ctrl.Result{RequeueAfter: 30*time.Second}, nil
+			return ctrl.Result{RequeueAfter: 30 * time.Second}, nil
 		case controllererrors.ErrInstanceNotFoundMsg:
 			controllerutil.RemoveFinalizer(replication, internal.FinalizerName)
 			fallthrough
@@ -104,8 +106,10 @@ func (r *ReplicationReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 	if err != nil {
 		return ctrl.Result{}, err
 	}
-	if err := r.Client.Status().Patch(ctx, replication, patch); err != nil {
-		return ctrl.Result{}, err
+	if !reflect.DeepEqual(original.ObjectMeta.OwnerReferences, replication.ObjectMeta.OwnerReferences) {
+		if err := r.Client.Patch(ctx, replication, patch); err != nil {
+			return ctrl.Result{}, err
+		}
 	}
 
 	// Build a client to connect to the harbor API
@@ -118,7 +122,7 @@ func (r *ReplicationReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 	err = internal.AssertHealthyHarborInstance(ctx, harborClient)
 	if err != nil {
 		reqLogger.Info("waiting till harbor instance is healthy")
-		return ctrl.Result{RequeueAfter: 30*time.Second}, nil
+		return ctrl.Result{RequeueAfter: 30 * time.Second}, nil
 	}
 
 	switch replication.Status.Phase {
@@ -150,7 +154,7 @@ func (r *ReplicationReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		}
 
 		// Set OwnerReference to the parent (source- or destination) registry
-		err = ctrl.SetControllerReference(&registry, replication, r.Scheme)
+		err = controllerutil.SetOwnerReference(replication, &registry, r.Scheme)
 		if err != nil {
 			return ctrl.Result{}, err
 		}
@@ -169,14 +173,13 @@ func (r *ReplicationReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 				PolicyID: replication.Status.ID,
 			}
 
-			if err := harborClient.TriggerReplicationExecution(ctx, replExec); err != nil {
-				reqLogger.Info(fmt.Sprintf("replication execution after creation could not be triggered: %s", err))
-				return ctrl.Result{}, err
+			if err := harborClient.TriggerReplicationExecution(ctx, replExec); err == nil {
+				replication.Status.Phase = v1alpha2.ReplicationStatusPhaseManualExecutionRunning
+				return ctrl.Result{}, r.Client.Status().Patch(ctx, replication, patch)
 			}
 
-			replication.Status.Phase = v1alpha2.ReplicationStatusPhaseManualExecutionRunning
-
-			return ctrl.Result{}, r.Client.Status().Patch(ctx, replication, patch)
+			// that shouldn't happen, but there is nothing we can do about it at this point
+			reqLogger.Error(err, "replication execution after creation could not be triggered. Skipping to next state")
 		}
 
 		replication.Status.Phase = v1alpha2.ReplicationStatusPhaseCompleted

diff --git a/controllers/registries/user_controller.go b/controllers/registries/user_controller.go
@@ -19,9 +19,11 @@ package registries
 import (
 	"context"
 	"errors"
-	controllererrors "github.com/mittwald/harbor-operator/controllers/registries/errors"
+	"reflect"
 	"time"
 
+	controllererrors "github.com/mittwald/harbor-operator/controllers/registries/errors"
+
 	"github.com/mittwald/goharbor-client/v5/apiv2/model"
 	clienterrors "github.com/mittwald/goharbor-client/v5/apiv2/pkg/errors"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
@@ -90,7 +92,8 @@ func (r *UserReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res c
 		return ctrl.Result{}, err
 	}
 
-	patch := client.MergeFrom(user.DeepCopy())
+	original := user.DeepCopy()
+	patch := client.MergeFrom(original)
 
 	if user.ObjectMeta.DeletionTimestamp != nil && user.Status.Phase != v1alpha2.UserStatusPhaseTerminating {
 		user.Status = v1alpha2.UserStatus{Phase: v1alpha2.UserStatusPhaseTerminating}
@@ -108,7 +111,7 @@ func (r *UserReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res c
 		switch err.Error() {
 		case controllererrors.ErrInstanceNotInstalledMsg:
 			reqLogger.Info("waiting till harbor instance is installed")
-			return ctrl.Result{RequeueAfter: 30*time.Second}, nil
+			return ctrl.Result{RequeueAfter: 30 * time.Second}, nil
 		case controllererrors.ErrInstanceNotFoundMsg:
 			controllerutil.RemoveFinalizer(user, internal.FinalizerName)
 			fallthrough
@@ -122,8 +125,10 @@ func (r *UserReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res c
 	if err != nil {
 		return ctrl.Result{}, err
 	}
-	if err := r.Client.Status().Patch(ctx, user, patch); err != nil {
-		return ctrl.Result{}, err
+	if !reflect.DeepEqual(original.ObjectMeta.OwnerReferences, user.ObjectMeta.OwnerReferences) {
+		if err := r.Client.Patch(ctx, user, patch); err != nil {
+			return ctrl.Result{}, err
+		}
 	}
 
 	// Build a client to connect to the harbor API