Update Output reconciliation logic (#345)

* Updates the output reconciliation logic of the Workspace controllers. * Workspace: The output will be synchronized when the run status is either applied or planned_and_finished, whereas previously it was only applied. * Workspace: the output will be synchronized faster and require fewer API calls. * Update controllers/workspace_controller.go Co-authored-by: Alex Somesan <alex.somesan@gmail.com> --------- Co-authored-by: Alex Somesan <alex.somesan@gmail.com>
hashicorp · Mar 6, 2024 · b4d5fd1 · b4d5fd1
1 parent 595ddcf
commit b4d5fd1
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 44 deletions.
diff --git a/.changes/unreleased/ENHANCEMENTS-345-20240227-142955.yaml b/.changes/unreleased/ENHANCEMENTS-345-20240227-142955.yaml
@@ -0,0 +1,6 @@
+kind: ENHANCEMENTS
+body: '`Workspace`: The output will be synchronized when the run status is either
+  `applied` or `planned_and_finished`, whereas previously it was only `applied`.'
+time: 2024-02-27T14:29:55.576978+01:00
+custom:
+  PR: "345"
diff --git a/.changes/unreleased/ENHANCEMENTS-345-20240227-143107.yaml b/.changes/unreleased/ENHANCEMENTS-345-20240227-143107.yaml
@@ -0,0 +1,5 @@
+kind: ENHANCEMENTS
+body: '`Workspace`: The output will be synchronized faster and require fewer API calls.'
+time: 2024-02-27T14:31:07.446525+01:00
+custom:
+  PR: "345"
diff --git a/controllers/workspace_controller.go b/controllers/workspace_controller.go
@@ -581,16 +581,6 @@ func (r *WorkspaceReconciler) reconcileWorkspace(ctx context.Context, w *workspa
 	w.log.Info("Reconcile Run Triggers", "msg", "successfully reconcilied run triggers")
 	r.Recorder.Eventf(&w.instance, corev1.EventTypeNormal, "ReconcileRunTriggers", "Reconcilied run triggers in workspace ID %s", w.instance.Status.WorkspaceID)
 
-	// Reconcile Outputs
-	err = r.reconcileOutputs(ctx, w, workspace)
-	if err != nil {
-		w.log.Error(err, "Reconcile Outputs", "msg", "failed to reconcile outputs")
-		r.Recorder.Eventf(&w.instance, corev1.EventTypeWarning, "ReconcileOutputs", "Failed to reconcile outputs in workspace ID %s", w.instance.Status.WorkspaceID)
-		return err
-	}
-	w.log.Info("Reconcile Outputs", "msg", "successfully reconcilied outputs")
-	r.Recorder.Eventf(&w.instance, corev1.EventTypeNormal, "ReconcileOutputs", "Successfully reconcilied outputs in workspace ID %s", w.instance.Status.WorkspaceID)
-
 	// Reconcile Team Access
 	err = r.reconcileTeamAccess(ctx, w)
 	if err != nil {
@@ -632,6 +622,7 @@ func (r *WorkspaceReconciler) reconcileWorkspace(ctx context.Context, w *workspa
 	r.Recorder.Eventf(&w.instance, corev1.EventTypeNormal, "ReconcileNotifications", "Reconcilied notifications in workspace ID %s", w.instance.Status.WorkspaceID)
 
 	// Reconsile Runs (Status)
+	// This reconciliation should always happen before `reconcileOutputs`
 	err = r.reconcileRuns(ctx, w, workspace)
 	if err != nil {
 		w.log.Error(err, "Reconcile Runs", "msg", "failed to reconcile runs")
@@ -641,5 +632,18 @@ func (r *WorkspaceReconciler) reconcileWorkspace(ctx context.Context, w *workspa
 	w.log.Info("Reconcile Runs", "msg", "successfully reconcilied runs")
 	r.Recorder.Eventf(&w.instance, corev1.EventTypeNormal, "ReconcileRuns", "Successfully reconcilied runs in workspace ID %s", w.instance.Status.WorkspaceID)
 
+	// Reconcile Outputs
+	// This reconciliation should always happen after `reconcileRuns`
+	// TODO:
+	// - move the output reconciliation to runs reconciliation since they depend on each other
+	err = r.reconcileOutputs(ctx, w)
+	if err != nil {
+		w.log.Error(err, "Reconcile Outputs", "msg", "failed to reconcile outputs")
+		r.Recorder.Eventf(&w.instance, corev1.EventTypeWarning, "ReconcileOutputs", "Failed to reconcile outputs in workspace ID %s", w.instance.Status.WorkspaceID)
+		return err
+	}
+	w.log.Info("Reconcile Outputs", "msg", "successfully reconcilied outputs")
+	r.Recorder.Eventf(&w.instance, corev1.EventTypeNormal, "ReconcileOutputs", "Successfully reconcilied outputs in workspace ID %s", w.instance.Status.WorkspaceID)
+
 	return r.updateStatus(ctx, w, workspace)
 }
diff --git a/controllers/workspace_controller_outputs.go b/controllers/workspace_controller_outputs.go
@@ -7,13 +7,12 @@ import (
 	"context"
 	"fmt"
 
+	tfc "github.com/hashicorp/go-tfe"
+	appv1alpha2 "github.com/hashicorp/terraform-cloud-operator/api/v1alpha2"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
-
-	tfc "github.com/hashicorp/go-tfe"
-	appv1alpha2 "github.com/hashicorp/terraform-cloud-operator/api/v1alpha2"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 )
 
@@ -65,8 +64,10 @@ func (r *WorkspaceReconciler) secretAvailable(ctx context.Context, instance *app
 func (r *WorkspaceReconciler) setOutputs(ctx context.Context, w *workspaceInstance) error {
 	workspace, err := w.tfClient.Client.Workspaces.ReadByID(ctx, w.instance.Status.WorkspaceID)
 	if err != nil {
+		w.log.Error(err, "Reconcile Outputs", "mgs", fmt.Sprintf("failed to read workspace by ID %q", w.instance.Status.WorkspaceID))
 		return err
 	}
+
 	if workspace.CurrentStateVersion == nil {
 		return fmt.Errorf("current workspace state version is not available")
 	}
@@ -83,6 +84,7 @@ func (r *WorkspaceReconciler) setOutputs(ctx context.Context, w *workspaceInstan
 
 	outputs, err := w.tfClient.Client.StateVersions.ListOutputs(ctx, workspace.CurrentStateVersion.ID, &tfc.StateVersionOutputsListOptions{})
 	if err != nil {
+		w.log.Error(err, "Reconcile Outputs", "mgs", fmt.Sprintf("failed to list outputs for state version %q", workspace.CurrentStateVersion.ID))
 		return err
 	}
 
@@ -91,7 +93,7 @@ func (r *WorkspaceReconciler) setOutputs(ctx context.Context, w *workspaceInstan
 	for _, o := range outputs.Items {
 		out, err := formatOutput(o)
 		if err != nil {
-			w.log.Error(err, "Reconcile Module Outputs", "mgs", fmt.Sprintf("failed to marshal JSON for %q", o.Name))
+			w.log.Error(err, "Reconcile Outputs", "mgs", fmt.Sprintf("failed to marshal JSON for %q", o.Name))
 			r.Recorder.Event(&w.instance, corev1.EventTypeWarning, "ReconcileOutputs", "failed to marshal JSON")
 			continue
 		}
@@ -150,41 +152,30 @@ func (r *WorkspaceReconciler) setOutputs(ctx context.Context, w *workspaceInstan
 	return nil
 }
 
-func (r *WorkspaceReconciler) runApplied(ctx context.Context, w *workspaceInstance, workspace *tfc.Workspace) (bool, error) {
-	run, err := w.tfClient.Client.Runs.Read(ctx, workspace.CurrentRun.ID)
-	if err != nil {
-		w.log.Error(err, "Reconcile Outputs", "mgs", fmt.Sprintf("failed to read current run ID %s", workspace.CurrentRun.ID))
-		return false, err
-	}
+func (r *WorkspaceReconciler) reconcileOutputs(ctx context.Context, w *workspaceInstance) error {
+	w.log.Info("Reconcile Outputs", "mgs", "new reconciliation event")
 
-	if run.Status == tfc.RunApplied {
-		return true, nil
+	if w.instance.Status.Run == nil {
+		w.log.Info("Reconcile Outputs", "mgs", "no need to update outputs")
+		return nil
 	}
 
-	return false, nil
-}
+	if !w.instance.Status.Run.RunApplied() {
+		w.log.Info("Reconcile Outputs", "message", "no need to update outputs")
+		return nil
+	}
 
-func (r *WorkspaceReconciler) reconcileOutputs(ctx context.Context, w *workspaceInstance, workspace *tfc.Workspace) error {
-	w.log.Info("Reconcile Outputs", "mgs", "new reconciliation event")
+	if w.instance.Status.Run.OutputRunID == w.instance.Status.Run.ID {
+		w.log.Info("Reconcile Outputs", "message", "no need to update outputs")
+		return nil
+	}
 
-	if workspace.CurrentRun != nil {
-		runApplied, err := r.runApplied(ctx, w, workspace)
-		if err != nil {
-			return err
-		}
-		if runApplied {
-			w.log.Info("Reconcile Outputs", "mgs", "run successfully applied")
-			if w.instance.Status.Run != nil && w.instance.Status.Run.OutputRunID != workspace.CurrentRun.ID {
-				w.log.Info("Reconcile Outputs", "mgs", "creating or updating outputs")
-				err = r.setOutputs(ctx, w)
-				if err != nil {
-					return err
-				}
-				w.instance.Status.Run.OutputRunID = workspace.CurrentRun.ID
-				return nil
-			}
-			w.log.Info("Reconcile Outputs", "mgs", "no need to update outputs")
-		}
+	w.log.Info("Reconcile Outputs", "mgs", "creating or updating outputs")
+	if err := r.setOutputs(ctx, w); err != nil {
+		return err
 	}
+
+	w.instance.Status.Run.OutputRunID = w.instance.Status.Run.ID
+
 	return nil
 }