host-device: increase test coverage to 1.0.0 and older spec versions

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

plugins/main/host-device/host-device_test.go

@@ -23,8 +23,8 @@ import (
 
 	"github.com/containernetworking/cni/pkg/skel"
 	"github.com/containernetworking/cni/pkg/types"
-	types040 "github.com/containernetworking/cni/pkg/types/040"
+	"github.com/containernetworking/cni/pkg/types/040"
-	current "github.com/containernetworking/cni/pkg/types/100"
+	"github.com/containernetworking/cni/pkg/types/100"
 	"github.com/containernetworking/cni/pkg/version"
 	"github.com/containernetworking/plugins/pkg/ns"
 	"github.com/containernetworking/plugins/pkg/testutils"
@@ -45,7 +45,7 @@ type Net struct {
 	IPAM          *IPAMConfig            `json:"ipam,omitempty"`
 	DNS           types.DNS              `json:"dns"`
 	RawPrevResult map[string]interface{} `json:"prevResult,omitempty"`
-	PrevResult    current.Result         `json:"-"`
+	PrevResult    types100.Result        `json:"-"`
 }
 
 type IPAMConfig struct {
@@ -215,14 +215,79 @@ func buildOneConfig(name, cniVersion string, orig *Net, prevResult types.Result)
 
 }
 
+type tester interface {
+	expectInterfaces(result types.Result, name, mac, sandbox string)
+}
+
+type testerBase struct{}
+
+type testerV10x testerBase
+type testerV04x testerBase
+type testerV03x testerBase
+
+func newTesterByVersion(version string) tester {
+	switch {
+	case strings.HasPrefix(version, "1.0."):
+		return &testerV10x{}
+	case strings.HasPrefix(version, "0.4."):
+		return &testerV04x{}
+	case strings.HasPrefix(version, "0.3."):
+		return &testerV03x{}
+	default:
+		Fail(fmt.Sprintf("unsupported config version %s", version))
+	}
+	return nil
+}
+
+func (t *testerV10x) expectInterfaces(result types.Result, name, mac, sandbox string) {
+	// check that the result was sane
+	res, err := types100.NewResultFromResult(result)
+	Expect(err).NotTo(HaveOccurred())
+	Expect(res.Interfaces).To(Equal([]*types100.Interface{
+		{
+			Name:    name,
+			Mac:     mac,
+			Sandbox: sandbox,
+		},
+	}))
+}
+
+func (t *testerV04x) expectInterfaces(result types.Result, name, mac, sandbox string) {
+	// check that the result was sane
+	res, err := types040.NewResultFromResult(result)
+	Expect(err).NotTo(HaveOccurred())
+	Expect(res.Interfaces).To(Equal([]*types040.Interface{
+		{
+			Name:    name,
+			Mac:     mac,
+			Sandbox: sandbox,
+		},
+	}))
+}
+
+func (t *testerV03x) expectInterfaces(result types.Result, name, mac, sandbox string) {
+	// check that the result was sane
+	res, err := types040.NewResultFromResult(result)
+	Expect(err).NotTo(HaveOccurred())
+	Expect(res.Interfaces).To(Equal([]*types040.Interface{
+		{
+			Name:    name,
+			Mac:     mac,
+			Sandbox: sandbox,
+		},
+	}))
+}
+
 var _ = Describe("base functionality", func() {
-	var originalNS ns.NetNS
+	var originalNS, targetNS ns.NetNS
 	var ifname string
 
 	BeforeEach(func() {
 		var err error
 		originalNS, err = testutils.NewNS()
 		Expect(err).NotTo(HaveOccurred())
+		targetNS, err = testutils.NewNS()
+		Expect(err).NotTo(HaveOccurred())
 
 		ifname = fmt.Sprintf("dummy-%x", rand.Int31())
 	})
@@ -230,9 +295,16 @@ var _ = Describe("base functionality", func() {
 	AfterEach(func() {
 		Expect(originalNS.Close()).To(Succeed())
 		Expect(testutils.UnmountNS(originalNS)).To(Succeed())
+		Expect(targetNS.Close()).To(Succeed())
+		Expect(testutils.UnmountNS(targetNS)).To(Succeed())
 	})
 
-	It("Works with a valid config without IPAM", func() {
+	for _, ver := range []string{"0.3.0", "0.3.1", "0.4.0", "1.0.0"} {
+		// Redefine ver inside for scope so real value is picked up by each dynamically defined It()
+		// See Gingkgo's "Patterns for dynamically generating tests" documentation.
+		ver := ver
+
+		It(fmt.Sprintf("[%s] works with a valid config without IPAM", ver), func() {
 			var origLink netlink.Link
 
 			// prepare ifname in original namespace
@@ -252,16 +324,13 @@ var _ = Describe("base functionality", func() {
 			})
 
 			// call CmdAdd
-		targetNS, err := testutils.NewNS()
-		Expect(err).NotTo(HaveOccurred())
-
 			cniName := "eth0"
 			conf := fmt.Sprintf(`{
 				"cniVersion": "%s",
 				"name": "cni-plugin-host-device-test",
 				"type": "host-device",
 				"device": %q
-		}`, current.ImplementedSpecVersion, ifname)
+			}`, ver, ifname)
 			args := &skel.CmdArgs{
 				ContainerID: "dummy",
 				Netns:       targetNS.Path(),
@@ -269,7 +338,7 @@ var _ = Describe("base functionality", func() {
 				StdinData:   []byte(conf),
 			}
 			var resI types.Result
-		err = originalNS.Do(func(ns.NetNS) error {
+			err := originalNS.Do(func(ns.NetNS) error {
 				defer GinkgoRecover()
 				var err error
 				resI, _, err = testutils.CmdAddWithArgs(args, func() error { return cmdAdd(args) })
@@ -278,15 +347,8 @@ var _ = Describe("base functionality", func() {
 			Expect(err).NotTo(HaveOccurred())
 
 			// check that the result was sane
-		res, err := current.NewResultFromResult(resI)
+			t := newTesterByVersion(ver)
-		Expect(err).NotTo(HaveOccurred())
+			t.expectInterfaces(resI, cniName, origLink.Attrs().HardwareAddr.String(), targetNS.Path())
-		Expect(res.Interfaces).To(Equal([]*current.Interface{
-			{
-				Name:    cniName,
-				Mac:     origLink.Attrs().HardwareAddr.String(),
-				Sandbox: targetNS.Path(),
-			},
-		}))
 
 			// assert that dummy0 is now in the target namespace
 			_ = targetNS.Do(func(ns.NetNS) error {
@@ -308,18 +370,18 @@ var _ = Describe("base functionality", func() {
 			// Check that deleting the device moves it back and restores the name
 			_ = originalNS.Do(func(ns.NetNS) error {
 				defer GinkgoRecover()
-			err = testutils.CmdDelWithArgs(args, func() error {
+				err := testutils.CmdDelWithArgs(args, func() error {
 					return cmdDel(args)
 				})
 				Expect(err).NotTo(HaveOccurred())
 
-			_, err := netlink.LinkByName(ifname)
+				_, err = netlink.LinkByName(ifname)
 				Expect(err).NotTo(HaveOccurred())
 				return nil
 			})
 		})
 
-	It("Test idempotence of CmdDel", func() {
+		It(fmt.Sprintf("[%s] ensures CmdDel is idempotent", ver), func() {
 			var (
 				origLink     netlink.Link
 				conflictLink netlink.Link
@@ -342,16 +404,13 @@ var _ = Describe("base functionality", func() {
 			})
 
 			// call CmdAdd
-		targetNS, err := testutils.NewNS()
-		Expect(err).NotTo(HaveOccurred())
-
 			cniName := "eth0"
 			conf := fmt.Sprintf(`{
 				"cniVersion": "%s",
 				"name": "cni-plugin-host-device-test",
 				"type": "host-device",
 				"device": %q
-		}`, current.ImplementedSpecVersion, ifname)
+			}`, ver, ifname)
 			args := &skel.CmdArgs{
 				ContainerID: "dummy",
 				Netns:       targetNS.Path(),
@@ -359,7 +418,7 @@ var _ = Describe("base functionality", func() {
 				StdinData:   []byte(conf),
 			}
 			var resI types.Result
-		err = originalNS.Do(func(ns.NetNS) error {
+			err := originalNS.Do(func(ns.NetNS) error {
 				defer GinkgoRecover()
 				var err error
 				resI, _, err = testutils.CmdAddWithArgs(args, func() error { return cmdAdd(args) })
@@ -367,16 +426,9 @@ var _ = Describe("base functionality", func() {
 			})
 			Expect(err).NotTo(HaveOccurred())
 
-		// check that the result is sane
+			// check that the result was sane
-		res, err := current.NewResultFromResult(resI)
+			t := newTesterByVersion(ver)
-		Expect(err).NotTo(HaveOccurred())
+			t.expectInterfaces(resI, cniName, origLink.Attrs().HardwareAddr.String(), targetNS.Path())
-		Expect(res.Interfaces).To(Equal([]*current.Interface{
-			{
-				Name:    cniName,
-				Mac:     origLink.Attrs().HardwareAddr.String(),
-				Sandbox: targetNS.Path(),
-			},
-		}))
 
 			// assert that dummy0 is now in the target namespace
 			_ = targetNS.Do(func(ns.NetNS) error {
@@ -457,7 +509,7 @@ var _ = Describe("base functionality", func() {
 			})
 		})
 
-	It("Works with a valid config with IPAM", func() {
+		It(fmt.Sprintf("Works with a valid %s config with IPAM", ver), func() {
 			var origLink netlink.Link
 
 			// prepare ifname in original namespace
@@ -477,8 +529,6 @@ var _ = Describe("base functionality", func() {
 			})
 
 			// call CmdAdd
-		targetNS, err := testutils.NewNS()
-		Expect(err).NotTo(HaveOccurred())
 			targetIP := "10.10.0.1/24"
 			cniName := "eth0"
 			conf := fmt.Sprintf(`{
@@ -494,7 +544,7 @@ var _ = Describe("base functionality", func() {
 					}]
 				},
 				"device": %q
-		}`, current.ImplementedSpecVersion, ifname)
+			}`, ver, ifname)
 			args := &skel.CmdArgs{
 				ContainerID: "dummy",
 				Netns:       targetNS.Path(),
@@ -502,7 +552,7 @@ var _ = Describe("base functionality", func() {
 				StdinData:   []byte(conf),
 			}
 			var resI types.Result
-		err = originalNS.Do(func(ns.NetNS) error {
+			err := originalNS.Do(func(ns.NetNS) error {
 				defer GinkgoRecover()
 				var err error
 				resI, _, err = testutils.CmdAddWithArgs(args, func() error { return cmdAdd(args) })
@@ -511,15 +561,8 @@ var _ = Describe("base functionality", func() {
 			Expect(err).NotTo(HaveOccurred())
 
 			// check that the result was sane
-		res, err := current.NewResultFromResult(resI)
+			t := newTesterByVersion(ver)
-		Expect(err).NotTo(HaveOccurred())
+			t.expectInterfaces(resI, cniName, origLink.Attrs().HardwareAddr.String(), targetNS.Path())
-		Expect(res.Interfaces).To(Equal([]*current.Interface{
-			{
-				Name:    cniName,
-				Mac:     origLink.Attrs().HardwareAddr.String(),
-				Sandbox: targetNS.Path(),
-			},
-		}))
 
 			// assert that dummy0 is now in the target namespace
 			_ = targetNS.Do(func(ns.NetNS) error {
@@ -560,12 +603,12 @@ var _ = Describe("base functionality", func() {
 			})
 		})
 
-	It("fails an invalid config", func() {
+		It(fmt.Sprintf("[%s] fails an invalid config", ver), func() {
 			conf := fmt.Sprintf(`{
 				"cniVersion": "%s",
 				"name": "cni-plugin-host-device-test",
 				"type": "host-device"
-		}`, current.ImplementedSpecVersion)
+			}`, ver)
 
 			args := &skel.CmdArgs{
 				ContainerID: "dummy",
@@ -578,7 +621,7 @@ var _ = Describe("base functionality", func() {
 
 		})
 
-	It("Works with a valid 0.4.0 config without IPAM", func() {
+		It(fmt.Sprintf("[%s] works with a valid config without IPAM", ver), func() {
 			var origLink netlink.Link
 
 			// prepare ifname in original namespace
@@ -598,16 +641,13 @@ var _ = Describe("base functionality", func() {
 			})
 
 			// call CmdAdd
-		targetNS, err := testutils.NewNS()
-		Expect(err).NotTo(HaveOccurred())
-
 			cniName := "eth0"
 			conf := fmt.Sprintf(`{
-			"cniVersion": "0.4.0",
+				"cniVersion": "%s",
 				"name": "cni-plugin-host-device-test",
 				"type": "host-device",
 				"device": %q
-		}`, ifname)
+			}`, ver, ifname)
 			args := &skel.CmdArgs{
 				ContainerID: "dummy",
 				Netns:       targetNS.Path(),
@@ -615,7 +655,7 @@ var _ = Describe("base functionality", func() {
 				StdinData:   []byte(conf),
 			}
 			var resI types.Result
-		err = originalNS.Do(func(ns.NetNS) error {
+			err := originalNS.Do(func(ns.NetNS) error {
 				defer GinkgoRecover()
 				var err error
 				resI, _, err = testutils.CmdAddWithArgs(args, func() error { return cmdAdd(args) })
@@ -624,15 +664,8 @@ var _ = Describe("base functionality", func() {
 			Expect(err).NotTo(HaveOccurred())
 
 			// check that the result was sane
-		res, err := types040.GetResult(resI)
+			t := newTesterByVersion(ver)
-		Expect(err).NotTo(HaveOccurred())
+			t.expectInterfaces(resI, cniName, origLink.Attrs().HardwareAddr.String(), targetNS.Path())
-		Expect(res.Interfaces).To(Equal([]*types040.Interface{
-			{
-				Name:    cniName,
-				Mac:     origLink.Attrs().HardwareAddr.String(),
-				Sandbox: targetNS.Path(),
-			},
-		}))
 
 			// assert that dummy0 is now in the target namespace
 			_ = targetNS.Do(func(ns.NetNS) error {
@@ -651,13 +684,13 @@ var _ = Describe("base functionality", func() {
 				return nil
 			})
 
+			if testutils.SpecVersionHasCHECK(ver) {
 				// call CmdCheck
 				n := &Net{}
 				err = json.Unmarshal([]byte(conf), &n)
 				Expect(err).NotTo(HaveOccurred())
 
-		cniVersion := "0.4.0"
+				newConf, err := buildOneConfig("testConfig", ver, n, resI)
-		newConf, err := buildOneConfig("testConfig", cniVersion, n, res)
 				Expect(err).NotTo(HaveOccurred())
 
 				confString, err := json.Marshal(newConf)
@@ -669,11 +702,10 @@ var _ = Describe("base functionality", func() {
 
 				err = originalNS.Do(func(ns.NetNS) error {
 					defer GinkgoRecover()
-			var err error
+					return testutils.CmdCheckWithArgs(args, func() error { return cmdCheck(args) })
-			err = testutils.CmdCheckWithArgs(args, func() error { return cmdCheck(args) })
-			return err
 				})
 				Expect(err).NotTo(HaveOccurred())
+			}
 
 			// Check that deleting the device moves it back and restores the name
 			_ = originalNS.Do(func(ns.NetNS) error {
@@ -689,7 +721,7 @@ var _ = Describe("base functionality", func() {
 			})
 		})
 
-	It("Works with a valid 0.4.0 config with IPAM", func() {
+		It(fmt.Sprintf("Works with a valid %s config with IPAM", ver), func() {
 			var origLink netlink.Link
 
 			// prepare ifname in original namespace
@@ -709,12 +741,10 @@ var _ = Describe("base functionality", func() {
 			})
 
 			// call CmdAdd
-		targetNS, err := testutils.NewNS()
-		Expect(err).NotTo(HaveOccurred())
 			targetIP := "10.10.0.1/24"
 			cniName := "eth0"
 			conf := fmt.Sprintf(`{
-			"cniVersion": "0.4.0",
+				"cniVersion": "%s",
 				"name": "cni-plugin-host-device-test",
 				"type": "host-device",
 				"ipam": {
@@ -726,7 +756,7 @@ var _ = Describe("base functionality", func() {
 					}]
 				},
 				"device": %q
-		}`, ifname)
+			}`, ver, ifname)
 			args := &skel.CmdArgs{
 				ContainerID: "dummy",
 				Netns:       targetNS.Path(),
@@ -734,7 +764,7 @@ var _ = Describe("base functionality", func() {
 				StdinData:   []byte(conf),
 			}
 			var resI types.Result
-		err = originalNS.Do(func(ns.NetNS) error {
+			err := originalNS.Do(func(ns.NetNS) error {
 				defer GinkgoRecover()
 				var err error
 				resI, _, err = testutils.CmdAddWithArgs(args, func() error { return cmdAdd(args) })
@@ -743,15 +773,8 @@ var _ = Describe("base functionality", func() {
 			Expect(err).NotTo(HaveOccurred())
 
 			// check that the result was sane
-		res, err := types040.GetResult(resI)
+			t := newTesterByVersion(ver)
-		Expect(err).NotTo(HaveOccurred())
+			t.expectInterfaces(resI, cniName, origLink.Attrs().HardwareAddr.String(), targetNS.Path())
-		Expect(res.Interfaces).To(Equal([]*types040.Interface{
-			{
-				Name:    cniName,
-				Mac:     origLink.Attrs().HardwareAddr.String(),
-				Sandbox: targetNS.Path(),
-			},
-		}))
 
 			// assert that dummy0 is now in the target namespace
 			_ = targetNS.Do(func(ns.NetNS) error {
@@ -786,8 +809,8 @@ var _ = Describe("base functionality", func() {
 			n.IPAM, _, err = LoadIPAMConfig([]byte(conf), "")
 			Expect(err).NotTo(HaveOccurred())
 
-		cniVersion := "0.4.0"
+			if testutils.SpecVersionHasCHECK(ver) {
-		newConf, err := buildOneConfig("testConfig", cniVersion, n, res)
+				newConf, err := buildOneConfig("testConfig", ver, n, resI)
 				Expect(err).NotTo(HaveOccurred())
 
 				confString, err := json.Marshal(newConf)
@@ -799,11 +822,10 @@ var _ = Describe("base functionality", func() {
 
 				err = originalNS.Do(func(ns.NetNS) error {
 					defer GinkgoRecover()
-			var err error
+					return testutils.CmdCheckWithArgs(args, func() error { return cmdCheck(args) })
-			err = testutils.CmdCheckWithArgs(args, func() error { return cmdCheck(args) })
-			return err
 				})
 				Expect(err).NotTo(HaveOccurred())
+			}
 
 			// Check that deleting the device moves it back and restores the name
 			_ = originalNS.Do(func(ns.NetNS) error {
@@ -819,7 +841,7 @@ var _ = Describe("base functionality", func() {
 			})
 		})
 
-	It("Test idempotence of CmdDel with 0.4.0 config", func() {
+		It(fmt.Sprintf("Test idempotence of CmdDel with %s config", ver), func() {
 			var (
 				origLink     netlink.Link
 				conflictLink netlink.Link
@@ -842,16 +864,13 @@ var _ = Describe("base functionality", func() {
 			})
 
 			// call CmdAdd
-		targetNS, err := testutils.NewNS()
-		Expect(err).NotTo(HaveOccurred())
-
 			cniName := "eth0"
 			conf := fmt.Sprintf(`{
-			"cniVersion": "0.4.0",
+				"cniVersion": "%s",
 				"name": "cni-plugin-host-device-test",
 				"type": "host-device",
 				"device": %q
-		}`, ifname)
+			}`, ver, ifname)
 			args := &skel.CmdArgs{
 				ContainerID: "dummy",
 				Netns:       targetNS.Path(),
@@ -859,7 +878,7 @@ var _ = Describe("base functionality", func() {
 				StdinData:   []byte(conf),
 			}
 			var resI types.Result
-		err = originalNS.Do(func(ns.NetNS) error {
+			err := originalNS.Do(func(ns.NetNS) error {
 				defer GinkgoRecover()
 				var err error
 				resI, _, err = testutils.CmdAddWithArgs(args, func() error { return cmdAdd(args) })
@@ -867,16 +886,9 @@ var _ = Describe("base functionality", func() {
 			})
 			Expect(err).NotTo(HaveOccurred())
 
-		// check that the result is sane
+			// check that the result was sane
-		res, err := types040.GetResult(resI)
+			t := newTesterByVersion(ver)
-		Expect(err).NotTo(HaveOccurred())
+			t.expectInterfaces(resI, cniName, origLink.Attrs().HardwareAddr.String(), targetNS.Path())
-		Expect(res.Interfaces).To(Equal([]*types040.Interface{
-			{
-				Name:    cniName,
-				Mac:     origLink.Attrs().HardwareAddr.String(),
-				Sandbox: targetNS.Path(),
-			},
-		}))
 
 			// assert that dummy0 is now in the target namespace
 			_ = targetNS.Do(func(ns.NetNS) error {
@@ -957,4 +969,5 @@ var _ = Describe("base functionality", func() {
 				return nil
 			})
 		})
+	}
 })