Skip to content

Commit aff6bdd

Browse files
ffromaniffromani
committed
nrt: use the new config code
Port the plugin application code to use the new configuration facilities. No expected changes in behavior. Signed-off-by: Francesco Romani <[email protected]>
1 parent 9b11ab6 commit aff6bdd

File tree

11 files changed

+117
-87
lines changed

11 files changed

+117
-87
lines changed

‎pkg/noderesourcetopology/cache/overreserve_test.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -509,6 +509,12 @@ func TestResyncMatchFingerprint(t *testing.T) {
509509
podfingerprint.Annotation: "pfp0v0019e0420efb37746c6",
510510
},
511511
},
512+
Attributes: topologyv1alpha2.AttributeList{
513+
{
514+
Name: podfingerprint.Attribute,
515+
Value: "pfp0v0019e0420efb37746c6",
516+
},
517+
},
512518
TopologyPolicies: []string{string(topologyv1alpha2.SingleNUMANodeContainerLevel)},
513519
Zones: topologyv1alpha2.ZoneList{
514520
{

‎pkg/noderesourcetopology/cache/store.go

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -195,10 +195,13 @@ func (cnt counter) Len() int {
195195
// podFingerprintForNodeTopology extracts without recomputing the pods fingerprint from
196196
// the provided Node Resource Topology object.
197197
func podFingerprintForNodeTopology(nrt *topologyv1alpha2.NodeResourceTopology) string {
198-
if nrt.Annotations == nil {
199-
return ""
198+
if attrValue, ok := findAttribute(nrt.Attributes, podfingerprint.Attribute); ok {
199+
return attrValue
200200
}
201-
return nrt.Annotations[podfingerprint.Annotation]
201+
if nrt.Annotations != nil {
202+
return nrt.Annotations[podfingerprint.Annotation]
203+
}
204+
return ""
202205
}
203206

204207
// checkPodFingerprintForNode verifies if the given pods fingeprint (usually from NRT update) matches the
@@ -222,3 +225,12 @@ func checkPodFingerprintForNode(logID string, indexer NodeIndexer, nodeName, pfp
222225

223226
return pfp.Check(pfpExpected)
224227
}
228+
229+
func findAttribute(attrs topologyv1alpha2.AttributeList, name string) (string, bool) {
230+
for _, attr := range attrs {
231+
if attr.Name == name {
232+
return attr.Value, true
233+
}
234+
}
235+
return "", false
236+
}

‎pkg/noderesourcetopology/cache/store_test.go

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,22 @@ func TestFingerprintFromNRT(t *testing.T) {
5151
t.Errorf("misdetected fingerprint from empty annotations")
5252
}
5353

54-
pfpTest := "test"
55-
nrt.Annotations[podfingerprint.Annotation] = pfpTest
54+
pfpTestAnn := "test-ann"
55+
nrt.Annotations[podfingerprint.Annotation] = pfpTestAnn
5656
pfp = podFingerprintForNodeTopology(nrt)
57-
if pfp != pfpTest {
58-
t.Errorf("misdetected fingerprint as %q expected %q", pfp, pfpTest)
57+
if pfp != pfpTestAnn {
58+
t.Errorf("misdetected fingerprint as %q expected %q", pfp, pfpTestAnn)
59+
}
60+
61+
// test attribute overrides annotation
62+
pfpTestAttr := "test-attr"
63+
nrt.Attributes = append(nrt.Attributes, topologyv1alpha2.AttributeInfo{
64+
Name: podfingerprint.Attribute,
65+
Value: pfpTestAttr,
66+
})
67+
pfp = podFingerprintForNodeTopology(nrt)
68+
if pfp != pfpTestAttr {
69+
t.Errorf("misdetected fingerprint as %q expected %q", pfp, pfpTestAttr)
5970
}
6071
}
6172

‎pkg/noderesourcetopology/filter.go

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"k8s.io/klog/v2"
2626
v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper"
2727
v1qos "k8s.io/kubernetes/pkg/apis/core/v1/helper/qos"
28+
kubeletconfig "k8s.io/kubernetes/pkg/kubelet/apis/config"
2829
bm "k8s.io/kubernetes/pkg/kubelet/cm/topologymanager/bitmask"
2930
"k8s.io/kubernetes/pkg/scheduler/framework"
3031

@@ -198,7 +199,6 @@ func (tm *TopologyMatch) Filter(ctx context.Context, cycleState *framework.Cycle
198199

199200
nodeName := nodeInfo.Node().Name
200201
nodeTopology, ok := tm.nrtCache.GetCachedNRTCopy(nodeName, pod)
201-
202202
if !ok {
203203
return framework.NewStatus(framework.Unschedulable, fmt.Sprintf("invalid node topology data for node %s", nodeName))
204204
}
@@ -207,15 +207,9 @@ func (tm *TopologyMatch) Filter(ctx context.Context, cycleState *framework.Cycle
207207
}
208208

209209
klog.V(5).InfoS("Found NodeResourceTopology", "nodeTopology", klog.KObj(nodeTopology))
210-
if len(nodeTopology.TopologyPolicies) == 0 {
211-
klog.V(2).InfoS("Cannot determine policy", "node", nodeName)
212-
return nil
213-
}
214210

215-
policyName := nodeTopology.TopologyPolicies[0]
216-
handler, ok := tm.filterHandlers[topologyv1alpha2.TopologyManagerPolicy(policyName)]
217-
if !ok {
218-
klog.V(4).InfoS("Policy handler not found", "policy", policyName)
211+
handler := filterHandlerFromTopologyManagerConfig(topologyManagerConfigFromNodeResourceTopology(nodeTopology))
212+
if handler == nil {
219213
return nil
220214
}
221215
status := handler(pod, nodeTopology.Zones, nodeInfo)
@@ -266,3 +260,16 @@ func hasNonNativeResource(pod *v1.Pod) bool {
266260
}
267261
return false
268262
}
263+
264+
func filterHandlerFromTopologyManagerConfig(conf TopologyManagerConfig) filterFn {
265+
if conf.Policy != kubeletconfig.SingleNumaNodeTopologyManagerPolicy {
266+
return nil
267+
}
268+
if conf.Scope == kubeletconfig.PodTopologyManagerScope {
269+
return singleNUMAPodLevelHandler
270+
}
271+
if conf.Scope == kubeletconfig.ContainerTopologyManagerScope {
272+
return singleNUMAContainerLevelHandler
273+
}
274+
return nil // cannot happen
275+
}

‎pkg/noderesourcetopology/filter_test.go

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -670,8 +670,7 @@ func TestNodeResourceTopology(t *testing.T) {
670670
}
671671

672672
tm := TopologyMatch{
673-
filterHandlers: newFilterHandlers(),
674-
nrtCache: nrtcache.NewPassthrough(fakeInformer.Lister()),
673+
nrtCache: nrtcache.NewPassthrough(fakeInformer.Lister()),
675674
}
676675

677676
for _, tt := range tests {
@@ -886,8 +885,7 @@ func TestNodeResourceTopologyMultiContainerPodScope(t *testing.T) {
886885
}
887886

888887
tm := TopologyMatch{
889-
filterHandlers: newFilterHandlers(),
890-
nrtCache: nrtcache.NewPassthrough(fakeInformer.Lister()),
888+
nrtCache: nrtcache.NewPassthrough(fakeInformer.Lister()),
891889
}
892890

893891
nodeInfo := framework.NewNodeInfo()
@@ -1144,8 +1142,7 @@ func TestNodeResourceTopologyMultiContainerContainerScope(t *testing.T) {
11441142
}
11451143

11461144
tm := TopologyMatch{
1147-
filterHandlers: newFilterHandlers(),
1148-
nrtCache: nrtcache.NewPassthrough(fakeInformer.Lister()),
1145+
nrtCache: nrtcache.NewPassthrough(fakeInformer.Lister()),
11491146
}
11501147

11511148
nodeInfo := framework.NewNodeInfo()

‎pkg/noderesourcetopology/plugin.go

Lines changed: 12 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -80,26 +80,12 @@ func subtractFromNUMAs(resources v1.ResourceList, numaNodes NUMANodeList, nodes
8080
type filterFn func(pod *v1.Pod, zones topologyv1alpha2.ZoneList, nodeInfo *framework.NodeInfo) *framework.Status
8181
type scoringFn func(*v1.Pod, topologyv1alpha2.ZoneList) (int64, *framework.Status)
8282

83-
type filterHandlersMap map[topologyv1alpha2.TopologyManagerPolicy]filterFn
84-
type scoreHandlersMap map[topologyv1alpha2.TopologyManagerPolicy]scoringFn
85-
86-
func leastNUMAscoreHandlers() scoreHandlersMap {
87-
return scoreHandlersMap{
88-
topologyv1alpha2.SingleNUMANodePodLevel: leastNUMAPodScopeScore,
89-
topologyv1alpha2.SingleNUMANodeContainerLevel: leastNUMAContainerScopeScore,
90-
topologyv1alpha2.BestEffortPodLevel: leastNUMAPodScopeScore,
91-
topologyv1alpha2.BestEffortContainerLevel: leastNUMAContainerScopeScore,
92-
topologyv1alpha2.RestrictedPodLevel: leastNUMAPodScopeScore,
93-
topologyv1alpha2.RestrictedContainerLevel: leastNUMAContainerScopeScore,
94-
}
95-
}
96-
9783
// TopologyMatch plugin which run simplified version of TopologyManager's admit handler
9884
type TopologyMatch struct {
99-
filterHandlers filterHandlersMap
100-
scoringHandlers scoreHandlersMap
10185
resourceToWeightMap resourceToWeightMap
10286
nrtCache nrtcache.Interface
87+
scoreStrategyFunc scoreStrategyFn
88+
scoreStrategyType apiconfig.ScoringStrategyType
10389
}
10490

10591
var _ framework.FilterPlugin = &TopologyMatch{}
@@ -131,24 +117,21 @@ func New(args runtime.Object, handle framework.Handle) (framework.Plugin, error)
131117
resToWeightMap[v1.ResourceName(resource.Name)] = resource.Weight
132118
}
133119

134-
var scoringHandlers scoreHandlersMap
135-
136-
if tcfg.ScoringStrategy.Type == apiconfig.LeastNUMANodes {
137-
scoringHandlers = leastNUMAscoreHandlers()
138-
} else {
139-
strategy, err := getScoringStrategyFunction(tcfg.ScoringStrategy.Type)
140-
if err != nil {
141-
return nil, err
142-
}
143-
144-
scoringHandlers = newScoringHandlers(strategy, resToWeightMap)
120+
// This is not strictly needed, but we do it here and we carry `scoreStrategyFunc` around
121+
// to be able to do as much parameter validation as possible here in this function.
122+
// We perform only the NRT-object-specific validation in `Filter()` and `Score()`
123+
// because we can't help it, being the earliest point in time on which we have access
124+
// to NRT instances.
125+
strategy, err := getScoringStrategyFunction(tcfg.ScoringStrategy.Type)
126+
if err != nil {
127+
return nil, err
145128
}
146129

147130
topologyMatch := &TopologyMatch{
148-
filterHandlers: newFilterHandlers(),
149-
scoringHandlers: scoringHandlers,
150131
resourceToWeightMap: resToWeightMap,
151132
nrtCache: nrtCache,
133+
scoreStrategyFunc: strategy,
134+
scoreStrategyType: tcfg.ScoringStrategy.Type,
152135
}
153136

154137
return topologyMatch, nil

‎pkg/noderesourcetopology/pluginhelpers.go

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -109,21 +109,3 @@ func extractResources(zone topologyv1alpha2.Zone) corev1.ResourceList {
109109
}
110110
return res
111111
}
112-
113-
func newFilterHandlers() filterHandlersMap {
114-
return filterHandlersMap{
115-
topologyv1alpha2.SingleNUMANodePodLevel: singleNUMAPodLevelHandler,
116-
topologyv1alpha2.SingleNUMANodeContainerLevel: singleNUMAContainerLevelHandler,
117-
}
118-
}
119-
120-
func newScoringHandlers(strategy scoreStrategyFn, resourceToWeightMap resourceToWeightMap) scoreHandlersMap {
121-
return scoreHandlersMap{
122-
topologyv1alpha2.SingleNUMANodePodLevel: func(pod *corev1.Pod, zones topologyv1alpha2.ZoneList) (int64, *framework.Status) {
123-
return podScopeScore(pod, zones, strategy, resourceToWeightMap)
124-
},
125-
topologyv1alpha2.SingleNUMANodeContainerLevel: func(pod *corev1.Pod, zones topologyv1alpha2.ZoneList) (int64, *framework.Status) {
126-
return containerScopeScore(pod, zones, strategy, resourceToWeightMap)
127-
},
128-
}
129-
}

‎pkg/noderesourcetopology/score.go

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
v1 "k8s.io/api/core/v1"
2626
"k8s.io/klog/v2"
2727
v1qos "k8s.io/kubernetes/pkg/apis/core/v1/helper/qos"
28+
kubeletconfig "k8s.io/kubernetes/pkg/kubelet/apis/config"
2829
"k8s.io/kubernetes/pkg/scheduler/framework"
2930
apiconfig "sigs.k8s.io/scheduler-plugins/apis/config"
3031

@@ -74,18 +75,11 @@ func (tm *TopologyMatch) Score(ctx context.Context, state *framework.CycleState,
7475
}
7576

7677
logNRT("noderesourcetopology found", nodeTopology)
77-
if len(nodeTopology.TopologyPolicies) == 0 {
78-
klog.V(2).InfoS("Cannot determine policy", "node", nodeName)
79-
return 0, nil
80-
}
8178

82-
policyName := nodeTopology.TopologyPolicies[0]
83-
handler, ok := tm.scoringHandlers[topologyv1alpha2.TopologyManagerPolicy(policyName)]
84-
if !ok {
85-
klog.V(4).InfoS("policy handler not found", "policy", policyName)
79+
handler := tm.scoringHandlerFromTopologyManagerConfig(topologyManagerConfigFromNodeResourceTopology(nodeTopology))
80+
if handler == nil {
8681
return 0, nil
8782
}
88-
8983
return handler(pod, nodeTopology.Zones)
9084
}
9185

@@ -119,6 +113,9 @@ func getScoringStrategyFunction(strategy apiconfig.ScoringStrategyType) (scoreSt
119113
return leastAllocatedScoreStrategy, nil
120114
case apiconfig.BalancedAllocation:
121115
return balancedAllocationScoreStrategy, nil
116+
case apiconfig.LeastNUMANodes:
117+
// this is a special case handled down the flow. We just need to NOT error out.
118+
return nil, nil
122119
default:
123120
return nil, fmt.Errorf("illegal scoring strategy found")
124121
}
@@ -152,3 +149,29 @@ func containerScopeScore(pod *v1.Pod, zones topologyv1alpha2.ZoneList, scorerFn
152149
klog.V(5).InfoS("container scope scoring final node score", "finalScore", finalScore)
153150
return finalScore, nil
154151
}
152+
153+
func (tm *TopologyMatch) scoringHandlerFromTopologyManagerConfig(conf TopologyManagerConfig) scoringFn {
154+
if tm.scoreStrategyType == apiconfig.LeastNUMANodes {
155+
if conf.Scope == kubeletconfig.PodTopologyManagerScope {
156+
return leastNUMAPodScopeScore
157+
}
158+
if conf.Scope == kubeletconfig.ContainerTopologyManagerScope {
159+
return leastNUMAContainerScopeScore
160+
}
161+
return nil // cannot happen
162+
}
163+
if conf.Policy != kubeletconfig.SingleNumaNodeTopologyManagerPolicy {
164+
return nil
165+
}
166+
if conf.Scope == kubeletconfig.PodTopologyManagerScope {
167+
return func(pod *v1.Pod, zones topologyv1alpha2.ZoneList) (int64, *framework.Status) {
168+
return podScopeScore(pod, zones, tm.scoreStrategyFunc, tm.resourceToWeightMap)
169+
}
170+
}
171+
if conf.Scope == kubeletconfig.ContainerTopologyManagerScope {
172+
return func(pod *v1.Pod, zones topologyv1alpha2.ZoneList) (int64, *framework.Status) {
173+
return containerScopeScore(pod, zones, tm.scoreStrategyFunc, tm.resourceToWeightMap)
174+
}
175+
}
176+
return nil // cannot happen
177+
}

‎pkg/noderesourcetopology/score_test.go

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2828
"k8s.io/kubernetes/pkg/scheduler/framework"
2929

30+
apiconfig "sigs.k8s.io/scheduler-plugins/apis/config"
3031
nrtcache "sigs.k8s.io/scheduler-plugins/pkg/noderesourcetopology/cache"
3132

3233
topologyv1alpha2 "github.com/k8stopologyawareschedwg/noderesourcetopology-api/pkg/apis/topology/v1alpha2"
@@ -192,12 +193,9 @@ func TestNodeResourceScorePlugin(t *testing.T) {
192193
for _, test := range tests {
193194
nodesMap, lister := initTest(topologyv1alpha2.SingleNUMANodeContainerLevel)
194195
t.Run(test.name, func(t *testing.T) {
195-
scoringHandlers := newScoringHandlers(test.strategy, nil)
196-
197196
tm := &TopologyMatch{
198-
filterHandlers: newFilterHandlers(),
199-
scoringHandlers: scoringHandlers,
200-
nrtCache: nrtcache.NewPassthrough(lister),
197+
scoreStrategyFunc: test.strategy,
198+
nrtCache: nrtcache.NewPassthrough(lister),
201199
}
202200

203201
for _, req := range test.requests {
@@ -442,9 +440,8 @@ func TestNodeResourceScorePluginLeastNUMA(t *testing.T) {
442440
nodesMap, lister := initTest(tc.policy)
443441

444442
tm := &TopologyMatch{
445-
filterHandlers: newFilterHandlers(),
446-
scoringHandlers: leastNUMAscoreHandlers(),
447-
nrtCache: nrtcache.NewPassthrough(lister),
443+
scoreStrategyType: apiconfig.LeastNUMANodes,
444+
nrtCache: nrtcache.NewPassthrough(lister),
448445
}
449446
nodeToScore := make(nodeToScoreMap, len(nodesMap))
450447
pod := makePodByResourceLists(tc.podRequests...)

‎test/integration/noderesourcetopology_cache_test.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -531,10 +531,17 @@ func TestTopologyCachePluginWithUpdates(t *testing.T) {
531531

532532
// we want to run concurrently with the resync loop is running.
533533
go func() {
534+
pfpSign := mkPFP("fake-node-cache-1", tt.podDescs[0].pod)
534535
updatedNRTs := []*topologyv1alpha2.NodeResourceTopology{
535536
MakeNRT().Name("fake-node-cache-1").Policy(topologyv1alpha2.SingleNUMANodeContainerLevel).
536537
Annotations(map[string]string{
537-
podfingerprint.Annotation: mkPFP("fake-node-cache-1", tt.podDescs[0].pod),
538+
podfingerprint.Annotation: pfpSign,
539+
}).
540+
Attributes(topologyv1alpha2.AttributeList{
541+
{
542+
Name: podfingerprint.Attribute,
543+
Value: pfpSign,
544+
},
538545
}).
539546
Zone(
540547
topologyv1alpha2.ResourceInfoList{

0 commit comments

Comments
 (0)