Skip to content

Commit b54ba7a

Browse files
bart0shbart0sh
authored
Merge pull request #1285 from tkatila/label-concat-fix
GPU nfdhook label split fix
2 parents 947e865 + d90e35a commit b54ba7a

File tree

4 files changed

+253
-31
lines changed

4 files changed

+253
-31
lines changed

‎cmd/gpu_nfdhook/labeler.go‎

Lines changed: 6 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ const (
4646
controlDeviceRE = `^controlD[0-9]+$`
4747
vendorString = "0x8086"
4848
labelMaxLength = 63
49+
labelControlChar = "Z"
4950
)
5051

5152
type labelMap map[string]string
@@ -342,27 +343,6 @@ func (l *labeler) createPCIGroupLabel(gpuNumList []string) string {
342343
return labelValue
343344
}
344345

345-
// split returns the given string cut to chunks of size up to maxLength size.
346-
// maxLength refers to the max length of the strings in the returned slice.
347-
// If the whole input string fits under maxLength, it is not split.
348-
// split("foo_bar", 4) returns []string{"foo_", "bar"}.
349-
func split(str string, maxLength uint) []string {
350-
remainingString := str
351-
results := []string{}
352-
353-
for len(remainingString) >= 0 {
354-
if uint(len(remainingString)) <= maxLength {
355-
results = append(results, remainingString)
356-
return results
357-
}
358-
359-
results = append(results, remainingString[:maxLength])
360-
remainingString = remainingString[maxLength:]
361-
}
362-
363-
return results
364-
}
365-
366346
// createLabels is the main function of plugin labeler, it creates label-value pairs for the gpus.
367347
func (l *labeler) createLabels() error {
368348
gpuNameList, err := l.scan()
@@ -412,11 +392,12 @@ func (l *labeler) createLabels() error {
412392

413393
if gpuCount > 0 {
414394
// add gpu list label (example: "card0.card1.card2") - deprecated
415-
l.labels[labelNamespace+gpuListLabelName] = split(strings.Join(gpuNameList, "."), labelMaxLength)[0]
395+
l.labels[labelNamespace+gpuListLabelName] = pluginutils.SplitAtLastAlphaNum(
396+
strings.Join(gpuNameList, "."), labelMaxLength, labelControlChar)[0]
416397

417398
// add gpu num list label(s) (example: "0.1.2", which is short form of "card0.card1.card2")
418399
allGPUs := strings.Join(gpuNumList, ".")
419-
gpuNumLists := split(allGPUs, labelMaxLength)
400+
gpuNumLists := pluginutils.SplitAtLastAlphaNum(allGPUs, labelMaxLength, labelControlChar)
420401

421402
l.labels[labelNamespace+gpuNumListLabelName] = gpuNumLists[0]
422403
for i := 1; i < len(gpuNumLists); i++ {
@@ -427,7 +408,7 @@ func (l *labeler) createLabels() error {
427408
// add numa node mapping to labels: gpu.intel.com/numa-gpu-map="0-0.1.2.3_1-4.5.6.7"
428409
numaMappingLabel := createNumaNodeMappingLabel(numaMapping)
429410

430-
numaMappingLabelList := split(numaMappingLabel, labelMaxLength)
411+
numaMappingLabelList := pluginutils.SplitAtLastAlphaNum(numaMappingLabel, labelMaxLength, labelControlChar)
431412

432413
l.labels[labelNamespace+numaMappingName] = numaMappingLabelList[0]
433414
for i := 1; i < len(numaMappingLabelList); i++ {
@@ -441,7 +422,7 @@ func (l *labeler) createLabels() error {
441422
// aa pci-group label(s), (two group example: "1.2.3.4_5.6.7.8")
442423
allPCIGroups := l.createPCIGroupLabel(gpuNumList)
443424
if allPCIGroups != "" {
444-
pciGroups := split(allPCIGroups, labelMaxLength)
425+
pciGroups := pluginutils.SplitAtLastAlphaNum(allPCIGroups, labelMaxLength, labelControlChar)
445426

446427
l.labels[labelNamespace+pciGroupLabelName] = pciGroups[0]
447428
for i := 1; i < len(gpuNumLists); i++ {

‎cmd/gpu_nfdhook/labeler_test.go‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ import (
2020
"reflect"
2121
"strconv"
2222
"testing"
23+
24+
"github.com/intel/intel-device-plugins-for-kubernetes/cmd/internal/pluginutils"
2325
)
2426

2527
type testcase struct {
@@ -413,8 +415,8 @@ func getTestCases() []testcase {
413415
"gpu.intel.com/millicores": "27000",
414416
"gpu.intel.com/memory.max": "432000000000",
415417
"gpu.intel.com/cards": "card0.card1.card10.card11.card12.card13.card14.card15.card16.ca",
416-
"gpu.intel.com/gpu-numbers": "0.1.10.11.12.13.14.15.16.17.18.19.2.20.21.22.23.24.25.26.3.4.5.",
417-
"gpu.intel.com/gpu-numbers2": "6.7.8.9",
418+
"gpu.intel.com/gpu-numbers": "0.1.10.11.12.13.14.15.16.17.18.19.2.20.21.22.23.24.25.26.3.4.5",
419+
"gpu.intel.com/gpu-numbers2": "Z.6.7.8.9",
418420
"gpu.intel.com/tiles": "27",
419421
},
420422
},
@@ -665,12 +667,12 @@ func getTestCases() []testcase {
665667
expectedRetval: nil,
666668
expectedLabels: labelMap{
667669
"gpu.intel.com/cards": "card0.card1.card10.card11.card12.card13.card14.card15.card16.ca",
668-
"gpu.intel.com/gpu-numbers": "0.1.10.11.12.13.14.15.16.17.18.19.2.20.21.22.23.24.25.26.3.4.5.",
669-
"gpu.intel.com/gpu-numbers2": "6.7.8.9",
670+
"gpu.intel.com/gpu-numbers": "0.1.10.11.12.13.14.15.16.17.18.19.2.20.21.22.23.24.25.26.3.4.5",
671+
"gpu.intel.com/gpu-numbers2": "Z.6.7.8.9",
670672
"gpu.intel.com/memory.max": "432000000000",
671673
"gpu.intel.com/millicores": "27000",
672674
"gpu.intel.com/numa-gpu-map": "0-0.1.2.3.4.5.6.7.8_1-13.14.15.16.17.18.19.20.21_2-10.11.12.9_3",
673-
"gpu.intel.com/numa-gpu-map2": "-22.23.24.25.26",
675+
"gpu.intel.com/numa-gpu-map2": "Z-22.23.24.25.26",
674676
"gpu.intel.com/tiles": "27",
675677
},
676678
},
@@ -729,7 +731,7 @@ func TestSplit(t *testing.T) {
729731
}
730732

731733
for _, test := range tests {
732-
result := split(test.str, test.maxLength)
734+
result := pluginutils.Split(test.str, test.maxLength)
733735
if !reflect.DeepEqual(test.expectedResult, result) {
734736
t.Errorf("\n%q ended up with unexpected result %v vs expected %v", test.name, result, test.expectedResult)
735737
}

‎cmd/internal/pluginutils/labels.go‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
// Copyright 2022 Intel Corporation. All Rights Reserved.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package pluginutils
16+
17+
import (
18+
"strings"
19+
20+
"k8s.io/klog/v2"
21+
)
22+
23+
// Split returns the given string cut to chunks of size up to maxLength size.
24+
// maxLength refers to the max length of the strings in the returned slice.
25+
// If the whole input string fits under maxLength, it is not split.
26+
// Split("foo_bar", 4) returns []string{"foo_", "bar"}.
27+
func Split(str string, maxLength uint) []string {
28+
remainingString := str
29+
results := []string{}
30+
31+
for len(remainingString) >= 0 {
32+
if uint(len(remainingString)) <= maxLength {
33+
results = append(results, remainingString)
34+
break
35+
}
36+
37+
results = append(results, remainingString[:maxLength])
38+
remainingString = remainingString[maxLength:]
39+
}
40+
41+
return results
42+
}
43+
44+
// SplitAtLastAlphaNum returns the given string cut to chunks of size up to maxLength.
45+
// Difference to the Split above, this cuts the string at the last alpha numeric character
46+
// (a-z0-9A-Z) and adds concatChars at the beginning of the next string chunk.
47+
func SplitAtLastAlphaNum(str string, maxLength uint, concatChars string) []string {
48+
remainingString := str
49+
results := []string{}
50+
51+
if maxLength <= uint(len(concatChars)) {
52+
klog.Errorf("SplitAtLastAlphaNum: maxLength cannot be smaller than concatChars: %d vs %d", maxLength, uint(len(concatChars)))
53+
54+
results = []string{}
55+
56+
return results
57+
}
58+
59+
isAlphaNum := func(c byte) bool {
60+
return c >= 'a' && c <= 'z' ||
61+
c >= 'A' && c <= 'Z' ||
62+
c >= '0' && c <= '9'
63+
}
64+
65+
strPrefix := ""
66+
67+
for len(remainingString) >= 0 {
68+
if uint(len(remainingString)) <= maxLength {
69+
results = append(results, (strPrefix + remainingString))
70+
break
71+
}
72+
73+
alphaNumIndex := int(maxLength) - 1
74+
for alphaNumIndex >= 0 && !isAlphaNum(remainingString[alphaNumIndex]) {
75+
alphaNumIndex--
76+
}
77+
78+
if alphaNumIndex < 0 {
79+
klog.Errorf("SplitAtLastAlphaNum: chunk without any alpha numeric characters: %s", remainingString)
80+
81+
results = []string{}
82+
83+
return results
84+
}
85+
86+
// increase by one to get the actual cut index
87+
alphaNumIndex++
88+
89+
results = append(results, strPrefix+remainingString[:alphaNumIndex])
90+
remainingString = remainingString[alphaNumIndex:]
91+
92+
if strPrefix == "" {
93+
maxLength -= uint(len(concatChars))
94+
strPrefix = concatChars
95+
}
96+
}
97+
98+
return results
99+
}
100+
101+
func ConcatAlphaNumSplitChunks(chunks []string, concatChars string) string {
102+
if len(chunks) == 1 {
103+
return chunks[0]
104+
}
105+
106+
s := chunks[0]
107+
108+
for _, chunk := range chunks[1:] {
109+
if !strings.HasPrefix(chunk, concatChars) {
110+
klog.Warningf("Chunk has invalid prefix: %s (should have %s)", chunk[:len(concatChars)], concatChars)
111+
}
112+
113+
s += chunk[len(concatChars):]
114+
}
115+
116+
return s
117+
}

‎cmd/internal/pluginutils/labels_test.go‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
// Copyright 2020-2021 Intel Corporation. All Rights Reserved.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package pluginutils
16+
17+
import (
18+
"testing"
19+
)
20+
21+
func TestSplitAlphaNumeric(t *testing.T) {
22+
type testData struct {
23+
label string
24+
prefix string
25+
expStrings []string
26+
maxLength int
27+
}
28+
29+
tds := []testData{
30+
{
31+
"0.0-1.0_0.1-1.1_0.1-1.1_0.0-1.0_0.1-1.1_0.0-1.0_0.1-1.1_0.0-1.0_0.1-1.1_0.0-1.0_0.1-1.1_0.0-1.0_0.1-1.1_0.1-1.1_1.0-0.0_1.1-0.1_1.0-0.0_1.1-0.1",
32+
"Z",
33+
[]string{
34+
"0.0-1.0_0.1-1.1_0.1-1.1_0.0-1.0_0.1-1.1_0.0-1.0_0.1-1.1_0.0-1.0",
35+
"Z_0.1-1.1_0.0-1.0_0.1-1.1_0.0-1.0_0.1-1.1_0.1-1.1_1.0-0.0_1.1-0",
36+
"Z.1_1.0-0.0_1.1-0.1",
37+
},
38+
63,
39+
},
40+
{
41+
"0.0-1.0_0.1-1.1_0.1-1.1_0.0-1.0_0.1-1.1_0.0-1.0_0.1-1.1_0.0-1.0_0.1-1.1_0.0-1.0_0.1-1.1_0.0-1.0_0.1-1.1_0.1-1.1_1.0-0.0_1.1-0.1_1.0-0.0_1.1-0.1",
42+
"ZZZ",
43+
[]string{
44+
"0.0-1.0_0.1",
45+
"ZZZ-1.1_0.1",
46+
"ZZZ-1.1_0.0",
47+
"ZZZ-1.0_0.1",
48+
"ZZZ-1.1_0.0",
49+
"ZZZ-1.0_0.1",
50+
"ZZZ-1.1_0.0",
51+
"ZZZ-1.0_0.1",
52+
"ZZZ-1.1_0.0",
53+
"ZZZ-1.0_0.1",
54+
"ZZZ-1.1_0.0",
55+
"ZZZ-1.0_0.1",
56+
"ZZZ-1.1_0.1",
57+
"ZZZ-1.1_1.0",
58+
"ZZZ-0.0_1.1",
59+
"ZZZ-0.1_1.0",
60+
"ZZZ-0.0_1.1",
61+
"ZZZ-0.1",
62+
},
63+
12,
64+
},
65+
{
66+
"0.0-1.0_0.1-1.1_0.1-1.1_0.0-1.0_0.1-1.1_0.0-1.0_0.1-1.1_0.0-15.0_0.1",
67+
"X",
68+
[]string{
69+
"0.0-1.0_0.1-1.1_0.1-1.1_0.0-1.0_0.1-1.1_0.0-1.0_0.1-1.1_0.0-15",
70+
"X.0_0.1",
71+
},
72+
63,
73+
},
74+
{
75+
"0.0-1.0_0.1-1.1_0.1-1.1_0.0-1.0_0.1-1.1_0.0-1.0_0.1-1._-_._-_..0_0.1",
76+
"XYZ",
77+
[]string{
78+
"0.0-1.0_0.1-1.1_0.1-1.1_0.0-1.0_0.1-1.1_0.0-1.0_0.1-1",
79+
"XYZ._-_._-_..0_0.1",
80+
},
81+
63,
82+
},
83+
{
84+
"A___B____C",
85+
"Z",
86+
[]string{},
87+
4,
88+
},
89+
{
90+
"A___B____C",
91+
"ZYYYYYYZZZZZ",
92+
[]string{},
93+
4,
94+
},
95+
}
96+
97+
for _, td := range tds {
98+
res := SplitAtLastAlphaNum(td.label, uint(td.maxLength), td.prefix)
99+
100+
if len(res) != len(td.expStrings) {
101+
t.Errorf("Got invalid amount of string chunks: %d", len(res))
102+
}
103+
104+
for i, s := range td.expStrings {
105+
if res[i] != s {
106+
t.Errorf("Invalid chunk from split %s (vs. %s)", res[i], s)
107+
}
108+
109+
if len(res[i]) > td.maxLength {
110+
t.Errorf("Chunk is too long %d (vs. %d)", len(res[i]), td.maxLength)
111+
}
112+
}
113+
}
114+
115+
for _, td := range tds[:4] {
116+
res := ConcatAlphaNumSplitChunks(td.expStrings, td.prefix)
117+
118+
if res != td.label {
119+
t.Errorf("Invalid concatenated string: %s vs. %s", res, td.label)
120+
}
121+
}
122+
}

0 commit comments

Comments
 (0)