Skip to content

Commit

Permalink
fix managed-by label being too long when the node name is long.
Browse files Browse the repository at this point in the history
Added a hashing function that only hashes the passed in base and
prefix if the combined size is > the maxLength passed into the function
In this case the maximum length of a label.

Signed-off-by: Alexander Wels <awels@redhat.com>
  • Loading branch information
awels committed Mar 30, 2022
1 parent daab12b commit d03a6ce
Show file tree
Hide file tree
Showing 3 changed files with 139 additions and 1 deletion.
3 changes: 2 additions & 1 deletion cmd/csi-provisioner/csi-provisioner.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime/schema"
validation "k8s.io/apimachinery/pkg/util/validation"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/client-go/informers"
"k8s.io/client-go/kubernetes"
Expand Down Expand Up @@ -458,7 +459,7 @@ func main() {

managedByID := "external-provisioner"
if *enableNodeDeployment {
managedByID += "-" + node
managedByID = getNameWithMaxLength(managedByID, node, validation.DNS1035LabelMaxLength)
}

// We only need objects from our own namespace. The normal factory would give
Expand Down
75 changes: 75 additions & 0 deletions cmd/csi-provisioner/util.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
/*
Copyright 2022 The Kubernetes Authors.
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 main

import (
"fmt"
"hash/fnv"
)

// getNameWithMaxLength returns a name given a base ("deployment-5") and a suffix ("deploy")
// It will first attempt to join them with a dash. If the resulting name is longer
// than maxLength: if the suffix is too long, it will truncate the base name and add
// an 8-character hash of the [base]-[suffix] string. If the suffix is not too long,
// it will truncate the base, add the hash of the base and return [base]-[hash]-[suffix]
// copied from https://github.com/openshift/library-go/blob/master/pkg/build/naming/namer.go
func getNameWithMaxLength(base, suffix string, maxLength int) string {
name := fmt.Sprintf("%s-%s", base, suffix)
if len(name) <= maxLength {
return name
}

baseLength := maxLength - 10 /*length of -hash-*/ - len(suffix)

// if the suffix is too long, ignore it
if baseLength < 0 {
prefix := base[0:min(len(base), max(0, maxLength-9))]
// Calculate hash on initial base-suffix string
shortName := fmt.Sprintf("%s-%s", prefix, hash(name))
return shortName[:min(maxLength, len(shortName))]
}

prefix := base[0:baseLength]
// Calculate hash on initial base-suffix string
return fmt.Sprintf("%s-%s-%s", prefix, hash(base), suffix)
}

// hash calculates the hexadecimal representation (8-chars)
// of the hash of the passed in string using the FNV-a algorithm
func hash(s string) string {
hash := fnv.New32a()
hash.Write([]byte(s))
intHash := hash.Sum32()
result := fmt.Sprintf("%08x", intHash)
return result
}

// max returns the greater of its 2 inputs
func max(a, b int) int {
if b > a {
return b
}
return a
}

// min returns the lesser of its 2 inputs
func min(a, b int) int {
if b < a {
return b
}
return a
}
62 changes: 62 additions & 0 deletions cmd/csi-provisioner/util_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
Copyright 2022 The Kubernetes Authors.
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 main

import (
"fmt"
"strings"
"testing"
)

const (
externalProvisioner = "external-provisioner"
)

func TestGetNameWithMaxLength(t *testing.T) {
testcases := map[string]struct {
expected string
nodeName string
}{
"unchanged": {
expected: fmt.Sprintf("%s-%s", externalProvisioner, "node01"),
nodeName: "node01",
},
"exactly63": {
// 20 (external-provisioner) + 1 (-) + 4 (node) + 38 = 63
expected: fmt.Sprintf("%s-%s", externalProvisioner, fmt.Sprintf("node%s", strings.Repeat("a", 38))),
nodeName: fmt.Sprintf("node%s", strings.Repeat("a", 38)),
},
"one over": {
expected: fmt.Sprintf("%s-%s-%s", "external-p", "53f40b57", fmt.Sprintf("node%s", strings.Repeat("a", 39))),
nodeName: fmt.Sprintf("node%s", strings.Repeat("a", 39)),
},
"very long, ignore suffix": {
expected: fmt.Sprintf("%s-%s", externalProvisioner, "df38e37f"),
nodeName: fmt.Sprintf("node%s", strings.Repeat("a", 63)),
},
}

for name, c := range testcases {
t.Run(name, func(t *testing.T) {
expected := c.expected
res := getNameWithMaxLength(externalProvisioner, c.nodeName, 63)
if expected != res {
t.Errorf("Expected: %s, does not match result: %s", expected, res)
}
})
}
}

0 comments on commit d03a6ce

Please sign in to comment.