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

@@ -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)
+}

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)
 			}

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