fix(controller): prevent unsuccessful promo from blocking next promo (a…

…kuity#3053) Signed-off-by: Kent Rancourt <kent.rancourt@gmail.com>
krancour · Dec 4, 2024 · 91f34a9 · 91f34a9
1 parent 3dcabac
commit 91f34a9
Show file tree

Hide file tree

Showing 9 changed files with 240 additions and 416 deletions.
diff --git a/api/v1alpha1/generated.pb.go b/api/v1alpha1/generated.pb.go
diff --git a/api/v1alpha1/generated.proto b/api/v1alpha1/generated.proto
diff --git a/api/v1alpha1/stage_types.go b/api/v1alpha1/stage_types.go
@@ -511,8 +511,6 @@ type Health struct {
 	// Issues clarifies why a Stage in any state other than Healthy is in that
 	// state. This field will always be the empty when a Stage is Healthy.
 	Issues []string `json:"issues,omitempty" protobuf:"bytes,2,rep,name=issues"`
-	// ArgoCDApps describes the current state of any related ArgoCD Applications.
-	ArgoCDApps []ArgoCDAppStatus `json:"argoCDApps,omitempty" protobuf:"bytes,3,rep,name=argoCDApps"`
 	// Config is the opaque configuration of all health checks performed on this
 	// Stage.
 	Config *apiextensionsv1.JSON `json:"config,omitempty" protobuf:"bytes,4,opt,name=config"`

diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go
diff --git a/charts/kargo/resources/crds/kargo.akuity.io_stages.yaml b/charts/kargo/resources/crds/kargo.akuity.io_stages.yaml
@@ -1101,49 +1101,6 @@ spec:
               health:
                 description: Health is the Stage's last observed health.
                 properties:
-                  argoCDApps:
-                    description: ArgoCDApps describes the current state of any related
-                      ArgoCD Applications.
-                    items:
-                      description: ArgoCDAppStatus describes the current state of
-                        a single ArgoCD Application.
-                      properties:
-                        healthStatus:
-                          description: HealthStatus is the health of the ArgoCD Application.
-                          properties:
-                            message:
-                              type: string
-                            status:
-                              type: string
-                          required:
-                          - status
-                          type: object
-                        name:
-                          description: Name is the name of the ArgoCD Application.
-                          type: string
-                        namespace:
-                          description: Namespace is the namespace of the ArgoCD Application.
-                          type: string
-                        syncStatus:
-                          description: SyncStatus is the sync status of the ArgoCD
-                            Application.
-                          properties:
-                            revision:
-                              type: string
-                            revisions:
-                              items:
-                                type: string
-                              type: array
-                            status:
-                              type: string
-                          required:
-                          - status
-                          type: object
-                      required:
-                      - name
-                      - namespace
-                      type: object
-                    type: array
                   config:
                     description: |-
                       Config is the opaque configuration of all health checks performed on this

diff --git a/internal/controller/stages/regular_stages.go b/internal/controller/stages/regular_stages.go
@@ -707,14 +707,18 @@ func (r *RegularStageReconciler) assessHealth(ctx context.Context, stage *kargoa
 	//  even if the current Promotion did not succeed (e.g. because it was
 	//  aborted).
 	if lastPromo.Status.Phase != kargoapi.PromotionPhaseSucceeded {
-		logger.Debug("Last promotion did not succeed: no health checks to perform")
+		logger.Debug("Last promotion did not succeed: defaulting Stage health to Unhealthy")
 		conditions.Set(&newStatus, &metav1.Condition{
 			Type:               kargoapi.ConditionTypeHealthy,
-			Status:             metav1.ConditionUnknown,
+			Status:             metav1.ConditionFalse,
 			Reason:             fmt.Sprintf("LastPromotion%s", lastPromo.Status.Phase),
-			Message:            "No health checks to perform for unsuccessful Promotion",
+			Message:            "Last Promotion did not succeed",
 			ObservedGeneration: stage.Generation,
 		})
+		newStatus.Health = &kargoapi.Health{
+			Status: kargoapi.HealthStateUnhealthy,
+			Issues: []string{"Last Promotion did not succeed"},
+		}
 		return newStatus
 	}
 

diff --git a/internal/controller/stages/regular_stages_test.go b/internal/controller/stages/regular_stages_test.go
@@ -1221,13 +1221,15 @@ func TestRegularStageReconciler_assessHealth(t *testing.T) {
 				},
 			},
 			assertions: func(t *testing.T, status kargoapi.StageStatus) {
-				assert.Nil(t, status.Health)
+				assert.NotNil(t, status.Health)
+				assert.Equal(t, kargoapi.HealthStateUnhealthy, status.Health.Status)
+				assert.Equal(t, []string{"Last Promotion did not succeed"}, status.Health.Issues)
 
 				healthyCond := conditions.Get(&status, kargoapi.ConditionTypeHealthy)
 				require.NotNil(t, healthyCond)
-				assert.Equal(t, metav1.ConditionUnknown, healthyCond.Status)
+				assert.Equal(t, metav1.ConditionFalse, healthyCond.Status)
 				assert.Equal(t, "LastPromotionAborted", healthyCond.Reason)
-				assert.Equal(t, "No health checks to perform for unsuccessful Promotion", healthyCond.Message)
+				assert.Equal(t, "Last Promotion did not succeed", healthyCond.Message)
 			},
 		},
 		{

diff --git a/ui/src/gen/schema/stages.kargo.akuity.io_v1alpha1.json b/ui/src/gen/schema/stages.kargo.akuity.io_v1alpha1.json
@@ -998,64 +998,6 @@
         "health": {
           "description": "Health is the Stage's last observed health.",
           "properties": {
-            "argoCDApps": {
-              "description": "ArgoCDApps describes the current state of any related ArgoCD Applications.",
-              "items": {
-                "description": "ArgoCDAppStatus describes the current state of a single ArgoCD Application.",
-                "properties": {
-                  "healthStatus": {
-                    "description": "HealthStatus is the health of the ArgoCD Application.",
-                    "properties": {
-                      "message": {
-                        "type": "string"
-                      },
-                      "status": {
-                        "type": "string"
-                      }
-                    },
-                    "required": [
-                      "status"
-                    ],
-                    "type": "object"
-                  },
-                  "name": {
-                    "description": "Name is the name of the ArgoCD Application.",
-                    "type": "string"
-                  },
-                  "namespace": {
-                    "description": "Namespace is the namespace of the ArgoCD Application.",
-                    "type": "string"
-                  },
-                  "syncStatus": {
-                    "description": "SyncStatus is the sync status of the ArgoCD Application.",
-                    "properties": {
-                      "revision": {
-                        "type": "string"
-                      },
-                      "revisions": {
-                        "items": {
-                          "type": "string"
-                        },
-                        "type": "array"
-                      },
-                      "status": {
-                        "type": "string"
-                      }
-                    },
-                    "required": [
-                      "status"
-                    ],
-                    "type": "object"
-                  }
-                },
-                "required": [
-                  "name",
-                  "namespace"
-                ],
-                "type": "object"
-              },
-              "type": "array"
-            },
             "config": {
               "description": "Config is the opaque configuration of all health checks performed on this\nStage.",
               "x-kubernetes-preserve-unknown-fields": true

diff --git a/ui/src/gen/v1alpha1/generated_pb.ts b/ui/src/gen/v1alpha1/generated_pb.ts
@@ -1766,13 +1766,6 @@ export class Health extends Message<Health> {
    */
   issues: string[] = [];
 
-  /**
-   * ArgoCDApps describes the current state of any related ArgoCD Applications.
-   *
-   * @generated from field: repeated git.luolix.top.akuity.kargo.api.v1alpha1.ArgoCDAppStatus argoCDApps = 3;
-   */
-  argoCDApps: ArgoCDAppStatus[] = [];
-
   /**
    * Config is the opaque configuration of all health checks performed on this
    * Stage.
@@ -1798,7 +1791,6 @@ export class Health extends Message<Health> {
   static readonly fields: FieldList = proto2.util.newFieldList(() => [
     { no: 1, name: "status", kind: "scalar", T: 9 /* ScalarType.STRING */, opt: true },
     { no: 2, name: "issues", kind: "scalar", T: 9 /* ScalarType.STRING */, repeated: true },
-    { no: 3, name: "argoCDApps", kind: "message", T: ArgoCDAppStatus, repeated: true },
     { no: 4, name: "config", kind: "message", T: JSON, opt: true },
     { no: 5, name: "output", kind: "message", T: JSON, opt: true },
   ]);