add tests for existing route case

.gitignore

@@ -13,3 +13,4 @@ tmp-cni
 k8s-install/scripts/tmp/
 install_cni.test
 *.swp
+*.coverprofile

Makefile

@@ -140,7 +140,7 @@ test-containerized: run-etcd run-k8s-apiserver build-containerized dist/host-loc
 	-v $(CURDIR):/go/src/github.com/projectcalico/cni-plugin:rw \
 	$(CALICO_BUILD) sh -c '\
 			cd  /go/src/github.com/projectcalico/cni-plugin && \
-			ginkgo'
+			make ut'
 	make stop-etcd
 
 # This does not currently work, kubernetes needs additional configuration
@@ -179,11 +179,11 @@ run-test-containerized-without-building: run-etcd run-k8s-apiserver
 	-v $(CURDIR):/go/src/github.com/projectcalico/cni-plugin:rw \
 	$(CALICO_BUILD) sh -c '\
 			cd  /go/src/github.com/projectcalico/cni-plugin && \
-			ginkgo'
+			make ut'
 	make stop-etcd
 
 ## Run the tests in a container (as root) for different CNI spec versions
-## to make sure we don't break backwards compatiblity.
+## to make sure we don't break backwards compatibility.
 .PHONY: test-containerized-cni-versions
 test-containerized-cni-versions: build-containerized dist/host-local;
 	for cniversion in "0.2.0" "0.3.1" ; do \
@@ -204,6 +204,26 @@ build-containerized: vendor
 			cd /go/src/github.com/projectcalico/cni-plugin && \
 			make binary'
 
+.PHONY: ut
+## Run the tests locally, must have local etcd running
+ut:
+	# Run tests recursively (-r).
+	ginkgo -cover -r -skipPackage vendor -skipPackage k8s-install
+
+	@echo
+	@echo '+==============+'
+	@echo '| All coverage |'
+	@echo '+==============+'
+	@echo
+	@find . -iname '*.coverprofile' | xargs -I _ go tool cover -func=_
+
+	@echo
+	@echo '+==================+'
+	@echo '| Missing coverage |'
+	@echo '+==================+'
+	@echo
+	@find . -iname '*.coverprofile' | xargs -I _ go tool cover -func=_ | grep -v '100.0%'
+
 ## Etcd is used by the tests
 run-etcd: stop-etcd
 	docker run --detach \

cni_plugin_suite_test.go

@@ -0,0 +1,13 @@
+package main_test
+
+import (
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+
+	"testing"
+)
+
+func TestCniPlugin(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "CniPlugin Suite")
+}

glide.yaml

@@ -18,7 +18,7 @@ import:
   subpackages:
   - gexec
 - package: github.com/projectcalico/libcalico-go
-  version: dae320cd652b66d908eb5d59209717e3c3514c26
+  version: b32402a368fade7a5c736bc2e0773ac67166f5f2
   subpackages:
   - lib/apiconfig
   - lib/apis/v2

utils/network.go

@@ -195,25 +195,32 @@ func DoNetworking(args *skel.CmdArgs, conf types.NetConf, result *current.Result
 	}
 
 	// Now that the host side of the veth is moved, state set to UP, and configured with sysctls, we can add the routes to it in the host namespace.
-	err = setupRoutes(hostVeth, result)
+	err = SetupRoutes(hostVeth, result)
 	if err != nil {
 		return "", "", fmt.Errorf("error adding host side routes for interface: %s, error: %s", hostVeth.Attrs().Name, err)
 	}
 
 	return hostVethName, contVethMAC, err
 }
 
-// setupRoutes sets up the routes for the host side of the veth pair.
-func setupRoutes(hostVeth netlink.Link, result *current.Result) error {
+// SetupRoutes sets up the routes for the host side of the veth pair.
+func SetupRoutes(hostVeth netlink.Link, result *current.Result) error {
 
 	// Go through all the IPs and add routes for each IP in the result.
 	for _, ipAddr := range result.IPs {
+		routes0, err := netlink.RouteList(hostVeth, netlink.FAMILY_ALL)
+		if err != nil {
+			return fmt.Errorf("error listing routes")
+		}
+
+		logrus.Printf("> list all routes before doing anything: %v\n", routes0)
 		route := netlink.Route{
 			LinkIndex: hostVeth.Attrs().Index,
 			Scope:     netlink.SCOPE_LINK,
 			Dst:       &ipAddr.Address,
 		}
-		err := netlink.RouteAdd(&route)
+		err = netlink.RouteAdd(&route)
+		logrus.Printf("> route add error: %v, Type: %T", err, err)
 
 		if err != nil {
 			switch err {
@@ -230,7 +237,9 @@ func setupRoutes(hostVeth netlink.Link, result *current.Result) error {
 				// exactly what we are intending to program.
 				// If the route we want is already there then most likely it's programmed by Felix, so we ignore it,
 				// and we return an error if none of the routes match the route we're trying to program.
+				logrus.Printf("> constructed route: %v , scope: %v \n", route, route.Scope)
 				for _, r := range routes {
+					logrus.Printf("> host route list: %v , scope: %v \n", r, r.Scope)
 					if r.LinkIndex == route.LinkIndex && reflect.DeepEqual(r.Dst, route.Dst) && reflect.DeepEqual(r.Scope, route.Scope) {
 						// Route was already present on the host.
 						logrus.Infof("CNI skipping add route. Route already exists for %s\n", hostVeth.Attrs().Name)

utils/utils_suite_test.go

@@ -0,0 +1,13 @@
+package utils_test
+
+import (
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+
+	"testing"
+)
+
+func TestUtils(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Utils Suite")
+}

utils/utils_test.go

@@ -0,0 +1,62 @@
+package utils_test
+
+import (
+	"fmt"
+	"os"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+	"github.com/onsi/gomega/gexec"
+	"github.com/projectcalico/cni-plugin/testutils"
+	"github.com/projectcalico/cni-plugin/utils"
+	log "github.com/sirupsen/logrus"
+	"github.com/vishvananda/netlink"
+)
+
+var _ = Describe("Utils", func() {
+	cniVersion := os.Getenv("CNI_SPEC_VERSION")
+	Describe("Run install-cni", func() {
+		Context("container route already exists on the host", func() {
+			netconf := fmt.Sprintf(`
+			{
+			  "cniVersion": "%s",
+			  "name": "net1",
+			  "type": "calico",
+			  "etcd_endpoints": "http://%s:2379",
+			  "hostname": "namedHostname",
+			  "datastore_type": "%s",
+			  "ipam": {
+			    "type": "host-local",
+			    "subnet": "10.0.0.0/8"
+			  },
+			  "log_level":"debug"
+			}`, cniVersion, os.Getenv("ETCD_IP"), os.Getenv("DATASTORE_TYPE"))
+
+			It("route setup should be resilient to existing route", func() {
+				By("creating a CNI networked container, which should also install the container route in the host namespace")
+				containerID, session, _, _, _, contNs, err := testutils.CreateContainerWithId(netconf, "", testutils.TEST_DEFAULT_NS, "", "meep123")
+				Expect(err).ShouldNot(HaveOccurred())
+				Eventually(session).Should(gexec.Exit())
+
+				// CNI plugin generates host side vEth name from containerID if used for "cni" orchestrator.
+				hostVethName := "cali" + containerID[:utils.Min(11, len(containerID))] //"cali" + containerID
+				hostVeth, err := netlink.LinkByName(hostVethName)
+				Expect(err).ToNot(HaveOccurred())
+
+				result, err := testutils.GetResultForCurrent(session, cniVersion)
+				if err != nil {
+					log.Fatalf("Error getting result from the session: %v\n", err)
+				}
+
+				log.Printf("Unmarshalled result: %v\n", result)
+
+				By("setting up the same route CNI plugin installed in the initial run for the hostVeth")
+				err = utils.SetupRoutes(hostVeth, result)
+				Expect(err).NotTo(HaveOccurred())
+
+				_, err = testutils.DeleteContainerWithId(netconf, contNs.Path(), "", testutils.TEST_DEFAULT_NS, containerID)
+				Expect(err).ShouldNot(HaveOccurred())
+			})
+		})
+	})
+})