Skip to content

Commit a7c57a8

Browse files
zhiying-linzhiying-lin
committed
fix the copilot code
1 parent 4c54ece commit a7c57a8

File tree

3 files changed

+56
-81
lines changed

3 files changed

+56
-81
lines changed

‎pkg/controllers/membercluster/v1beta1/membercluster_controller.go‎

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -63,13 +63,6 @@ const (
6363
reasonMemberClusterJoined = "MemberClusterJoined"
6464
reasonMemberClusterLeft = "MemberClusterLeft"
6565
reasonMemberClusterUnknown = "MemberClusterJoinStateUnknown"
66-
67-
// Messages for member cluster conditions.
68-
messageMemberClusterReadyToJoin = "Member cluster is ready to join the fleet"
69-
messageMemberClusterNotReadyToJoin = "Member cluster is not ready to join the fleet"
70-
messageMemberClusterJoined = "Member cluster has successfully joined the fleet"
71-
messageMemberClusterLeft = "Member cluster has left the fleet"
72-
messageMemberClusterUnknown = "Member cluster join state is unknown"
7366
)
7467

7568
// Reconciler reconciles a MemberCluster object
@@ -555,8 +548,10 @@ func (r *Reconciler) updateMemberClusterStatus(ctx context.Context, mc *clusterv
555548
// aggregateJoinedCondition is used to calculate and mark the joined or left status for member cluster based on join conditions from all agents.
556549
func (r *Reconciler) aggregateJoinedCondition(mc *clusterv1beta1.MemberCluster) {
557550
klog.V(2).InfoS("Aggregate joined condition from all agents", "memberCluster", klog.KObj(mc))
551+
var unknownMessage string
558552
if len(mc.Status.AgentStatus) < len(r.agents) {
559-
markMemberClusterUnknown(r.recorder, mc)
553+
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))
554+
markMemberClusterUnknown(r.recorder, mc, unknownMessage)
560555
return
561556
}
562557
joined := true
@@ -569,7 +564,8 @@ func (r *Reconciler) aggregateJoinedCondition(mc *clusterv1beta1.MemberCluster)
569564
}
570565
condition := meta.FindStatusCondition(agentStatus.Conditions, string(clusterv1beta1.AgentJoined))
571566
if condition == nil {
572-
markMemberClusterUnknown(r.recorder, mc)
567+
unknownMessage = fmt.Sprintf("Member cluster %s has not reported the join condition for agent %s", mc.Name, agentStatus.Type)
568+
markMemberClusterUnknown(r.recorder, mc, unknownMessage)
573569
return
574570
}
575571

@@ -579,7 +575,8 @@ func (r *Reconciler) aggregateJoinedCondition(mc *clusterv1beta1.MemberCluster)
579575
}
580576

581577
if len(reportedAgents) < len(r.agents) {
582-
markMemberClusterUnknown(r.recorder, mc)
578+
unknownMessage = fmt.Sprintf("Member cluster %s has not reported all the expected agents, expected %d, got %d", mc.Name, len(r.agents), len(reportedAgents))
579+
markMemberClusterUnknown(r.recorder, mc, unknownMessage)
583580
return
584581
}
585582

@@ -588,7 +585,8 @@ func (r *Reconciler) aggregateJoinedCondition(mc *clusterv1beta1.MemberCluster)
588585
} else if !joined && left {
589586
markMemberClusterLeft(r.recorder, mc)
590587
} else {
591-
markMemberClusterUnknown(r.recorder, mc)
588+
unknownMessage = "Member agents are not in a consistent state"
589+
markMemberClusterUnknown(r.recorder, mc, unknownMessage)
592590
}
593591
}
594592

@@ -599,7 +597,7 @@ func markMemberClusterReadyToJoin(recorder record.EventRecorder, mc apis.Conditi
599597
Type: string(clusterv1beta1.ConditionTypeMemberClusterReadyToJoin),
600598
Status: metav1.ConditionTrue,
601599
Reason: reasonMemberClusterReadyToJoin,
602-
Message: messageMemberClusterReadyToJoin,
600+
Message: "Member cluster is ready to join the fleet",
603601
ObservedGeneration: mc.GetGeneration(),
604602
}
605603

@@ -620,7 +618,7 @@ func markMemberClusterJoined(recorder record.EventRecorder, mc apis.ConditionedO
620618
Type: string(clusterv1beta1.ConditionTypeMemberClusterJoined),
621619
Status: metav1.ConditionTrue,
622620
Reason: reasonMemberClusterJoined,
623-
Message: messageMemberClusterJoined,
621+
Message: "Member cluster has successfully joined the fleet",
624622
ObservedGeneration: mc.GetGeneration(),
625623
}
626624

@@ -642,14 +640,14 @@ func markMemberClusterLeft(recorder record.EventRecorder, mc apis.ConditionedObj
642640
Type: string(clusterv1beta1.ConditionTypeMemberClusterJoined),
643641
Status: metav1.ConditionFalse,
644642
Reason: reasonMemberClusterLeft,
645-
Message: messageMemberClusterLeft,
643+
Message: "Member cluster has left the fleet",
646644
ObservedGeneration: mc.GetGeneration(),
647645
}
648646
notReadyCondition := metav1.Condition{
649647
Type: string(clusterv1beta1.ConditionTypeMemberClusterReadyToJoin),
650648
Status: metav1.ConditionFalse,
651649
Reason: reasonMemberClusterNotReadyToJoin,
652-
Message: messageMemberClusterNotReadyToJoin,
650+
Message: "Member cluster is not ready to join the fleet",
653651
ObservedGeneration: mc.GetGeneration(),
654652
}
655653

@@ -665,13 +663,13 @@ func markMemberClusterLeft(recorder record.EventRecorder, mc apis.ConditionedObj
665663
}
666664

667665
// markMemberClusterUnknown is used to update the status of the member cluster to have the left condition.
668-
func markMemberClusterUnknown(recorder record.EventRecorder, mc apis.ConditionedObj) {
666+
func markMemberClusterUnknown(recorder record.EventRecorder, mc apis.ConditionedObj, unknownMessage string) {
669667
klog.V(2).InfoS("Mark the member cluster join condition unknown", "memberCluster", klog.KObj(mc))
670668
newCondition := metav1.Condition{
671669
Type: string(clusterv1beta1.ConditionTypeMemberClusterJoined),
672670
Status: metav1.ConditionUnknown,
673671
Reason: reasonMemberClusterUnknown,
674-
Message: messageMemberClusterUnknown,
672+
Message: unknownMessage,
675673
ObservedGeneration: mc.GetGeneration(),
676674
}
677675

‎pkg/controllers/membercluster/v1beta1/membercluster_controller_integration_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ const (
4444
)
4545

4646
var (
47-
ignoreOption = cmpopts.IgnoreFields(metav1.Condition{}, "LastTransitionTime")
47+
ignoreOption = cmpopts.IgnoreFields(metav1.Condition{}, "LastTransitionTime", "Message")
4848
)
4949

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

‎pkg/controllers/membercluster/v1beta1/membercluster_controller_test.go‎

Lines changed: 39 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -695,7 +695,7 @@ func TestMarkMemberClusterJoined(t *testing.T) {
695695

696696
// Check expected conditions.
697697
expectedConditions := []metav1.Condition{
698-
{Type: string(clusterv1beta1.ConditionTypeMemberClusterJoined), Status: metav1.ConditionTrue, Reason: reasonMemberClusterJoined, Message: messageMemberClusterJoined},
698+
{Type: string(clusterv1beta1.ConditionTypeMemberClusterJoined), Status: metav1.ConditionTrue, Reason: reasonMemberClusterJoined, Message: "Member cluster has successfully joined the fleet"},
699699
}
700700

701701
for i := range expectedConditions {
@@ -794,22 +794,19 @@ func TestSyncInternalMemberClusterStatus(t *testing.T) {
794794
Status: clusterv1beta1.MemberClusterStatus{
795795
Conditions: []metav1.Condition{
796796
{
797-
Type: string(clusterv1beta1.ConditionTypeMemberClusterJoined),
798-
Status: metav1.ConditionTrue,
799-
Reason: reasonMemberClusterJoined,
800-
Message: messageMemberClusterJoined,
797+
Type: string(clusterv1beta1.ConditionTypeMemberClusterJoined),
798+
Status: metav1.ConditionTrue,
799+
Reason: reasonMemberClusterJoined,
801800
},
802801
{
803-
Type: propertyProviderConditionType1,
804-
Status: propertyProviderConditionStatus1,
805-
Reason: propertyProviderConditionReason1,
806-
Message: propertyProviderConditionMessage1,
802+
Type: propertyProviderConditionType1,
803+
Status: propertyProviderConditionStatus1,
804+
Reason: propertyProviderConditionReason1,
807805
},
808806
{
809-
Type: propertyProviderConditionType2,
810-
Status: propertyProviderConditionStatus2,
811-
Reason: propertyProviderConditionReason2,
812-
Message: propertyProviderConditionMessage2,
807+
Type: propertyProviderConditionType2,
808+
Status: propertyProviderConditionStatus2,
809+
Reason: propertyProviderConditionReason2,
813810
},
814811
},
815812
Properties: map[clusterv1beta1.PropertyName]clusterv1beta1.PropertyValue{
@@ -914,16 +911,13 @@ func TestSyncInternalMemberClusterStatus(t *testing.T) {
914911
Status: clusterv1beta1.MemberClusterStatus{
915912
Conditions: []metav1.Condition{
916913
{
917-
Type: string(clusterv1beta1.ConditionTypeMemberClusterJoined),
918-
Status: metav1.ConditionFalse,
919-
Reason: reasonMemberClusterLeft,
920-
Message: messageMemberClusterLeft,
921-
},
914+
Type: string(clusterv1beta1.ConditionTypeMemberClusterJoined),
915+
Status: metav1.ConditionFalse,
916+
Reason: reasonMemberClusterLeft},
922917
{
923-
Type: string(clusterv1beta1.ConditionTypeMemberClusterReadyToJoin),
924-
Status: metav1.ConditionFalse,
925-
Reason: reasonMemberClusterNotReadyToJoin,
926-
Message: messageMemberClusterNotReadyToJoin,
918+
Type: string(clusterv1beta1.ConditionTypeMemberClusterReadyToJoin),
919+
Status: metav1.ConditionFalse,
920+
Reason: reasonMemberClusterNotReadyToJoin,
927921
},
928922
},
929923
ResourceUsage: clusterv1beta1.ResourceUsage{
@@ -1014,8 +1008,7 @@ func TestSyncInternalMemberClusterStatus(t *testing.T) {
10141008
{
10151009
Type: string(clusterv1beta1.ConditionTypeMemberClusterJoined),
10161010
Status: metav1.ConditionUnknown,
1017-
Reason: reasonMemberClusterUnknown,
1018-
Message: messageMemberClusterUnknown,
1011+
Reason: reasonMemberClusterUnknown,
10191012
},
10201013
},
10211014
ResourceUsage: clusterv1beta1.ResourceUsage{
@@ -1088,10 +1081,9 @@ func TestSyncInternalMemberClusterStatus(t *testing.T) {
10881081
},
10891082
Conditions: []metav1.Condition{
10901083
{
1091-
Type: string(clusterv1beta1.ConditionTypeMemberClusterJoined),
1092-
Status: metav1.ConditionUnknown,
1093-
Reason: reasonMemberClusterUnknown,
1094-
Message: messageMemberClusterUnknown,
1084+
Type: string(clusterv1beta1.ConditionTypeMemberClusterJoined),
1085+
Status: metav1.ConditionUnknown,
1086+
Reason: reasonMemberClusterUnknown,
10951087
},
10961088
},
10971089
},
@@ -1169,10 +1161,9 @@ func TestSyncInternalMemberClusterStatus(t *testing.T) {
11691161
Status: clusterv1beta1.MemberClusterStatus{
11701162
Conditions: []metav1.Condition{
11711163
{
1172-
Type: string(clusterv1beta1.ConditionTypeMemberClusterJoined),
1173-
Status: metav1.ConditionTrue,
1174-
Reason: reasonMemberClusterJoined,
1175-
Message: messageMemberClusterJoined,
1164+
Type: string(clusterv1beta1.ConditionTypeMemberClusterJoined),
1165+
Status: metav1.ConditionTrue,
1166+
Reason: reasonMemberClusterJoined,
11761167
},
11771168
},
11781169
ResourceUsage: clusterv1beta1.ResourceUsage{
@@ -1263,8 +1254,7 @@ func TestSyncInternalMemberClusterStatus(t *testing.T) {
12631254
{
12641255
Type: string(clusterv1beta1.ConditionTypeMemberClusterJoined),
12651256
Status: metav1.ConditionUnknown,
1266-
Reason: reasonMemberClusterUnknown,
1267-
Message: messageMemberClusterUnknown,
1257+
Reason: reasonMemberClusterUnknown,
12681258
},
12691259
},
12701260
ResourceUsage: clusterv1beta1.ResourceUsage{
@@ -1337,8 +1327,7 @@ func TestSyncInternalMemberClusterStatus(t *testing.T) {
13371327
{
13381328
Type: string(clusterv1beta1.ConditionTypeMemberClusterJoined),
13391329
Status: metav1.ConditionUnknown,
1340-
Reason: reasonMemberClusterUnknown,
1341-
Message: messageMemberClusterUnknown,
1330+
Reason: reasonMemberClusterUnknown,
13421331
},
13431332
},
13441333
ResourceUsage: clusterv1beta1.ResourceUsage{
@@ -1415,8 +1404,7 @@ func TestSyncInternalMemberClusterStatus(t *testing.T) {
14151404
{
14161405
Type: string(clusterv1beta1.ConditionTypeMemberClusterJoined),
14171406
Status: metav1.ConditionUnknown,
1418-
Reason: reasonMemberClusterUnknown,
1419-
Message: messageMemberClusterUnknown,
1407+
Reason: reasonMemberClusterUnknown,
14201408
},
14211409
},
14221410
ResourceUsage: clusterv1beta1.ResourceUsage{
@@ -1453,30 +1441,19 @@ func TestSyncInternalMemberClusterStatus(t *testing.T) {
14531441
for testName, tt := range tests {
14541442
t.Run(testName, func(t *testing.T) {
14551443
tt.r.syncInternalMemberClusterStatus(tt.internalMemberCluster, tt.memberCluster)
1456-
1457-
// Compare the Joined condition.
1458-
diff := cmp.Diff(tt.wantedMemberCluster.GetCondition(string(clusterv1beta1.ConditionTypeMemberClusterJoined)),
1459-
tt.memberCluster.GetCondition(string(clusterv1beta1.ConditionTypeMemberClusterJoined)),
1460-
cmpopts.IgnoreTypes(time.Time{}))
1461-
assert.Equal(t, "", diff)
1462-
1463-
// Compare the property provider conditions (if present).
1464-
diff = cmp.Diff(tt.wantedMemberCluster.GetCondition(propertyProviderConditionType1),
1465-
tt.memberCluster.GetCondition(propertyProviderConditionType1),
1466-
cmpopts.IgnoreTypes(time.Time{}))
1467-
assert.Equal(t, "", diff)
1468-
1469-
diff = cmp.Diff(tt.wantedMemberCluster.GetCondition(propertyProviderConditionType2),
1470-
tt.memberCluster.GetCondition(propertyProviderConditionType2),
1471-
cmpopts.IgnoreTypes(time.Time{}))
1472-
assert.Equal(t, "", diff)
1473-
1474-
// Compare the properties (if present).
1475-
assert.Equal(t, tt.wantedMemberCluster.Status.Properties, tt.memberCluster.Status.Properties)
1476-
// Compare the resource usage.
1477-
assert.Equal(t, tt.wantedMemberCluster.Status.ResourceUsage, tt.memberCluster.Status.ResourceUsage)
1478-
// Compare the agent status.
1479-
assert.Equal(t, tt.wantedMemberCluster.Status.AgentStatus, tt.memberCluster.Status.AgentStatus)
1444+
// Compare the entire MemberCluster status struct, ignoring time.Time fields.
1445+
cmpOptions := cmp.Options{
1446+
cmpopts.IgnoreFields(metav1.Condition{}, "Message", "LastTransitionTime"),
1447+
cmpopts.SortSlices(func(c1, c2 metav1.Condition) bool {
1448+
return c1.Type < c2.Type
1449+
}),
1450+
cmpopts.SortSlices(func(a1, a2 clusterv1beta1.AgentStatus) bool {
1451+
return a1.Type < a2.Type
1452+
}),
1453+
}
1454+
if diff := cmp.Diff(tt.wantedMemberCluster.Status, tt.memberCluster.Status, cmpOptions); diff != "" {
1455+
t.Errorf("syncInternalMemberClusterStatus() mismatch (-want +got):\n%s", diff)
1456+
}
14801457
})
14811458
}
14821459
}

0 commit comments

Comments
 (0)