Skip to content

Commit dc3082d

Browse files
jwttyjwtty
authored
feat: support multi-version reporting in CRP for updateRun (#72)
1 parent d9e86f2 commit dc3082d

21 files changed

+3646
-551
lines changed

‎pkg/controllers/clusterresourceplacement/controller.go‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"github.com/prometheus/client_golang/prometheus"
2929
corev1 "k8s.io/api/core/v1"
3030
apierrors "k8s.io/apimachinery/pkg/api/errors"
31+
"k8s.io/apimachinery/pkg/api/meta"
3132
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3233
"k8s.io/apimachinery/pkg/types"
3334
utilerrors "k8s.io/apimachinery/pkg/util/errors"
@@ -975,10 +976,124 @@ func (r *Reconciler) setPlacementStatus(
975976
return false, nil
976977
}
977978

979+
if crp.Spec.Strategy.Type == fleetv1beta1.ExternalRolloutStrategyType {
980+
// For external rollout strategy, if clusters observe different resource snapshot versions,
981+
// we set RolloutStarted to Unknown without any other conditions since we do not know exactly which version is rolling out.
982+
// We also need to reset ObservedResourceIndex and selectedResources.
983+
rolloutStartedUnknown, err := r.determineRolloutStateForCRPWithExternalRolloutStrategy(ctx, crp, selected, allRPS, selectedResourceIDs)
984+
if err != nil || rolloutStartedUnknown {
985+
return true, err
986+
}
987+
}
988+
978989
setCRPConditions(crp, allRPS, rpsSetCondTypeCounter, expectedCondTypes)
979990
return true, nil
980991
}
981992

993+
func (r *Reconciler) determineRolloutStateForCRPWithExternalRolloutStrategy(
994+
ctx context.Context,
995+
crp *fleetv1beta1.ClusterResourcePlacement,
996+
selected []*fleetv1beta1.ClusterDecision,
997+
allRPS []fleetv1beta1.ResourcePlacementStatus,
998+
selectedResourceIDs []fleetv1beta1.ResourceIdentifier,
999+
) (bool, error) {
1000+
if len(selected) == 0 {
1001+
// This should not happen as we already checked in setPlacementStatus.
1002+
err := controller.NewUnexpectedBehaviorError(fmt.Errorf("selected cluster list is empty for placement %s when checking per-cluster rollout state", crp.Name))
1003+
klog.ErrorS(err, "Should not happen: selected cluster list is empty in determineRolloutStateForCRPWithExternalRolloutStrategy()")
1004+
return false, err
1005+
}
1006+
1007+
differentResourceIndicesObserved := false
1008+
observedResourceIndex := allRPS[0].ObservedResourceIndex
1009+
for i := range len(selected) - 1 {
1010+
if allRPS[i].ObservedResourceIndex != allRPS[i+1].ObservedResourceIndex {
1011+
differentResourceIndicesObserved = true
1012+
break
1013+
}
1014+
}
1015+
1016+
if differentResourceIndicesObserved {
1017+
// If clusters observe different resource snapshot versions, we set RolloutStarted condition to Unknown.
1018+
// ObservedResourceIndex and selectedResources are reset, too.
1019+
klog.V(2).InfoS("Placement has External rollout strategy and different resource snapshot versions are observed across clusters, set RolloutStarted condition to Unknown", "clusterResourcePlacement", klog.KObj(crp))
1020+
crp.Status.ObservedResourceIndex = ""
1021+
crp.Status.SelectedResources = []fleetv1beta1.ResourceIdentifier{}
1022+
crp.SetConditions(metav1.Condition{
1023+
Type: string(fleetv1beta1.ClusterResourcePlacementRolloutStartedConditionType),
1024+
Status: metav1.ConditionUnknown,
1025+
Reason: condition.RolloutControlledByExternalControllerReason,
1026+
Message: "Rollout is controlled by an external controller and different resource snapshot versions are observed across clusters",
1027+
ObservedGeneration: crp.Generation,
1028+
})
1029+
// As CRP status will refresh even if the spec has not changed, we reset any unused conditions
1030+
// to avoid confusion.
1031+
for i := condition.RolloutStartedCondition + 1; i < condition.TotalCondition; i++ {
1032+
meta.RemoveStatusCondition(&crp.Status.Conditions, string(i.ClusterResourcePlacementConditionType()))
1033+
}
1034+
return true, nil
1035+
}
1036+
1037+
if observedResourceIndex == "" {
1038+
// All bindings have empty resource snapshot name, we set the rollout condition to Unknown.
1039+
// ObservedResourceIndex and selectedResources are reset, too.
1040+
klog.V(2).InfoS("Placement has External rollout strategy and no resource snapshot name is observed across clusters, set RolloutStarted condition to Unknown", "clusterResourcePlacement", klog.KObj(crp))
1041+
crp.Status.ObservedResourceIndex = ""
1042+
crp.Status.SelectedResources = []fleetv1beta1.ResourceIdentifier{}
1043+
crp.SetConditions(metav1.Condition{
1044+
Type: string(fleetv1beta1.ClusterResourcePlacementRolloutStartedConditionType),
1045+
Status: metav1.ConditionUnknown,
1046+
Reason: condition.RolloutControlledByExternalControllerReason,
1047+
Message: "Rollout is controlled by an external controller and no resource snapshot name is observed across clusters, probably rollout has not started yet",
1048+
ObservedGeneration: crp.Generation,
1049+
})
1050+
// As CRP status will refresh even if the spec has not changed, we reset any unused conditions
1051+
// to avoid confusion.
1052+
for i := condition.RolloutStartedCondition + 1; i < condition.TotalCondition; i++ {
1053+
meta.RemoveStatusCondition(&crp.Status.Conditions, string(i.ClusterResourcePlacementConditionType()))
1054+
}
1055+
return true, nil
1056+
}
1057+
1058+
// All bindings have the same observed resource snapshot.
1059+
// We only set the ObservedResourceIndex and selectedResources, as the conditions will be set with setCRPConditions.
1060+
// If all clusters observe the latest resource snapshot, we do not need to go through all the resource snapshots again to collect selected resources.
1061+
if observedResourceIndex == crp.Status.ObservedResourceIndex {
1062+
crp.Status.SelectedResources = selectedResourceIDs
1063+
} else {
1064+
crp.Status.ObservedResourceIndex = observedResourceIndex
1065+
selectedResources, err := controller.CollectResourceIdentifiersFromClusterResourceSnapshot(ctx, r.Client, crp.Name, observedResourceIndex)
1066+
if err != nil {
1067+
klog.ErrorS(err, "Failed to collect resource identifiers from clusterResourceSnapshot", "clusterResourcePlacement", klog.KObj(crp), "resourceSnapshotIndex", observedResourceIndex)
1068+
return false, err
1069+
}
1070+
crp.Status.SelectedResources = selectedResources
1071+
}
1072+
1073+
for i := range len(selected) {
1074+
rolloutStartedCond := meta.FindStatusCondition(allRPS[i].Conditions, string(fleetv1beta1.ResourceRolloutStartedConditionType))
1075+
if !condition.IsConditionStatusTrue(rolloutStartedCond, crp.Generation) &&
1076+
!condition.IsConditionStatusFalse(rolloutStartedCond, crp.Generation) {
1077+
klog.V(2).InfoS("Placement has External rollout strategy and some cluster is in RolloutStarted Unknown state, set RolloutStarted condition to Unknown",
1078+
"clusterName", allRPS[i].ClusterName, "observedResourceIndex", observedResourceIndex, "clusterResourcePlacement", klog.KObj(crp))
1079+
crp.SetConditions(metav1.Condition{
1080+
Type: string(fleetv1beta1.ClusterResourcePlacementRolloutStartedConditionType),
1081+
Status: metav1.ConditionUnknown,
1082+
Reason: condition.RolloutControlledByExternalControllerReason,
1083+
Message: fmt.Sprintf("Rollout is controlled by an external controller and cluster %s is in RolloutStarted Unknown state", allRPS[i].ClusterName),
1084+
ObservedGeneration: crp.Generation,
1085+
})
1086+
// As CRP status will refresh even if the spec has not changed, we reset any unused conditions
1087+
// to avoid confusion.
1088+
for i := condition.RolloutStartedCondition + 1; i < condition.TotalCondition; i++ {
1089+
meta.RemoveStatusCondition(&crp.Status.Conditions, string(i.ClusterResourcePlacementConditionType()))
1090+
}
1091+
return true, nil
1092+
}
1093+
}
1094+
return false, nil
1095+
}
1096+
9821097
func buildScheduledCondition(crp *fleetv1beta1.ClusterResourcePlacement, latestSchedulingPolicySnapshot *fleetv1beta1.ClusterSchedulingPolicySnapshot) metav1.Condition {
9831098
scheduledCondition := latestSchedulingPolicySnapshot.GetCondition(string(fleetv1beta1.PolicySnapshotScheduled))
9841099

‎pkg/controllers/clusterresourceplacement/controller_integration_test.go‎

Lines changed: 36 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -585,7 +585,8 @@ var _ = Describe("Test ClusterResourcePlacement Controller", func() {
585585
},
586586
PlacementStatuses: []placementv1beta1.ResourcePlacementStatus{
587587
{
588-
ClusterName: member1Name,
588+
ClusterName: member1Name,
589+
ObservedResourceIndex: "0",
589590
Conditions: []metav1.Condition{
590591
{
591592
Status: metav1.ConditionTrue,
@@ -610,7 +611,8 @@ var _ = Describe("Test ClusterResourcePlacement Controller", func() {
610611
},
611612
},
612613
{
613-
ClusterName: member2Name,
614+
ClusterName: member2Name,
615+
ObservedResourceIndex: "", // Empty as the binding is not created yet.
614616
Conditions: []metav1.Condition{
615617
{
616618
Status: metav1.ConditionTrue,
@@ -686,7 +688,8 @@ var _ = Describe("Test ClusterResourcePlacement Controller", func() {
686688
}
687689
wantCRP.Status.PlacementStatuses = []placementv1beta1.ResourcePlacementStatus{
688690
{
689-
ClusterName: member1Name,
691+
ClusterName: member1Name,
692+
ObservedResourceIndex: "0",
690693
Conditions: []metav1.Condition{
691694
{
692695
Status: metav1.ConditionTrue,
@@ -711,7 +714,8 @@ var _ = Describe("Test ClusterResourcePlacement Controller", func() {
711714
},
712715
},
713716
{
714-
ClusterName: member2Name,
717+
ClusterName: member2Name,
718+
ObservedResourceIndex: "0",
715719
Conditions: []metav1.Condition{
716720
{
717721
Status: metav1.ConditionTrue,
@@ -788,7 +792,8 @@ var _ = Describe("Test ClusterResourcePlacement Controller", func() {
788792
},
789793
PlacementStatuses: []placementv1beta1.ResourcePlacementStatus{
790794
{
791-
ClusterName: member1Name,
795+
ClusterName: member1Name,
796+
ObservedResourceIndex: "0",
792797
Conditions: []metav1.Condition{
793798
{
794799
Status: metav1.ConditionTrue,
@@ -813,7 +818,8 @@ var _ = Describe("Test ClusterResourcePlacement Controller", func() {
813818
},
814819
},
815820
{
816-
ClusterName: member2Name,
821+
ClusterName: member2Name,
822+
ObservedResourceIndex: "", // Empty as the binding is not created yet.
817823
Conditions: []metav1.Condition{
818824
{
819825
Status: metav1.ConditionTrue,
@@ -887,7 +893,8 @@ var _ = Describe("Test ClusterResourcePlacement Controller", func() {
887893
}
888894
wantCRP.Status.PlacementStatuses = []placementv1beta1.ResourcePlacementStatus{
889895
{
890-
ClusterName: member1Name,
896+
ClusterName: member1Name,
897+
ObservedResourceIndex: "0",
891898
Conditions: []metav1.Condition{
892899
{
893900
Status: metav1.ConditionTrue,
@@ -912,7 +919,8 @@ var _ = Describe("Test ClusterResourcePlacement Controller", func() {
912919
},
913920
},
914921
{
915-
ClusterName: member2Name,
922+
ClusterName: member2Name,
923+
ObservedResourceIndex: "0",
916924
Conditions: []metav1.Condition{
917925
{
918926
Status: metav1.ConditionTrue,
@@ -1054,7 +1062,8 @@ var _ = Describe("Test ClusterResourcePlacement Controller", func() {
10541062
}
10551063
wantCRP.Status.PlacementStatuses = []placementv1beta1.ResourcePlacementStatus{
10561064
{
1057-
ClusterName: member1Name,
1065+
ClusterName: member1Name,
1066+
ObservedResourceIndex: "0",
10581067
Conditions: []metav1.Condition{
10591068
{
10601069
Status: metav1.ConditionTrue,
@@ -1083,7 +1092,8 @@ var _ = Describe("Test ClusterResourcePlacement Controller", func() {
10831092
},
10841093
},
10851094
{
1086-
ClusterName: member2Name,
1095+
ClusterName: member2Name,
1096+
ObservedResourceIndex: "0",
10871097
Conditions: []metav1.Condition{
10881098
{
10891099
Status: metav1.ConditionTrue,
@@ -1181,7 +1191,8 @@ var _ = Describe("Test ClusterResourcePlacement Controller", func() {
11811191
},
11821192
PlacementStatuses: []placementv1beta1.ResourcePlacementStatus{
11831193
{
1184-
ClusterName: member1Name,
1194+
ClusterName: member1Name,
1195+
ObservedResourceIndex: "0",
11851196
Conditions: []metav1.Condition{
11861197
{
11871198
Status: metav1.ConditionTrue,
@@ -1211,7 +1222,8 @@ var _ = Describe("Test ClusterResourcePlacement Controller", func() {
12111222
},
12121223
},
12131224
{
1214-
ClusterName: member2Name,
1225+
ClusterName: member2Name,
1226+
ObservedResourceIndex: "0",
12151227
Conditions: []metav1.Condition{
12161228
{
12171229
Status: metav1.ConditionTrue,
@@ -1475,7 +1487,8 @@ var _ = Describe("Test ClusterResourcePlacement Controller", func() {
14751487
},
14761488
PlacementStatuses: []placementv1beta1.ResourcePlacementStatus{
14771489
{
1478-
ClusterName: member1Name,
1490+
ClusterName: member1Name,
1491+
ObservedResourceIndex: "0",
14791492
Conditions: []metav1.Condition{
14801493
{
14811494
Status: metav1.ConditionTrue,
@@ -1500,7 +1513,8 @@ var _ = Describe("Test ClusterResourcePlacement Controller", func() {
15001513
},
15011514
},
15021515
{
1503-
ClusterName: member2Name,
1516+
ClusterName: member2Name,
1517+
ObservedResourceIndex: "0",
15041518
Conditions: []metav1.Condition{
15051519
{
15061520
Status: metav1.ConditionTrue,
@@ -1652,7 +1666,8 @@ var _ = Describe("Test ClusterResourcePlacement Controller", func() {
16521666
},
16531667
PlacementStatuses: []placementv1beta1.ResourcePlacementStatus{
16541668
{
1655-
ClusterName: member1Name,
1669+
ClusterName: member1Name,
1670+
ObservedResourceIndex: "0",
16561671
Conditions: []metav1.Condition{
16571672
{
16581673
Status: metav1.ConditionTrue,
@@ -1677,7 +1692,8 @@ var _ = Describe("Test ClusterResourcePlacement Controller", func() {
16771692
},
16781693
},
16791694
{
1680-
ClusterName: member2Name,
1695+
ClusterName: member2Name,
1696+
ObservedResourceIndex: "0",
16811697
Conditions: []metav1.Condition{
16821698
{
16831699
Status: metav1.ConditionTrue,
@@ -1780,7 +1796,8 @@ var _ = Describe("Test ClusterResourcePlacement Controller", func() {
17801796
}
17811797
wantCRP.Status.PlacementStatuses = []placementv1beta1.ResourcePlacementStatus{
17821798
{
1783-
ClusterName: member1Name,
1799+
ClusterName: member1Name,
1800+
ObservedResourceIndex: "0",
17841801
Conditions: []metav1.Condition{
17851802
{
17861803
Status: metav1.ConditionTrue,
@@ -1810,7 +1827,8 @@ var _ = Describe("Test ClusterResourcePlacement Controller", func() {
18101827
},
18111828
},
18121829
{
1813-
ClusterName: member2Name,
1830+
ClusterName: member2Name,
1831+
ObservedResourceIndex: "0",
18141832
Conditions: []metav1.Condition{
18151833
{
18161834
Status: metav1.ConditionTrue,

0 commit comments

Comments
 (0)