🚀 Add event filtering to the Workspace controller

CHANGELOG.md

@@ -12,6 +12,7 @@ ENHANCEMENT:
 
 ENHANCEMENT:
 
+* `Workspace`: add event filtering to reduce the number of unnecessary reconciliations. [[GH-194](https://github.com/hashicorp/terraform-cloud-operator/pull/194)]
 * `Workspace`: add Terraform version utilized in the Workspace to the status: `status.TerraformVersion`. [[GH-206](https://github.com/hashicorp/terraform-cloud-operator/pull/206)]
 
 DOCS:

controllers/agentpool_controller.go

@@ -16,9 +16,7 @@ import (
 	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/event"
 	"sigs.k8s.io/controller-runtime/pkg/log"
-	"sigs.k8s.io/controller-runtime/pkg/predicate"
 
 	"github.com/go-logr/logr"
 	tfc "github.com/hashicorp/go-tfe"
@@ -106,35 +104,7 @@ func (r *AgentPoolReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 func (r *AgentPoolReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&appv1alpha2.AgentPool{}).
-		WithEventFilter(func() predicate.Predicate {
-			return predicate.Funcs{
-				CreateFunc: func(e event.CreateEvent) bool {
-					return true
-				},
-				UpdateFunc: func(e event.UpdateEvent) bool {
-					if e.ObjectOld == nil || e.ObjectNew == nil {
-						return false
-					}
-
-					// if Generations of new and old objects are not equal this is an update of the object
-					// if Generations and ResourceVersions of new and old objects are equal this is a periodic reconciliation
-					if e.ObjectOld.GetGeneration() != e.ObjectNew.GetGeneration() {
-						return true
-					} else if e.ObjectOld.GetResourceVersion() == e.ObjectNew.GetResourceVersion() {
-						return true
-					}
-
-					// Do not call reconciliation in all other cases
-					return false
-				},
-				DeleteFunc: func(e event.DeleteEvent) bool {
-					return true
-				},
-				GenericFunc: func(e event.GenericEvent) bool {
-					return true
-				},
-			}
-		}()).
+		WithEventFilter(handlePredicates()).
 		Complete(r)
 }
 
@@ -301,7 +271,7 @@ func (r *AgentPoolReconciler) reconcileAgentPool(ctx context.Context, ap *agentP
 		agentPool, err = r.createAgentPool(ctx, ap)
 		if err != nil {
 			ap.log.Error(err, "Reconcile Agent Pool", "msg", "failed to create a new agent pool")
-			r.Recorder.Event(&ap.instance, corev1.EventTypeWarning, "RemoveFinalizer", "Failed to create a new agent pool")
+			r.Recorder.Event(&ap.instance, corev1.EventTypeWarning, "ReconcileAgentPool", "Failed to create a new agent pool")
 			return err
 		}
 		ap.log.Info("Reconcile Agent Pool", "msg", fmt.Sprintf("successfully created a new agent pool with ID %s", agentPool.ID))

controllers/module_controller.go

@@ -19,9 +19,7 @@ import (
 	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/event"
 	"sigs.k8s.io/controller-runtime/pkg/log"
-	"sigs.k8s.io/controller-runtime/pkg/predicate"
 
 	"github.com/go-logr/logr"
 	"github.com/hashicorp/go-slug"
@@ -125,35 +123,7 @@ func (r *ModuleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 func (r *ModuleReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&appv1alpha2.Module{}).
-		WithEventFilter(func() predicate.Predicate {
-			return predicate.Funcs{
-				CreateFunc: func(e event.CreateEvent) bool {
-					return true
-				},
-				UpdateFunc: func(e event.UpdateEvent) bool {
-					if e.ObjectOld == nil || e.ObjectNew == nil {
-						return false
-					}
-
-					// if Generations of new and old objects are not equal this is an update of the object
-					// if Generations and ResourceVersions of new and old objects are equal this is a periodic reconciliation
-					if e.ObjectOld.GetGeneration() != e.ObjectNew.GetGeneration() {
-						return true
-					} else if e.ObjectOld.GetResourceVersion() == e.ObjectNew.GetResourceVersion() {
-						return true
-					}
-
-					// Do not call reconciliation in all other cases
-					return false
-				},
-				DeleteFunc: func(e event.DeleteEvent) bool {
-					return true
-				},
-				GenericFunc: func(e event.GenericEvent) bool {
-					return true
-				},
-			}
-		}()).
+		WithEventFilter(handlePredicates()).
 		Complete(r)
 }
 

controllers/predicate.go

@@ -0,0 +1,49 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: MPL-2.0
+
+package controllers
+
+import (
+	"sigs.k8s.io/controller-runtime/pkg/event"
+	"sigs.k8s.io/controller-runtime/pkg/predicate"
+)
+
+func handlePredicates() predicate.Predicate {
+	return predicate.Funcs{
+		CreateFunc: func(e event.CreateEvent) bool {
+			return true
+		},
+		UpdateFunc: func(e event.UpdateEvent) bool {
+			// Update event has no old object to update.
+			if e.ObjectOld == nil {
+				return false
+			}
+
+			// Update event has no new object to update.
+			if e.ObjectNew == nil {
+				return false
+			}
+
+			// Generation of an object changes when .spec has been updated.
+			// If Generations of new and old objects are not equal the object has be to reconcile.
+			if e.ObjectOld.GetGeneration() != e.ObjectNew.GetGeneration() {
+				return true
+			}
+
+			// ResourceVersion of an object changes when any part of the object has been updated.
+			// If ResourceVersions of new and old objects are equal this is a periodic reconciliation.
+			if e.ObjectOld.GetResourceVersion() == e.ObjectNew.GetResourceVersion() {
+				return true
+			}
+
+			// Do not call reconciliation in all other cases
+			return false
+		},
+		DeleteFunc: func(e event.DeleteEvent) bool {
+			return true
+		},
+		GenericFunc: func(e event.GenericEvent) bool {
+			return true
+		},
+	}
+}

controllers/workspace_controller.go

@@ -115,6 +115,7 @@ func (r *WorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 func (r *WorkspaceReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&appv1alpha2.Workspace{}).
+		WithEventFilter(handlePredicates()).
 		Complete(r)
 }
 
@@ -253,7 +254,8 @@ func (r *WorkspaceReconciler) updateStatus(ctx context.Context, w *workspaceInst
 	return r.Status().Update(ctx, &w.instance)
 }
 
-func (r *WorkspaceReconciler) createWorkspace(ctx context.Context, w *workspaceInstance) error {
+// WORKSPACES
+func (r *WorkspaceReconciler) createWorkspace(ctx context.Context, w *workspaceInstance) (*tfc.Workspace, error) {
 	spec := w.instance.Spec
 	options := tfc.WorkspaceCreateOptions{
 		Name:             tfc.String(spec.Name),
@@ -270,7 +272,7 @@ func (r *WorkspaceReconciler) createWorkspace(ctx context.Context, w *workspaceI
 		if err != nil {
 			w.log.Error(err, "Reconcile Workspace", "msg", "failed to get agent pool ID")
 			r.Recorder.Event(&w.instance, corev1.EventTypeWarning, "ReconcileWorkspace", "Failed to get agent pool ID")
-			return err
+			return nil, err
 		}
 		w.log.Info("Reconcile Workspace", "msg", fmt.Sprintf("agent pool ID %s will be used", agentPoolID))
 		options.AgentPoolID = tfc.String(agentPoolID)
@@ -293,23 +295,14 @@ func (r *WorkspaceReconciler) createWorkspace(ctx context.Context, w *workspaceI
 	if err != nil {
 		w.log.Error(err, "Reconcile Workspace", "msg", "failed to create a new workspace")
 		r.Recorder.Event(&w.instance, corev1.EventTypeWarning, "ReconcileWorkspace", "Failed to create a new workspace")
-		return err
+		return nil, err
 	}
-	w.log.Info("Reconcile Workspace", "msg", "successfully created a new workspace")
-	r.Recorder.Eventf(&w.instance, corev1.EventTypeNormal, "ReconcileWorkspace", "Successfully created a new workspace with ID %s", workspace.ID)
 
-	ws, err := r.reconcileSSHKey(ctx, w, workspace)
-	if err != nil {
-		w.log.Error(err, "Reconcile SSH Key", "msg", "failed to assign ssh key ID")
-		r.Recorder.Eventf(&w.instance, corev1.EventTypeWarning, "ReconcileSSHKey", "Failed to assign SSH Key ID")
-	} else {
-		w.log.Info("Reconcile SSH Key", "msg", "successfully assigned ssh key to the workspace")
-		r.Recorder.Eventf(&w.instance, corev1.EventTypeNormal, "ReconcileSSHKey", "Successfully assigned SSH Key to the workspace with ID %s", workspace.ID)
-		workspace = ws
+	w.instance.Status = appv1alpha2.WorkspaceStatus{
+		WorkspaceID: workspace.ID,
 	}
 
-	// Update status once a workspace has been successfully created
-	return r.updateStatus(ctx, w, workspace)
+	return workspace, nil
 }
 
 func (r *WorkspaceReconciler) readWorkspace(ctx context.Context, w *workspaceInstance) (*tfc.Workspace, error) {
@@ -437,7 +430,14 @@ func (r *WorkspaceReconciler) reconcileWorkspace(ctx context.Context, w *workspa
 	if w.instance.IsCreationCandidate() {
 		w.log.Info("Reconcile Workspace", "msg", "status.WorkspaceID is empty, creating a new workspace")
 		r.Recorder.Event(&w.instance, corev1.EventTypeNormal, "ReconcileWorkspace", "Status.WorkspaceID is empty, creating a new workspace")
-		return r.createWorkspace(ctx, w)
+		_, err = r.createWorkspace(ctx, w)
+		if err != nil {
+			w.log.Error(err, "Reconcile Workspace", "msg", "failed to create a new workspace")
+			r.Recorder.Event(&w.instance, corev1.EventTypeWarning, "ReconcileWorkspace", "Failed to create a new workspace")
+			return err
+		}
+		w.log.Info("Reconcile Workspace", "msg", "successfully created a new workspace")
+		r.Recorder.Eventf(&w.instance, corev1.EventTypeNormal, "ReconcileWorkspace", "Successfully created a new workspace with ID %s", w.instance.Status.WorkspaceID)
 	}
 
 	// read the Terraform Cloud workspace to compare it with the Kubernetes object spec
@@ -447,7 +447,14 @@ func (r *WorkspaceReconciler) reconcileWorkspace(ctx context.Context, w *workspa
 		if err == tfc.ErrResourceNotFound {
 			w.log.Info("Reconcile Workspace", "msg", "workspace not found, creating a new workspace")
 			r.Recorder.Eventf(&w.instance, corev1.EventTypeWarning, "ReconcileWorkspace", "Workspace ID %s not found, creating a new workspace", w.instance.Status.WorkspaceID)
-			return r.createWorkspace(ctx, w)
+			workspace, err = r.createWorkspace(ctx, w)
+			if err != nil {
+				w.log.Error(err, "Reconcile Workspace", "msg", "failed to create a new workspace")
+				r.Recorder.Event(&w.instance, corev1.EventTypeWarning, "ReconcileWorkspace", "Failed to create a new workspace")
+				return err
+			}
+			w.log.Info("Reconcile Workspace", "msg", "successfully created a new workspace")
+			r.Recorder.Eventf(&w.instance, corev1.EventTypeNormal, "ReconcileWorkspace", "Successfully created a new workspace with ID %s", w.instance.Status.WorkspaceID)
 		} else {
 			w.log.Error(err, "Reconcile Workspace", "msg", fmt.Sprintf("failed to read workspace ID %s", w.instance.Status.WorkspaceID))
 			r.Recorder.Eventf(&w.instance, corev1.EventTypeWarning, "ReconcileWorkspace", "Failed to read workspace ID %s", w.instance.Status.WorkspaceID)
@@ -464,20 +471,21 @@ func (r *WorkspaceReconciler) reconcileWorkspace(ctx context.Context, w *workspa
 			r.Recorder.Eventf(&w.instance, corev1.EventTypeWarning, "ReconcileWorkspace", "Failed to update workspace ID %s", w.instance.Status.WorkspaceID)
 			return err
 		}
-		// reconcile SSH key
-		workspace, err = r.reconcileSSHKey(ctx, w, workspace)
-		if err != nil {
-			w.log.Error(err, "Reconcile SSH Key", "msg", "failed to assign ssh key ID")
-			r.Recorder.Eventf(&w.instance, corev1.EventTypeWarning, "ReconcileSSHKey", "Failed to assign SSH Key ID")
-			return err
-		} else {
-			w.log.Info("Reconcile SSH Key", "msg", "successfully reconcile ssh key")
-			r.Recorder.Event(&w.instance, corev1.EventTypeNormal, "ReconcileSSHKey", "Successfully reconcile SSH Key")
-		}
 	} else {
 		w.log.Info("Reconcile Workspace", "msg", fmt.Sprintf("observed and desired states are matching, no need to update workspace ID %s", w.instance.Status.WorkspaceID))
 	}
 
+	// reconcile SSH key
+	err = r.reconcileSSHKey(ctx, w, workspace)
+	if err != nil {
+		w.log.Error(err, "Reconcile SSH Key", "msg", "failed to assign ssh key ID")
+		r.Recorder.Eventf(&w.instance, corev1.EventTypeWarning, "ReconcileSSHKey", "Failed to assign SSH Key ID")
+		return err
+	} else {
+		w.log.Info("Reconcile SSH Key", "msg", "successfully reconcile ssh key")
+		r.Recorder.Event(&w.instance, corev1.EventTypeNormal, "ReconcileSSHKey", "Successfully reconcile SSH Key")
+	}
+
 	// Reconcile Tags
 	err = r.reconcileTags(ctx, w, workspace)
 	if err != nil {
@@ -558,6 +566,5 @@ func (r *WorkspaceReconciler) reconcileWorkspace(ctx context.Context, w *workspa
 	w.log.Info("Reconcile Notifications", "msg", "successfully reconcilied notifications")
 	r.Recorder.Eventf(&w.instance, corev1.EventTypeNormal, "ReconcileNotifications", "Reconcilied notifications in workspace ID %s", w.instance.Status.WorkspaceID)
 
-	// Update status once a workspace has been successfully updated
 	return r.updateStatus(ctx, w, workspace)
 }

controllers/workspace_controller_sshkey.go

@@ -39,37 +39,40 @@ func (r *WorkspaceReconciler) getSSHKeyID(ctx context.Context, w *workspaceInsta
 	return specSSHKey.ID, nil
 }
 
-func (r *WorkspaceReconciler) reconcileSSHKey(ctx context.Context, w *workspaceInstance, workspace *tfc.Workspace) (*tfc.Workspace, error) {
+func (r *WorkspaceReconciler) reconcileSSHKey(ctx context.Context, w *workspaceInstance, workspace *tfc.Workspace) error {
 	if w.instance.Spec.SSHKey == nil {
 		// Verify whether a Workspace has an SSH key and unassign it if so
 		if workspace.SSHKey != nil {
 			w.log.Info("Reconcile SSH Key", "msg", "unassigning the ssh key")
-			return w.tfClient.Client.Workspaces.UnassignSSHKey(ctx, workspace.ID)
+			_, err := w.tfClient.Client.Workspaces.UnassignSSHKey(ctx, workspace.ID)
+			return err
 		}
 
-		return workspace, nil
+		return nil
 	}
 
 	SSHKeyID, err := r.getSSHKeyID(ctx, w)
 	if err != nil {
-		return workspace, err
+		return err
 	}
 
 	// Assign an SSH key to a workspace if nothing is assigned
 	if workspace.SSHKey == nil {
 		w.log.Info("Reconcile SSH Key", "msg", "assigning the ssh key")
-		return w.tfClient.Client.Workspaces.AssignSSHKey(ctx, workspace.ID, tfc.WorkspaceAssignSSHKeyOptions{
+		_, err := w.tfClient.Client.Workspaces.AssignSSHKey(ctx, workspace.ID, tfc.WorkspaceAssignSSHKeyOptions{
 			SSHKeyID: tfc.String(SSHKeyID),
 		})
+		return err
 	}
 
 	// Assign an SSH key to a workspace if it is different from the one in spec
 	if workspace.SSHKey.ID != SSHKeyID {
 		w.log.Info("Reconcile SSH Key", "msg", "assigning the ssh key")
-		return w.tfClient.Client.Workspaces.AssignSSHKey(ctx, workspace.ID, tfc.WorkspaceAssignSSHKeyOptions{
+		_, err := w.tfClient.Client.Workspaces.AssignSSHKey(ctx, workspace.ID, tfc.WorkspaceAssignSSHKeyOptions{
 			SSHKeyID: tfc.String(SSHKeyID),
 		})
+		return err
 	}
 
-	return workspace, nil
+	return nil
 }