From b272f826ac4e67928e92523ee2bb2a0d72319c1f Mon Sep 17 00:00:00 2001 From: gunjan5 Date: Wed, 1 Nov 2017 15:36:40 -0700 Subject: [PATCH] should work now, hopefully --- Makefile | 8 ++--- calico_cni_test.go | 44 +++++++++++++++++++++++++++ cni_plugin_suite_test.go | 13 -------- glide.lock | 2 +- utils/network.go | 16 +++------- utils/utils_suite_test.go | 13 -------- utils/utils_test.go | 62 --------------------------------------- 7 files changed, 53 insertions(+), 105 deletions(-) delete mode 100644 cni_plugin_suite_test.go delete mode 100644 utils/utils_suite_test.go delete mode 100644 utils/utils_test.go diff --git a/Makefile b/Makefile index 19dd1f796..a3c7de081 100644 --- a/Makefile +++ b/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 && \ - make ut' + make run-tests' make stop-etcd # This does not currently work, kubernetes needs additional configuration @@ -179,7 +179,7 @@ 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 && \ - make ut' + make run-tests' make stop-etcd ## Run the tests in a container (as root) for different CNI spec versions @@ -204,9 +204,9 @@ build-containerized: vendor cd /go/src/github.com/projectcalico/cni-plugin && \ make binary' -.PHONY: ut +.PHONY: run-tests ## Run the tests locally, must have local etcd running -ut: +run-tests: # Run tests recursively (-r). ginkgo -cover -r -skipPackage vendor -skipPackage k8s-install diff --git a/calico_cni_test.go b/calico_cni_test.go index 706e46410..0be320608 100644 --- a/calico_cni_test.go +++ b/calico_cni_test.go @@ -413,4 +413,48 @@ var _ = Describe("CalicoCni", func() { }) }) }) + + Describe("SetupRoutes works fine when the route is already programmed", func() { + Context("container route already exists on the host", func() { + netconf := fmt.Sprintf(` + { + "cniVersion": "%s", + "name": "net1", + "type": "calico", + "etcd_endpoints": "http://%s:2379", + "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, "", "meep1337") + 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()) + }) + }) + }) }) diff --git a/cni_plugin_suite_test.go b/cni_plugin_suite_test.go deleted file mode 100644 index 6e958383b..000000000 --- a/cni_plugin_suite_test.go +++ /dev/null @@ -1,13 +0,0 @@ -package main_test - -import ( - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - - "testing" -) - -func TestCniPlugin(t *testing.T) { - RegisterFailHandler(Fail) - RunSpecs(t, "CniPlugin Suite") -} diff --git a/glide.lock b/glide.lock index b3123102d..e3d6117fc 100644 --- a/glide.lock +++ b/glide.lock @@ -1,5 +1,5 @@ hash: b38f9a84a04a54a9153a5e081a0622f979842a10f38d53a84d197776a2c6e55f -updated: 2017-11-01T09:27:22.905163563-07:00 +updated: 2017-11-01T15:33:14.273267925-07:00 imports: - name: cloud.google.com/go version: 3b1ae45394a234c385be014e9a488f2bb6eef821 diff --git a/utils/network.go b/utils/network.go index e1c7f948a..91c8658a6 100644 --- a/utils/network.go +++ b/utils/network.go @@ -5,7 +5,6 @@ import ( "io" "net" "os" - "reflect" "syscall" "github.com/containernetworking/cni/pkg/ip" @@ -208,19 +207,12 @@ 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) - logrus.Printf("> route add error: %v, Type: %T", err, err) + err := netlink.RouteAdd(&route) if err != nil { switch err { @@ -237,10 +229,10 @@ 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) + logrus.Debugf("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) { + logrus.Debugf("Routes on the interface '%s': %v , scope: %v \n", hostVeth.Attrs().Name, r, r.Scope) + if r.LinkIndex == route.LinkIndex && r.Dst.IP.Equal(route.Dst.IP) && 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) return nil diff --git a/utils/utils_suite_test.go b/utils/utils_suite_test.go deleted file mode 100644 index f160db602..000000000 --- a/utils/utils_suite_test.go +++ /dev/null @@ -1,13 +0,0 @@ -package utils_test - -import ( - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - - "testing" -) - -func TestUtils(t *testing.T) { - RegisterFailHandler(Fail) - RunSpecs(t, "Utils Suite") -} diff --git a/utils/utils_test.go b/utils/utils_test.go deleted file mode 100644 index 927bf1a8e..000000000 --- a/utils/utils_test.go +++ /dev/null @@ -1,62 +0,0 @@ -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()) - }) - }) - }) -})