Merge pull request #369 from Random-Liu/cherrypick-#366-0.7

[Backport v0.7] Fix: failed to set bridge addr: could not add IP address to \"cni0\": file exists

pkg/ip/link_linux.go

@@ -42,8 +42,14 @@ func makeVethPair(name, peer string, mtu int) (netlink.Link, error) {
 	if err := netlink.LinkAdd(veth); err != nil {
 		return nil, err
 	}
+	// Re-fetch the link to get its creation-time parameters, e.g. index and mac
+	veth2, err := netlink.LinkByName(name)
+	if err != nil {
+		netlink.LinkDel(veth) // try and clean up the link if possible.
+		return nil, err
+	}
 
-	return veth, nil
+	return veth2, nil
 }
 
 func peerExists(name string) bool {

plugins/main/bridge/bridge.go

@@ -170,7 +170,7 @@ func ensureBridgeAddr(br *netlink.Bridge, family int, ipn *net.IPNet, forceAddre
 	}
 
 	addr := &netlink.Addr{IPNet: ipn, Label: ""}
-	if err := netlink.AddrAdd(br, addr); err != nil {
+	if err := netlink.AddrAdd(br, addr); err != nil && err != syscall.EEXIST {
 		return fmt.Errorf("could not add IP address to %q: %v", br.Name, err)
 	}
 

plugins/main/bridge/bridge_test.go

@@ -257,7 +257,14 @@ func (tester *testerV03x) cmdAddTest(tc testCase) {
 
 		Expect(len(result.Interfaces)).To(Equal(3))
 		Expect(result.Interfaces[0].Name).To(Equal(BRNAME))
+		Expect(result.Interfaces[0].Mac).To(HaveLen(17))
+
+		Expect(result.Interfaces[1].Name).To(HavePrefix("veth"))
+		Expect(result.Interfaces[1].Mac).To(HaveLen(17))
+
 		Expect(result.Interfaces[2].Name).To(Equal(IFNAME))
+		Expect(result.Interfaces[2].Mac).To(HaveLen(17)) //mac is random
+		Expect(result.Interfaces[2].Sandbox).To(Equal(tester.targetNS.Path()))
 
 		// Make sure bridge link exists
 		link, err := netlink.LinkByName(result.Interfaces[0].Name)

plugins/main/ptp/ptp_test.go

@@ -78,12 +78,14 @@ var _ = Describe("ptp Operations", func() {
 		// Make sure ptp link exists in the target namespace
 		// Then, ping the gateway
 		seenIPs := 0
+
+		wantMac := ""
 		err = targetNs.Do(func(ns.NetNS) error {
 			defer GinkgoRecover()
 
 			link, err := netlink.LinkByName(IFNAME)
 			Expect(err).NotTo(HaveOccurred())
-			Expect(link.Attrs().Name).To(Equal(IFNAME))
+			wantMac = link.Attrs().HardwareAddr.String()
 
 			for _, ipc := range res.IPs {
 				if *ipc.Interface != 1 {
@@ -105,6 +107,17 @@ var _ = Describe("ptp Operations", func() {
 
 		Expect(seenIPs).To(Equal(numIPs))
 
+		// make sure the interfaces are correct
+		Expect(res.Interfaces).To(HaveLen(2))
+
+		Expect(res.Interfaces[0].Name).To(HavePrefix("veth"))
+		Expect(res.Interfaces[0].Mac).To(HaveLen(17))
+		Expect(res.Interfaces[0].Sandbox).To(BeEmpty())
+
+		Expect(res.Interfaces[1].Name).To(Equal(IFNAME))
+		Expect(res.Interfaces[1].Mac).To(Equal(wantMac))
+		Expect(res.Interfaces[1].Sandbox).To(Equal(targetNs.Path()))
+
 		// Call the plugins with the DEL command, deleting the veth endpoints
 		err = originalNS.Do(func(ns.NetNS) error {
 			defer GinkgoRecover()

plugins/meta/portmap/chain.go

@@ -29,6 +29,8 @@ type chain struct {
 
 	entryRules [][]string // the rules that "point" to this chain
 	rules      [][]string // the rules this chain contains
+
+	prependEntry bool // whether or not the entry rules should be prepended
 }
 
 // setup idempotently creates the chain. It will not error if the chain exists.
@@ -45,19 +47,19 @@ func (c *chain) setup(ipt *iptables.IPTables) error {
 	}
 
 	// Add the rules to the chain
-	for i := len(c.rules) - 1; i >= 0; i-- {
+	for _, rule := range c.rules {
-		if err := prependUnique(ipt, c.table, c.name, c.rules[i]); err != nil {
+		if err := insertUnique(ipt, c.table, c.name, false, rule); err != nil {
 			return err
 		}
 	}
 
 	// Add the entry rules to the entry chains
 	for _, entryChain := range c.entryChains {
-		for i := len(c.entryRules) - 1; i >= 0; i-- {
+		for _, rule := range c.entryRules {
 			r := []string{}
-			r = append(r, c.entryRules[i]...)
+			r = append(r, rule...)
 			r = append(r, "-j", c.name)
-			if err := prependUnique(ipt, c.table, entryChain, r); err != nil {
+			if err := insertUnique(ipt, c.table, entryChain, c.prependEntry, r); err != nil {
 				return err
 			}
 		}
@@ -105,8 +107,9 @@ func (c *chain) teardown(ipt *iptables.IPTables) error {
 	return nil
 }
 
-// prependUnique will prepend a rule to a chain, if it does not already exist
+// insertUnique will add a rule to a chain if it does not already exist.
-func prependUnique(ipt *iptables.IPTables, table, chain string, rule []string) error {
+// By default the rule is appended, unless prepend is true.
+func insertUnique(ipt *iptables.IPTables, table, chain string, prepend bool, rule []string) error {
 	exists, err := ipt.Exists(table, chain, rule...)
 	if err != nil {
 		return err
@@ -115,7 +118,11 @@ func prependUnique(ipt *iptables.IPTables, table, chain string, rule []string) e
 		return nil
 	}
 
-	return ipt.Insert(table, chain, 1, rule...)
+	if prepend {
+		return ipt.Insert(table, chain, 1, rule...)
+	} else {
+		return ipt.Append(table, chain, rule...)
+	}
 }
 
 func chainExists(ipt *iptables.IPTables, tableName, chainName string) (bool, error) {

plugins/meta/portmap/chain_test.go

@@ -116,8 +116,8 @@ var _ = Describe("chain tests", func() {
 		Expect(err).NotTo(HaveOccurred())
 		Expect(haveRules).To(Equal([]string{
 			"-N " + tlChainName,
-			"-A " + tlChainName + " -d 203.0.113.1/32 -j " + testChain.name,
 			"-A " + tlChainName + ` -m comment --comment "canary value" -j ACCEPT`,
+			"-A " + tlChainName + " -d 203.0.113.1/32 -j " + testChain.name,
 		}))
 
 		// Check that the chain and rule was created

plugins/meta/portmap/portmap.go

@@ -255,6 +255,10 @@ func genMarkMasqChain(markBit int) chain {
 		table:       "nat",
 		name:        MarkMasqChainName,
 		entryChains: []string{"POSTROUTING"},
+		// Only this entry chain needs to be prepended, because otherwise it is
+		// stomped on by the masquerading rules created by the CNI ptp and bridge
+		// plugins.
+		prependEntry: true,
 		entryRules: [][]string{{
 			"-m", "comment",
 			"--comment", "CNI portfwd requiring masquerade",

plugins/meta/portmap/portmap_integ_test.go

@@ -163,6 +163,12 @@ var _ = Describe("portmap integration tests", func() {
 		fmt.Fprintf(GinkgoWriter, "hostIP: %s:%d, contIP: %s:%d\n",
 			hostIP, hostPort, contIP, containerPort)
 
+		// dump iptables-save output for debugging
+		cmd = exec.Command("iptables-save")
+		cmd.Stderr = GinkgoWriter
+		cmd.Stdout = GinkgoWriter
+		Expect(cmd.Run()).To(Succeed())
+
 		// Sanity check: verify that the container is reachable directly
 		contOK := testEchoServer(contIP.String(), containerPort, "")
 

plugins/meta/portmap/portmap_test.go

@@ -323,6 +323,7 @@ var _ = Describe("portmapping configuration", func() {
 						"--mark", "0x20/0x20",
 						"-j", "MASQUERADE",
 					}},
+					prependEntry: true,
 				}))
 			})
 		})