Skip to content

Commit

Permalink
update decision group logic
Browse files Browse the repository at this point in the history 
Signed-off-by: haoqing0110 <qhao@redhat.com>
  • Loading branch information
@haoqing0110
haoqing0110 committed Jul 6, 2023
1 parent a6e8d8b commit 9b14b19
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 63 deletions.
60 changes: 39 additions & 21 deletions pkg/placement/controllers/scheduling/scheduling_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -465,11 +465,6 @@ func (c *schedulingController) generatePlacementDecisionsAndStatus(
// generate decision group
decisionGroups, status := c.generateDecisionGroups(placement, clusters)

// generate at least on empty decisionGroup, this is to ensure there's an empty placement decision if no cluster selected.
if len(decisionGroups) == 0 {
decisionGroups = append(decisionGroups, clusterDecisionGroup{})
}

// generate placement decision for each decision group
for decisionGroupIndex, decisionGroup := range decisionGroups {
// sort clusterdecisions by cluster name
Expand All @@ -495,34 +490,31 @@ func (c *schedulingController) generateDecisionGroups(
clusters []*clusterapiv1.ManagedCluster,
) (clusterDecisionGroups, *framework.Status) {
groups := []clusterDecisionGroup{}
// Record the cluster names
clusterNames := sets.NewString()
for _, cluster := range clusters {
clusterNames.Insert(cluster.Name)
}

// Calculate the group length
length, status := calculateLength(&placement.Spec.DecisionStrategy.GroupStrategy.ClustersPerDecisionGroup, len(clusters))
if status.IsError() {
return groups, status
}

// If no decision group defined in placement, all the clusters will be put into a group with empty name.
decisionStrategy := placement.Spec.DecisionStrategy.GroupStrategy.DeepCopy()
decisionGroups := decisionStrategy.DecisionGroups
decisionGroups = append(decisionGroups, clusterapiv1beta1.DecisionGroup{})
// Record the cluster names
clusterNames := sets.NewString()
for _, cluster := range clusters {
clusterNames.Insert(cluster.Name)
}

// Groups the clusters by decision groups.
for _, d := range decisionGroups {
for _, d := range placement.Spec.DecisionStrategy.GroupStrategy.DecisionGroups {
// create cluster label selector
clusterSelector, err := helpers.NewClusterSelector(d.ClusterSelector)
if err != nil {
status = framework.NewStatus("", framework.Misconfigured, err.Error())
return groups, status
}

// filter clusters by label selector
groupName := d.GroupName
matched := []clusterapiv1beta1.ClusterDecision{}

for _, cluster := range clusters {
if ok := clusterSelector.Matches(cluster.Labels, helpers.GetClusterClaims(cluster)); !ok {
continue
Expand All @@ -535,11 +527,31 @@ func (c *schedulingController) generateDecisionGroups(
ClusterName: cluster.Name,
})
clusterNames.Delete(cluster.Name)
}

// clusters number in each group should be less than ClustersPerDecisionGroup
if len(matched) > 0 {
decisionGroup := clusterDecisionGroup{
decisionGroupName: groupName,
clusterDecisions: matched,
}
groups = append(groups, decisionGroup)
}
}

// The clusters not belong to any decision group will be put into group with empty name.
// The group length depends on ClustersPerDecisionGroup.
if len(clusterNames) > 0 {
matched := []clusterapiv1beta1.ClusterDecision{}
for _, cluster := range clusters {
if !clusterNames.Has(cluster.Name) {
continue
}
matched = append(matched, clusterapiv1beta1.ClusterDecision{
ClusterName: cluster.Name,
})
if len(matched) == length {
decisionGroup := clusterDecisionGroup{
decisionGroupName: groupName,
decisionGroupName: "",
clusterDecisions: matched,
}
groups = append(groups, decisionGroup)
Expand All @@ -549,13 +561,18 @@ func (c *schedulingController) generateDecisionGroups(

if len(matched) > 0 {
decisionGroup := clusterDecisionGroup{
decisionGroupName: groupName,
decisionGroupName: "",
clusterDecisions: matched,
}
groups = append(groups, decisionGroup)
}
}

// generate at least on empty decisionGroup, this is to ensure there's an empty placement decision if no cluster selected.
if len(groups) == 0 {
groups = append(groups, clusterDecisionGroup{})
}

return groups, framework.NewStatus("", framework.Success, "")
}

Expand Down Expand Up @@ -771,7 +788,8 @@ func (c *schedulingController) createOrUpdatePlacementDecision(
}

func calculateLength(intOrStr *intstr.IntOrString, total int) (int, *framework.Status) {
var length int
length := total

switch intOrStr.Type {
case intstr.Int:
length = intOrStr.IntValue()
Expand All @@ -790,7 +808,7 @@ func calculateLength(intOrStr *intstr.IntOrString, total int) (int, *framework.S
}
}

if length > total {
if length < 0 || length > total {
length = total
}
return length, framework.NewStatus("", framework.Success, "")
Expand Down
71 changes: 29 additions & 42 deletions pkg/placement/controllers/scheduling/scheduling_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,18 @@ func TestSchedulingController_sync(t *testing.T) {
if placement.Status.NumberOfSelectedClusters != int32(3) {
t.Errorf("expecte %d cluster selected, but got %d", 3, placement.Status.NumberOfSelectedClusters)
}
if len(placement.Status.DecisionGroups) != 1 || placement.Status.DecisionGroups[0].ClustersCount != 3 {
t.Errorf("expecte %d cluster decision gorups, but got %d", 1, len(placement.Status.DecisionGroups))
expectDecisionGroups := []clusterapiv1beta1.DecisionGroupStatus{
{
DecisionGroupIndex: 0,
DecisionGroupName: "",
Decisions: []string{"placement1-decision-0"},
ClustersCount: 3,
},
}
if !reflect.DeepEqual(placement.Status.DecisionGroups, expectDecisionGroups) {
t.Errorf("expect %v cluster decision gorups, but got %v", expectDecisionGroups, placement.Status.DecisionGroups)
}

util.HasCondition(
placement.Status.Conditions,
clusterapiv1beta1.PlacementConditionSatisfied,
Expand Down Expand Up @@ -138,7 +147,7 @@ func TestSchedulingController_sync(t *testing.T) {
{
name: "placement with canary group strategy",
placement: testinghelpers.NewPlacement(placementNamespace, placementName).WithGroupStrategy(clusterapiv1beta1.GroupStrategy{
ClustersPerDecisionGroup: intstr.FromInt(2),
ClustersPerDecisionGroup: intstr.FromInt(1),
DecisionGroups: []clusterapiv1beta1.DecisionGroup{
{
GroupName: "canary",
Expand All @@ -157,7 +166,7 @@ func TestSchedulingController_sync(t *testing.T) {
scheduledDecisions: []*clusterapiv1.ManagedCluster{
testinghelpers.NewManagedCluster("cluster1").WithLabel("cloud", "Amazon").Build(),
testinghelpers.NewManagedCluster("cluster2").WithLabel("cloud", "Amazon").Build(),
testinghelpers.NewManagedCluster("cluster3").WithLabel("cloud", "Amazon").Build(),
testinghelpers.NewManagedCluster("cluster3").WithLabel("cloud", "Azure").Build(),
testinghelpers.NewManagedCluster("cluster4").WithLabel("cloud", "Azure").Build(),
},
unscheduledDecisions: 0,
Expand All @@ -180,13 +189,13 @@ func TestSchedulingController_sync(t *testing.T) {
DecisionGroupIndex: 0,
DecisionGroupName: "canary",
Decisions: []string{"placement1-decision-0"},
ClustersCount: 1,
ClustersCount: 2,
},
{
DecisionGroupIndex: 1,
DecisionGroupName: "",
Decisions: []string{"placement1-decision-1"},
ClustersCount: 2,
ClustersCount: 1,
},
{
DecisionGroupIndex: 2,
Expand All @@ -210,7 +219,6 @@ func TestSchedulingController_sync(t *testing.T) {
{
name: "placement with multiple group strategy",
placement: testinghelpers.NewPlacement(placementNamespace, placementName).WithGroupStrategy(clusterapiv1beta1.GroupStrategy{
ClustersPerDecisionGroup: intstr.FromString("25%"),
DecisionGroups: []clusterapiv1beta1.DecisionGroup{
{
GroupName: "group1",
Expand Down Expand Up @@ -239,9 +247,9 @@ func TestSchedulingController_sync(t *testing.T) {
unscheduledDecisions: 0,
},
validateActions: func(t *testing.T, actions []clienttesting.Action) {
testingcommon.AssertActions(t, actions, "create", "create", "create", "update")
testingcommon.AssertActions(t, actions, "create", "create", "update")
// check if Placement has been updated
actual := actions[3].(clienttesting.UpdateActionImpl).Object
actual := actions[2].(clienttesting.UpdateActionImpl).Object
placement, ok := actual.(*clusterapiv1beta1.Placement)
if !ok {
t.Errorf("expected Placement was updated")
Expand All @@ -256,18 +264,12 @@ func TestSchedulingController_sync(t *testing.T) {
DecisionGroupIndex: 0,
DecisionGroupName: "group1",
Decisions: []string{"placement1-decision-0"},
ClustersCount: 1,
ClustersCount: 2,
},
{
DecisionGroupIndex: 1,
DecisionGroupName: "group1",
Decisions: []string{"placement1-decision-1"},
ClustersCount: 1,
},
{
DecisionGroupIndex: 2,
DecisionGroupName: "group2",
Decisions: []string{"placement1-decision-2"},
Decisions: []string{"placement1-decision-1"},
ClustersCount: 1,
},
}
Expand Down Expand Up @@ -1035,7 +1037,7 @@ func TestBind(t *testing.T) {
{
name: "create placementdecision with canary group strategy",
placement: testinghelpers.NewPlacement(placementNamespace, placementName).WithGroupStrategy(clusterapiv1beta1.GroupStrategy{
ClustersPerDecisionGroup: intstr.FromInt(2),
ClustersPerDecisionGroup: intstr.FromString("25%"),
DecisionGroups: []clusterapiv1beta1.DecisionGroup{
{
GroupName: "canary",
Expand All @@ -1047,7 +1049,7 @@ func TestBind(t *testing.T) {
clusters: []*clusterapiv1.ManagedCluster{
testinghelpers.NewManagedCluster("cluster1").WithLabel("cloud", "Amazon").Build(),
testinghelpers.NewManagedCluster("cluster2").WithLabel("cloud", "Amazon").Build(),
testinghelpers.NewManagedCluster("cluster3").WithLabel("cloud", "Amazon").Build(),
testinghelpers.NewManagedCluster("cluster3").WithLabel("cloud", "Azure").Build(),
testinghelpers.NewManagedCluster("cluster4").WithLabel("cloud", "Azure").Build(),
},
validateActions: func(t *testing.T, actions []clienttesting.Action) {
Expand All @@ -1058,7 +1060,7 @@ func TestBind(t *testing.T) {
if !ok {
t.Errorf("expected PlacementDecision was created")
}
assertClustersSelected(t, placementDecision.Status.Decisions, selectedClusters[3:4]...)
assertClustersSelected(t, placementDecision.Status.Decisions, selectedClusters[2:]...)
if placementDecision.Labels[clusterapiv1beta1.DecisionGroupIndexLabel] != "0" {
t.Errorf("unexpected PlacementDecision labels %v", placementDecision.Labels)
}
Expand All @@ -1071,7 +1073,7 @@ func TestBind(t *testing.T) {
if !ok {
t.Errorf("expected PlacementDecision was created")
}
assertClustersSelected(t, placementDecision.Status.Decisions, selectedClusters[0:2]...)
assertClustersSelected(t, placementDecision.Status.Decisions, selectedClusters[0:1]...)
if placementDecision.Labels[clusterapiv1beta1.DecisionGroupIndexLabel] != "1" {
t.Errorf("unexpected PlacementDecision labels %v", placementDecision.Labels)
}
Expand All @@ -1084,7 +1086,7 @@ func TestBind(t *testing.T) {
if !ok {
t.Errorf("expected PlacementDecision was created")
}
assertClustersSelected(t, placementDecision.Status.Decisions, selectedClusters[2:3]...)
assertClustersSelected(t, placementDecision.Status.Decisions, selectedClusters[1:2]...)
if placementDecision.Labels[clusterapiv1beta1.DecisionGroupIndexLabel] != "2" {
t.Errorf("unexpected PlacementDecision labels %v", placementDecision.Labels)
}
Expand All @@ -1096,7 +1098,6 @@ func TestBind(t *testing.T) {
{
name: "create placementdecision when no cluster selected by canary group strategy",
placement: testinghelpers.NewPlacement(placementNamespace, placementName).WithGroupStrategy(clusterapiv1beta1.GroupStrategy{
ClustersPerDecisionGroup: intstr.FromInt(2),
DecisionGroups: []clusterapiv1beta1.DecisionGroup{
{
GroupName: "canary",
Expand All @@ -1111,8 +1112,8 @@ func TestBind(t *testing.T) {
testinghelpers.NewManagedCluster("cluster3").WithLabel("cloud", "Amazon").Build(),
},
validateActions: func(t *testing.T, actions []clienttesting.Action) {
testingcommon.AssertActions(t, actions, "create", "create")
selectedClusters := newSelectedClusters(2)
testingcommon.AssertActions(t, actions, "create")
selectedClusters := newSelectedClusters(3)
actual := actions[0].(clienttesting.CreateActionImpl).Object
placementDecision, ok := actual.(*clusterapiv1beta1.PlacementDecision)
if !ok {
Expand All @@ -1130,7 +1131,6 @@ func TestBind(t *testing.T) {
{
name: "create placementdecision with multiple group strategy",
placement: testinghelpers.NewPlacement(placementNamespace, placementName).WithGroupStrategy(clusterapiv1beta1.GroupStrategy{
ClustersPerDecisionGroup: intstr.FromString("25%"),
DecisionGroups: []clusterapiv1beta1.DecisionGroup{
{
GroupName: "group1",
Expand All @@ -1151,14 +1151,14 @@ func TestBind(t *testing.T) {
testinghelpers.NewManagedCluster("cluster3").WithLabel("cloud", "Azure").Build(),
},
validateActions: func(t *testing.T, actions []clienttesting.Action) {
testingcommon.AssertActions(t, actions, "create", "create", "create")
testingcommon.AssertActions(t, actions, "create", "create")
selectedClusters := newSelectedClusters(4)
actual := actions[0].(clienttesting.CreateActionImpl).Object
placementDecision, ok := actual.(*clusterapiv1beta1.PlacementDecision)
if !ok {
t.Errorf("expected PlacementDecision was created")
}
assertClustersSelected(t, placementDecision.Status.Decisions, selectedClusters[0:1]...)
assertClustersSelected(t, placementDecision.Status.Decisions, selectedClusters[0:2]...)
if placementDecision.Labels[clusterapiv1beta1.DecisionGroupIndexLabel] != "0" {
t.Errorf("unexpected PlacementDecision labels %v", placementDecision.Labels)
}
Expand All @@ -1171,21 +1171,8 @@ func TestBind(t *testing.T) {
if !ok {
t.Errorf("expected PlacementDecision was created")
}
assertClustersSelected(t, placementDecision.Status.Decisions, selectedClusters[1:2]...)
if placementDecision.Labels[clusterapiv1beta1.DecisionGroupIndexLabel] != "1" {
t.Errorf("unexpected PlacementDecision labels %v", placementDecision.Labels)
}
if placementDecision.Labels[clusterapiv1beta1.DecisionGroupNameLabel] != "group1" {
t.Errorf("unexpected PlacementDecision labels %v", placementDecision.Labels)
}

actual = actions[2].(clienttesting.CreateActionImpl).Object
placementDecision, ok = actual.(*clusterapiv1beta1.PlacementDecision)
if !ok {
t.Errorf("expected PlacementDecision was created")
}
assertClustersSelected(t, placementDecision.Status.Decisions, selectedClusters[2:3]...)
if placementDecision.Labels[clusterapiv1beta1.DecisionGroupIndexLabel] != "2" {
if placementDecision.Labels[clusterapiv1beta1.DecisionGroupIndexLabel] != "1" {
t.Errorf("unexpected PlacementDecision labels %v", placementDecision.Labels)
}
if placementDecision.Labels[clusterapiv1beta1.DecisionGroupNameLabel] != "group2" {
Expand Down

0 comments on commit 9b14b19

Please sign in to comment.