Adding Kubelet changes to enable SetHostnameAsFQDN feature

These changes allow to set FQDN as hostname of pods for pods
that set the new PodSpec field setHostnameAsFQDN to true. The PodSpec
new field was added in related PR.

This is PART2 (last) of the changes to enable KEP #1797 and addresses #91036

pkg/kubelet/kubelet_pods.go

@@ -58,6 +58,7 @@ import (
 	"k8s.io/kubernetes/pkg/kubelet/images"
 	"k8s.io/kubernetes/pkg/kubelet/status"
 	kubetypes "k8s.io/kubernetes/pkg/kubelet/types"
+	"k8s.io/kubernetes/pkg/kubelet/util"
 	"k8s.io/kubernetes/pkg/kubelet/util/format"
 	volumeutil "k8s.io/kubernetes/pkg/volume/util"
 	"k8s.io/kubernetes/pkg/volume/util/hostutil"
@@ -442,12 +443,18 @@ func (kl *Kubelet) GenerateRunContainerOptions(pod *v1.Pod, container *v1.Contai
 	if err != nil {
 		return nil, nil, err
 	}
-
+	// The value of hostname is the short host name and it is sent to makeMounts to create /etc/hosts file.
 	hostname, hostDomainName, err := kl.GeneratePodHostNameAndDomain(pod)
 	if err != nil {
 		return nil, nil, err
 	}
-	opts.Hostname = hostname
+	// nodename will be equals to hostname if SetHostnameAsFQDN is nil or false. If SetHostnameFQDN
+	// is true and hostDomainName is defined, nodename will be the FQDN (hostname.hostDomainName)
+	nodename, err := util.GetNodenameForKernel(hostname, hostDomainName, pod.Spec.SetHostnameAsFQDN)
+	if err != nil {
+		return nil, nil, err
+	}
+	opts.Hostname = nodename
 	podName := volumeutil.GetUniquePodName(pod)
 	volumes := kl.volumeManager.GetMountedVolumesForPod(podName)
 

pkg/kubelet/kuberuntime/kuberuntime_sandbox.go

@@ -30,6 +30,7 @@ import (
 	"k8s.io/kubernetes/pkg/features"
 	kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
 	"k8s.io/kubernetes/pkg/kubelet/types"
+	"k8s.io/kubernetes/pkg/kubelet/util"
 	"k8s.io/kubernetes/pkg/kubelet/util/format"
 )
 
@@ -96,11 +97,15 @@ func (m *kubeGenericRuntimeManager) generatePodSandboxConfig(pod *v1.Pod, attemp
 
 	if !kubecontainer.IsHostNetworkPod(pod) {
 		// TODO: Add domain support in new runtime interface
-		hostname, _, err := m.runtimeHelper.GeneratePodHostNameAndDomain(pod)
+		podHostname, podDomain, err := m.runtimeHelper.GeneratePodHostNameAndDomain(pod)
 		if err != nil {
 			return nil, err
 		}
-		podSandboxConfig.Hostname = hostname
+		podHostname, err = util.GetNodenameForKernel(podHostname, podDomain, pod.Spec.SetHostnameAsFQDN)
+		if err != nil {
+			return nil, err
+		}
+		podSandboxConfig.Hostname = podHostname
 	}
 
 	logDir := BuildPodLogsDirectory(pod.Namespace, pod.Name, pod.UID)

pkg/kubelet/util/util.go

@@ -17,6 +17,8 @@ limitations under the License.
 package util
 
 import (
+	"fmt"
+
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -25,3 +27,19 @@ import (
 func FromApiserverCache(opts *metav1.GetOptions) {
 	opts.ResourceVersion = "0"
 }
+
+// GetNodenameForKernel gets hostname value to set in the hostname field (the nodename field of struct utsname) of the pod.
+func GetNodenameForKernel(hostname string, hostDomainName string, setHostnameAsFQDN *bool) (string, error) {
+	kernelHostname := hostname
+	// FQDN has to be 64 chars to fit in the Linux nodename kernel field (specification 64 chars and the null terminating char).
+	const fqdnMaxLen = 64
+	if len(hostDomainName) > 0 && setHostnameAsFQDN != nil && *setHostnameAsFQDN == true {
+		fqdn := fmt.Sprintf("%s.%s", hostname, hostDomainName)
+		// FQDN has to be shorter than hostnameMaxLen characters.
+		if len(fqdn) > fqdnMaxLen {
+			return "", fmt.Errorf("Failed to construct FQDN from pod hostname and cluster domain, FQDN %s is too long (%d characters is the max, %d characters requested)", fqdn, fqdnMaxLen, len(fqdn))
+		}
+		kernelHostname = fqdn
+	}
+	return kernelHostname, nil
+}

pkg/kubelet/util/util_test.go

@@ -0,0 +1,88 @@
+/*
+Copyright 2020 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 util
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGetNodenameForKernel(t *testing.T) {
+	testcases := []struct {
+		description       string
+		hostname          string
+		hostDomain        string
+		setHostnameAsFQDN bool
+		expectedHostname  string
+		expectError       bool
+	}{{
+		description:       "no hostDomain, setHostnameAsFQDN false",
+		hostname:          "test.pod.hostname",
+		hostDomain:        "",
+		setHostnameAsFQDN: false,
+		expectedHostname:  "test.pod.hostname",
+		expectError:       false,
+	}, {
+		description:       "no hostDomain, setHostnameAsFQDN true",
+		hostname:          "test.pod.hostname",
+		hostDomain:        "",
+		setHostnameAsFQDN: true,
+		expectedHostname:  "test.pod.hostname",
+		expectError:       false,
+	}, {
+		description:       "valid hostDomain, setHostnameAsFQDN false",
+		hostname:          "test.pod.hostname",
+		hostDomain:        "svc.subdomain.local",
+		setHostnameAsFQDN: false,
+		expectedHostname:  "test.pod.hostname",
+		expectError:       false,
+	}, {
+		description:       "valid hostDomain, setHostnameAsFQDN true",
+		hostname:          "test.pod.hostname",
+		hostDomain:        "svc.subdomain.local",
+		setHostnameAsFQDN: true,
+		expectedHostname:  "test.pod.hostname.svc.subdomain.local",
+		expectError:       false,
+	}, {
+		description:       "FQDN is too long, setHostnameAsFQDN false",
+		hostname:          "1234567.1234567",                                         //8*2-1=15 chars
+		hostDomain:        "1234567.1234567.1234567.1234567.1234567.1234567.1234567", //8*7-1=55 chars
+		setHostnameAsFQDN: false,                                                     //FQDN=15 + 1(dot) + 55 = 71 chars
+		expectedHostname:  "1234567.1234567",
+		expectError:       false,
+	}, {
+		description:       "FQDN is too long, setHostnameAsFQDN true",
+		hostname:          "1234567.1234567",                                         //8*2-1=15 chars
+		hostDomain:        "1234567.1234567.1234567.1234567.1234567.1234567.1234567", //8*7-1=55 chars
+		setHostnameAsFQDN: true,                                                      //FQDN=15 + 1(dot) + 55 = 71 chars
+		expectedHostname:  "",
+		expectError:       true,
+	}}
+
+	for _, tc := range testcases {
+		t.Logf("TestCase: %q", tc.description)
+		outputHostname, err := GetNodenameForKernel(tc.hostname, tc.hostDomain, &tc.setHostnameAsFQDN)
+		if tc.expectError {
+			assert.Error(t, err)
+		} else {
+			assert.NoError(t, err)
+		}
+		assert.Equal(t, tc.expectedHostname, outputHostname)
+	}
+
+}

test/e2e/node/BUILD

@@ -12,6 +12,7 @@ go_library(
         "mount_propagation.go",
         "node_problem_detector.go",
         "pod_gc.go",
+        "pod_hostnamefqdn.go",
         "pods.go",
         "pre_stop.go",
         "runtimeclass.go",

test/e2e/node/pod_hostnamefqdn.go

@@ -0,0 +1,143 @@
+/*
+Copyright 2020 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.
+*/
+
+/* This test check that setHostnameAsFQDN PodSpec field works as
+ * expected.
+ */
+
+package node
+
+import (
+	"crypto/rand"
+	"fmt"
+	"math/big"
+
+	v1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/kubernetes/test/e2e/framework"
+	imageutils "k8s.io/kubernetes/test/utils/image"
+
+	"github.com/onsi/ginkgo"
+)
+
+func generatePodName(base string) string {
+	id, err := rand.Int(rand.Reader, big.NewInt(214748))
+	if err != nil {
+		return base
+	}
+	return fmt.Sprintf("%s-%d", base, id)
+}
+
+func testPod() *v1.Pod {
+	podName := generatePodName("hostfqdn")
+	pod := &v1.Pod{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:        podName,
+			Labels:      map[string]string{"name": podName},
+			Annotations: map[string]string{},
+		},
+		Spec: v1.PodSpec{
+			Containers: []v1.Container{
+				{
+					Name:  "test-container",
+					Image: imageutils.GetE2EImage(imageutils.BusyBox),
+				},
+			},
+			RestartPolicy: v1.RestartPolicyNever,
+		},
+	}
+
+	return pod
+}
+
+var _ = SIGDescribe("Hostname of Pod [Feature:SetHostnameAsFQDN]", func() {
+	f := framework.NewDefaultFramework("hostfqdn")
+
+	/*
+	   Release : v1.19
+	   Testname: Create Pod without fully qualified domain name (FQDN)
+	   Description: A Pod that does not define the subdomain field in it spec, does not have FQDN.
+	*/
+	ginkgo.It("a pod without subdomain field does not have FQDN [Feature:SetHostnameAsFQDN]", func() {
+		pod := testPod()
+		pod.Spec.Containers[0].Command = []string{"sh", "-c", "echo $(hostname)';'$(hostname -f)';'"}
+		output := []string{fmt.Sprintf("%s;%s;", pod.ObjectMeta.Name, pod.ObjectMeta.Name)}
+		// Create Pod
+		f.TestContainerOutput("shotname only", pod, 0, output)
+	})
+
+	/*
+	   Release : v1.19
+	   Testname: Create Pod without FQDN, setHostnameAsFQDN field set to true
+	   Description: A Pod that does not define the subdomain field in it spec, does not have FQDN.
+	                Hence, SetHostnameAsFQDN feature has no effect.
+	*/
+	ginkgo.It("a pod without FQDN is not affected by SetHostnameAsFQDN field [Feature:SetHostnameAsFQDN]", func() {
+		pod := testPod()
+		// Setting setHostnameAsFQDN field to true should have no effect.
+		setHostnameAsFQDN := true
+		pod.Spec.SetHostnameAsFQDN = &setHostnameAsFQDN
+		pod.Spec.Containers[0].Command = []string{"sh", "-c", "echo $(hostname)';'$(hostname -f)';'"}
+		output := []string{fmt.Sprintf("%s;%s;", pod.ObjectMeta.Name, pod.ObjectMeta.Name)}
+		// Create Pod
+		f.TestContainerOutput("shotname only", pod, 0, output)
+	})
+
+	/*
+	   Release : v1.19
+	   Testname: Create Pod with FQDN, setHostnameAsFQDN field not defined.
+	   Description: A Pod that defines the subdomain field in it spec has FQDN.
+	                hostname command returns shortname (pod name in this case), and hostname -f returns FQDN.
+	*/
+	ginkgo.It("a pod with subdomain field has FQDN, hostname is shortname [Feature:SetHostnameAsFQDN]", func() {
+		pod := testPod()
+		pod.Spec.Containers[0].Command = []string{"sh", "-c", "echo $(hostname)';'$(hostname -f)';'"}
+		subdomain := "t"
+		// Set PodSpec subdomain field to generate FQDN for pod
+		pod.Spec.Subdomain = subdomain
+		// Expected Pod FQDN
+		hostFQDN := fmt.Sprintf("%s.%s.%s.svc.%s", pod.ObjectMeta.Name, subdomain, f.Namespace.Name, framework.TestContext.ClusterDNSDomain)
+		output := []string{fmt.Sprintf("%s;%s;", pod.ObjectMeta.Name, hostFQDN)}
+		// Create Pod
+		f.TestContainerOutput("shotname and fqdn", pod, 0, output)
+	})
+
+	/*
+	   Release : v1.19
+	   Testname: Create Pod with FQDN, setHostnameAsFQDN field set to true.
+	   Description: A Pod that defines the subdomain field in it spec has FQDN. When setHostnameAsFQDN: true, the
+	                hostname is set to be the FQDN. In this case, both commands hostname and hostname -f return the FQDN of the Pod.
+	*/
+	ginkgo.It("a pod with subdomain field has FQDN, when setHostnameAsFQDN is set to true, the FQDN is set as hostname [Feature:SetHostnameAsFQDN]", func() {
+		pod := testPod()
+		pod.Spec.Containers[0].Command = []string{"sh", "-c", "echo $(hostname)';'$(hostname -f)';'"}
+		subdomain := "t"
+		// Set PodSpec subdomain field to generate FQDN for pod
+		pod.Spec.Subdomain = subdomain
+		// Set PodSpec setHostnameAsFQDN to set FQDN as hostname
+		setHostnameAsFQDN := true
+		pod.Spec.SetHostnameAsFQDN = &setHostnameAsFQDN
+		// Expected Pod FQDN
+		hostFQDN := fmt.Sprintf("%s.%s.%s.svc.%s", pod.ObjectMeta.Name, subdomain, f.Namespace.Name, framework.TestContext.ClusterDNSDomain)
+		// Fail if FQDN is longer than 64 characters, otherwise the Pod will remain pending until test timeout.
+		// In Linux, 64 characters is the limit of the hostname kernel field, which this test sets to the pod FQDN.
+		framework.ExpectEqual(len(hostFQDN) < 65, true, fmt.Sprintf("The FQDN of the Pod cannot be longer than 64 characters, requested %s which is %d characters long.", hostFQDN, len(hostFQDN)))
+		output := []string{fmt.Sprintf("%s;%s;", hostFQDN, hostFQDN)}
+		// Create Pod
+		f.TestContainerOutput("fqdn and fqdn", pod, 0, output)
+	})
+
+})