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>

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: degradedApp,
+			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: childApp,
 		}, {}}
 		resourceStatuses := initStatuses(resources)