Skip to content

Commit

Permalink
add test
Browse files Browse the repository at this point in the history
Signed-off-by: KunWuLuan <kunwuluan@gmail.com>
  • Loading branch information
KunWuLuan committed Mar 6, 2024
1 parent dd70b1a commit 6de91a0
Show file tree
Hide file tree
Showing 4 changed files with 216 additions and 37 deletions.
21 changes: 3 additions & 18 deletions pkg/scheduler/frameworkext/topologymanager/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,24 +82,9 @@ func (m *topologyManager) Admit(ctx context.Context, cycleState *framework.Cycle
func (m *topologyManager) calculateAffinity(ctx context.Context, cycleState *framework.CycleState, policy Policy, pod *corev1.Pod, nodeName string, exclusivePolicy apiext.NUMATopologyExclusive, allNUMANodeStatus []apiext.NUMANodeStatus) (NUMATopologyHint, bool) {
providersHints := m.accumulateProvidersHints(ctx, cycleState, pod, nodeName)
bestHint, admit := policy.Merge(providersHints, exclusivePolicy, allNUMANodeStatus)
// check bestHint again if default hint is the best
if exclusivePolicy == apiext.NUMATopologyExclusiveRequired {
if bestHint.NUMANodeAffinity.Count() > 1 {
// we should make sure no numa is in single state
for _, nodeid := range bestHint.NUMANodeAffinity.GetBits() {
if allNUMANodeStatus[nodeid] == apiext.NUMANodeStatusSingle {
klog.V(5).Infof("bestHint violated the exclusivePolicy requirement: bestHint: %v, policy: %v, numaStatus: %v, nodeName: %v, pod: %v",
bestHint, exclusivePolicy, allNUMANodeStatus, nodeName, pod.Name)
return bestHint, false
}
}
} else {
if allNUMANodeStatus[bestHint.NUMANodeAffinity.GetBits()[0]] == apiext.NUMANodeStatusShared {
klog.V(5).Infof("bestHint violated the exclusivePolicy requirement: bestHint: %v, policy: %v, numaStatus: %v, nodeName: %v, pod: %v",
bestHint, exclusivePolicy, allNUMANodeStatus, nodeName, pod.Name)
return bestHint, false
}
}
if !checkExclusivePolicy(bestHint, exclusivePolicy, allNUMANodeStatus) {
klog.V(5).Infof("bestHint violated the exclusivePolicy requirement: bestHint: %v, policy: %v, numaStatus: %v, nodeName: %v, pod: %v",
bestHint, exclusivePolicy, allNUMANodeStatus, nodeName, pod.Name)
}
klog.V(5).Infof("PodTopologyHint: %v", bestHint)
return bestHint, admit
Expand Down
35 changes: 22 additions & 13 deletions pkg/scheduler/frameworkext/topologymanager/policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,26 @@ func (th *NUMATopologyHint) LessThan(other NUMATopologyHint) bool {
return th.NUMANodeAffinity.IsNarrowerThan(other.NUMANodeAffinity)
}

// Check if the affinity match the exclusive policy, return true if match or false otherwise.
func checkExclusivePolicy(affinity NUMATopologyHint, exclusivePolicy apiext.NUMATopologyExclusive, allNUMANodeStatus []apiext.NUMANodeStatus) bool {
// check bestHint again if default hint is the best
if exclusivePolicy == apiext.NUMATopologyExclusiveRequired {
if affinity.NUMANodeAffinity.Count() > 1 {
// we should make sure no numa is in single state
for _, nodeid := range affinity.NUMANodeAffinity.GetBits() {
if allNUMANodeStatus[nodeid] == apiext.NUMANodeStatusSingle {
return false
}
}
} else {
if allNUMANodeStatus[affinity.NUMANodeAffinity.GetBits()[0]] == apiext.NUMANodeStatusShared {
return false
}
}
}
return true
}

// Merge a TopologyHints permutation to a single hint by performing a bitwise-AND
// of their affinity masks. The hint shall be preferred if all hits in the permutation
// are preferred.
Expand Down Expand Up @@ -143,19 +163,8 @@ func mergeFilteredHints(numaNodes []int, filteredHints [][]NUMATopologyHint, exc
if mergedHint.NUMANodeAffinity.Count() == 0 {
return
}
if exclusivePolicy == apiext.NUMATopologyExclusiveRequired {
if mergedHint.NUMANodeAffinity.Count() > 1 {
// we should make sure no numa is in single state
for _, nodeid := range mergedHint.NUMANodeAffinity.GetBits() {
if allNUMANodeStatus[nodeid] == apiext.NUMANodeStatusSingle {
return
}
}
} else {
if allNUMANodeStatus[mergedHint.NUMANodeAffinity.GetBits()[0]] == apiext.NUMANodeStatusShared {
return
}
}
if !checkExclusivePolicy(mergedHint, exclusivePolicy, allNUMANodeStatus) {
return
}

for _, v := range permutation {
Expand Down
127 changes: 124 additions & 3 deletions pkg/scheduler/frameworkext/topologymanager/policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ import (
"reflect"
"testing"

"github.com/koordinator-sh/koordinator/apis/extension"

Check failure on line 25 in pkg/scheduler/frameworkext/topologymanager/policy_test.go

View workflow job for this annotation

GitHub Actions / golangci-lint

File is not `goimports`-ed with -local github.com/koordinator-sh/koordinator (goimports)
apiext "github.com/koordinator-sh/koordinator/apis/extension"
"github.com/koordinator-sh/koordinator/pkg/util/bitmask"
"github.com/stretchr/testify/assert"
corev1 "k8s.io/api/core/v1"
"k8s.io/kubernetes/pkg/scheduler/framework"

"github.com/koordinator-sh/koordinator/apis/extension"
"github.com/koordinator-sh/koordinator/pkg/util/bitmask"
)

var _ NUMATopologyHintProvider = &mockNUMATopologyHintProvider{}
Expand Down Expand Up @@ -898,3 +898,124 @@ func testPolicyMerge(policy Policy, tcases []policyMergeTestCase, t *testing.T)
}
}
}

func Test_checkExclusivePolicy(t *testing.T) {
type args struct {
affinity NUMATopologyHint
exclusivePolicy apiext.NUMATopologyExclusive
allNUMANodeStatus []apiext.NUMANodeStatus
}
tests := []struct {
name string
args args
want bool
}{
// TODO: Add test cases.
{
name: "preferred policy 1",
args: args{
affinity: NUMATopologyHint{NUMANodeAffinity: NewTestBitMask(0), Preferred: true, Score: 0},
exclusivePolicy: apiext.NUMATopologyExclusivePreferred,
allNUMANodeStatus: []apiext.NUMANodeStatus{apiext.NUMANodeStatusShared, apiext.NUMANodeStatusShared},
},
want: true,
},
{
name: "preferred policy 2",
args: args{
affinity: NUMATopologyHint{NUMANodeAffinity: NewTestBitMask(0, 1), Preferred: true, Score: 0},
exclusivePolicy: apiext.NUMATopologyExclusivePreferred,
allNUMANodeStatus: []apiext.NUMANodeStatus{apiext.NUMANodeStatusShared, apiext.NUMANodeStatusShared},
},
want: true,
},
{
name: "preferred policy 3",
args: args{
affinity: NUMATopologyHint{NUMANodeAffinity: NewTestBitMask(0), Preferred: true, Score: 0},
exclusivePolicy: apiext.NUMATopologyExclusivePreferred,
allNUMANodeStatus: []apiext.NUMANodeStatus{apiext.NUMANodeStatusIdle, apiext.NUMANodeStatusSingle},
},
want: true,
},
{
name: "preferred policy 4",
args: args{
affinity: NUMATopologyHint{NUMANodeAffinity: NewTestBitMask(0, 1), Preferred: true, Score: 0},
exclusivePolicy: apiext.NUMATopologyExclusivePreferred,
allNUMANodeStatus: []apiext.NUMANodeStatus{apiext.NUMANodeStatusIdle, apiext.NUMANodeStatusSingle},
},
want: true,
},
{
name: "required policy 1",
args: args{
affinity: NUMATopologyHint{NUMANodeAffinity: NewTestBitMask(0), Preferred: true, Score: 0},
exclusivePolicy: apiext.NUMATopologyExclusiveRequired,
allNUMANodeStatus: []apiext.NUMANodeStatus{apiext.NUMANodeStatusIdle, apiext.NUMANodeStatusSingle},
},
want: true,
},
{
name: "required policy 2",
args: args{
affinity: NUMATopologyHint{NUMANodeAffinity: NewTestBitMask(0), Preferred: true, Score: 0},
exclusivePolicy: apiext.NUMATopologyExclusiveRequired,
allNUMANodeStatus: []apiext.NUMANodeStatus{apiext.NUMANodeStatusShared, apiext.NUMANodeStatusSingle},
},
want: false,
},
{
name: "required policy 3",
args: args{
affinity: NUMATopologyHint{NUMANodeAffinity: NewTestBitMask(0), Preferred: true, Score: 0},
exclusivePolicy: apiext.NUMATopologyExclusiveRequired,
allNUMANodeStatus: []apiext.NUMANodeStatus{apiext.NUMANodeStatusShared, apiext.NUMANodeStatusShared},
},
want: false,
},
{
name: "required policy 4",
args: args{
affinity: NUMATopologyHint{NUMANodeAffinity: NewTestBitMask(0), Preferred: true, Score: 0},
exclusivePolicy: apiext.NUMATopologyExclusiveRequired,
allNUMANodeStatus: []apiext.NUMANodeStatus{apiext.NUMANodeStatusSingle, apiext.NUMANodeStatusShared},
},
want: true,
},
{
name: "required policy 5",
args: args{
affinity: NUMATopologyHint{NUMANodeAffinity: NewTestBitMask(0, 1), Preferred: true, Score: 0},
exclusivePolicy: apiext.NUMATopologyExclusiveRequired,
allNUMANodeStatus: []apiext.NUMANodeStatus{apiext.NUMANodeStatusShared, apiext.NUMANodeStatusSingle},
},
want: false,
},
{
name: "required policy 6",
args: args{
affinity: NUMATopologyHint{NUMANodeAffinity: NewTestBitMask(0, 1), Preferred: true, Score: 0},
exclusivePolicy: apiext.NUMATopologyExclusiveRequired,
allNUMANodeStatus: []apiext.NUMANodeStatus{apiext.NUMANodeStatusShared, apiext.NUMANodeStatusIdle},
},
want: true,
},
{
name: "required policy 7",
args: args{
affinity: NUMATopologyHint{NUMANodeAffinity: NewTestBitMask(0, 1), Preferred: true, Score: 0},
exclusivePolicy: apiext.NUMATopologyExclusiveRequired,
allNUMANodeStatus: []apiext.NUMANodeStatus{apiext.NUMANodeStatusShared, apiext.NUMANodeStatusShared},
},
want: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := checkExclusivePolicy(tt.args.affinity, tt.args.exclusivePolicy, tt.args.allNUMANodeStatus); got != tt.want {
t.Errorf("checkExclusivePolicy() = %v, want %v", got, tt.want)
}
})
}
}
70 changes: 67 additions & 3 deletions pkg/scheduler/plugins/nodenumaresource/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,17 @@ limitations under the License.
package nodenumaresource

import (
"errors"
"reflect"
"testing"

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/koordinator-sh/koordinator/apis/extension"

Check failure on line 26 in pkg/scheduler/plugins/nodenumaresource/util_test.go

View workflow job for this annotation

GitHub Actions / golangci-lint

File is not `goimports`-ed with -local github.com/koordinator-sh/koordinator (goimports)
schedulingconfig "github.com/koordinator-sh/koordinator/pkg/scheduler/apis/config"
"github.com/stretchr/testify/assert"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/koordinator-sh/koordinator/apis/extension"
schedulingconfig "github.com/koordinator-sh/koordinator/pkg/scheduler/apis/config"
)

func Test_getCPUBindPolicy(t *testing.T) {
Expand Down Expand Up @@ -99,3 +102,64 @@ func Test_getCPUBindPolicy(t *testing.T) {
})
}
}

func Test_mergeTopologyPolicy(t *testing.T) {
type args struct {
nodePolicy extension.NUMATopologyPolicy
podPolicy extension.NUMATopologyPolicy
}
tests := []struct {
name string
args args
want extension.NUMATopologyPolicy
wantErr error
}{
// TODO: Add test cases.
{
name: "no policy on pod",
args: args{
nodePolicy: extension.NUMATopologyPolicyRestricted,
podPolicy: extension.NUMATopologyPolicyNone,
},
want: extension.NUMATopologyPolicyRestricted,
},
{
name: "policy on pod",
args: args{
nodePolicy: extension.NUMATopologyPolicyRestricted,
podPolicy: extension.NUMATopologyPolicyRestricted,
},
want: extension.NUMATopologyPolicyRestricted,
},
{
name: "policy on pod not match policy on node",
args: args{
nodePolicy: extension.NUMATopologyPolicyRestricted,
podPolicy: extension.NUMATopologyPolicyBestEffort,
},
want: extension.NUMATopologyPolicyNone,
wantErr: errors.New(ErrNotMatchNUMATopology),
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := mergeTopologyPolicy(tt.args.nodePolicy, tt.args.podPolicy)
if err == nil && err != tt.wantErr {
t.Errorf("mergeTopologyPolicy() error = %v, wantErr %v", err, tt.wantErr)
return
} else if tt.wantErr == nil && err != tt.wantErr {
t.Errorf("mergeTopologyPolicy() error = %v, wantErr %v", err, tt.wantErr)
return
}
if err != nil && tt.wantErr != nil {
if diff := cmp.Diff(err.Error(), tt.wantErr.Error(), cmpopts.EquateErrors()); diff != "" {
t.Errorf("mergeTopologyPolicy() error = %v, wantErr %v, diff: %v", err, tt.wantErr, diff)
return
}
}
if !reflect.DeepEqual(got, tt.want) {
t.Errorf("mergeTopologyPolicy() = %v, want %v", got, tt.want)
}
})
}
}

0 comments on commit 6de91a0

Please sign in to comment.