fix(health): app missing health only when all resources are missing (#23995) (#25962)

Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Co-authored-by: Peter Jiang <35584807+pjiang-dev@users.noreply.github.com>

Author: agaudreault

controller/health.go

@@ -8,7 +8,6 @@ import (
 	"github.com/argoproj/gitops-engine/pkg/sync/ignore"
 	kubeutil "github.com/argoproj/gitops-engine/pkg/utils/kube"
 	log "github.com/sirupsen/logrus"
-	"k8s.io/apimachinery/pkg/runtime/schema"
 
 	"github.com/argoproj/argo-cd/v3/common"
 	"github.com/argoproj/argo-cd/v3/pkg/apis/application"
@@ -21,27 +20,35 @@ import (
 func setApplicationHealth(resources []managedResource, statuses []appv1.ResourceStatus, resourceOverrides map[string]appv1.ResourceOverride, app *appv1.Application, persistResourceHealth bool) (health.HealthStatusCode, error) {
 	var savedErr error
 	var errCount uint
+	var containsResources, containsLiveResources bool
 
 	appHealthStatus := health.HealthStatusHealthy
 	for i, res := range resources {
 		if res.Target != nil && hookutil.Skip(res.Target) {
 			continue
 		}
-		if res.Live != nil && res.Live.GetAnnotations() != nil && res.Live.GetAnnotations()[common.AnnotationIgnoreHealthCheck] == "true" {
+		if res.Live != nil && (hookutil.IsHook(res.Live) || ignore.Ignore(res.Live)) {
 			continue
 		}
-		if res.Live != nil && (hookutil.IsHook(res.Live) || ignore.Ignore(res.Live)) {
+
+		// Contains actual resources that are not hooks
+		containsResources = true
+		if res.Live != nil {
+			containsLiveResources = true
+		}
+
+		// Do not aggregate the health of the resource if the annotation to ignore health check is set to true
+		if res.Live != nil && res.Live.GetAnnotations() != nil && res.Live.GetAnnotations()[common.AnnotationIgnoreHealthCheck] == "true" {
 			continue
 		}
 
 		var healthStatus *health.HealthStatus
 		var err error
 		healthOverrides := lua.ResourceHealthOverrides(resourceOverrides)
-		gvk := schema.GroupVersionKind{Group: res.Group, Version: res.Version, Kind: res.Kind}
 		if res.Live == nil {
 			healthStatus = &health.HealthStatus{Status: health.HealthStatusMissing}
 		} else {
-			// App the manages itself should not affect own health
+			// App that manages itself should not affect own health
 			if isSelfReferencedApp(app, kubeutil.GetObjectRef(res.Live)) {
 				continue
 			}
@@ -65,8 +72,8 @@ func setApplicationHealth(resources []managedResource, statuses []appv1.Resource
 			statuses[i].Health = nil
 		}
 
-		// Is health status is missing but resource has not built-in/custom health check then it should not affect parent app health
-		if _, hasOverride := healthOverrides[lua.GetConfigMapKey(gvk)]; healthStatus.Status == health.HealthStatusMissing && !hasOverride && health.GetHealthCheckFunc(gvk) == nil {
+		// Missing resources should not affect parent app health - the OutOfSync status already indicates resources are missing
+		if res.Live == nil && healthStatus.Status == health.HealthStatusMissing {
 			continue
 		}
 
@@ -79,6 +86,12 @@ func setApplicationHealth(resources []managedResource, statuses []appv1.Resource
 			appHealthStatus = healthStatus.Status
 		}
 	}
+
+	// If the app is expected to have resources but does not contain any live resources, set the app health to missing
+	if containsResources && !containsLiveResources && health.IsWorse(appHealthStatus, health.HealthStatusMissing) {
+		appHealthStatus = health.HealthStatusMissing
+	}
+
 	if persistResourceHealth {
 		app.Status.ResourceHealthSource = appv1.ResourceHealthLocationInline
 	} else {

controller/health_test.go

@@ -16,6 +16,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"sigs.k8s.io/yaml"
 
+	"github.com/argoproj/argo-cd/v3/common"
 	"github.com/argoproj/argo-cd/v3/pkg/apis/application"
 	appv1 "github.com/argoproj/argo-cd/v3/pkg/apis/application/v1alpha1"
 	"github.com/argoproj/argo-cd/v3/util/lua"
@@ -103,12 +104,103 @@ func TestSetApplicationHealth_ResourceHealthNotPersisted(t *testing.T) {
 	assert.Nil(t, resourceStatuses[0].Health)
 }
 
+func TestSetApplicationHealth_NoResource(t *testing.T) {
+	resources := []managedResource{}
+	resourceStatuses := initStatuses(resources)
+
+	healthStatus, err := setApplicationHealth(resources, resourceStatuses, lua.ResourceHealthOverrides{}, app, true)
+	require.NoError(t, err)
+	assert.Equal(t, health.HealthStatusHealthy, healthStatus)
+}
+
+func TestSetApplicationHealth_OnlyHooks(t *testing.T) {
+	pod := resourceFromFile("./testdata/pod-running-restart-always.yaml")
+	pod.SetAnnotations(map[string]string{synccommon.AnnotationKeyHook: string(synccommon.HookTypeSync)})
+
+	resources := []managedResource{{
+		Group: "", Version: "v1", Kind: "Pod", Target: &pod, Live: &pod,
+	}}
+	resourceStatuses := initStatuses(resources)
+
+	healthStatus, err := setApplicationHealth(resources, resourceStatuses, lua.ResourceHealthOverrides{}, app, true)
+	require.NoError(t, err)
+	assert.Equal(t, health.HealthStatusHealthy, healthStatus)
+}
+
 func TestSetApplicationHealth_MissingResource(t *testing.T) {
 	pod := resourceFromFile("./testdata/pod-running-restart-always.yaml")
+	pod2 := pod.DeepCopy()
+	pod2.SetName("pod2")
+
+	resources := []managedResource{
+		{Group: "", Version: "v1", Kind: "Pod", Target: &pod},
+		{Group: "", Version: "v1", Kind: "Pod", Target: pod2, Live: pod2},
+	}
+	resourceStatuses := initStatuses(resources)
+
+	healthStatus, err := setApplicationHealth(resources, resourceStatuses, lua.ResourceHealthOverrides{}, app, true)
+	require.NoError(t, err)
+	assert.Equal(t, health.HealthStatusHealthy, healthStatus)
+}
+
+func TestSetApplicationHealth_MissingResource_WithIgnoreHealthcheck(t *testing.T) {
+	pod := resourceFromFile("./testdata/pod-running-restart-always.yaml")
+	pod2 := pod.DeepCopy()
+	pod2.SetName("pod2")
+	pod2.SetAnnotations(map[string]string{common.AnnotationIgnoreHealthCheck: "true"})
+
+	resources := []managedResource{
+		{Group: "", Version: "v1", Kind: "Pod", Target: &pod},
+		{Group: "", Version: "v1", Kind: "Pod", Target: pod2, Live: pod2},
+	}
+	resourceStatuses := initStatuses(resources)
+
+	healthStatus, err := setApplicationHealth(resources, resourceStatuses, lua.ResourceHealthOverrides{}, app, true)
+	require.NoError(t, err)
+	assert.Equal(t, health.HealthStatusHealthy, healthStatus)
+}
+
+func TestSetApplicationHealth_MissingResource_WithChildApp(t *testing.T) {
+	childApp := newAppLiveObj(health.HealthStatusUnknown)
+	pod := resourceFromFile("./testdata/pod-running-restart-always.yaml")
+	resources := []managedResource{
+		{Group: application.Group, Version: "v1alpha1", Kind: application.ApplicationKind, Target: childApp, Live: childApp},
+		{Group: "", Version: "v1", Kind: "Pod", Target: &pod},
+	}
+	resourceStatuses := initStatuses(resources)
+
+	healthStatus, err := setApplicationHealth(resources, resourceStatuses, lua.ResourceHealthOverrides{}, app, true)
+	require.NoError(t, err)
+	assert.Equal(t, health.HealthStatusHealthy, healthStatus)
+}
+
+func TestSetApplicationHealth_AllMissingResources(t *testing.T) {
+	pod := resourceFromFile("./testdata/pod-running-restart-always.yaml")
+	pod2 := pod.DeepCopy()
+	pod2.SetName("pod2")
+
+	resources := []managedResource{
+		{Group: "", Version: "v1", Kind: "Pod", Target: &pod},
+		{Group: "", Version: "v1", Kind: "Pod", Target: pod2},
+	}
+	resourceStatuses := initStatuses(resources)
+
+	healthStatus, err := setApplicationHealth(resources, resourceStatuses, lua.ResourceHealthOverrides{}, app, true)
+	require.NoError(t, err)
+	assert.Equal(t, health.HealthStatusMissing, healthStatus)
+}
+
+func TestSetApplicationHealth_AllMissingResources_WithHooks(t *testing.T) {
+	pod := resourceFromFile("./testdata/pod-running-restart-always.yaml")
+	pod2 := pod.DeepCopy()
+	pod2.SetName("pod2")
+	pod2.SetAnnotations(map[string]string{synccommon.AnnotationKeyHook: string(synccommon.HookTypeSync)})
 
 	resources := []managedResource{{
 		Group: "", Version: "v1", Kind: "Pod", Target: &pod,
-	}, {}}
+	}, {
+		Group: "", Version: "v1", Kind: "Pod", Target: pod2, Live: pod2,
+	}}
 	resourceStatuses := initStatuses(resources)
 
 	healthStatus, err := setApplicationHealth(resources, resourceStatuses, lua.ResourceHealthOverrides{}, app, true)
@@ -149,32 +241,6 @@ func TestSetApplicationHealth_HealthImproves(t *testing.T) {
 	}
 }
 
-func TestSetApplicationHealth_MissingResourceNoBuiltHealthCheck(t *testing.T) {
-	cm := resourceFromFile("./testdata/configmap.yaml")
-
-	resources := []managedResource{{
-		Group: "", Version: "v1", Kind: "ConfigMap", Target: &cm,
-	}}
-	resourceStatuses := initStatuses(resources)
-
-	t.Run("NoOverride", func(t *testing.T) {
-		healthStatus, err := setApplicationHealth(resources, resourceStatuses, lua.ResourceHealthOverrides{}, app, true)
-		require.NoError(t, err)
-		assert.Equal(t, health.HealthStatusHealthy, healthStatus)
-		assert.Equal(t, health.HealthStatusMissing, resourceStatuses[0].Health.Status)
-	})
-
-	t.Run("HasOverride", func(t *testing.T) {
-		healthStatus, err := setApplicationHealth(resources, resourceStatuses, lua.ResourceHealthOverrides{
-			lua.GetConfigMapKey(schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"}): appv1.ResourceOverride{
-				HealthLua: "some health check",
-			},
-		}, app, true)
-		require.NoError(t, err)
-		assert.Equal(t, health.HealthStatusMissing, healthStatus)
-	})
-}
-
 func newAppLiveObj(status health.HealthStatusCode) *unstructured.Unstructured {
 	app := appv1.Application{
 		ObjectMeta: metav1.ObjectMeta{
@@ -214,9 +280,9 @@ return hs`,
 	}
 
 	t.Run("ChildAppDegraded", func(t *testing.T) {
-		degradedApp := newAppLiveObj(health.HealthStatusDegraded)
+		childApp := newAppLiveObj(health.HealthStatusDegraded)
 		resources := []managedResource{{
-			Group: application.Group, Version: "v1alpha1", Kind: application.ApplicationKind, Live: degradedApp,
+			Group: application.Group, Version: "v1alpha1", Kind: application.ApplicationKind, Live: childApp,
 		}, {}}
 		resourceStatuses := initStatuses(resources)
 
@@ -226,9 +292,21 @@ return hs`,
 	})
 
 	t.Run("ChildAppMissing", func(t *testing.T) {
-		degradedApp := newAppLiveObj(health.HealthStatusMissing)
+		childApp := newAppLiveObj(health.HealthStatusMissing)
+		resources := []managedResource{{
+			Group: application.Group, Version: "v1alpha1", Kind: application.ApplicationKind, Live: childApp,
+		}, {}}
+		resourceStatuses := initStatuses(resources)
+
+		healthStatus, err := setApplicationHealth(resources, resourceStatuses, overrides, app, true)
+		require.NoError(t, err)
+		assert.Equal(t, health.HealthStatusHealthy, healthStatus)
+	})
+
+	t.Run("ChildAppUnknown", func(t *testing.T) {
+		childApp := newAppLiveObj(health.HealthStatusUnknown)
 		resources := []managedResource{{
-			Group: application.Group, Version: "v1alpha1", Kind: application.ApplicationKind, Live: degradedApp,
+			Group: application.Group, Version: "v1alpha1", Kind: application.ApplicationKind, Live: childApp,
 		}, {}}
 		resourceStatuses := initStatuses(resources)
 

docs/operator-manual/upgrading/3.3-3.4.md

@@ -0,0 +1,19 @@
+# v3.3 to 3.4
+
+## Applications with `Missing` health status
+
+The behavior of Application health status has changed to be more consistent and informative. Previously, Applications would show `Missing` health status inconsistently depending on whether missing resources had built-in or custom health checks defined.
+
+**New behavior:**
+
+- Applications now show `Missing` health **only when ALL resources are missing** (e.g., before the first sync)
+- Individual missing resources no longer affect the Application's overall health status
+- The health status now reflects the aggregated health of **existing resources**
+- The `OutOfSync` status already indicates when resources are missing, making the health status redundant for individual missing resources
+- If the defined resource health check is explicitly returning `Missing` for an existing resource, that will still be reflected in the overall Application health
+
+**Impact:**
+
+- Applications with some missing resources will now show the health of their existing resources (e.g., `Healthy`, `Progressing`, `Degraded`) instead of `Missing`
+- Automation relying on the Application Health status to detect missing resources should now check the Sync status for `OutOfSync` instead, and optionally inspect individual resource health if needed.
+- Users can now distinguish between an Application that has never been synced (all resources missing = `Missing` health) vs. an Application with some resources deleted (shows health of remaining resources)

docs/operator-manual/upgrading/overview.md

@@ -39,6 +39,7 @@ kubectl apply -n argocd --server-side --force-conflicts -f https://raw.githubuse
 
 <hr/>
 
+- [v3.3 to v3.4](./3.3-3.4.md)
 - [v3.2 to v3.3](./3.2-3.3.md)
 - [v3.1 to v3.2](./3.1-3.2.md)
 - [v3.0 to v3.1](./3.0-3.1.md)

mkdocs.yml

@@ -135,6 +135,7 @@ nav:
     - operator-manual/server-commands/additional-configuration-method.md
   - Upgrading:
     - operator-manual/upgrading/overview.md
+    - operator-manual/upgrading/3.3-3.4.md
     - operator-manual/upgrading/3.2-3.3.md
     - operator-manual/upgrading/3.1-3.2.md
     - operator-manual/upgrading/3.0-3.1.md

test/e2e/app_management_ns_test.go

@@ -291,24 +291,24 @@ func TestNamespacedAppCreationWithoutForceUpdate(t *testing.T) {
 
 func TestNamespacedDeleteAppResource(t *testing.T) {
 	ctx := Given(t)
-
-	ctx.
-		Path(guestbookPath).
+	ctx.Path(guestbookPath).
 		SetTrackingMethod("annotation").
 		SetAppNamespace(fixture.AppNamespace()).
 		When().
 		CreateApp().
 		Sync().
 		Then().
 		Expect(SyncStatusIs(SyncStatusCodeSynced)).
+		Expect(HealthIs(health.HealthStatusHealthy)).
 		And(func(_ *Application) {
 			// app should be listed
 			if _, err := fixture.RunCli("app", "delete-resource", ctx.AppQualifiedName(), "--kind", "Service", "--resource-name", "guestbook-ui"); err != nil {
 				require.NoError(t, err)
 			}
 		}).
 		Expect(SyncStatusIs(SyncStatusCodeOutOfSync)).
-		Expect(HealthIs(health.HealthStatusMissing))
+		Expect(HealthIs(health.HealthStatusHealthy)).
+		Expect(ResourceHealthIs("Service", "guestbook-ui", health.HealthStatusMissing))
 }
 
 // demonstrate that we cannot use a standard sync when an immutable field is changed, we must use "force"

test/e2e/app_management_test.go

@@ -487,7 +487,8 @@ func TestDeleteAppResource(t *testing.T) {
 			}
 		}).
 		Expect(SyncStatusIs(SyncStatusCodeOutOfSync)).
-		Expect(HealthIs(health.HealthStatusMissing))
+		Expect(HealthIs(health.HealthStatusHealthy)).
+		Expect(ResourceHealthIs("Service", "guestbook-ui", health.HealthStatusMissing))
 }
 
 // Fix for issue #2677, support PATCH in HTTP service