🐛 Fix health check to support partial probe results with multi-probe configuration

When multiple health check probes are configured, if the first probe
returns empty feedback results, the health check was incorrectly marking
the addon as Unknown and returning early, preventing other probes from
being evaluated.

This fix changes the logic to:
- Collect all empty probe fields instead of returning immediately
- Only mark the addon as Unknown if ALL probes have no results
- Allow partial probe results to be considered valid
- Remove redundant check for non-empty results after healthChecker

Fixes #341

Signed-off-by: Claude <[email protected]>
Signed-off-by: zhujian <[email protected]>

Author: zhujian7

pkg/addonmanager/controllers/agentdeploy/healthcheck_sync.go

@@ -188,28 +188,20 @@ func (s *healthCheckSyncer) probeAddonStatusByWorks(
 	}
 
 	var FieldResults []agent.FieldResult
+	var emptyProbeFields []workapiv1.ResourceIdentifier
 
 	for _, field := range probeFields {
 		results := findResultsByIdentifier(field.ResourceIdentifier, manifestConditions)
 		// if no results are returned. it is possible that work agent has not returned the feedback value.
-		// mark condition to unknown
+		// collect these fields and check if all probes are empty later
 		if len(results) == 0 {
-			meta.SetStatusCondition(&addon.Status.Conditions, metav1.Condition{
-				Type:   addonapiv1alpha1.ManagedClusterAddOnConditionAvailable,
-				Status: metav1.ConditionUnknown,
-				Reason: addonapiv1alpha1.AddonAvailableReasonNoProbeResult,
-				Message: fmt.Sprintf("Probe results are not returned for %s/%s: %s/%s",
-					field.ResourceIdentifier.Group, field.ResourceIdentifier.Resource,
-					field.ResourceIdentifier.Namespace, field.ResourceIdentifier.Name),
-			})
-			return nil
+			emptyProbeFields = append(emptyProbeFields, field.ResourceIdentifier)
+			continue
 		}
 
 		// healthCheck will be ignored if healthChecker is set
 		if healthChecker != nil {
-			if len(results) != 0 {
-				FieldResults = append(FieldResults, results...)
-			}
+			FieldResults = append(FieldResults, results...)
 			continue
 		}
 
@@ -238,6 +230,28 @@ func (s *healthCheckSyncer) probeAddonStatusByWorks(
 
 	}
 
+	// If all probe fields have no results, mark condition to unknown
+	if len(emptyProbeFields) == len(probeFields) && len(probeFields) > 0 {
+		var msg string
+		if len(emptyProbeFields) == 1 {
+			msg = fmt.Sprintf("Probe results are not returned for %s/%s: %s/%s",
+				emptyProbeFields[0].Group, emptyProbeFields[0].Resource,
+				emptyProbeFields[0].Namespace, emptyProbeFields[0].Name)
+		} else {
+			msg = fmt.Sprintf("Probe results are not returned for %d probe fields", len(emptyProbeFields))
+		}
+		meta.SetStatusCondition(&addon.Status.Conditions, metav1.Condition{
+			Type:    addonapiv1alpha1.ManagedClusterAddOnConditionAvailable,
+			Status:  metav1.ConditionUnknown,
+			Reason:  addonapiv1alpha1.AddonAvailableReasonNoProbeResult,
+			Message: msg,
+		})
+		return nil
+	}
+
+	// If we have FieldResults but some probes are empty, still proceed with healthChecker
+	// This allows partial probe results to be considered valid
+
 	if healthChecker != nil {
 		if err := healthChecker(FieldResults, cluster, addon); err != nil {
 			meta.SetStatusCondition(&addon.Status.Conditions, metav1.Condition{

pkg/addonmanager/controllers/agentdeploy/healthcheck_sync_test.go

@@ -1259,6 +1259,74 @@ func TestHealthCheckReconcile(t *testing.T) {
 				Message: "test add-on is available.",
 			},
 		},
+		{
+			name: "Health check mode is work with multi probes where first probe has empty results but second has valid results",
+			testAddon: &healthCheckTestAgent{name: "test",
+				health: newDeploymentsCheckAllProber(
+					types.NamespacedName{Name: "test-deployment-nonexistent", Namespace: "default"},
+					types.NamespacedName{Name: "test-deployment1", Namespace: "default"}),
+			},
+			addon: addontesting.NewAddonWithConditions("test", "cluster1", manifestAppliedCondition),
+			existingWork: []runtime.Object{
+				&v1.ManifestWork{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "addon-test-deploy-01",
+						Namespace: "cluster1",
+						Labels: map[string]string{
+							"open-cluster-management.io/addon-name": "test",
+						},
+					},
+					Spec: v1.ManifestWorkSpec{},
+					Status: v1.ManifestWorkStatus{
+						ResourceStatus: v1.ManifestResourceStatus{
+							Manifests: []v1.ManifestCondition{
+								{
+									ResourceMeta: v1.ManifestResourceMeta{
+										Ordinal:   0,
+										Group:     "apps",
+										Version:   "",
+										Kind:      "",
+										Resource:  "deployments",
+										Name:      "test-deployment1",
+										Namespace: "default",
+									},
+									StatusFeedbacks: v1.StatusFeedbackResult{
+										Values: []v1.FeedbackValue{
+											{
+												Name: "Replicas",
+												Value: v1.FieldValue{
+													Integer: boolPtr(1),
+												},
+											},
+											{
+												Name: "ReadyReplicas",
+												Value: v1.FieldValue{
+													Integer: boolPtr(1),
+												},
+											},
+										},
+									},
+								},
+							},
+						},
+						Conditions: []metav1.Condition{
+							{
+								Type:   v1.WorkAvailable,
+								Status: metav1.ConditionTrue,
+							},
+						},
+					},
+				},
+			},
+			expectedErr:             nil,
+			expectedHealthCheckMode: addonapiv1alpha1.HealthCheckModeCustomized,
+			expectAvailableCondition: metav1.Condition{
+				Type:    addonapiv1alpha1.ManagedClusterAddOnConditionAvailable,
+				Status:  metav1.ConditionTrue,
+				Reason:  addonapiv1alpha1.AddonAvailableReasonProbeAvailable,
+				Message: "test add-on is available.",
+			},
+		},
 	}
 
 	for _, c := range cases {