Merge pull request #180 from gibizer/refactor-defer

Refactor our defer calls
openstack-k8s-operators · Dec 15, 2022 · 7c4ca6b · 7c4ca6b
2 parents 765b05f + 8b6fe3d
commit 7c4ca6b
Show file tree

Hide file tree

Showing 5 changed files with 44 additions and 100 deletions.
diff --git a/controllers/common.go b/controllers/common.go
@@ -274,3 +274,39 @@ func (r *ReconcilerBase) GenerateConfigs(
 	// to enable unit testing at some point.
 	return configmap.EnsureConfigMaps(ctx, h, instance, cms, envVars)
 }
+
+func (r *ReconcilerBase) patchInstance(ctx context.Context, h *helper.Helper, instance client.Object) error {
+	var err error
+
+	if err = h.SetAfter(instance); err != nil {
+		util.LogErrorForObject(h, err, "Set after and calc patch/diff", instance)
+		return err
+	}
+
+	changes := h.GetChanges()
+	patch := client.MergeFrom(h.GetBeforeObject())
+
+	if changes["metadata"] {
+		err = r.Client.Patch(ctx, instance, patch)
+		if k8s_errors.IsConflict(err) {
+			util.LogForObject(h, "Metadata update conflict", instance)
+			return err
+		} else if err != nil && !k8s_errors.IsNotFound(err) {
+			util.LogErrorForObject(h, err, "Metadate update failed", instance)
+			return err
+		}
+	}
+
+	if changes["status"] {
+		err = r.Client.Status().Patch(ctx, instance, patch)
+		if k8s_errors.IsConflict(err) {
+			util.LogForObject(h, "Status update conflict", instance)
+			return err
+
+		} else if err != nil && !k8s_errors.IsNotFound(err) {
+			util.LogErrorForObject(h, err, "Status update failed", instance)
+			return err
+		}
+	}
+	return nil
+}
diff --git a/controllers/nova_controller.go b/controllers/nova_controller.go
@@ -25,7 +25,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
-	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 
@@ -112,40 +111,11 @@ func (r *NovaReconciler) Reconcile(ctx context.Context, req ctrl.Request) (resul
 			instance.Status.Conditions.MarkTrue(
 				condition.ReadyCondition, condition.ReadyMessage)
 		}
-		if err := h.SetAfter(instance); err != nil {
-			util.LogErrorForObject(h, err, "Set after and calc patch/diff", instance)
+		err := r.patchInstance(ctx, h, instance)
+		if err != nil {
 			_err = err
 			return
 		}
-
-		changes := h.GetChanges()
-		patch := client.MergeFrom(h.GetBeforeObject())
-
-		if changes["metadata"] {
-			err = r.Client.Patch(ctx, instance, patch)
-			if k8s_errors.IsConflict(err) {
-				util.LogForObject(h, "Metadata update conflict", instance)
-				_err = err
-				return
-			} else if err != nil && !k8s_errors.IsNotFound(err) {
-				util.LogErrorForObject(h, err, "Metadate update failed", instance)
-				_err = err
-				return
-			}
-		}
-
-		if changes["status"] {
-			err = r.Client.Status().Patch(ctx, instance, patch)
-			if k8s_errors.IsConflict(err) {
-				util.LogForObject(h, "Status update conflict", instance)
-				_err = err
-				return
-			} else if err != nil && !k8s_errors.IsNotFound(err) {
-				util.LogErrorForObject(h, err, "Status update failed", instance)
-				_err = err
-				return
-			}
-		}
 	}()
 
 	if !instance.DeletionTimestamp.IsZero() {

diff --git a/controllers/novaapi_controller.go b/controllers/novaapi_controller.go
@@ -23,7 +23,6 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
-	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 
@@ -116,40 +115,11 @@ func (r *NovaAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request) (re
 			instance.Status.Conditions.MarkTrue(
 				condition.ReadyCondition, condition.ReadyMessage)
 		}
-		if err := h.SetAfter(instance); err != nil {
-			util.LogErrorForObject(h, err, "Set after and calc patch/diff", instance)
+		err := r.patchInstance(ctx, h, instance)
+		if err != nil {
 			_err = err
 			return
 		}
-
-		changes := h.GetChanges()
-		patch := client.MergeFrom(h.GetBeforeObject())
-
-		if changes["metadata"] {
-			err = r.Client.Patch(ctx, instance, patch)
-			if k8s_errors.IsConflict(err) {
-				util.LogForObject(h, "Metadata update conflict", instance)
-				_err = err
-				return
-			} else if err != nil && !k8s_errors.IsNotFound(err) {
-				util.LogErrorForObject(h, err, "Metadate update failed", instance)
-				_err = err
-				return
-			}
-		}
-
-		if changes["status"] {
-			err = r.Client.Status().Patch(ctx, instance, patch)
-			if k8s_errors.IsConflict(err) {
-				util.LogForObject(h, "Status update conflict", instance)
-				_err = err
-				return
-			} else if err != nil && !k8s_errors.IsNotFound(err) {
-				util.LogErrorForObject(h, err, "Status update failed", instance)
-				_err = err
-				return
-			}
-		}
 	}()
 
 	if !instance.DeletionTimestamp.IsZero() {

diff --git a/controllers/novacell_controller.go b/controllers/novacell_controller.go
@@ -23,7 +23,6 @@ import (
 	k8s_errors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	ctrl "sigs.k8s.io/controller-runtime"
-	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 
@@ -97,26 +96,11 @@ func (r *NovaCellReconciler) Reconcile(ctx context.Context, req ctrl.Request) (r
 			instance.Status.Conditions.MarkTrue(
 				condition.ReadyCondition, condition.ReadyMessage)
 		}
-		if err := h.SetAfter(instance); err != nil {
-			util.LogErrorForObject(h, err, "Set after and calc patch/diff", instance)
+		err := r.patchInstance(ctx, h, instance)
+		if err != nil {
 			_err = err
 			return
 		}
-
-		if changed := h.GetChanges()["status"]; changed {
-			patch := client.MergeFrom(h.GetBeforeObject())
-
-			err = r.Client.Status().Patch(ctx, instance, patch)
-			if k8s_errors.IsConflict(err) {
-				util.LogForObject(h, "Status update conflict", instance)
-				_err = err
-				return
-			} else if err != nil && !k8s_errors.IsNotFound(err) {
-				util.LogErrorForObject(h, err, "Status update failed", instance)
-				_err = err
-				return
-			}
-		}
 	}()
 
 	result, err = r.ensureConductor(ctx, h, instance)

diff --git a/controllers/novaconductor_controller.go b/controllers/novaconductor_controller.go
@@ -24,7 +24,6 @@ import (
 	k8s_errors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
-	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 
 	common "github.com/openstack-k8s-operators/lib-common/modules/common"
@@ -107,26 +106,11 @@ func (r *NovaConductorReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 			instance.Status.Conditions.MarkTrue(
 				condition.ReadyCondition, condition.ReadyMessage)
 		}
-		if err := h.SetAfter(instance); err != nil {
-			util.LogErrorForObject(h, err, "Set after and calc patch/diff", instance)
+		err := r.patchInstance(ctx, h, instance)
+		if err != nil {
 			_err = err
 			return
 		}
-
-		if changed := h.GetChanges()["status"]; changed {
-			patch := client.MergeFrom(h.GetBeforeObject())
-
-			err = r.Client.Status().Patch(ctx, instance, patch)
-			if k8s_errors.IsConflict(err) {
-				util.LogForObject(h, "Status update conflict", instance)
-				_err = err
-				return
-			} else if err != nil && !k8s_errors.IsNotFound(err) {
-				util.LogErrorForObject(h, err, "Status update failed", instance)
-				_err = err
-				return
-			}
-		}
 	}()
 
 	// TODO(gibi): Can we use a simple map[string][string] for hashes?