From 766eb6f0f730937efe4a26f6b58faaf012f09be8 Mon Sep 17 00:00:00 2001 From: Euan Kemp Date: Thu, 26 May 2016 17:52:16 -0700 Subject: [PATCH 1/6] kubenet: Fix bug where shaper.Reset wasn't called The error check was inverse what it should have been, causing shaper.Reset to only get called with invalid cidrs. --- pkg/kubelet/network/cni/testing/mock_cni.go | 39 +++++++++++++++ pkg/kubelet/network/kubenet/kubenet_linux.go | 4 +- .../network/kubenet/kubenet_linux_test.go | 48 ++++++++++++++++++- 3 files changed, 87 insertions(+), 4 deletions(-) create mode 100644 pkg/kubelet/network/cni/testing/mock_cni.go diff --git a/pkg/kubelet/network/cni/testing/mock_cni.go b/pkg/kubelet/network/cni/testing/mock_cni.go new file mode 100644 index 00000000000..622edefd577 --- /dev/null +++ b/pkg/kubelet/network/cni/testing/mock_cni.go @@ -0,0 +1,39 @@ +/* +Copyright 2014 The Kubernetes Authors 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. +*/ + +// mock_cni is a mock of the `libcni.CNI` interface. It's a handwritten mock +// because there are only two functions to deal with. +package mock_cni + +import ( + "github.com/appc/cni/libcni" + "github.com/appc/cni/pkg/types" + "github.com/stretchr/testify/mock" +) + +type MockCNI struct { + mock.Mock +} + +func (m *MockCNI) AddNetwork(net *libcni.NetworkConfig, rt *libcni.RuntimeConf) (*types.Result, error) { + args := m.Called(net, rt) + return args.Get(0).(*types.Result), args.Error(1) +} + +func (m *MockCNI) DelNetwork(net *libcni.NetworkConfig, rt *libcni.RuntimeConf) error { + args := m.Called(net, rt) + return args.Error(0) +} diff --git a/pkg/kubelet/network/kubenet/kubenet_linux.go b/pkg/kubelet/network/kubenet/kubenet_linux.go index cdc83c54cca..497f770dc58 100644 --- a/pkg/kubelet/network/kubenet/kubenet_linux.go +++ b/pkg/kubelet/network/kubenet/kubenet_linux.go @@ -67,7 +67,7 @@ type kubenetNetworkPlugin struct { host network.Host netConfig *libcni.NetworkConfig loConfig *libcni.NetworkConfig - cniConfig *libcni.CNIConfig + cniConfig libcni.CNI shaper bandwidth.BandwidthShaper podCIDRs map[kubecontainer.ContainerID]string MTU int @@ -374,7 +374,7 @@ func (plugin *kubenetNetworkPlugin) TearDownPod(namespace string, name string, i if hasCIDR { glog.V(5).Infof("Removing pod CIDR %s from shaper", cidr) // shaper wants /32 - if addr, _, err := net.ParseCIDR(cidr); err != nil { + if addr, _, err := net.ParseCIDR(cidr); err == nil { if err = plugin.shaper.Reset(fmt.Sprintf("%s/32", addr.String())); err != nil { glog.Warningf("Failed to remove pod CIDR %s from shaper: %v", cidr, err) } diff --git a/pkg/kubelet/network/kubenet/kubenet_linux_test.go b/pkg/kubelet/network/kubenet/kubenet_linux_test.go index a16e11e7fa7..bd83ad06218 100644 --- a/pkg/kubelet/network/kubenet/kubenet_linux_test.go +++ b/pkg/kubelet/network/kubenet/kubenet_linux_test.go @@ -19,14 +19,24 @@ package kubenet import ( "fmt" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + + "testing" + kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" "k8s.io/kubernetes/pkg/kubelet/network" + "k8s.io/kubernetes/pkg/kubelet/network/cni/testing" nettest "k8s.io/kubernetes/pkg/kubelet/network/testing" + "k8s.io/kubernetes/pkg/util/bandwidth" "k8s.io/kubernetes/pkg/util/exec" - "testing" + ipttest "k8s.io/kubernetes/pkg/util/iptables/testing" ) -func newFakeKubenetPlugin(initMap map[kubecontainer.ContainerID]string, execer exec.Interface, host network.Host) network.NetworkPlugin { +// test it fulfills the NetworkPlugin interface +var _ network.NetworkPlugin = &kubenetNetworkPlugin{} + +func newFakeKubenetPlugin(initMap map[kubecontainer.ContainerID]string, execer exec.Interface, host network.Host) *kubenetNetworkPlugin { return &kubenetNetworkPlugin{ podCIDRs: initMap, execer: execer, @@ -111,4 +121,38 @@ func TestGetPodNetworkStatus(t *testing.T) { } } +// TestTeardownBeforeSetUp tests that a `TearDown` call does call +// `shaper.Reset` +func TestTeardownCallsShaper(t *testing.T) { + fexec := &exec.FakeExec{ + CommandScript: []exec.FakeCommandAction{}, + LookPathFunc: func(file string) (string, error) { + return fmt.Sprintf("/fake-bin/%s", file), nil + }, + } + fhost := nettest.NewFakeHost(nil) + fshaper := &bandwidth.FakeShaper{} + mockcni := &mock_cni.MockCNI{} + kubenet := newFakeKubenetPlugin(map[kubecontainer.ContainerID]string{}, fexec, fhost) + kubenet.cniConfig = mockcni + kubenet.shaper = fshaper + kubenet.iptables = ipttest.NewFake() + + mockcni.On("DelNetwork", mock.AnythingOfType("*libcni.NetworkConfig"), mock.AnythingOfType("*libcni.RuntimeConf")).Return(nil) + + details := make(map[string]interface{}) + details[network.NET_PLUGIN_EVENT_POD_CIDR_CHANGE_DETAIL_CIDR] = "10.0.0.1/24" + kubenet.Event(network.NET_PLUGIN_EVENT_POD_CIDR_CHANGE, details) + + existingContainerID := kubecontainer.BuildContainerID("docker", "123") + kubenet.podCIDRs[existingContainerID] = "10.0.0.1/24" + + if err := kubenet.TearDownPod("namespace", "name", existingContainerID); err != nil { + t.Fatalf("Unexpected error in TearDownPod: %v", err) + } + assert.Equal(t, []string{"10.0.0.1/32"}, fshaper.ResetCIDRs, "shaper.Reset should have been called") + + mockcni.AssertExpectations(t) +} + //TODO: add unit test for each implementation of network plugin interface From 2f5e738dc18c9c603c2974dc4032aa1344df4917 Mon Sep 17 00:00:00 2001 From: Euan Kemp Date: Thu, 26 May 2016 18:47:22 -0700 Subject: [PATCH 2/6] kubenet: Fix inconsistent cidr usage/parsing Before this change, the podCIDRs map contained both cidrs and ips depending on which code path entered a container into it. Specifically, SetUpPod would enter a CIDR while GetPodNetworkStatus would enter an IP. This normalizes both of them to always enter just IP addresses. This also removes the now-redundant cidr parsing that was used to get the ip before --- pkg/kubelet/network/kubenet/kubenet_linux.go | 52 ++++++++----------- .../network/kubenet/kubenet_linux_test.go | 14 ++--- 2 files changed, 28 insertions(+), 38 deletions(-) diff --git a/pkg/kubelet/network/kubenet/kubenet_linux.go b/pkg/kubelet/network/kubenet/kubenet_linux.go index 497f770dc58..e1c707f7659 100644 --- a/pkg/kubelet/network/kubenet/kubenet_linux.go +++ b/pkg/kubelet/network/kubenet/kubenet_linux.go @@ -69,9 +69,9 @@ type kubenetNetworkPlugin struct { loConfig *libcni.NetworkConfig cniConfig libcni.CNI shaper bandwidth.BandwidthShaper - podCIDRs map[kubecontainer.ContainerID]string + mu sync.Mutex //Mutex for protecting podIPs map and netConfig + podIPs map[kubecontainer.ContainerID]string MTU int - mu sync.Mutex //Mutex for protecting podCIDRs map and netConfig execer utilexec.Interface nsenterPath string hairpinMode componentconfig.HairpinMode @@ -86,7 +86,7 @@ func NewPlugin() network.NetworkPlugin { iptInterface := utiliptables.New(execer, dbus, protocol) return &kubenetNetworkPlugin{ - podCIDRs: make(map[kubecontainer.ContainerID]string), + podIPs: make(map[kubecontainer.ContainerID]string), hostPortMap: make(map[hostport]closeable), MTU: 1460, //TODO: don't hardcode this execer: utilexec.New(), @@ -317,10 +317,10 @@ func (plugin *kubenetNetworkPlugin) SetUpPod(namespace string, name string, id k if err != nil { return err } - if res.IP4 == nil || res.IP4.IP.String() == "" { + if res.IP4 == nil || res.IP4.IP.IP.String() == "" { return fmt.Errorf("CNI plugin reported no IPv4 address for container %v.", id) } - plugin.podCIDRs[id] = res.IP4.IP.String() + plugin.podIPs[id] = res.IP4.IP.IP.String() // Put the container bridge into promiscuous mode to force it to accept hairpin packets. // TODO: Remove this once the kernel bug (#20096) is fixed. @@ -346,8 +346,8 @@ func (plugin *kubenetNetworkPlugin) SetUpPod(namespace string, name string, id k } if egress != nil || ingress != nil { - ipAddr, _, _ := net.ParseCIDR(plugin.podCIDRs[id]) - if err = plugin.shaper.ReconcileCIDR(fmt.Sprintf("%s/32", ipAddr.String()), egress, ingress); err != nil { + ipAddr := plugin.podIPs[id] + if err := plugin.shaper.ReconcileCIDR(fmt.Sprintf("%s/32", ipAddr), egress, ingress); err != nil { return fmt.Errorf("Failed to add pod to shaper: %v", err) } } @@ -369,26 +369,24 @@ func (plugin *kubenetNetworkPlugin) TearDownPod(namespace string, name string, i return fmt.Errorf("Kubenet needs a PodCIDR to tear down pods") } - // no cached CIDR is Ok during teardown - cidr, hasCIDR := plugin.podCIDRs[id] - if hasCIDR { - glog.V(5).Infof("Removing pod CIDR %s from shaper", cidr) + // no cached IP is Ok during teardown + podIP, hasIP := plugin.podIPs[id] + if hasIP { + glog.V(5).Infof("Removing pod IP %s from shaper", podIP) // shaper wants /32 - if addr, _, err := net.ParseCIDR(cidr); err == nil { - if err = plugin.shaper.Reset(fmt.Sprintf("%s/32", addr.String())); err != nil { - glog.Warningf("Failed to remove pod CIDR %s from shaper: %v", cidr, err) - } + if err := plugin.shaper.Reset(fmt.Sprintf("%s/32", podIP)); err != nil { + glog.Warningf("Failed to remove pod IP %s from shaper: %v", podIP, err) } } if err := plugin.delContainerFromNetwork(plugin.netConfig, network.DefaultInterfaceName, namespace, name, id); err != nil { // This is to prevent returning error when TearDownPod is called twice on the same pod. This helps to reduce event pollution. - if !hasCIDR { + if !hasIP { glog.Warningf("Failed to delete container from kubenet: %v", err) return nil } return err } - delete(plugin.podCIDRs, id) + delete(plugin.podIPs, id) plugin.syncHostportsRules() return nil @@ -400,11 +398,8 @@ func (plugin *kubenetNetworkPlugin) GetPodNetworkStatus(namespace string, name s plugin.mu.Lock() defer plugin.mu.Unlock() // Assuming the ip of pod does not change. Try to retrieve ip from kubenet map first. - if cidr, ok := plugin.podCIDRs[id]; ok { - ip, _, err := net.ParseCIDR(strings.Trim(cidr, "\n")) - if err == nil { - return &network.PodNetworkStatus{IP: ip}, nil - } + if podIP, ok := plugin.podIPs[id]; ok { + return &network.PodNetworkStatus{IP: net.ParseIP(podIP)}, nil } netnsPath, err := plugin.host.GetRuntime().GetNetNS(id) @@ -429,7 +424,7 @@ func (plugin *kubenetNetworkPlugin) GetPodNetworkStatus(namespace string, name s if err != nil { return nil, fmt.Errorf("Kubenet failed to parse ip from output %s due to %v", output, err) } - plugin.podCIDRs[id] = ip.String() + plugin.podIPs[id] = ip.String() return &network.PodNetworkStatus{IP: ip}, nil } @@ -549,7 +544,7 @@ func (plugin *kubenetNetworkPlugin) openPodHostports(pod *api.Pod) (map[hostport //syncHostportMap syncs newly opened hostports to kubenet on successful pod setup. If pod setup failed, then clean up. func (plugin *kubenetNetworkPlugin) syncHostportMap(id kubecontainer.ContainerID, hostportMap map[hostport]closeable) { // if pod ip cannot be retrieved from podCIDR, then assume pod setup failed. - if _, ok := plugin.podCIDRs[id]; !ok { + if _, ok := plugin.podIPs[id]; !ok { for hp, socket := range hostportMap { err := socket.Close() if err != nil { @@ -580,23 +575,18 @@ func (plugin *kubenetNetworkPlugin) gatherAllHostports() (map[api.ContainerPort] } } // Assuming if kubenet has the pod's ip, the pod is alive and its host port should be presented. - cidr, ok := plugin.podCIDRs[podInfraContainerId] + podIP, ok := plugin.podIPs[podInfraContainerId] if !ok { // The POD has been delete. Ignore continue } - podIP, _, err := net.ParseCIDR(strings.Trim(cidr, "\n")) - if err != nil { - glog.V(3).Info("Failed to retrieve pod ip for %s-%s: %v", p.Namespace, p.Name, err) - continue - } // Need the complete api.Pod object pod, ok := plugin.host.GetPodByName(p.Namespace, p.Name) if ok { for _, container := range pod.Spec.Containers { for _, port := range container.Ports { if port.HostPort != 0 { - podHostportMap[port] = targetPod{podFullName: kubecontainer.GetPodFullName(pod), podIP: podIP.String()} + podHostportMap[port] = targetPod{podFullName: kubecontainer.GetPodFullName(pod), podIP: podIP} } } } diff --git a/pkg/kubelet/network/kubenet/kubenet_linux_test.go b/pkg/kubelet/network/kubenet/kubenet_linux_test.go index bd83ad06218..c55a9e576ec 100644 --- a/pkg/kubelet/network/kubenet/kubenet_linux_test.go +++ b/pkg/kubelet/network/kubenet/kubenet_linux_test.go @@ -38,17 +38,17 @@ var _ network.NetworkPlugin = &kubenetNetworkPlugin{} func newFakeKubenetPlugin(initMap map[kubecontainer.ContainerID]string, execer exec.Interface, host network.Host) *kubenetNetworkPlugin { return &kubenetNetworkPlugin{ - podCIDRs: initMap, - execer: execer, - MTU: 1460, - host: host, + podIPs: initMap, + execer: execer, + MTU: 1460, + host: host, } } func TestGetPodNetworkStatus(t *testing.T) { podIPMap := make(map[kubecontainer.ContainerID]string) - podIPMap[kubecontainer.ContainerID{ID: "1"}] = "10.245.0.2/32" - podIPMap[kubecontainer.ContainerID{ID: "2"}] = "10.245.0.3/32" + podIPMap[kubecontainer.ContainerID{ID: "1"}] = "10.245.0.2" + podIPMap[kubecontainer.ContainerID{ID: "2"}] = "10.245.0.3" testCases := []struct { id string @@ -145,7 +145,7 @@ func TestTeardownCallsShaper(t *testing.T) { kubenet.Event(network.NET_PLUGIN_EVENT_POD_CIDR_CHANGE, details) existingContainerID := kubecontainer.BuildContainerID("docker", "123") - kubenet.podCIDRs[existingContainerID] = "10.0.0.1/24" + kubenet.podIPs[existingContainerID] = "10.0.0.1" if err := kubenet.TearDownPod("namespace", "name", existingContainerID); err != nil { t.Fatalf("Unexpected error in TearDownPod: %v", err) From 7e0b9bfa66b7aeebfc8a18feb0d82dffe96a85c0 Mon Sep 17 00:00:00 2001 From: Euan Kemp Date: Thu, 26 May 2016 19:07:20 -0700 Subject: [PATCH 3/6] kubenet: Fix panic when teardown run before setup Teardown can run before Setup when the kubelet is restarted... in that case, the shaper was nil and thus calling the shaper resulted in a panic This fixes that by ensuring the shaper is always set... +1 level of indirection and all that. --- pkg/kubelet/network/kubenet/kubenet_linux.go | 54 ++++++++++--------- .../network/kubenet/kubenet_linux_test.go | 2 +- 2 files changed, 31 insertions(+), 25 deletions(-) diff --git a/pkg/kubelet/network/kubenet/kubenet_linux.go b/pkg/kubelet/network/kubenet/kubenet_linux.go index e1c707f7659..a82b879093c 100644 --- a/pkg/kubelet/network/kubenet/kubenet_linux.go +++ b/pkg/kubelet/network/kubenet/kubenet_linux.go @@ -64,19 +64,19 @@ const ( type kubenetNetworkPlugin struct { network.NoopNetworkPlugin - host network.Host - netConfig *libcni.NetworkConfig - loConfig *libcni.NetworkConfig - cniConfig libcni.CNI - shaper bandwidth.BandwidthShaper - mu sync.Mutex //Mutex for protecting podIPs map and netConfig - podIPs map[kubecontainer.ContainerID]string - MTU int - execer utilexec.Interface - nsenterPath string - hairpinMode componentconfig.HairpinMode - hostPortMap map[hostport]closeable - iptables utiliptables.Interface + host network.Host + netConfig *libcni.NetworkConfig + loConfig *libcni.NetworkConfig + cniConfig libcni.CNI + bandwidthShaper bandwidth.BandwidthShaper + mu sync.Mutex //Mutex for protecting podIPs map, netConfig, and shaper initialization + podIPs map[kubecontainer.ContainerID]string + MTU int + execer utilexec.Interface + nsenterPath string + hairpinMode componentconfig.HairpinMode + hostPortMap map[hostport]closeable + iptables utiliptables.Interface } func NewPlugin() network.NetworkPlugin { @@ -335,19 +335,13 @@ func (plugin *kubenetNetworkPlugin) SetUpPod(namespace string, name string, id k } } - // The first SetUpPod call creates the bridge; ensure shaping is enabled - if plugin.shaper == nil { - plugin.shaper = bandwidth.NewTCShaper(BridgeName) - if plugin.shaper == nil { - return fmt.Errorf("Failed to create bandwidth shaper!") - } - plugin.ensureBridgeTxQueueLen() - plugin.shaper.ReconcileInterface() - } + // The first SetUpPod call creates the bridge; get a shaper for the sake of + // initialization + shaper := plugin.shaper() if egress != nil || ingress != nil { ipAddr := plugin.podIPs[id] - if err := plugin.shaper.ReconcileCIDR(fmt.Sprintf("%s/32", ipAddr), egress, ingress); err != nil { + if err := shaper.ReconcileCIDR(fmt.Sprintf("%s/32", ipAddr), egress, ingress); err != nil { return fmt.Errorf("Failed to add pod to shaper: %v", err) } } @@ -374,7 +368,7 @@ func (plugin *kubenetNetworkPlugin) TearDownPod(namespace string, name string, i if hasIP { glog.V(5).Infof("Removing pod IP %s from shaper", podIP) // shaper wants /32 - if err := plugin.shaper.Reset(fmt.Sprintf("%s/32", podIP)); err != nil { + if err := plugin.shaper().Reset(fmt.Sprintf("%s/32", podIP)); err != nil { glog.Warningf("Failed to remove pod IP %s from shaper: %v", podIP, err) } } @@ -811,3 +805,15 @@ func (plugin *kubenetNetworkPlugin) cleanupHostportMap(containerPortMap map[api. } } } + +// shaper retrieves the bandwidth shaper and, if it hasn't been fetched before, +// initializes it and ensures the bridge is appropriately configured +// This function should only be called while holding the `plugin.mu` lock +func (plugin *kubenetNetworkPlugin) shaper() bandwidth.BandwidthShaper { + if plugin.bandwidthShaper == nil { + plugin.bandwidthShaper = bandwidth.NewTCShaper(BridgeName) + plugin.ensureBridgeTxQueueLen() + plugin.bandwidthShaper.ReconcileInterface() + } + return plugin.bandwidthShaper +} diff --git a/pkg/kubelet/network/kubenet/kubenet_linux_test.go b/pkg/kubelet/network/kubenet/kubenet_linux_test.go index c55a9e576ec..051a0f2494f 100644 --- a/pkg/kubelet/network/kubenet/kubenet_linux_test.go +++ b/pkg/kubelet/network/kubenet/kubenet_linux_test.go @@ -135,8 +135,8 @@ func TestTeardownCallsShaper(t *testing.T) { mockcni := &mock_cni.MockCNI{} kubenet := newFakeKubenetPlugin(map[kubecontainer.ContainerID]string{}, fexec, fhost) kubenet.cniConfig = mockcni - kubenet.shaper = fshaper kubenet.iptables = ipttest.NewFake() + kubenet.bandwidthShaper = fshaper mockcni.On("DelNetwork", mock.AnythingOfType("*libcni.NetworkConfig"), mock.AnythingOfType("*libcni.RuntimeConf")).Return(nil) From c4b8959a752ded10fbc8b0bcf4c5f7c43d93cdb1 Mon Sep 17 00:00:00 2001 From: Euan Kemp Date: Thu, 26 May 2016 19:42:56 -0700 Subject: [PATCH 4/6] kubenet: Reduce loglevel of spammy message When no shaping is enabled, that warning would always be printed. --- pkg/kubelet/network/kubenet/kubenet_linux.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/kubelet/network/kubenet/kubenet_linux.go b/pkg/kubelet/network/kubenet/kubenet_linux.go index a82b879093c..c4fa04dad19 100644 --- a/pkg/kubelet/network/kubenet/kubenet_linux.go +++ b/pkg/kubelet/network/kubenet/kubenet_linux.go @@ -369,7 +369,8 @@ func (plugin *kubenetNetworkPlugin) TearDownPod(namespace string, name string, i glog.V(5).Infof("Removing pod IP %s from shaper", podIP) // shaper wants /32 if err := plugin.shaper().Reset(fmt.Sprintf("%s/32", podIP)); err != nil { - glog.Warningf("Failed to remove pod IP %s from shaper: %v", podIP, err) + // Possible bandwidth shaping wasn't enabled for this pod anyways + glog.V(4).Infof("Failed to remove pod IP %s from shaper: %v", podIP, err) } } if err := plugin.delContainerFromNetwork(plugin.netConfig, network.DefaultInterfaceName, namespace, name, id); err != nil { From 93487867ac50f69f1cd6a71a420b86c012d72a47 Mon Sep 17 00:00:00 2001 From: Euan Kemp Date: Fri, 27 May 2016 10:15:16 -0700 Subject: [PATCH 5/6] kubenet: Update empty ip check The previous check was incorrect because the `IP.String` method returns `` and other non-empty-strings on error conditions. --- pkg/kubelet/network/kubenet/kubenet_linux.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/kubelet/network/kubenet/kubenet_linux.go b/pkg/kubelet/network/kubenet/kubenet_linux.go index c4fa04dad19..9ff155b2df8 100644 --- a/pkg/kubelet/network/kubenet/kubenet_linux.go +++ b/pkg/kubelet/network/kubenet/kubenet_linux.go @@ -317,7 +317,7 @@ func (plugin *kubenetNetworkPlugin) SetUpPod(namespace string, name string, id k if err != nil { return err } - if res.IP4 == nil || res.IP4.IP.IP.String() == "" { + if res.IP4 == nil || len(res.IP4.IP.IP) != net.IPv4len { return fmt.Errorf("CNI plugin reported no IPv4 address for container %v.", id) } plugin.podIPs[id] = res.IP4.IP.IP.String() From c83ad19ae920dc6b3a97e26c4b99fb3b0c19e87e Mon Sep 17 00:00:00 2001 From: Euan Kemp Date: Fri, 27 May 2016 16:25:14 -0700 Subject: [PATCH 6/6] kubenet: Fix ipv4 validity check The length of an IP can be 4 or 16, and even if 16 it can be a valid ipv4 address. This check is the more-correct way to handle this, and it also provides more granular error messages. --- pkg/kubelet/network/kubenet/kubenet_linux.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/pkg/kubelet/network/kubenet/kubenet_linux.go b/pkg/kubelet/network/kubenet/kubenet_linux.go index 9ff155b2df8..db22b3afd7b 100644 --- a/pkg/kubelet/network/kubenet/kubenet_linux.go +++ b/pkg/kubelet/network/kubenet/kubenet_linux.go @@ -317,10 +317,14 @@ func (plugin *kubenetNetworkPlugin) SetUpPod(namespace string, name string, id k if err != nil { return err } - if res.IP4 == nil || len(res.IP4.IP.IP) != net.IPv4len { + if res.IP4 == nil { return fmt.Errorf("CNI plugin reported no IPv4 address for container %v.", id) } - plugin.podIPs[id] = res.IP4.IP.IP.String() + ip4 := res.IP4.IP.IP.To4() + if ip4 == nil { + return fmt.Errorf("CNI plugin reported an invalid IPv4 address for container %v: %+v.", id, res.IP4) + } + plugin.podIPs[id] = ip4.String() // Put the container bridge into promiscuous mode to force it to accept hairpin packets. // TODO: Remove this once the kernel bug (#20096) is fixed.