fix: add missing Message field in member cluster conditions (#92)

---------
Signed-off-by: Zhiying Lin <[email protected]>
Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: britaniar <[email protected]>

Author: Copilot

pkg/controllers/membercluster/v1beta1/membercluster_controller.go

@@ -548,8 +548,10 @@ func (r *Reconciler) updateMemberClusterStatus(ctx context.Context, mc *clusterv
 // aggregateJoinedCondition is used to calculate and mark the joined or left status for member cluster based on join conditions from all agents.
 func (r *Reconciler) aggregateJoinedCondition(mc *clusterv1beta1.MemberCluster) {
 	klog.V(2).InfoS("Aggregate joined condition from all agents", "memberCluster", klog.KObj(mc))
+	var unknownMessage string
 	if len(mc.Status.AgentStatus) < len(r.agents) {
-		markMemberClusterUnknown(r.recorder, mc)
+		unknownMessage = fmt.Sprintf("Member cluster %s has not reported all the expected agents, expected %d, got %d", mc.Name, len(r.agents), len(mc.Status.AgentStatus))
+		markMemberClusterUnknown(r.recorder, mc, unknownMessage)
 		return
 	}
 	joined := true
@@ -562,7 +564,8 @@ func (r *Reconciler) aggregateJoinedCondition(mc *clusterv1beta1.MemberCluster)
 		}
 		condition := meta.FindStatusCondition(agentStatus.Conditions, string(clusterv1beta1.AgentJoined))
 		if condition == nil {
-			markMemberClusterUnknown(r.recorder, mc)
+			unknownMessage = fmt.Sprintf("Member cluster %s has not reported the join condition for agent %s", mc.Name, agentStatus.Type)
+			markMemberClusterUnknown(r.recorder, mc, unknownMessage)
 			return
 		}
 
@@ -572,7 +575,8 @@ func (r *Reconciler) aggregateJoinedCondition(mc *clusterv1beta1.MemberCluster)
 	}
 
 	if len(reportedAgents) < len(r.agents) {
-		markMemberClusterUnknown(r.recorder, mc)
+		unknownMessage = fmt.Sprintf("Member cluster %s has not reported all the expected agents, expected %d, got %d", mc.Name, len(r.agents), len(reportedAgents))
+		markMemberClusterUnknown(r.recorder, mc, unknownMessage)
 		return
 	}
 
@@ -581,7 +585,8 @@ func (r *Reconciler) aggregateJoinedCondition(mc *clusterv1beta1.MemberCluster)
 	} else if !joined && left {
 		markMemberClusterLeft(r.recorder, mc)
 	} else {
-		markMemberClusterUnknown(r.recorder, mc)
+		unknownMessage = "Member agents are not in a consistent state"
+		markMemberClusterUnknown(r.recorder, mc, unknownMessage)
 	}
 }
 
@@ -592,6 +597,7 @@ func markMemberClusterReadyToJoin(recorder record.EventRecorder, mc apis.Conditi
 		Type:               string(clusterv1beta1.ConditionTypeMemberClusterReadyToJoin),
 		Status:             metav1.ConditionTrue,
 		Reason:             reasonMemberClusterReadyToJoin,
+		Message:            "Member cluster is ready to join the fleet",
 		ObservedGeneration: mc.GetGeneration(),
 	}
 
@@ -612,6 +618,7 @@ func markMemberClusterJoined(recorder record.EventRecorder, mc apis.ConditionedO
 		Type:               string(clusterv1beta1.ConditionTypeMemberClusterJoined),
 		Status:             metav1.ConditionTrue,
 		Reason:             reasonMemberClusterJoined,
+		Message:            "Member cluster has successfully joined the fleet",
 		ObservedGeneration: mc.GetGeneration(),
 	}
 
@@ -633,12 +640,14 @@ func markMemberClusterLeft(recorder record.EventRecorder, mc apis.ConditionedObj
 		Type:               string(clusterv1beta1.ConditionTypeMemberClusterJoined),
 		Status:             metav1.ConditionFalse,
 		Reason:             reasonMemberClusterLeft,
+		Message:            "Member cluster has left the fleet",
 		ObservedGeneration: mc.GetGeneration(),
 	}
 	notReadyCondition := metav1.Condition{
 		Type:               string(clusterv1beta1.ConditionTypeMemberClusterReadyToJoin),
 		Status:             metav1.ConditionFalse,
 		Reason:             reasonMemberClusterNotReadyToJoin,
+		Message:            "Member cluster is not ready to join the fleet",
 		ObservedGeneration: mc.GetGeneration(),
 	}
 
@@ -654,12 +663,13 @@ func markMemberClusterLeft(recorder record.EventRecorder, mc apis.ConditionedObj
 }
 
 // markMemberClusterUnknown is used to update the status of the member cluster to have the left condition.
-func markMemberClusterUnknown(recorder record.EventRecorder, mc apis.ConditionedObj) {
+func markMemberClusterUnknown(recorder record.EventRecorder, mc apis.ConditionedObj, unknownMessage string) {
 	klog.V(2).InfoS("Mark the member cluster join condition unknown", "memberCluster", klog.KObj(mc))
 	newCondition := metav1.Condition{
 		Type:               string(clusterv1beta1.ConditionTypeMemberClusterJoined),
 		Status:             metav1.ConditionUnknown,
 		Reason:             reasonMemberClusterUnknown,
+		Message:            unknownMessage,
 		ObservedGeneration: mc.GetGeneration(),
 	}
 

pkg/controllers/membercluster/v1beta1/membercluster_controller_integration_test.go

@@ -44,7 +44,7 @@ const (
 )
 
 var (
-	ignoreOption = cmpopts.IgnoreFields(metav1.Condition{}, "LastTransitionTime")
+	ignoreOption = cmpopts.IgnoreFields(metav1.Condition{}, "LastTransitionTime", "Message")
 )
 
 var _ = Describe("Test MemberCluster Controller", func() {
@@ -502,7 +502,7 @@ var _ = Describe("Test MemberCluster Controller", func() {
 				ResourceUsage: imc.Status.ResourceUsage,
 				AgentStatus:   imc.Status.AgentStatus,
 			}
-			options := cmpopts.IgnoreFields(metav1.Condition{}, "LastTransitionTime", "ObservedGeneration")
+			options := cmpopts.IgnoreFields(metav1.Condition{}, "LastTransitionTime", "ObservedGeneration", "Message")
 			// ignore the ObservedGeneration here cause controller won't update the ReadyToJoin condition.
 			Expect(cmp.Diff(wantMC, mc.Status, options)).Should(BeEmpty(), "mc status mismatch, (-want, +got)")
 

pkg/controllers/membercluster/v1beta1/membercluster_controller_test.go

@@ -695,7 +695,7 @@ func TestMarkMemberClusterJoined(t *testing.T) {
 
 	// Check expected conditions.
 	expectedConditions := []metav1.Condition{
-		{Type: string(clusterv1beta1.ConditionTypeMemberClusterJoined), Status: metav1.ConditionTrue, Reason: reasonMemberClusterJoined},
+		{Type: string(clusterv1beta1.ConditionTypeMemberClusterJoined), Status: metav1.ConditionTrue, Reason: reasonMemberClusterJoined, Message: "Member cluster has successfully joined the fleet"},
 	}
 
 	for i := range expectedConditions {
@@ -799,16 +799,14 @@ func TestSyncInternalMemberClusterStatus(t *testing.T) {
 							Reason: reasonMemberClusterJoined,
 						},
 						{
-							Type:    propertyProviderConditionType1,
-							Status:  propertyProviderConditionStatus1,
-							Reason:  propertyProviderConditionReason1,
-							Message: propertyProviderConditionMessage1,
+							Type:   propertyProviderConditionType1,
+							Status: propertyProviderConditionStatus1,
+							Reason: propertyProviderConditionReason1,
 						},
 						{
-							Type:    propertyProviderConditionType2,
-							Status:  propertyProviderConditionStatus2,
-							Reason:  propertyProviderConditionReason2,
-							Message: propertyProviderConditionMessage2,
+							Type:   propertyProviderConditionType2,
+							Status: propertyProviderConditionStatus2,
+							Reason: propertyProviderConditionReason2,
 						},
 					},
 					Properties: map[clusterv1beta1.PropertyName]clusterv1beta1.PropertyValue{
@@ -1444,30 +1442,19 @@ func TestSyncInternalMemberClusterStatus(t *testing.T) {
 	for testName, tt := range tests {
 		t.Run(testName, func(t *testing.T) {
 			tt.r.syncInternalMemberClusterStatus(tt.internalMemberCluster, tt.memberCluster)
-
-			// Compare the Joined condition.
-			diff := cmp.Diff(tt.wantedMemberCluster.GetCondition(string(clusterv1beta1.ConditionTypeMemberClusterJoined)),
-				tt.memberCluster.GetCondition(string(clusterv1beta1.ConditionTypeMemberClusterJoined)),
-				cmpopts.IgnoreTypes(time.Time{}))
-			assert.Equal(t, "", diff)
-
-			// Compare the property provider conditions (if present).
-			diff = cmp.Diff(tt.wantedMemberCluster.GetCondition(propertyProviderConditionType1),
-				tt.memberCluster.GetCondition(propertyProviderConditionType1),
-				cmpopts.IgnoreTypes(time.Time{}))
-			assert.Equal(t, "", diff)
-
-			diff = cmp.Diff(tt.wantedMemberCluster.GetCondition(propertyProviderConditionType2),
-				tt.memberCluster.GetCondition(propertyProviderConditionType2),
-				cmpopts.IgnoreTypes(time.Time{}))
-			assert.Equal(t, "", diff)
-
-			// Compare the properties (if present).
-			assert.Equal(t, tt.wantedMemberCluster.Status.Properties, tt.memberCluster.Status.Properties)
-			// Compare the resource usage.
-			assert.Equal(t, tt.wantedMemberCluster.Status.ResourceUsage, tt.memberCluster.Status.ResourceUsage)
-			// Compare the agent status.
-			assert.Equal(t, tt.wantedMemberCluster.Status.AgentStatus, tt.memberCluster.Status.AgentStatus)
+			// Compare the entire MemberCluster status struct, ignoring time.Time fields.
+			cmpOptions := cmp.Options{
+				cmpopts.IgnoreFields(metav1.Condition{}, "Message", "LastTransitionTime"),
+				cmpopts.SortSlices(func(c1, c2 metav1.Condition) bool {
+					return c1.Type < c2.Type
+				}),
+				cmpopts.SortSlices(func(a1, a2 clusterv1beta1.AgentStatus) bool {
+					return a1.Type < a2.Type
+				}),
+			}
+			if diff := cmp.Diff(tt.wantedMemberCluster.Status, tt.memberCluster.Status, cmpOptions); diff != "" {
+				t.Errorf("syncInternalMemberClusterStatus() mismatch (-want +got):\n%s", diff)
+			}
 		})
 	}
 }