Merge pull request #59849 from yue9944882/forcibly-lower-staticpod-name

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. lowercase node name in generated static pod name **What this PR does / why we need it**: Cast appended node name to lowercase when generating static pod name on kubelet starting. **Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*: Fixes #59801 **Special notes for your reviewer**: Not sure about how to deal with other illegal node names e.g. containing invalid no-alphabetic characters. Maybe just let it fail-hard is not a bad idea. But considering that containing uppercase letter in the hostname is somehow a usual case even in the production environment of some companies, tolerating uppercase and cast it implicitly should be good. **Release note**: ```release-note force node name lowercase on static pod name generating ```

Merge pull request #59849 from yue9944882/forcibly-lower-staticpod-name
52b7aab0 · Kubernetes Submit Queue · GitHub · 62c5f21d · fdd36017 · 52b7aab0
Unverified Commit 52b7aab0 authored Feb 25, 2018 by Kubernetes Submit Queue Committed by GitHub Feb 25, 2018
Hide whitespace changes
Inline Side-by-side

Showing with 64 additions and 1 deletion

BUILD pkg/kubelet/config/BUILD +1 -0

common.go pkg/kubelet/config/common.go +2 -1

common_test.go pkg/kubelet/config/common_test.go +61 -0

No files found.
--- a/pkg/kubelet/config/BUILD
+++ b/pkg/kubelet/config/BUILD
@@ -107,6 +107,7 @@ go_test(
        "//pkg/apis/core/validation:go_default_library",
        "//pkg/kubelet/types:go_default_library",
        "//pkg/securitycontext:go_default_library",
+        "//vendor/github.com/stretchr/testify/assert:go_default_library",
        "//vendor/k8s.io/api/core/v1:go_default_library",
        "//vendor/k8s.io/apimachinery/pkg/api/equality:go_default_library",
        "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",

--- a/pkg/kubelet/config/common.go
+++ b/pkg/kubelet/config/common.go
@@ -21,6 +21,7 @@ import (
 	"crypto/md5"
 	"encoding/hex"
 	"fmt"
+	"strings"

 	"k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -44,7 +45,7 @@ import (

 // Generate a pod name that is unique among nodes by appending the nodeName.
 func generatePodName(name string, nodeName types.NodeName) string {
-	return fmt.Sprintf("%s-%s", name, nodeName)
+	return fmt.Sprintf("%s-%s", name, strings.ToLower(string(nodeName)))
 }

 func applyDefaults(pod *api.Pod, source string, isFile bool, nodeName types.NodeName) error {

--- a/pkg/kubelet/config/common_test.go
+++ b/pkg/kubelet/config/common_test.go
@@ -20,12 +20,17 @@ import (
 	"reflect"
 	"testing"

+	"github.com/stretchr/testify/assert"
+
 	"k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/kubernetes/pkg/api/legacyscheme"
 	"k8s.io/kubernetes/pkg/api/testapi"
+	"k8s.io/kubernetes/pkg/apis/core"
 	api "k8s.io/kubernetes/pkg/apis/core"
+	"k8s.io/kubernetes/pkg/apis/core/validation"
 	"k8s.io/kubernetes/pkg/securitycontext"
 )

@@ -189,3 +194,59 @@ func TestGetSelfLink(t *testing.T) {
 		}
 	}
 }
+
+func TestStaticPodNameGenerate(t *testing.T) {
+	testCases := []struct {
+		nodeName  types.NodeName
+		podName   string
+		expected  string
+		overwrite string
+		shouldErr bool
+	}{
+		{
+			"node1",
+			"static-pod1",
+			"static-pod1-node1",
+			"",
+			false,
+		},
+		{
+			"Node1",
+			"static-pod1",
+			"static-pod1-node1",
+			"",
+			false,
+		},
+		{
+			"NODE1",
+			"static-pod1",
+			"static-pod1-node1",
+			"static-pod1-NODE1",
+			true,
+		},
+	}
+	for _, c := range testCases {
+		assert.Equal(t, c.expected, generatePodName(c.podName, c.nodeName), "wrong pod name generated")
+		pod := &core.Pod{}
+		pod.Name = c.podName
+		if c.overwrite != "" {
+			pod.Name = c.overwrite
+		}
+		errs := validation.ValidatePod(pod)
+		if c.shouldErr {
+			specNameErrored := false
+			for _, err := range errs {
+				if err.Field == "metadata.name" {
+					specNameErrored = true
+				}
+			}
+			assert.NotEmpty(t, specNameErrored, "expecting error")
+		} else {
+			for _, err := range errs {
+				if err.Field == "metadata.name" {
+					t.Fail()
+				}
+			}
+		}
+	}
+}