dhcp: increase test coverage to 1.0.0 and older spec versions

Signed-off-by: Dan Williams <dcbw@redhat.com>

plugins/ipam/dhcp/dhcp_test.go

@@ -15,7 +15,9 @@
 package main
 
 import (
+	"bytes"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"net"
 	"os"
@@ -25,7 +27,7 @@ import (
 	"time"
 
 	"github.com/containernetworking/cni/pkg/skel"
-	current "github.com/containernetworking/cni/pkg/types/100"
+	"github.com/containernetworking/cni/pkg/types/100"
 	"github.com/containernetworking/plugins/pkg/ns"
 	"github.com/containernetworking/plugins/pkg/testutils"
 
@@ -208,6 +210,11 @@ var _ = Describe("DHCP Operations", func() {
 		dhcpPluginPath, err := exec.LookPath("dhcp")
 		Expect(err).NotTo(HaveOccurred())
 		clientCmd = exec.Command(dhcpPluginPath, "daemon", "-socketpath", socketPath)
+
+		// copy dhcp client's stdout/stderr to test stdout
+		clientCmd.Stdout = os.Stdout
+		clientCmd.Stderr = os.Stderr
+
 		err = clientCmd.Start()
 		Expect(err).NotTo(HaveOccurred())
 		Expect(clientCmd.Process).NotTo(BeNil())
@@ -226,118 +233,127 @@ var _ = Describe("DHCP Operations", func() {
 		clientCmd.Wait()
 
 		Expect(originalNS.Close()).To(Succeed())
+		Expect(testutils.UnmountNS(originalNS)).To(Succeed())
 		Expect(targetNS.Close()).To(Succeed())
-		defer os.RemoveAll(tmpDir)
+		Expect(testutils.UnmountNS(targetNS)).To(Succeed())
+
+		Expect(os.RemoveAll(tmpDir)).To(Succeed())
 	})
 
-	It("configures and deconfigures a link with ADD/DEL", func() {
+	for _, ver := range testutils.AllSpecVersions {
-		conf := fmt.Sprintf(`{
+		// Redefine ver inside for scope so real value is picked up by each dynamically defined It()
-    "cniVersion": "0.3.1",
+		// See Gingkgo's "Patterns for dynamically generating tests" documentation.
-    "name": "mynet",
+		ver := ver
-    "type": "ipvlan",
-    "ipam": {
-        "type": "dhcp",
-	"daemonSocketPath": "%s"
-    }
-}`, socketPath)
 
-		args := &skel.CmdArgs{
+		It(fmt.Sprintf("[%s] configures and deconfigures a link with ADD/DEL", ver), func() {
-			ContainerID: "dummy",
+			conf := fmt.Sprintf(`{
-			Netns:       targetNS.Path(),
+			    "cniVersion": "%s",
-			IfName:      contVethName,
+			    "name": "mynet",
-			StdinData:   []byte(conf),
+			    "type": "ipvlan",
-		}
+			    "ipam": {
+				"type": "dhcp",
+				"daemonSocketPath": "%s"
+			    }
+			}`, ver, socketPath)
 
-		var addResult *current.Result
+			args := &skel.CmdArgs{
-		err := originalNS.Do(func(ns.NetNS) error {
+				ContainerID: "dummy",
-			defer GinkgoRecover()
+				Netns:       targetNS.Path(),
+				IfName:      contVethName,
+				StdinData:   []byte(conf),
+			}
 
-			r, _, err := testutils.CmdAddWithArgs(args, func() error {
+			var addResult *types100.Result
-				return cmdAdd(args)
+			err := originalNS.Do(func(ns.NetNS) error {
-			})
-			Expect(err).NotTo(HaveOccurred())
-
-			addResult, err = current.GetResult(r)
-			Expect(err).NotTo(HaveOccurred())
-			Expect(len(addResult.IPs)).To(Equal(1))
-			Expect(addResult.IPs[0].Address.String()).To(Equal("192.168.1.5/24"))
-			return nil
-		})
-		Expect(err).NotTo(HaveOccurred())
-
-		err = originalNS.Do(func(ns.NetNS) error {
-			return testutils.CmdDelWithArgs(args, func() error {
-				return cmdDel(args)
-			})
-		})
-		Expect(err).NotTo(HaveOccurred())
-	})
-
-	It("correctly handles multiple DELs for the same container", func() {
-		conf := fmt.Sprintf(`{
-    "cniVersion": "0.3.1",
-    "name": "mynet",
-    "type": "ipvlan",
-    "ipam": {
-        "type": "dhcp",
-	"daemonSocketPath": "%s"
-    }
-}`, socketPath)
-
-		args := &skel.CmdArgs{
-			ContainerID: "dummy",
-			Netns:       targetNS.Path(),
-			IfName:      contVethName,
-			StdinData:   []byte(conf),
-		}
-
-		var addResult *current.Result
-		err := originalNS.Do(func(ns.NetNS) error {
-			defer GinkgoRecover()
-
-			r, _, err := testutils.CmdAddWithArgs(args, func() error {
-				return cmdAdd(args)
-			})
-			Expect(err).NotTo(HaveOccurred())
-
-			addResult, err = current.GetResult(r)
-			Expect(err).NotTo(HaveOccurred())
-			Expect(len(addResult.IPs)).To(Equal(1))
-			Expect(addResult.IPs[0].Address.String()).To(Equal("192.168.1.5/24"))
-			return nil
-		})
-		Expect(err).NotTo(HaveOccurred())
-
-		wg := sync.WaitGroup{}
-		wg.Add(3)
-		started := sync.WaitGroup{}
-		started.Add(3)
-		for i := 0; i < 3; i++ {
-			go func() {
 				defer GinkgoRecover()
 
-				// Wait until all goroutines are running
+				r, _, err := testutils.CmdAddWithArgs(args, func() error {
-				started.Done()
+					return cmdAdd(args)
-				started.Wait()
-
-				err = originalNS.Do(func(ns.NetNS) error {
-					return testutils.CmdDelWithArgs(args, func() error {
-						return cmdDel(args)
-					})
 				})
 				Expect(err).NotTo(HaveOccurred())
-				wg.Done()
-			}()
-		}
-		wg.Wait()
 
-		err = originalNS.Do(func(ns.NetNS) error {
+				addResult, err = types100.GetResult(r)
-			return testutils.CmdDelWithArgs(args, func() error {
+				Expect(err).NotTo(HaveOccurred())
-				return cmdDel(args)
+				Expect(len(addResult.IPs)).To(Equal(1))
+				Expect(addResult.IPs[0].Address.String()).To(Equal("192.168.1.5/24"))
+				return nil
 			})
+			Expect(err).NotTo(HaveOccurred())
+
+			err = originalNS.Do(func(ns.NetNS) error {
+				return testutils.CmdDelWithArgs(args, func() error {
+					return cmdDel(args)
+				})
+			})
+			Expect(err).NotTo(HaveOccurred())
 		})
-		Expect(err).NotTo(HaveOccurred())
+
-	})
+		It(fmt.Sprintf("[%s] correctly handles multiple DELs for the same container", ver), func() {
+			conf := fmt.Sprintf(`{
+			    "cniVersion": "%s",
+			    "name": "mynet",
+			    "type": "ipvlan",
+			    "ipam": {
+				"type": "dhcp",
+				"daemonSocketPath": "%s"
+			    }
+			}`, ver, socketPath)
+
+			args := &skel.CmdArgs{
+				ContainerID: "dummy",
+				Netns:       targetNS.Path(),
+				IfName:      contVethName,
+				StdinData:   []byte(conf),
+			}
+
+			var addResult *types100.Result
+			err := originalNS.Do(func(ns.NetNS) error {
+				defer GinkgoRecover()
+
+				r, _, err := testutils.CmdAddWithArgs(args, func() error {
+					return cmdAdd(args)
+				})
+				Expect(err).NotTo(HaveOccurred())
+
+				addResult, err = types100.GetResult(r)
+				Expect(err).NotTo(HaveOccurred())
+				Expect(len(addResult.IPs)).To(Equal(1))
+				Expect(addResult.IPs[0].Address.String()).To(Equal("192.168.1.5/24"))
+				return nil
+			})
+			Expect(err).NotTo(HaveOccurred())
+
+			wg := sync.WaitGroup{}
+			wg.Add(3)
+			started := sync.WaitGroup{}
+			started.Add(3)
+			for i := 0; i < 3; i++ {
+				go func() {
+					defer GinkgoRecover()
+
+					// Wait until all goroutines are running
+					started.Done()
+					started.Wait()
+
+					err = originalNS.Do(func(ns.NetNS) error {
+						return testutils.CmdDelWithArgs(args, func() error {
+							return cmdDel(args)
+						})
+					})
+					Expect(err).NotTo(HaveOccurred())
+					wg.Done()
+				}()
+			}
+			wg.Wait()
+
+			err = originalNS.Do(func(ns.NetNS) error {
+				return testutils.CmdDelWithArgs(args, func() error {
+					return cmdDel(args)
+				})
+			})
+			Expect(err).NotTo(HaveOccurred())
+		})
+	}
 })
 
 const (
@@ -508,7 +524,19 @@ var _ = Describe("DHCP Lease Unavailable Operations", func() {
 		// Start the DHCP client daemon
 		dhcpPluginPath, err := exec.LookPath("dhcp")
 		Expect(err).NotTo(HaveOccurred())
-		clientCmd = exec.Command(dhcpPluginPath, "daemon", "-socketpath", socketPath)
+		// Use very short timeouts for lease-unavailable operations because
+		// the same test is run many times, and the delays will exceed the
+		// `go test` timeout with default delays. Since our DHCP server
+		// and client daemon are local processes anyway, we can depend on
+		// them to respond very quickly.
+		clientCmd = exec.Command(dhcpPluginPath, "daemon", "-socketpath", socketPath, "-timeout", "2s", "-resendmax", "8s")
+
+		// copy dhcp client's stdout/stderr to test stdout
+		var b bytes.Buffer
+		mw := io.MultiWriter(os.Stdout, &b)
+		clientCmd.Stdout = mw
+		clientCmd.Stderr = mw
+
 		err = clientCmd.Start()
 		Expect(err).NotTo(HaveOccurred())
 		Expect(clientCmd.Process).NotTo(BeNil())
@@ -527,92 +555,101 @@ var _ = Describe("DHCP Lease Unavailable Operations", func() {
 		clientCmd.Wait()
 
 		Expect(originalNS.Close()).To(Succeed())
+		Expect(testutils.UnmountNS(originalNS)).To(Succeed())
 		Expect(targetNS.Close()).To(Succeed())
-		defer os.RemoveAll(tmpDir)
+		Expect(testutils.UnmountNS(targetNS)).To(Succeed())
+
+		Expect(os.RemoveAll(tmpDir)).To(Succeed())
 	})
 
-	It("Configures multiple links with multiple ADD with second lease unavailable", func() {
+	for _, ver := range testutils.AllSpecVersions {
-		conf := fmt.Sprintf(`{
+		// Redefine ver inside for scope so real value is picked up by each dynamically defined It()
-	    "cniVersion": "0.3.1",
+		// See Gingkgo's "Patterns for dynamically generating tests" documentation.
-	    "name": "mynet",
+		ver := ver
-	    "type": "bridge",
-	    "bridge": "%s",
-	    "ipam": {
-	        "type": "dhcp",
-		"daemonSocketPath": "%s"
-	    }
-	}`, hostBridgeName, socketPath)
 
-		args := &skel.CmdArgs{
+		It(fmt.Sprintf("[%s] configures multiple links with multiple ADD with second lease unavailable", ver), func() {
-			ContainerID: "dummy",
+			conf := fmt.Sprintf(`{
-			Netns:       targetNS.Path(),
+			    "cniVersion": "%s",
-			IfName:      contVethName0,
+			    "name": "mynet",
-			StdinData:   []byte(conf),
+			    "type": "bridge",
-		}
+			    "bridge": "%s",
+			    "ipam": {
+				"type": "dhcp",
+				"daemonSocketPath": "%s"
+			    }
+			}`, ver, hostBridgeName, socketPath)
 
-		var addResult *current.Result
+			args := &skel.CmdArgs{
-		err := originalNS.Do(func(ns.NetNS) error {
+				ContainerID: "dummy",
-			defer GinkgoRecover()
+				Netns:       targetNS.Path(),
+				IfName:      contVethName0,
+				StdinData:   []byte(conf),
+			}
 
-			r, _, err := testutils.CmdAddWithArgs(args, func() error {
+			var addResult *types100.Result
-				return cmdAdd(args)
+			err := originalNS.Do(func(ns.NetNS) error {
+				defer GinkgoRecover()
+
+				r, _, err := testutils.CmdAddWithArgs(args, func() error {
+					return cmdAdd(args)
+				})
+				Expect(err).NotTo(HaveOccurred())
+
+				addResult, err = types100.GetResult(r)
+				Expect(err).NotTo(HaveOccurred())
+				Expect(len(addResult.IPs)).To(Equal(1))
+				Expect(addResult.IPs[0].Address.String()).To(Equal("192.168.1.5/24"))
+				return nil
 			})
 			Expect(err).NotTo(HaveOccurred())
 
-			addResult, err = current.GetResult(r)
+			args = &skel.CmdArgs{
+				ContainerID: "dummy",
+				Netns:       targetNS.Path(),
+				IfName:      contVethName1,
+				StdinData:   []byte(conf),
+			}
+
+			err = originalNS.Do(func(ns.NetNS) error {
+				defer GinkgoRecover()
+
+				_, _, err := testutils.CmdAddWithArgs(args, func() error {
+					return cmdAdd(args)
+				})
+				Expect(err).To(HaveOccurred())
+				println(err.Error())
+				Expect(err.Error()).To(Equal("error calling DHCP.Allocate: no more tries"))
+				return nil
+			})
 			Expect(err).NotTo(HaveOccurred())
-			Expect(len(addResult.IPs)).To(Equal(1))
-			Expect(addResult.IPs[0].Address.String()).To(Equal("192.168.1.5/24"))
-			return nil
-		})
-		Expect(err).NotTo(HaveOccurred())
 
-		args = &skel.CmdArgs{
+			args = &skel.CmdArgs{
-			ContainerID: "dummy",
+				ContainerID: "dummy",
-			Netns:       targetNS.Path(),
+				Netns:       targetNS.Path(),
-			IfName:      contVethName1,
+				IfName:      contVethName1,
-			StdinData:   []byte(conf),
+				StdinData:   []byte(conf),
-		}
+			}
 
-		err = originalNS.Do(func(ns.NetNS) error {
+			err = originalNS.Do(func(ns.NetNS) error {
-			defer GinkgoRecover()
+				return testutils.CmdDelWithArgs(args, func() error {
-
+					return cmdDel(args)
-			_, _, err := testutils.CmdAddWithArgs(args, func() error {
+				})
-				return cmdAdd(args)
 			})
-			Expect(err).To(HaveOccurred())
+			Expect(err).NotTo(HaveOccurred())
-			println(err.Error())
-			Expect(err.Error()).To(Equal("error calling DHCP.Allocate: no more tries"))
-			return nil
-		})
-		Expect(err).NotTo(HaveOccurred())
 
-		args = &skel.CmdArgs{
+			args = &skel.CmdArgs{
-			ContainerID: "dummy",
+				ContainerID: "dummy",
-			Netns:       targetNS.Path(),
+				Netns:       targetNS.Path(),
-			IfName:      contVethName1,
+				IfName:      contVethName0,
-			StdinData:   []byte(conf),
+				StdinData:   []byte(conf),
-		}
+			}
 
-		err = originalNS.Do(func(ns.NetNS) error {
+			err = originalNS.Do(func(ns.NetNS) error {
-			return testutils.CmdDelWithArgs(args, func() error {
+				return testutils.CmdDelWithArgs(args, func() error {
-				return cmdDel(args)
+					return cmdDel(args)
+				})
 			})
+			Expect(err).NotTo(HaveOccurred())
 		})
-		Expect(err).NotTo(HaveOccurred())
+	}
-
-		args = &skel.CmdArgs{
-			ContainerID: "dummy",
-			Netns:       targetNS.Path(),
-			IfName:      contVethName0,
-			StdinData:   []byte(conf),
-		}
-
-		err = originalNS.Do(func(ns.NetNS) error {
-			return testutils.CmdDelWithArgs(args, func() error {
-				return cmdDel(args)
-			})
-		})
-		Expect(err).NotTo(HaveOccurred())
-	})
 })