From 5fd1651fc1280e7a8feb74f3bf551dfba50a5c63 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Tue, 12 Jan 2021 13:21:15 -0500 Subject: [PATCH] Make podIPs order match node IP family preference --- pkg/kubelet/kubelet_pods.go | 35 ++++++-- pkg/kubelet/kubelet_pods_test.go | 134 +++++++++++++++++++++++++++++++ 2 files changed, 162 insertions(+), 7 deletions(-) diff --git a/pkg/kubelet/kubelet_pods.go b/pkg/kubelet/kubelet_pods.go index 292f8a71b6e..db2be71c777 100644 --- a/pkg/kubelet/kubelet_pods.go +++ b/pkg/kubelet/kubelet_pods.go @@ -66,6 +66,7 @@ import ( "k8s.io/kubernetes/pkg/volume/util/volumepathhandler" volumevalidation "k8s.io/kubernetes/pkg/volume/validation" "k8s.io/kubernetes/third_party/forked/golang/expansion" + utilnet "k8s.io/utils/net" ) const ( @@ -1576,16 +1577,36 @@ func (kl *Kubelet) generateAPIPodStatus(pod *v1.Pod, podStatus *kubecontainer.Po // alter the kubelet state at all. func (kl *Kubelet) convertStatusToAPIStatus(pod *v1.Pod, podStatus *kubecontainer.PodStatus) *v1.PodStatus { var apiPodStatus v1.PodStatus - apiPodStatus.PodIPs = make([]v1.PodIP, 0, len(podStatus.IPs)) + + // The runtime pod status may have an arbitrary number of IPs, in an arbitrary + // order. Pick out the first returned IP of the same IP family as the node IP + // first, followed by the first IP of the opposite IP family (if any). + podIPs := make([]v1.PodIP, 0, len(podStatus.IPs)) + var validPrimaryIP, validSecondaryIP func(ip string) bool + if len(kl.nodeIPs) == 0 || utilnet.IsIPv4(kl.nodeIPs[0]) { + validPrimaryIP = utilnet.IsIPv4String + validSecondaryIP = utilnet.IsIPv6String + } else { + validPrimaryIP = utilnet.IsIPv6String + validSecondaryIP = utilnet.IsIPv4String + } for _, ip := range podStatus.IPs { - apiPodStatus.PodIPs = append(apiPodStatus.PodIPs, v1.PodIP{ - IP: ip, - }) + if validPrimaryIP(ip) { + podIPs = append(podIPs, v1.PodIP{IP: ip}) + break + } + } + for _, ip := range podStatus.IPs { + if validSecondaryIP(ip) { + podIPs = append(podIPs, v1.PodIP{IP: ip}) + break + } + } + apiPodStatus.PodIPs = podIPs + if len(podIPs) > 0 { + apiPodStatus.PodIP = podIPs[0].IP } - if len(apiPodStatus.PodIPs) > 0 { - apiPodStatus.PodIP = apiPodStatus.PodIPs[0].IP - } // set status for Pods created on versions of kube older than 1.6 apiPodStatus.QOSClass = v1qos.GetPodQOS(pod) diff --git a/pkg/kubelet/kubelet_pods_test.go b/pkg/kubelet/kubelet_pods_test.go index d2ddbf7c2ee..337c94928f9 100644 --- a/pkg/kubelet/kubelet_pods_test.go +++ b/pkg/kubelet/kubelet_pods_test.go @@ -20,6 +20,7 @@ import ( "errors" "fmt" "io/ioutil" + "net" "os" "path/filepath" "reflect" @@ -2545,3 +2546,136 @@ func TestGenerateAPIPodStatusHostNetworkPodIPs(t *testing.T) { }) } } + +func TestGenerateAPIPodStatusPodIPs(t *testing.T) { + testcases := []struct { + name string + nodeIP string + criPodIPs []string + podIPs []v1.PodIP + }{ + { + name: "Simple", + nodeIP: "", + criPodIPs: []string{"10.0.0.1"}, + podIPs: []v1.PodIP{ + {IP: "10.0.0.1"}, + }, + }, + { + name: "Dual-stack", + nodeIP: "", + criPodIPs: []string{"10.0.0.1", "fd01::1234"}, + podIPs: []v1.PodIP{ + {IP: "10.0.0.1"}, + {IP: "fd01::1234"}, + }, + }, + { + name: "Dual-stack with explicit node IP", + nodeIP: "192.168.1.1", + criPodIPs: []string{"10.0.0.1", "fd01::1234"}, + podIPs: []v1.PodIP{ + {IP: "10.0.0.1"}, + {IP: "fd01::1234"}, + }, + }, + { + name: "Dual-stack with CRI returning wrong family first", + nodeIP: "", + criPodIPs: []string{"fd01::1234", "10.0.0.1"}, + podIPs: []v1.PodIP{ + {IP: "10.0.0.1"}, + {IP: "fd01::1234"}, + }, + }, + { + name: "Dual-stack with explicit node IP with CRI returning wrong family first", + nodeIP: "192.168.1.1", + criPodIPs: []string{"fd01::1234", "10.0.0.1"}, + podIPs: []v1.PodIP{ + {IP: "10.0.0.1"}, + {IP: "fd01::1234"}, + }, + }, + { + name: "Dual-stack with IPv6 node IP", + nodeIP: "fd00::5678", + criPodIPs: []string{"10.0.0.1", "fd01::1234"}, + podIPs: []v1.PodIP{ + {IP: "fd01::1234"}, + {IP: "10.0.0.1"}, + }, + }, + { + name: "Dual-stack with IPv6 node IP, other CRI order", + nodeIP: "fd00::5678", + criPodIPs: []string{"fd01::1234", "10.0.0.1"}, + podIPs: []v1.PodIP{ + {IP: "fd01::1234"}, + {IP: "10.0.0.1"}, + }, + }, + { + name: "No Pod IP matching Node IP", + nodeIP: "fd00::5678", + criPodIPs: []string{"10.0.0.1"}, + podIPs: []v1.PodIP{ + {IP: "10.0.0.1"}, + }, + }, + { + name: "No Pod IP matching (unspecified) Node IP", + nodeIP: "", + criPodIPs: []string{"fd01::1234"}, + podIPs: []v1.PodIP{ + {IP: "fd01::1234"}, + }, + }, + { + name: "Multiple IPv4 IPs", + nodeIP: "", + criPodIPs: []string{"10.0.0.1", "10.0.0.2", "10.0.0.3"}, + podIPs: []v1.PodIP{ + {IP: "10.0.0.1"}, + }, + }, + { + name: "Multiple Dual-Stack IPs", + nodeIP: "", + criPodIPs: []string{"10.0.0.1", "10.0.0.2", "fd01::1234", "10.0.0.3", "fd01::5678"}, + podIPs: []v1.PodIP{ + {IP: "10.0.0.1"}, + {IP: "fd01::1234"}, + }, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + testKubelet := newTestKubelet(t, false /* controllerAttachDetachEnabled */) + defer testKubelet.Cleanup() + kl := testKubelet.kubelet + if tc.nodeIP != "" { + kl.nodeIPs = []net.IP{net.ParseIP(tc.nodeIP)} + } + + pod := podWithUIDNameNs("12345", "test-pod", "test-namespace") + + criStatus := &kubecontainer.PodStatus{ + ID: pod.UID, + Name: pod.Name, + Namespace: pod.Namespace, + IPs: tc.criPodIPs, + } + + status := kl.generateAPIPodStatus(pod, criStatus) + if !reflect.DeepEqual(status.PodIPs, tc.podIPs) { + t.Fatalf("Expected PodIPs %#v, got %#v", tc.podIPs, status.PodIPs) + } + if status.PodIP != status.PodIPs[0].IP { + t.Fatalf("Expected PodIP %q to equal PodIPs[0].IP %q", status.PodIP, status.PodIPs[0].IP) + } + }) + } +}