fix(controller): fix assessment of how app sync state impacts stage h…

…ealth (#1413)

Signed-off-by: Kent Rancourt <[email protected]>

internal/controller/stages/health.go

@@ -169,22 +169,26 @@ func stageHealthForAppSync(
 	app *argocd.Application,
 	revision string,
 ) (kargoapi.HealthState, string) {
-	if revision != "" && app.Status.Sync.Revision != revision {
-		if app.Operation != nil && app.Operation.Sync != nil {
-			return kargoapi.HealthStateProgressing,
-				fmt.Sprintf(
-					"Argo CD Application %q in namespace %q is being synced",
-					app.Name,
-					app.Namespace,
-				)
-		}
-		return kargoapi.HealthStateUnhealthy,
-			fmt.Sprintf(
-				"Argo CD Application %q in namespace %q is not synced to revision %q",
-				app.Name,
-				app.Namespace,
-				revision,
-			)
+	if revision == "" {
+		// No specific revision required
+		return kargoapi.HealthStateHealthy, ""
+	}
+	if app.Operation != nil && app.Operation.Sync != nil {
+		// Still syncing
+		return kargoapi.HealthStateProgressing, fmt.Sprintf(
+			"Argo CD Application %q in namespace %q is being synced",
+			app.Name,
+			app.Namespace,
+		)
+	}
+	// Done syncing
+	if app.Status.Sync.Revision != revision {
+		return kargoapi.HealthStateUnhealthy, fmt.Sprintf(
+			"Argo CD Application %q in namespace %q is not synced to revision %q",
+			app.Name,
+			app.Namespace,
+			revision,
+		)
 	}
 	return kargoapi.HealthStateHealthy, ""
 }

internal/controller/stages/health_test.go

@@ -372,3 +372,90 @@ func TestCheckHealth(t *testing.T) {
 		})
 	}
 }
+
+func TestStageHealthForAppSync(t *testing.T) {
+	testCases := []struct {
+		name       string
+		app        *argocd.Application
+		revision   string
+		assertions func(kargoapi.HealthState)
+	}{
+		{
+			name: "revision is empty",
+			assertions: func(health kargoapi.HealthState) {
+				require.Equal(t, kargoapi.HealthStateHealthy, health)
+			},
+		},
+		{
+			name: "revision is specified; does not match app; still syncing",
+			app: &argocd.Application{
+				Operation: &argocd.Operation{
+					Sync: &argocd.SyncOperation{},
+				},
+				Status: argocd.ApplicationStatus{
+					Sync: argocd.SyncStatus{
+						Revision: "not-the-right-commit",
+					},
+				},
+			},
+			revision: "fake-commit",
+			assertions: func(health kargoapi.HealthState) {
+				require.Equal(t, kargoapi.HealthStateProgressing, health)
+			},
+		},
+		{
+			name: "revision is specified; does not match app; done syncing",
+			app: &argocd.Application{
+				Status: argocd.ApplicationStatus{
+					Sync: argocd.SyncStatus{
+						Revision: "not-the-right-commit",
+					},
+				},
+			},
+			revision: "fake-commit",
+			assertions: func(health kargoapi.HealthState) {
+				require.Equal(t, kargoapi.HealthStateUnhealthy, health)
+			},
+		},
+		{
+			name: "revision is specified; matches app; still syncing",
+			app: &argocd.Application{
+				Operation: &argocd.Operation{
+					Sync: &argocd.SyncOperation{},
+				},
+				Status: argocd.ApplicationStatus{
+					Sync: argocd.SyncStatus{
+						Revision: "fake-commit",
+					},
+				},
+			},
+			revision: "fake-commit",
+			assertions: func(health kargoapi.HealthState) {
+				require.Equal(t, kargoapi.HealthStateProgressing, health)
+			},
+		},
+		{
+			name: "revision is specified; matches app; done syncing",
+			app: &argocd.Application{
+				Status: argocd.ApplicationStatus{
+					Sync: argocd.SyncStatus{
+						Revision: "fake-commit",
+					},
+				},
+			},
+			revision: "fake-commit",
+			assertions: func(health kargoapi.HealthState) {
+				require.Equal(t, kargoapi.HealthStateHealthy, health)
+			},
+		},
+	}
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			health, _ := stageHealthForAppSync(
+				testCase.app,
+				testCase.revision,
+			)
+			testCase.assertions(health)
+		})
+	}
+}