fix(#281): check the reason before updating the status

- since the condition type is a bit generic
  ("ConfigureWorkflowCompleted"), we can see a world where, in the
  future, we start setting that condition to true for other reasons
- Condition.Reason is the kubernetes recommended way to differentiate
  between the different reasons a Status has a particular value.

Co-authored-by: Sapphire Mason-Brown <[email protected]>

controllers/dynamic_resource_request_controller.go

@@ -159,7 +159,7 @@ func (r *DynamicResourceRequestController) Reconcile(ctx context.Context, req ct
 	}
 
 	workflowCompletedCondition := resourceutil.GetCondition(rr, resourceutil.ConfigureWorkflowCompletedCondition)
-	if workflowCompletedCondition != nil && workflowCompletedCondition.Status == v1.ConditionTrue {
+	if workflowCompletedCondition != nil && workflowCompletedCondition.Status == v1.ConditionTrue && workflowCompletedCondition.Reason == resourceutil.PipelinesExecutedSuccessfully {
 		lastTransitionTime := workflowCompletedCondition.LastTransitionTime.Format(time.RFC3339)
 		lastSuccessfulTime := resourceutil.GetStatus(rr, "lastSuccessfulConfigureWorkflowTime")
 

controllers/dynamic_resource_request_controller_test.go

@@ -284,7 +284,7 @@ var _ = Describe("DynamicResourceRequestController", func() {
 					Expect(status).To(BeEmpty())
 				})
 
-				It("is set to the time the workflow finished successfully", func() {
+				It("is set to the time the workflow finished with the right reason", func() {
 					lastTransitionTime := time.Now().Add(-time.Minute)
 					setConfigureWorkflowStatus(resReq, v1.ConditionTrue, lastTransitionTime)
 
@@ -325,6 +325,28 @@ var _ = Describe("DynamicResourceRequestController", func() {
 					Expect(before).To(Equal(after))
 				})
 
+				It("remains the same when the condition is True but not for the right Reason", func() {
+					before := resourceutil.GetStatus(resReq, "lastSuccessfulConfigureWorkflowTime")
+					Expect(before).NotTo(BeEmpty())
+					resourceutil.SetCondition(resReq, &clusterv1.Condition{
+						Type:               resourceutil.ConfigureWorkflowCompletedCondition,
+						Status:             v1.ConditionTrue,
+						Reason:             "SomeOtherReason",
+						Message:            fmt.Sprintf("some-reason-%s", time.Now().Format(time.RFC3339)),
+						LastTransitionTime: metav1.NewTime(time.Now()),
+					})
+					Expect(fakeK8sClient.Status().Update(ctx, resReq)).To(Succeed())
+
+					result, err := t.reconcileUntilCompletion(reconciler, resReq)
+					Expect(err).NotTo(HaveOccurred())
+					Expect(result).To(Equal(ctrl.Result{}))
+
+					Expect(fakeK8sClient.Get(ctx, resReqNameNamespace, resReq)).To(Succeed())
+					after := resourceutil.GetStatus(resReq, "lastSuccessfulConfigureWorkflowTime")
+					Expect(after).NotTo(BeEmpty())
+					Expect(before).To(Equal(after))
+				})
+
 				It("is updated when the workflow finishes successfully at a later time", func() {
 					before := resourceutil.GetStatus(resReq, "lastSuccessfulConfigureWorkflowTime")
 					Expect(before).NotTo(BeEmpty())
@@ -360,8 +382,8 @@ func setConfigureWorkflowStatus(resReq *unstructured.Unstructured, status v1.Con
 	resourceutil.SetCondition(resReq, &clusterv1.Condition{
 		Type:               resourceutil.ConfigureWorkflowCompletedCondition,
 		Status:             status,
-		Message:            "some-message",
-		Reason:             fmt.Sprintf("some-reason-%s", t.Format(time.RFC3339)),
+		Reason:             resourceutil.PipelinesExecutedSuccessfully,
+		Message:            fmt.Sprintf("some-reason-%s", t.Format(time.RFC3339)),
 		LastTransitionTime: metav1.NewTime(t),
 	})
 	Expect(fakeK8sClient.Status().Update(ctx, resReq)).To(Succeed())

lib/resourceutil/util.go

@@ -18,6 +18,7 @@ import (
 const (
 	ConfigureWorkflowCompletedCondition    = clusterv1.ConditionType("ConfigureWorkflowCompleted")
 	ConfigureWorkflowCompletedFailedReason = "ConfigureWorkflowFailed"
+	PipelinesExecutedSuccessfully          = "PipelinesExecutedSuccessfully"
 	ManualReconciliationLabel              = "kratix.io/manual-reconciliation"
 	ReconcileResourcesLabel                = "kratix.io/reconcile-resources"
 	promiseAvailableCondition              = clusterv1.ConditionType("PromiseAvailable")