Add a custom split method to overcome issues with k8s labels

If k8s label starts or ends with a non alphanumeric char, it is
ignored. This new split method cuts labels from the last alphanum
characters and adds a control character to the beginning of the next
chunk. The entity that uses these labels needs to then remove the
control character before concatenating the chunks.

Co-authored-by: Ukri Niemimuukko <ukri.niemimuukko@intel.com>
Signed-off-by: Tuomas Katila <tuomas.katila@intel.com>

cmd/gpu_nfdhook/labeler.go

@@ -46,6 +46,7 @@ const (
 	controlDeviceRE     = `^controlD[0-9]+$`
 	vendorString        = "0x8086"
 	labelMaxLength      = 63
+	labelControlChar    = "Z"
 )
 
 type labelMap map[string]string
@@ -391,11 +392,12 @@ func (l *labeler) createLabels() error {
 
 	if gpuCount > 0 {
 		// add gpu list label (example: "card0.card1.card2") - deprecated
-		l.labels[labelNamespace+gpuListLabelName] = pluginutils.Split(strings.Join(gpuNameList, "."), labelMaxLength)[0]
+		l.labels[labelNamespace+gpuListLabelName] = pluginutils.SplitAtLastAlphaNum(
+			strings.Join(gpuNameList, "."), labelMaxLength, labelControlChar)[0]
 
 		// add gpu num list label(s) (example: "0.1.2", which is short form of "card0.card1.card2")
 		allGPUs := strings.Join(gpuNumList, ".")
-		gpuNumLists := pluginutils.Split(allGPUs, labelMaxLength)
+		gpuNumLists := pluginutils.SplitAtLastAlphaNum(allGPUs, labelMaxLength, labelControlChar)
 
 		l.labels[labelNamespace+gpuNumListLabelName] = gpuNumLists[0]
 		for i := 1; i < len(gpuNumLists); i++ {
@@ -406,7 +408,7 @@ func (l *labeler) createLabels() error {
 			// add numa node mapping to labels: gpu.intel.com/numa-gpu-map="0-0.1.2.3_1-4.5.6.7"
 			numaMappingLabel := createNumaNodeMappingLabel(numaMapping)
 
-			numaMappingLabelList := pluginutils.Split(numaMappingLabel, labelMaxLength)
+			numaMappingLabelList := pluginutils.SplitAtLastAlphaNum(numaMappingLabel, labelMaxLength, labelControlChar)
 
 			l.labels[labelNamespace+numaMappingName] = numaMappingLabelList[0]
 			for i := 1; i < len(numaMappingLabelList); i++ {
@@ -420,7 +422,7 @@ func (l *labeler) createLabels() error {
 		// aa pci-group label(s), (two group example: "1.2.3.4_5.6.7.8")
 		allPCIGroups := l.createPCIGroupLabel(gpuNumList)
 		if allPCIGroups != "" {
-			pciGroups := pluginutils.Split(allPCIGroups, labelMaxLength)
+			pciGroups := pluginutils.SplitAtLastAlphaNum(allPCIGroups, labelMaxLength, labelControlChar)
 
 			l.labels[labelNamespace+pciGroupLabelName] = pciGroups[0]
 			for i := 1; i < len(gpuNumLists); i++ {

cmd/gpu_nfdhook/labeler_test.go

@@ -415,8 +415,8 @@ func getTestCases() []testcase {
 				"gpu.intel.com/millicores":   "27000",
 				"gpu.intel.com/memory.max":   "432000000000",
 				"gpu.intel.com/cards":        "card0.card1.card10.card11.card12.card13.card14.card15.card16.ca",
-				"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.",
+				"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",
-				"gpu.intel.com/gpu-numbers2": "6.7.8.9",
+				"gpu.intel.com/gpu-numbers2": "Z.6.7.8.9",
 				"gpu.intel.com/tiles":        "27",
 			},
 		},
@@ -667,12 +667,12 @@ func getTestCases() []testcase {
 			expectedRetval: nil,
 			expectedLabels: labelMap{
 				"gpu.intel.com/cards":         "card0.card1.card10.card11.card12.card13.card14.card15.card16.ca",
-				"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.",
+				"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",
-				"gpu.intel.com/gpu-numbers2":  "6.7.8.9",
+				"gpu.intel.com/gpu-numbers2":  "Z.6.7.8.9",
 				"gpu.intel.com/memory.max":    "432000000000",
 				"gpu.intel.com/millicores":    "27000",
 				"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",
-				"gpu.intel.com/numa-gpu-map2": "-22.23.24.25.26",
+				"gpu.intel.com/numa-gpu-map2": "Z-22.23.24.25.26",
 				"gpu.intel.com/tiles":         "27",
 			},
 		},

cmd/internal/pluginutils/labels.go

@@ -14,6 +14,12 @@
 
 package pluginutils
 
+import (
+	"strings"
+
+	"k8s.io/klog/v2"
+)
+
 // Split returns the given string cut to chunks of size up to maxLength size.
 // maxLength refers to the max length of the strings in the returned slice.
 // If the whole input string fits under maxLength, it is not split.
@@ -25,7 +31,7 @@ func Split(str string, maxLength uint) []string {
 	for len(remainingString) >= 0 {
 		if uint(len(remainingString)) <= maxLength {
 			results = append(results, remainingString)
-			return results
+			break
 		}
 
 		results = append(results, remainingString[:maxLength])
@@ -34,3 +40,78 @@ func Split(str string, maxLength uint) []string {
 
 	return results
 }
+
+// SplitAtLastAlphaNum returns the given string cut to chunks of size up to maxLength.
+// Difference to the Split above, this cuts the string at the last alpha numeric character
+// (a-z0-9A-Z) and adds concatChars at the beginning of the next string chunk.
+func SplitAtLastAlphaNum(str string, maxLength uint, concatChars string) []string {
+	remainingString := str
+	results := []string{}
+
+	if maxLength <= uint(len(concatChars)) {
+		klog.Errorf("SplitAtLastAlphaNum: maxLength cannot be smaller than concatChars: %d vs %d", maxLength, uint(len(concatChars)))
+
+		results = []string{}
+
+		return results
+	}
+
+	isAlphaNum := func(c byte) bool {
+		return c >= 'a' && c <= 'z' ||
+			c >= 'A' && c <= 'Z' ||
+			c >= '0' && c <= '9'
+	}
+
+	strPrefix := ""
+
+	for len(remainingString) >= 0 {
+		if uint(len(remainingString)) <= maxLength {
+			results = append(results, (strPrefix + remainingString))
+			break
+		}
+
+		alphaNumIndex := int(maxLength) - 1
+		for alphaNumIndex >= 0 && !isAlphaNum(remainingString[alphaNumIndex]) {
+			alphaNumIndex--
+		}
+
+		if alphaNumIndex < 0 {
+			klog.Errorf("SplitAtLastAlphaNum: chunk without any alpha numeric characters: %s", remainingString)
+
+			results = []string{}
+
+			return results
+		}
+
+		// increase by one to get the actual cut index
+		alphaNumIndex++
+
+		results = append(results, strPrefix+remainingString[:alphaNumIndex])
+		remainingString = remainingString[alphaNumIndex:]
+
+		if strPrefix == "" {
+			maxLength -= uint(len(concatChars))
+			strPrefix = concatChars
+		}
+	}
+
+	return results
+}
+
+func ConcatAlphaNumSplitChunks(chunks []string, concatChars string) string {
+	if len(chunks) == 1 {
+		return chunks[0]
+	}
+
+	s := chunks[0]
+
+	for _, chunk := range chunks[1:] {
+		if !strings.HasPrefix(chunk, concatChars) {
+			klog.Warningf("Chunk has invalid prefix: %s (should have %s)", chunk[:len(concatChars)], concatChars)
+		}
+
+		s += chunk[len(concatChars):]
+	}
+
+	return s
+}

cmd/internal/pluginutils/labels_test.go

@@ -0,0 +1,122 @@
+// Copyright 2020-2021 Intel Corporation. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package pluginutils
+
+import (
+	"testing"
+)
+
+func TestSplitAlphaNumeric(t *testing.T) {
+	type testData struct {
+		label      string
+		prefix     string
+		expStrings []string
+		maxLength  int
+	}
+
+	tds := []testData{
+		{
+			"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",
+			"Z",
+			[]string{
+				"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",
+				"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",
+				"Z.1_1.0-0.0_1.1-0.1",
+			},
+			63,
+		},
+		{
+			"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",
+			"ZZZ",
+			[]string{
+				"0.0-1.0_0.1",
+				"ZZZ-1.1_0.1",
+				"ZZZ-1.1_0.0",
+				"ZZZ-1.0_0.1",
+				"ZZZ-1.1_0.0",
+				"ZZZ-1.0_0.1",
+				"ZZZ-1.1_0.0",
+				"ZZZ-1.0_0.1",
+				"ZZZ-1.1_0.0",
+				"ZZZ-1.0_0.1",
+				"ZZZ-1.1_0.0",
+				"ZZZ-1.0_0.1",
+				"ZZZ-1.1_0.1",
+				"ZZZ-1.1_1.0",
+				"ZZZ-0.0_1.1",
+				"ZZZ-0.1_1.0",
+				"ZZZ-0.0_1.1",
+				"ZZZ-0.1",
+			},
+			12,
+		},
+		{
+			"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",
+			"X",
+			[]string{
+				"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",
+				"X.0_0.1",
+			},
+			63,
+		},
+		{
+			"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",
+			"XYZ",
+			[]string{
+				"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",
+				"XYZ._-_._-_..0_0.1",
+			},
+			63,
+		},
+		{
+			"A___B____C",
+			"Z",
+			[]string{},
+			4,
+		},
+		{
+			"A___B____C",
+			"ZYYYYYYZZZZZ",
+			[]string{},
+			4,
+		},
+	}
+
+	for _, td := range tds {
+		res := SplitAtLastAlphaNum(td.label, uint(td.maxLength), td.prefix)
+
+		if len(res) != len(td.expStrings) {
+			t.Errorf("Got invalid amount of string chunks: %d", len(res))
+		}
+
+		for i, s := range td.expStrings {
+			if res[i] != s {
+				t.Errorf("Invalid chunk from split %s (vs. %s)", res[i], s)
+			}
+
+			if len(res[i]) > td.maxLength {
+				t.Errorf("Chunk is too long %d (vs. %d)", len(res[i]), td.maxLength)
+			}
+		}
+	}
+
+	for _, td := range tds[:4] {
+		res := ConcatAlphaNumSplitChunks(td.expStrings, td.prefix)
+
+		if res != td.label {
+			t.Errorf("Invalid concatenated string: %s vs. %s", res, td.label)
+		}
+	}
+}