From a2b3c6d54c0b2cb8661a321efc99ae6c810031c9 Mon Sep 17 00:00:00 2001 From: Qing Hao Date: Wed, 23 Aug 2023 14:15:27 +0800 Subject: [PATCH] revert placement decision start index from 1 to keep backward compatibility (#253) Signed-off-by: haoqing0110 --- .../controllers/scheduling/schedule_test.go | 17 ++++----- .../scheduling/scheduling_controller.go | 6 ++-- .../scheduling/scheduling_controller_test.go | 36 +++++++++---------- pkg/placement/helpers/testing/builders.go | 6 +++- test/integration/placement/placement_test.go | 35 ++++++++++-------- 5 files changed, 53 insertions(+), 47 deletions(-) diff --git a/pkg/placement/controllers/scheduling/schedule_test.go b/pkg/placement/controllers/scheduling/schedule_test.go index 85f3b6921..7be3f7588 100644 --- a/pkg/placement/controllers/scheduling/schedule_test.go +++ b/pkg/placement/controllers/scheduling/schedule_test.go @@ -3,7 +3,6 @@ package scheduling import ( "context" "encoding/json" - "fmt" "reflect" "sort" "testing" @@ -148,7 +147,7 @@ func TestSchedule(t *testing.T) { initObjs: []runtime.Object{ testinghelpers.NewClusterSet(clusterSetName).Build(), testinghelpers.NewClusterSetBinding(placementNamespace, clusterSetName), - testinghelpers.NewPlacementDecision(placementNamespace, placementDecisionName(placementName, 1)). + testinghelpers.NewPlacementDecision(placementNamespace, testinghelpers.PlacementDecisionName(placementName, 1)). WithLabel(clusterapiv1beta1.PlacementLabel, placementName). WithDecisions("cluster1", "cluster2").Build(), }, @@ -420,7 +419,7 @@ func TestSchedule(t *testing.T) { initObjs: []runtime.Object{ testinghelpers.NewClusterSet(clusterSetName).Build(), testinghelpers.NewClusterSetBinding(placementNamespace, clusterSetName), - testinghelpers.NewPlacementDecision(placementNamespace, placementDecisionName(placementName, 1)). + testinghelpers.NewPlacementDecision(placementNamespace, testinghelpers.PlacementDecisionName(placementName, 1)). WithLabel(clusterapiv1beta1.PlacementLabel, placementName). WithDecisions("cluster1").Build(), }, @@ -467,7 +466,7 @@ func TestSchedule(t *testing.T) { initObjs: []runtime.Object{ testinghelpers.NewClusterSet(clusterSetName).Build(), testinghelpers.NewClusterSetBinding(placementNamespace, clusterSetName), - testinghelpers.NewPlacementDecision(placementNamespace, placementDecisionName("others", 1)). + testinghelpers.NewPlacementDecision(placementNamespace, testinghelpers.PlacementDecisionName("others", 1)). WithDecisions("cluster1", "cluster2").Build(), }, clusters: []*clusterapiv1.ManagedCluster{ @@ -513,11 +512,11 @@ func TestSchedule(t *testing.T) { initObjs: []runtime.Object{ testinghelpers.NewClusterSet(clusterSetName).Build(), testinghelpers.NewClusterSetBinding(placementNamespace, clusterSetName), - testinghelpers.NewPlacementDecision(placementNamespace, placementDecisionName("others", 1)). + testinghelpers.NewPlacementDecision(placementNamespace, testinghelpers.PlacementDecisionName("others", 1)). WithDecisions("cluster2", "cluster3").Build(), - testinghelpers.NewPlacementDecision(placementNamespace, placementDecisionName("others", 2)). + testinghelpers.NewPlacementDecision(placementNamespace, testinghelpers.PlacementDecisionName("others", 2)). WithDecisions("cluster1", "cluster2").Build(), - testinghelpers.NewPlacementDecision(placementNamespace, placementDecisionName(placementName, 1)). + testinghelpers.NewPlacementDecision(placementNamespace, testinghelpers.PlacementDecisionName(placementName, 1)). WithLabel(clusterapiv1beta1.PlacementLabel, placementName). WithDecisions("cluster3").Build(), }, @@ -605,10 +604,6 @@ func TestSchedule(t *testing.T) { } } -func placementDecisionName(placementName string, index int) string { - return fmt.Sprintf("%s-decision-%d", placementName, index) -} - func TestFilterResults(t *testing.T) { } diff --git a/pkg/placement/controllers/scheduling/scheduling_controller.go b/pkg/placement/controllers/scheduling/scheduling_controller.go index edcdeeb2c..96a1dce62 100644 --- a/pkg/placement/controllers/scheduling/scheduling_controller.go +++ b/pkg/placement/controllers/scheduling/scheduling_controller.go @@ -456,7 +456,7 @@ func (c *schedulingController) generatePlacementDecisionsAndStatus( placement *clusterapiv1beta1.Placement, clusters []*clusterapiv1.ManagedCluster, ) ([]*clusterapiv1beta1.PlacementDecision, []*clusterapiv1beta1.DecisionGroupStatus, *framework.Status) { - placementDecisionIndex := 0 + placementDecisionIndex := 1 var placementDecisions []*clusterapiv1beta1.PlacementDecision var decisionGroupStatus []*clusterapiv1beta1.DecisionGroupStatus @@ -465,7 +465,9 @@ func (c *schedulingController) generatePlacementDecisionsAndStatus( // generate placement decision for each decision group for decisionGroupIndex, decisionGroup := range decisionGroups { - // generate placement decisions and status, group and placement name index starts from 0. + // generate placement decisions and status, decision group index starts from 0 + // placement name index starts from 1 to keep backward compatibility + // TODO: should be consistent with index or using a random generate name when version bumps pds, groupStatus := c.generateDecision(placement, decisionGroup, decisionGroupIndex, placementDecisionIndex) placementDecisions = append(placementDecisions, pds...) diff --git a/pkg/placement/controllers/scheduling/scheduling_controller_test.go b/pkg/placement/controllers/scheduling/scheduling_controller_test.go index 57ae8aa8e..0cfb0b734 100644 --- a/pkg/placement/controllers/scheduling/scheduling_controller_test.go +++ b/pkg/placement/controllers/scheduling/scheduling_controller_test.go @@ -86,7 +86,7 @@ func TestSchedulingController_sync(t *testing.T) { { DecisionGroupIndex: 0, DecisionGroupName: "", - Decisions: []string{"placement1-decision-0"}, + Decisions: []string{testinghelpers.PlacementDecisionName(placementName, 1)}, ClustersCount: 3, }, } @@ -194,25 +194,25 @@ func TestSchedulingController_sync(t *testing.T) { { DecisionGroupIndex: 0, DecisionGroupName: "canary", - Decisions: []string{"placement1-decision-0"}, + Decisions: []string{testinghelpers.PlacementDecisionName(placementName, 1)}, ClustersCount: 1, }, { DecisionGroupIndex: 1, DecisionGroupName: "canary", - Decisions: []string{"placement1-decision-1"}, + Decisions: []string{testinghelpers.PlacementDecisionName(placementName, 2)}, ClustersCount: 1, }, { DecisionGroupIndex: 2, DecisionGroupName: "", - Decisions: []string{"placement1-decision-2"}, + Decisions: []string{testinghelpers.PlacementDecisionName(placementName, 3)}, ClustersCount: 1, }, { DecisionGroupIndex: 3, DecisionGroupName: "", - Decisions: []string{"placement1-decision-3"}, + Decisions: []string{testinghelpers.PlacementDecisionName(placementName, 4)}, ClustersCount: 1, }, } @@ -276,13 +276,13 @@ func TestSchedulingController_sync(t *testing.T) { { DecisionGroupIndex: 0, DecisionGroupName: "group1", - Decisions: []string{"placement1-decision-0"}, + Decisions: []string{testinghelpers.PlacementDecisionName(placementName, 1)}, ClustersCount: 2, }, { DecisionGroupIndex: 1, DecisionGroupName: "group2", - Decisions: []string{"placement1-decision-1"}, + Decisions: []string{testinghelpers.PlacementDecisionName(placementName, 2)}, ClustersCount: 1, }, } @@ -334,19 +334,19 @@ func TestSchedulingController_sync(t *testing.T) { { DecisionGroupIndex: 0, DecisionGroupName: "", - Decisions: []string{"placement1-decision-0"}, + Decisions: []string{testinghelpers.PlacementDecisionName(placementName, 1)}, ClustersCount: 1, }, { DecisionGroupIndex: 1, DecisionGroupName: "", - Decisions: []string{"placement1-decision-1"}, + Decisions: []string{testinghelpers.PlacementDecisionName(placementName, 2)}, ClustersCount: 1, }, { DecisionGroupIndex: 2, DecisionGroupName: "", - Decisions: []string{"placement1-decision-2"}, + Decisions: []string{testinghelpers.PlacementDecisionName(placementName, 3)}, ClustersCount: 1, }, } @@ -510,7 +510,7 @@ func TestSchedulingController_sync(t *testing.T) { testinghelpers.NewClusterSet("clusterset1").Build(), testinghelpers.NewClusterSetBinding(placementNamespace, "clusterset1"), testinghelpers.NewManagedCluster("cluster1").WithLabel(clusterapiv1beta2.ClusterSetLabel, "clusterset1").Build(), - testinghelpers.NewPlacementDecision(placementNamespace, placementDecisionName(placementName, 0)). + testinghelpers.NewPlacementDecision(placementNamespace, testinghelpers.PlacementDecisionName(placementName, 1)). WithLabel(clusterapiv1beta1.PlacementLabel, placementName). WithLabel(clusterapiv1beta1.DecisionGroupNameLabel, ""). WithLabel(clusterapiv1beta1.DecisionGroupIndexLabel, "0"). @@ -1265,12 +1265,12 @@ func TestBind(t *testing.T) { placement: testinghelpers.NewPlacement(placementNamespace, placementName).Build(), clusters: newClusters(128), initObjs: []runtime.Object{ - testinghelpers.NewPlacementDecision(placementNamespace, placementDecisionName(placementName, 0)). + testinghelpers.NewPlacementDecision(placementNamespace, testinghelpers.PlacementDecisionName(placementName, 1)). WithLabel(clusterapiv1beta1.PlacementLabel, placementName). WithLabel(clusterapiv1beta1.DecisionGroupNameLabel, ""). WithLabel(clusterapiv1beta1.DecisionGroupIndexLabel, "0"). WithDecisions(newSelectedClusters(128)[:100]...).Build(), - testinghelpers.NewPlacementDecision(placementNamespace, placementDecisionName(placementName, 1)). + testinghelpers.NewPlacementDecision(placementNamespace, testinghelpers.PlacementDecisionName(placementName, 2)). WithLabel(clusterapiv1beta1.PlacementLabel, placementName). WithLabel(clusterapiv1beta1.DecisionGroupNameLabel, ""). WithLabel(clusterapiv1beta1.DecisionGroupIndexLabel, "0"). @@ -1283,7 +1283,7 @@ func TestBind(t *testing.T) { placement: testinghelpers.NewPlacement(placementNamespace, placementName).Build(), clusters: newClusters(128), initObjs: []runtime.Object{ - testinghelpers.NewPlacementDecision(placementNamespace, placementDecisionName(placementName, 0)). + testinghelpers.NewPlacementDecision(placementNamespace, testinghelpers.PlacementDecisionName(placementName, 1)). WithLabel(clusterapiv1beta1.PlacementLabel, placementName). WithLabel(clusterapiv1beta1.DecisionGroupNameLabel, "fakegroup"). WithLabel(clusterapiv1beta1.DecisionGroupIndexLabel, "0"). @@ -1317,12 +1317,12 @@ func TestBind(t *testing.T) { placement: testinghelpers.NewPlacement(placementNamespace, placementName).Build(), clusters: newClusters(10), initObjs: []runtime.Object{ - testinghelpers.NewPlacementDecision(placementNamespace, placementDecisionName(placementName, 0)). + testinghelpers.NewPlacementDecision(placementNamespace, testinghelpers.PlacementDecisionName(placementName, 1)). WithLabel(clusterapiv1beta1.PlacementLabel, placementName). WithLabel(clusterapiv1beta1.DecisionGroupNameLabel, ""). WithLabel(clusterapiv1beta1.DecisionGroupIndexLabel, "0"). WithDecisions(newSelectedClusters(128)[:100]...).Build(), - testinghelpers.NewPlacementDecision(placementNamespace, placementDecisionName(placementName, 1)). + testinghelpers.NewPlacementDecision(placementNamespace, testinghelpers.PlacementDecisionName(placementName, 2)). WithLabel(clusterapiv1beta1.PlacementLabel, placementName). WithLabel(clusterapiv1beta1.DecisionGroupNameLabel, ""). WithLabel(clusterapiv1beta1.DecisionGroupIndexLabel, "0"). @@ -1344,12 +1344,12 @@ func TestBind(t *testing.T) { placement: testinghelpers.NewPlacement(placementNamespace, placementName).Build(), clusters: newClusters(0), initObjs: []runtime.Object{ - testinghelpers.NewPlacementDecision(placementNamespace, placementDecisionName(placementName, 0)). + testinghelpers.NewPlacementDecision(placementNamespace, testinghelpers.PlacementDecisionName(placementName, 1)). WithLabel(clusterapiv1beta1.PlacementLabel, placementName). WithLabel(clusterapiv1beta1.DecisionGroupNameLabel, ""). WithLabel(clusterapiv1beta1.DecisionGroupIndexLabel, "0"). WithDecisions(newSelectedClusters(128)[:100]...).Build(), - testinghelpers.NewPlacementDecision(placementNamespace, placementDecisionName(placementName, 1)). + testinghelpers.NewPlacementDecision(placementNamespace, testinghelpers.PlacementDecisionName(placementName, 2)). WithLabel(clusterapiv1beta1.PlacementLabel, placementName). WithLabel(clusterapiv1beta1.DecisionGroupNameLabel, ""). WithLabel(clusterapiv1beta1.DecisionGroupIndexLabel, "0"). diff --git a/pkg/placement/helpers/testing/builders.go b/pkg/placement/helpers/testing/builders.go index 244318510..1b44b7fdc 100644 --- a/pkg/placement/helpers/testing/builders.go +++ b/pkg/placement/helpers/testing/builders.go @@ -130,7 +130,7 @@ func (b *PlacementBuilder) WithNumOfSelectedClusters(nosc int, placementName str DecisionGroupIndex: 0, DecisionGroupName: "", ClustersCount: int32(nosc), - Decisions: []string{fmt.Sprintf("%s-decision-%d", placementName, 0)}, + Decisions: []string{PlacementDecisionName(placementName, 1)}, }, } return b @@ -375,3 +375,7 @@ func (a *AddOnPlacementScoreBuilder) WithValidUntil(validUntil time.Time) *AddOn func (a *AddOnPlacementScoreBuilder) Build() *clusterapiv1alpha1.AddOnPlacementScore { return a.addOnPlacementScore } + +func PlacementDecisionName(placementName string, index int) string { + return fmt.Sprintf("%s-decision-%d", placementName, index) +} diff --git a/test/integration/placement/placement_test.go b/test/integration/placement/placement_test.go index 57d4d209c..d4b1ef8c1 100644 --- a/test/integration/placement/placement_test.go +++ b/test/integration/placement/placement_test.go @@ -101,7 +101,7 @@ var _ = ginkgo.Describe("Placement", func() { assertPlacementDecisionNumbers(placementName, namespace, 5, 1) assertPlacementStatusDecisionGroups( placementName, namespace, - []clusterapiv1beta1.DecisionGroupStatus{{Decisions: []string{placementName + "-decision-0"}, ClustersCount: 5}}) + []clusterapiv1beta1.DecisionGroupStatus{{Decisions: []string{testinghelpers.PlacementDecisionName(placementName, 1)}, ClustersCount: 5}}) }) ginkgo.It("Should create empty placementdecision when no cluster selected", func() { @@ -109,7 +109,7 @@ var _ = ginkgo.Describe("Placement", func() { assertCreatingPlacementWithDecision(placement, 0, 1) assertPlacementStatusDecisionGroups( placementName, namespace, - []clusterapiv1beta1.DecisionGroupStatus{{Decisions: []string{placementName + "-decision-0"}, ClustersCount: 0}}) + []clusterapiv1beta1.DecisionGroupStatus{{Decisions: []string{testinghelpers.PlacementDecisionName(placementName, 1)}, ClustersCount: 0}}) }) ginkgo.It("Should create multiple placementdecisions once scheduled", func() { @@ -122,7 +122,12 @@ var _ = ginkgo.Describe("Placement", func() { assertPlacementDecisionNumbers(placementName, namespace, nod, 2) assertPlacementStatusDecisionGroups( placementName, namespace, - []clusterapiv1beta1.DecisionGroupStatus{{Decisions: []string{placementName + "-decision-0", placementName + "-decision-1"}, ClustersCount: 101}}) + []clusterapiv1beta1.DecisionGroupStatus{{ + Decisions: []string{ + testinghelpers.PlacementDecisionName(placementName, 1), + testinghelpers.PlacementDecisionName(placementName, 2), + }, + ClustersCount: 101}}) assertPlacementConditionSatisfied(placementName, namespace, nod, true) }) @@ -165,7 +170,7 @@ var _ = ginkgo.Describe("Placement", func() { assertPlacementDecisionNumbers(placementName, namespace, 3, 1) assertPlacementStatusDecisionGroups( placementName, namespace, - []clusterapiv1beta1.DecisionGroupStatus{{Decisions: []string{placementName + "-decision-0"}, ClustersCount: 3}}) + []clusterapiv1beta1.DecisionGroupStatus{{Decisions: []string{testinghelpers.PlacementDecisionName(placementName, 1)}, ClustersCount: 3}}) ginkgo.By("change the predicates") // change the predicates @@ -352,17 +357,17 @@ var _ = ginkgo.Describe("Placement", func() { { DecisionGroupIndex: 0, DecisionGroupName: "canary", - Decisions: []string{placementName + "-decision-0"}, + Decisions: []string{testinghelpers.PlacementDecisionName(placementName, 1)}, ClustersCount: 2, }, { DecisionGroupIndex: 1, - Decisions: []string{placementName + "-decision-1"}, + Decisions: []string{testinghelpers.PlacementDecisionName(placementName, 2)}, ClustersCount: 2, }, { DecisionGroupIndex: 2, - Decisions: []string{placementName + "-decision-2"}, + Decisions: []string{testinghelpers.PlacementDecisionName(placementName, 3)}, ClustersCount: 1, }, }) @@ -373,12 +378,12 @@ var _ = ginkgo.Describe("Placement", func() { { DecisionGroupIndex: 0, DecisionGroupName: "canary", - Decisions: []string{placementName + "-decision-0"}, + Decisions: []string{testinghelpers.PlacementDecisionName(placementName, 1)}, ClustersCount: 1, }, { DecisionGroupIndex: 1, - Decisions: []string{placementName + "-decision-1"}, + Decisions: []string{testinghelpers.PlacementDecisionName(placementName, 2)}, ClustersCount: 2, }, }) @@ -391,12 +396,12 @@ var _ = ginkgo.Describe("Placement", func() { { DecisionGroupIndex: 0, DecisionGroupName: "canary", - Decisions: []string{placementName + "-decision-0"}, + Decisions: []string{testinghelpers.PlacementDecisionName(placementName, 1)}, ClustersCount: 2, }, { DecisionGroupIndex: 1, - Decisions: []string{placementName + "-decision-1"}, + Decisions: []string{testinghelpers.PlacementDecisionName(placementName, 2)}, ClustersCount: 2, }, }) @@ -408,17 +413,17 @@ var _ = ginkgo.Describe("Placement", func() { { DecisionGroupIndex: 0, DecisionGroupName: "canary", - Decisions: []string{placementName + "-decision-0"}, + Decisions: []string{testinghelpers.PlacementDecisionName(placementName, 1)}, ClustersCount: 2, }, { DecisionGroupIndex: 1, - Decisions: []string{placementName + "-decision-1"}, + Decisions: []string{testinghelpers.PlacementDecisionName(placementName, 2)}, ClustersCount: 2, }, { DecisionGroupIndex: 2, - Decisions: []string{placementName + "-decision-2"}, + Decisions: []string{testinghelpers.PlacementDecisionName(placementName, 3)}, ClustersCount: 1, }, }) @@ -430,7 +435,7 @@ var _ = ginkgo.Describe("Placement", func() { assertPlacementDecisionNumbers(placementName, namespace, 0, 1) assertPlacementStatusDecisionGroups( placementName, namespace, - []clusterapiv1beta1.DecisionGroupStatus{{Decisions: []string{placementName + "-decision-0"}, ClustersCount: 0}}) + []clusterapiv1beta1.DecisionGroupStatus{{Decisions: []string{testinghelpers.PlacementDecisionName(placementName, 1)}, ClustersCount: 0}}) }) ginkgo.It("Should schedule successfully once clusters belong to global(empty labelselector) clusterset are added/deleted)", func() {