[release-1.1] 🐛 Fix health check to support partial probe results with multi-probe configuration (#343)

* 🐛 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]>

* Refactor health check probe result handling

Simplify the probe result collection logic by:
- Collecting all probe results upfront, regardless of healthChecker
- Checking for empty results based on FieldResults length
- Removing unnecessary tracking of empty probe fields
- Updating test expectations for multi-probe scenarios

This makes the code cleaner and maintains the fix for supporting
partial probe results with multi-probe configurations.

Signed-off-by: Jia Zhu <[email protected]>

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

---------

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

Author: 3 people

pkg/addonmanager/controllers/agentdeploy/healthcheck_sync.go

@@ -187,29 +187,19 @@ func (s *healthCheckSyncer) probeAddonStatusByWorks(
 		return err
 	}
 
-	var FieldResults []agent.FieldResult
+	var fieldResults []agent.FieldResult
 
 	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
+			continue
 		}
 
+		fieldResults = append(fieldResults, results...)
 		// healthCheck will be ignored if healthChecker is set
 		if healthChecker != nil {
-			if len(results) != 0 {
-				FieldResults = append(FieldResults, results...)
-			}
 			continue
 		}
 
@@ -238,8 +228,22 @@ func (s *healthCheckSyncer) probeAddonStatusByWorks(
 
 	}
 
+	// If all probe fields have no results, mark condition to unknown
+	if len(probeFields) > 0 && len(fieldResults) == 0 {
+		meta.SetStatusCondition(&addon.Status.Conditions, metav1.Condition{
+			Type:    addonapiv1alpha1.ManagedClusterAddOnConditionAvailable,
+			Status:  metav1.ConditionUnknown,
+			Reason:  addonapiv1alpha1.AddonAvailableReasonNoProbeResult,
+			Message: "Probe results are not returned",
+		})
+		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 {
+		if err := healthChecker(fieldResults, cluster, addon); err != nil {
 			meta.SetStatusCondition(&addon.Status.Conditions, metav1.Condition{
 				Type:    addonapiv1alpha1.ManagedClusterAddOnConditionAvailable,
 				Status:  metav1.ConditionFalse,

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 {

test/integration/kube/agent_deploy_test.go

@@ -922,8 +922,8 @@ var _ = ginkgo.Describe("Agent deploy", func() {
 				return err
 			}
 
-			// the daemonset is not probed, so the addon available condition is unknown
-			if !meta.IsStatusConditionPresentAndEqual(addon.Status.Conditions, "Available", metav1.ConditionUnknown) {
+			// the daemonset is not probed, so the addon available condition is false
+			if !meta.IsStatusConditionPresentAndEqual(addon.Status.Conditions, "Available", metav1.ConditionFalse) {
 				return fmt.Errorf("Unexpected addon available condition, %v", addon.Status.Conditions)
 			}
 			return nil