From 26c787531e5fda71df12293ca08fd9dcd0af1a7d Mon Sep 17 00:00:00 2001 From: whitewindmills Date: Fri, 31 Mar 2023 23:57:59 +0800 Subject: [PATCH] add placement helper Signed-off-by: whitewindmills --- .../policy/v1alpha1/propagation_helper.go | 10 +++++ .../v1alpha1/propagation_helper_test.go | 39 ++++++++++++++++-- pkg/descheduler/core/filter.go | 2 +- pkg/util/helper/policy.go | 11 ++--- pkg/util/helper/policy_test.go | 41 ++++++++++++++++++- .../clusterpropagationpolicy/mutating.go | 2 +- pkg/webhook/propagationpolicy/mutating.go | 2 +- 7 files changed, 94 insertions(+), 13 deletions(-) diff --git a/pkg/apis/policy/v1alpha1/propagation_helper.go b/pkg/apis/policy/v1alpha1/propagation_helper.go index b261aa9c7..b4480aa3f 100644 --- a/pkg/apis/policy/v1alpha1/propagation_helper.go +++ b/pkg/apis/policy/v1alpha1/propagation_helper.go @@ -18,3 +18,13 @@ func (p *PropagationPolicy) ExplicitPriority() int32 { func (p *ClusterPropagationPolicy) ExplicitPriority() int32 { return p.Spec.ExplicitPriority() } + +// ReplicaSchedulingType returns the replica assignment strategy which is +// "Duplicated" or "Divided". Returns "Duplicated" if the replica strategy is nil. +func (p *Placement) ReplicaSchedulingType() ReplicaSchedulingType { + if p.ReplicaScheduling == nil { + return ReplicaSchedulingTypeDuplicated + } + + return p.ReplicaScheduling.ReplicaSchedulingType +} diff --git a/pkg/apis/policy/v1alpha1/propagation_helper_test.go b/pkg/apis/policy/v1alpha1/propagation_helper_test.go index 18ea8e5e0..d15f842d9 100644 --- a/pkg/apis/policy/v1alpha1/propagation_helper_test.go +++ b/pkg/apis/policy/v1alpha1/propagation_helper_test.go @@ -13,11 +13,11 @@ func TestPropagationPolicy_ExplicitPriority(t *testing.T) { expectedPriority int32 }{ { - name: "no priority declared should defaults to zero", + name: "expected to be zero in pp if no priority declared", expectedPriority: 0, }, { - name: "no priority declared should defaults to zero", + name: "expected to be declared priority in pp", declaredPriority: pointer.Int32(20), expectedPriority: 20, }, @@ -41,11 +41,11 @@ func TestClusterPropagationPolicy_ExplicitPriority(t *testing.T) { expectedPriority int32 }{ { - name: "no priority declared should defaults to zero", + name: "expected to be zero in cpp if no priority declared", expectedPriority: 0, }, { - name: "no priority declared should defaults to zero", + name: "expected to be declared priority in cpp", declaredPriority: pointer.Int32(20), expectedPriority: 20, }, @@ -61,3 +61,34 @@ func TestClusterPropagationPolicy_ExplicitPriority(t *testing.T) { }) } } + +func TestPlacement_ReplicaSchedulingType(t *testing.T) { + var tests = []struct { + name string + declaredReplicaSchedulingType ReplicaSchedulingType + expectedReplicaSchedulingType ReplicaSchedulingType + }{ + { + name: "no replica scheduling strategy declared", + expectedReplicaSchedulingType: ReplicaSchedulingTypeDuplicated, + }, + { + name: "replica scheduling strategy is 'Divided'", + declaredReplicaSchedulingType: ReplicaSchedulingTypeDivided, + expectedReplicaSchedulingType: ReplicaSchedulingTypeDivided, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + p := &Placement{} + if test.declaredReplicaSchedulingType != "" { + p.ReplicaScheduling = &ReplicaSchedulingStrategy{ReplicaSchedulingType: test.declaredReplicaSchedulingType} + } + got := p.ReplicaSchedulingType() + if test.expectedReplicaSchedulingType != got { + t.Fatalf("Expected：%s, but got: %s", test.expectedReplicaSchedulingType, got) + } + }) + } +} diff --git a/pkg/descheduler/core/filter.go b/pkg/descheduler/core/filter.go index a1778b806..4f5aae606 100644 --- a/pkg/descheduler/core/filter.go +++ b/pkg/descheduler/core/filter.go @@ -45,5 +45,5 @@ func validatePlacement(binding *workv1alpha2.ResourceBinding) bool { if placement == nil { return false } - return helper.IsReplicaDynamicDivided(placement.ReplicaScheduling) + return helper.IsReplicaDynamicDivided(placement) } diff --git a/pkg/util/helper/policy.go b/pkg/util/helper/policy.go index 914f26f44..b288f733a 100644 --- a/pkg/util/helper/policy.go +++ b/pkg/util/helper/policy.go @@ -103,11 +103,12 @@ func GenerateResourceSelectorForServiceImport(svcImport policyv1alpha1.ResourceS } // IsReplicaDynamicDivided checks if a PropagationPolicy schedules replicas as dynamic. -func IsReplicaDynamicDivided(strategy *policyv1alpha1.ReplicaSchedulingStrategy) bool { - if strategy == nil || strategy.ReplicaSchedulingType != policyv1alpha1.ReplicaSchedulingTypeDivided { +func IsReplicaDynamicDivided(placement *policyv1alpha1.Placement) bool { + if placement.ReplicaSchedulingType() != policyv1alpha1.ReplicaSchedulingTypeDivided { return false } + strategy := placement.ReplicaScheduling switch strategy.ReplicaDivisionPreference { case policyv1alpha1.ReplicaDivisionPreferenceWeighted: return strategy.WeightPreference != nil && len(strategy.WeightPreference.DynamicWeight) != 0 @@ -132,9 +133,9 @@ func GetAppliedPlacement(annotations map[string]string) (*policyv1alpha1.Placeme } // SetReplicaDivisionPreferenceWeighted Set the default value of ReplicaDivisionPreference to Weighted -func SetReplicaDivisionPreferenceWeighted(strategy *policyv1alpha1.ReplicaSchedulingStrategy) { - if strategy == nil || strategy.ReplicaSchedulingType != policyv1alpha1.ReplicaSchedulingTypeDivided || strategy.ReplicaDivisionPreference != "" { +func SetReplicaDivisionPreferenceWeighted(placement *policyv1alpha1.Placement) { + if placement.ReplicaSchedulingType() != policyv1alpha1.ReplicaSchedulingTypeDivided || placement.ReplicaScheduling.ReplicaDivisionPreference != "" { return } - strategy.ReplicaDivisionPreference = policyv1alpha1.ReplicaDivisionPreferenceWeighted + placement.ReplicaScheduling.ReplicaDivisionPreference = policyv1alpha1.ReplicaDivisionPreferenceWeighted } diff --git a/pkg/util/helper/policy_test.go b/pkg/util/helper/policy_test.go index 815be3fa0..e67a50af3 100644 --- a/pkg/util/helper/policy_test.go +++ b/pkg/util/helper/policy_test.go @@ -315,7 +315,7 @@ func TestIsReplicaDynamicDivided(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - res := IsReplicaDynamicDivided(tt.strategy) + res := IsReplicaDynamicDivided(&policyv1alpha1.Placement{ReplicaScheduling: tt.strategy}) if res != tt.expected { t.Errorf("expected %v, but got %v", tt.expected, res) } @@ -361,3 +361,42 @@ func TestGetAppliedPlacement(t *testing.T) { }) } } + +func TestSetReplicaDivisionPreferenceWeighted(t *testing.T) { + tests := []struct { + name string + strategy *policyv1alpha1.ReplicaSchedulingStrategy + expectedWeighted bool + }{ + { + name: "no replica scheduling strategy declared", + expectedWeighted: false, + }, + { + name: "specified aggregated division preference", + strategy: &policyv1alpha1.ReplicaSchedulingStrategy{ + ReplicaSchedulingType: policyv1alpha1.ReplicaSchedulingTypeDivided, + ReplicaDivisionPreference: policyv1alpha1.ReplicaDivisionPreferenceAggregated, + }, + expectedWeighted: false, + }, + { + name: "unspecified replica division preference", + strategy: &policyv1alpha1.ReplicaSchedulingStrategy{ + ReplicaSchedulingType: policyv1alpha1.ReplicaSchedulingTypeDivided, + }, + expectedWeighted: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + p := &policyv1alpha1.Placement{ReplicaScheduling: tt.strategy} + SetReplicaDivisionPreferenceWeighted(p) + if (p.ReplicaScheduling != nil && + p.ReplicaScheduling.ReplicaDivisionPreference == policyv1alpha1.ReplicaDivisionPreferenceWeighted) != tt.expectedWeighted { + t.Errorf("expectedWeighted %v, but got %v", tt.expectedWeighted, !tt.expectedWeighted) + } + }) + } +} diff --git a/pkg/webhook/clusterpropagationpolicy/mutating.go b/pkg/webhook/clusterpropagationpolicy/mutating.go index c66fcf932..d3396ec44 100644 --- a/pkg/webhook/clusterpropagationpolicy/mutating.go +++ b/pkg/webhook/clusterpropagationpolicy/mutating.go @@ -57,7 +57,7 @@ func (a *MutatingAdmission) Handle(ctx context.Context, req admission.Request) a } // When ReplicaSchedulingType is Divided, set the default value of ReplicaDivisionPreference to Weighted. - helper.SetReplicaDivisionPreferenceWeighted(policy.Spec.Placement.ReplicaScheduling) + helper.SetReplicaDivisionPreferenceWeighted(&policy.Spec.Placement) marshaledBytes, err := json.Marshal(policy) if err != nil { diff --git a/pkg/webhook/propagationpolicy/mutating.go b/pkg/webhook/propagationpolicy/mutating.go index 55d82b081..7a80fbdfa 100644 --- a/pkg/webhook/propagationpolicy/mutating.go +++ b/pkg/webhook/propagationpolicy/mutating.go @@ -69,7 +69,7 @@ func (a *MutatingAdmission) Handle(ctx context.Context, req admission.Request) a } // When ReplicaSchedulingType is Divided, set the default value of ReplicaDivisionPreference to Weighted. - helper.SetReplicaDivisionPreferenceWeighted(policy.Spec.Placement.ReplicaScheduling) + helper.SetReplicaDivisionPreferenceWeighted(&policy.Spec.Placement) marshaledBytes, err := json.Marshal(policy) if err != nil {