From 7e0b9bfa66b7aeebfc8a18feb0d82dffe96a85c0 Mon Sep 17 00:00:00 2001 From: Euan Kemp Date: Thu, 26 May 2016 19:07:20 -0700 Subject: [PATCH] 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)