Ensure the bandwith plugin chooses the host veth device

When chained with a plugin that returns multiple devices, the bandwidth
plugin chooses the host veth device.

Signed-off-by: Tyler Schultz <tschultz@pivotal.io>

integration/integration_linux_test.go

@@ -20,64 +20,181 @@ import (
 	"os/exec"
 	"path/filepath"
 
+	"bytes"
+	"io"
+	"net"
+	"regexp"
+	"strconv"
+	"strings"
+	"time"
+
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
+	"github.com/onsi/gomega/gbytes"
 	"github.com/onsi/gomega/gexec"
 )
 
 var _ = Describe("Basic PTP using cnitool", func() {
 	var (
-		env        TestEnv
-		hostNS     NSShortName
-		contNS     NSShortName
 		cnitoolBin string
+		cniPath    string
 	)
 
 	BeforeEach(func() {
-		cniPath, err := filepath.Abs("../bin")
+		var err error
-		Expect(err).NotTo(HaveOccurred())
+		cniPath, err = filepath.Abs("../bin")
-		netConfPath, err := filepath.Abs("./testdata")
 		Expect(err).NotTo(HaveOccurred())
 		cnitoolBin, err = exec.LookPath("cnitool")
 		Expect(err).NotTo(HaveOccurred(), "expected to find cnitool in your PATH")
+	})
 
-		env = TestEnv([]string{
+	Context("basic cases", func() {
-			"CNI_PATH=" + cniPath,
+		var (
-			"NETCONFPATH=" + netConfPath,
+			env    TestEnv
-			"PATH=" + os.Getenv("PATH"),
+			hostNS Namespace
+			contNS Namespace
+		)
+
+		BeforeEach(func() {
+			var err error
+
+			netConfPath, err := filepath.Abs("./testdata")
+			Expect(err).NotTo(HaveOccurred())
+
+			env = TestEnv([]string{
+				"CNI_PATH=" + cniPath,
+				"NETCONFPATH=" + netConfPath,
+				"PATH=" + os.Getenv("PATH"),
+			})
+
+			hostNS = Namespace(fmt.Sprintf("cni-test-host-%x", rand.Int31()))
+			hostNS.Add()
+
+			contNS = Namespace(fmt.Sprintf("cni-test-cont-%x", rand.Int31()))
+			contNS.Add()
 		})
 
-		hostNS = NSShortName(fmt.Sprintf("cni-test-host-%x", rand.Int31()))
+		AfterEach(func() {
-		hostNS.Add()
+			contNS.Del()
+			hostNS.Del()
+		})
 
-		contNS = NSShortName(fmt.Sprintf("cni-test-cont-%x", rand.Int31()))
+		basicAssertion := func(netName, expectedIPPrefix string) {
-		contNS.Add()
+			env.runInNS(hostNS, cnitoolBin, "add", netName, contNS.LongName())
+
+			addrOutput := env.runInNS(contNS, "ip", "addr")
+			Expect(addrOutput).To(ContainSubstring(expectedIPPrefix))
+
+			env.runInNS(hostNS, cnitoolBin, "del", netName, contNS.LongName())
+		}
+
+		It("supports basic network add and del operations", func() {
+			basicAssertion("basic-ptp", "10.1.2.")
+		})
+
+		It("supports add and del with ptp + bandwidth", func() {
+			basicAssertion("chained-ptp-bandwidth", "10.9.2.")
+		})
 	})
 
-	AfterEach(func() {
+	Context("when the bandwidth plugin is chained with a plugin that returns multiple adapters", func() {
-		contNS.Del()
+		var (
-		hostNS.Del()
+			hostNS                                            Namespace
-	})
+			contNS1                                           Namespace
+			contNS2                                           Namespace
+			basicBridgeEnv                                    TestEnv
+			chainedBridgeBandwidthEnv                         TestEnv
+			chainedBridgeBandwidthSession, basicBridgeSession *gexec.Session
+			chainedBridgeBandwidthPort, basicBridgePort       int
+			chainedBridgeIP, basicBridgeIP                    string
+			runtimeWithLimit, runtimeWithoutLimit             time.Duration
+		)
 
-	basicAssertion := func(netName, expectedIPPrefix string) {
+		BeforeEach(func() {
-		env.runInNS(hostNS, cnitoolBin, "add", netName, contNS.LongName())
+			hostNS = Namespace(fmt.Sprintf("cni-test-host-%x", rand.Int31()))
+			hostNS.Add()
 
-		addrOutput := env.runInNS(contNS, "ip", "addr")
+			contNS1 = Namespace(fmt.Sprintf("cni-test-cont1-%x", rand.Int31()))
-		Expect(addrOutput).To(ContainSubstring(expectedIPPrefix))
+			contNS1.Add()
 
-		env.runInNS(hostNS, cnitoolBin, "del", netName, contNS.LongName())
+			contNS2 = Namespace(fmt.Sprintf("cni-test-cont2-%x", rand.Int31()))
-	}
+			contNS2.Add()
 
-	It("supports basic network add and del operations", func() {
+			basicBridgeNetConfPath, err := filepath.Abs("./testdata/basic-bridge")
-		basicAssertion("basic-ptp", "10.1.2.")
+			Expect(err).NotTo(HaveOccurred())
-	})
 
-	It("supports add and del with ptp + bandwidth", func() {
+			basicBridgeEnv = TestEnv([]string{
-		basicAssertion("chained-ptp-bandwidth", "10.9.2.")
+				"CNI_PATH=" + cniPath,
-	})
+				"NETCONFPATH=" + basicBridgeNetConfPath,
+				"PATH=" + os.Getenv("PATH"),
+			})
 
-	It("supports add and del with bridge + bandwidth", func() {
+			chainedBridgeBandwidthNetConfPath, err := filepath.Abs("./testdata/chained-bridge-bandwidth")
-		basicAssertion("chained-bridge-bandwidth", "10.11.2.")
+			Expect(err).NotTo(HaveOccurred())
+
+			chainedBridgeBandwidthEnv = TestEnv([]string{
+				"CNI_PATH=" + cniPath,
+				"NETCONFPATH=" + chainedBridgeBandwidthNetConfPath,
+				"PATH=" + os.Getenv("PATH"),
+			})
+		})
+
+		AfterEach(func() {
+			if chainedBridgeBandwidthSession != nil {
+				chainedBridgeBandwidthSession.Kill()
+			}
+			if basicBridgeSession != nil {
+				basicBridgeSession.Kill()
+			}
+
+			chainedBridgeBandwidthEnv.runInNS(hostNS, cnitoolBin, "del", "network-chain-test", contNS1.LongName())
+			basicBridgeEnv.runInNS(hostNS, cnitoolBin, "del", "network-chain-test", contNS2.LongName())
+		})
+
+		Measure("limits traffic only on the restricted bandwith veth device", func(b Benchmarker) {
+			ipRegexp := regexp.MustCompile("10\\.11\\.2\\.\\d{1,3}")
+
+			By(fmt.Sprintf("adding %s to %s\n\n", "chained-bridge-bandwidth", contNS1.ShortName()), func() {
+				chainedBridgeBandwidthEnv.runInNS(hostNS, cnitoolBin, "add", "network-chain-test", contNS1.LongName())
+				chainedBridgeIP = ipRegexp.FindString(chainedBridgeBandwidthEnv.runInNS(contNS1, "ip", "addr"))
+				Expect(chainedBridgeIP).To(ContainSubstring("10.11.2."))
+			})
+
+			By(fmt.Sprintf("adding %s to %s\n\n", "basic-bridge", contNS2.ShortName()), func() {
+				basicBridgeEnv.runInNS(hostNS, cnitoolBin, "add", "network-chain-test", contNS2.LongName())
+				basicBridgeIP = ipRegexp.FindString(basicBridgeEnv.runInNS(contNS2, "ip", "addr"))
+				Expect(basicBridgeIP).To(ContainSubstring("10.11.2."))
+			})
+
+			var err error
+
+			By(fmt.Sprintf("starting echo server in %s\n\n", contNS1.ShortName()), func() {
+				chainedBridgeBandwidthPort, chainedBridgeBandwidthSession, err = startEchoServerInNamespace(contNS1)
+				Expect(err).ToNot(HaveOccurred())
+			})
+
+			By(fmt.Sprintf("starting echo server in %s\n\n", contNS2.ShortName()), func() {
+				basicBridgePort, basicBridgeSession, err = startEchoServerInNamespace(contNS2)
+				Expect(err).ToNot(HaveOccurred())
+			})
+
+			packetInBytes := 20000 // The shaper needs to 'warm'. Send enough to cause it to throttle,
+			// balanced by run time.
+
+			By(fmt.Sprintf("sending tcp traffic to the chained, bridged, traffic shaped container on ip address '%s:%d'\n\n", chainedBridgeIP, chainedBridgeBandwidthPort), func() {
+				runtimeWithLimit = b.Time("with chained bridge and bandwidth plugins", func() {
+					makeTcpClientInNS(hostNS.ShortName(), chainedBridgeIP, chainedBridgeBandwidthPort, packetInBytes)
+				})
+			})
+
+			By(fmt.Sprintf("sending tcp traffic to the basic bridged container on ip address '%s:%d'\n\n", basicBridgeIP, basicBridgePort), func() {
+				runtimeWithoutLimit = b.Time("with basic bridged plugin", func() {
+					makeTcpClientInNS(hostNS.ShortName(), basicBridgeIP, basicBridgePort, packetInBytes)
+				})
+			})
+
+			Expect(runtimeWithLimit).To(BeNumerically(">", runtimeWithoutLimit+1000*time.Millisecond))
+		}, 1)
 	})
 })
 
@@ -92,21 +209,70 @@ func (e TestEnv) run(bin string, args ...string) string {
 	return string(session.Out.Contents())
 }
 
-func (e TestEnv) runInNS(nsShortName NSShortName, bin string, args ...string) string {
+func (e TestEnv) runInNS(nsShortName Namespace, bin string, args ...string) string {
 	a := append([]string{"netns", "exec", string(nsShortName), bin}, args...)
 	return e.run("ip", a...)
 }
 
-type NSShortName string
+type Namespace string
 
-func (n NSShortName) LongName() string {
+func (n Namespace) LongName() string {
 	return fmt.Sprintf("/var/run/netns/%s", n)
 }
 
-func (n NSShortName) Add() {
+func (n Namespace) ShortName() string {
+	return string(n)
+}
+
+func (n Namespace) Add() {
 	(TestEnv{}).run("ip", "netns", "add", string(n))
 }
 
-func (n NSShortName) Del() {
+func (n Namespace) Del() {
 	(TestEnv{}).run("ip", "netns", "del", string(n))
 }
+
+func makeTcpClientInNS(netns string, address string, port int, numBytes int) {
+	message := bytes.Repeat([]byte{'a'}, numBytes)
+
+	bin, err := exec.LookPath("nc")
+	Expect(err).NotTo(HaveOccurred())
+	var cmd *exec.Cmd
+	if netns != "" {
+		netns = filepath.Base(netns)
+		cmd = exec.Command("ip", "netns", "exec", netns, bin, "-v", address, strconv.Itoa(port))
+	} else {
+		cmd = exec.Command("nc", address, strconv.Itoa(port))
+	}
+	cmd.Stdin = bytes.NewBuffer([]byte(message))
+	cmd.Stderr = GinkgoWriter
+	out, err := cmd.Output()
+
+	Expect(err).NotTo(HaveOccurred())
+	Expect(string(out)).To(Equal(string(message)))
+}
+
+func startEchoServerInNamespace(netNS Namespace) (int, *gexec.Session, error) {
+	session, err := startInNetNS(echoServerBinaryPath, netNS)
+	Expect(err).NotTo(HaveOccurred())
+
+	// wait for it to print it's address on stdout
+	Eventually(session.Out).Should(gbytes.Say("\n"))
+	_, portString, err := net.SplitHostPort(strings.TrimSpace(string(session.Out.Contents())))
+	Expect(err).NotTo(HaveOccurred())
+
+	port, err := strconv.Atoi(portString)
+	Expect(err).NotTo(HaveOccurred())
+
+	go func() {
+		// print out echoserver output to ginkgo to capture any errors that might be occurring.
+		io.Copy(GinkgoWriter, io.MultiReader(session.Out, session.Err))
+	}()
+
+	return port, session, nil
+}
+
+func startInNetNS(binPath string, namespace Namespace) (*gexec.Session, error) {
+	cmd := exec.Command("ip", "netns", "exec", namespace.ShortName(), binPath)
+	return gexec.Start(cmd, GinkgoWriter, GinkgoWriter)
+}

integration/integration_suite_test.go

@@ -20,6 +20,7 @@ import (
 	. "github.com/onsi/ginkgo"
 	"github.com/onsi/ginkgo/config"
 	. "github.com/onsi/gomega"
+	"github.com/onsi/gomega/gexec"
 )
 
 func TestIntegration(t *testing.T) {
@@ -27,6 +28,17 @@ func TestIntegration(t *testing.T) {
 	RunSpecs(t, "Integration Suite")
 }
 
-var _ = BeforeSuite(func() {
+var echoServerBinaryPath string
+
+var _ = SynchronizedBeforeSuite(func() []byte {
+	binaryPath, err := gexec.Build("github.com/containernetworking/plugins/pkg/testutils/echosvr")
+	Expect(err).NotTo(HaveOccurred())
+	return []byte(binaryPath)
+}, func(data []byte) {
+	echoServerBinaryPath = string(data)
 	rand.Seed(config.GinkgoConfig.RandomSeed + int64(GinkgoParallelNode()))
 })
+
+var _ = SynchronizedAfterSuite(func() {}, func() {
+	gexec.CleanupBuildArtifacts()
+})

integration/testdata/basic-bridge/network-chain-test.json

@@ -0,0 +1,12 @@
+{
+  "cniVersion": "0.3.1",
+  "name": "network-chain-test",
+  "type": "bridge",
+  "bridge": "test-bridge-0",
+  "isDefaultGateway": true,
+  "ipam": {
+    "type": "host-local",
+    "subnet": "10.11.2.0/24",
+    "dataDir": "/tmp/foo"
+  }
+}

integration/testdata/chained-bridge-bandwidth.conflist

@@ -1,20 +0,0 @@
-{
-    "cniVersion": "0.3.1",
-    "name": "chained-bridge-bandwidth",
-    "plugins": [
-        {
-            "type": "bridge",
-            "ipam": {
-                "type": "host-local",
-                "subnet": "10.11.2.0/24"
-            }
-        },
-        {
-            "type": "bandwidth",
-            "ingressRate": 800,
-            "ingressBurst": 200,
-            "egressRate": 800,
-            "egressBurst": 200
-        }
-    ]
-}

integration/testdata/chained-bridge-bandwidth/network-chain-test.conflist

@@ -0,0 +1,23 @@
+{
+    "cniVersion": "0.3.1",
+    "name": "network-chain-test",
+    "plugins": [
+        {
+            "type": "bridge",
+            "bridge": "test-bridge-0",
+            "isDefaultGateway": true,
+            "ipam": {
+                "type": "host-local",
+                "subnet": "10.11.2.0/24",
+                "dataDir": "/tmp/foo"
+            }
+        },
+        {
+            "type": "bandwidth",
+            "ingressRate": 8000,
+            "ingressBurst": 16000,
+            "egressRate": 8000,
+            "egressBurst": 16000
+        }
+    ]
+}

plugins/meta/bandwidth/main.go

@@ -17,7 +17,6 @@ package main
 import (
 	"crypto/sha1"
 	"encoding/json"
-	"errors"
 	"fmt"
 
 	"github.com/containernetworking/cni/pkg/skel"
@@ -25,7 +24,9 @@ import (
 	"github.com/containernetworking/cni/pkg/types/current"
 	"github.com/containernetworking/cni/pkg/version"
 
+	"github.com/containernetworking/plugins/pkg/ip"
 	"github.com/vishvananda/netlink"
+	"errors"
 )
 
 type PluginConf struct {
@@ -112,15 +113,16 @@ func getMTU(deviceName string) (int, error) {
 }
 
 func getHostInterface(interfaces []*current.Interface) (*current.Interface, error) {
-	for _, prevIface := range interfaces {
+	var err error
-		if prevIface.Sandbox != "" {
+	for _, iface := range interfaces {
-			continue
+		if iface.Sandbox == "" { // host interface
+			_, _, err = ip.GetVethPeerIfindex(iface.Name)
+			if err == nil {
+				return iface, err
+			}
 		}
-
-		return prevIface, nil
 	}
-
+	return nil, errors.New("no host interface found: " + err.Error())
-	return nil, errors.New("no host interface found")
 }
 
 func cmdAdd(args *skel.CmdArgs) error {