From 536cb5b99b8a83f83b6efc13e5fdee36ad82c8cb Mon Sep 17 00:00:00 2001 From: Gabe Rosenhouse Date: Sun, 21 Aug 2016 23:48:04 -0700 Subject: [PATCH 1/8] versioning: plugins report a list of supported versions Further progress on versioning support (Issue #266). Bump CNI spec version to 0.3.0 --- SPEC.md | 20 ++++++--- libcni/api.go | 12 ++++++ libcni/api_test.go | 19 ++++++++ pkg/invoke/exec.go | 10 +++++ pkg/skel/skel.go | 26 ++++++----- pkg/skel/skel_test.go | 45 ++++++++++--------- pkg/version/version.go | 62 +++++++++++++++++++++----- pkg/version/version_suite_test.go | 27 ++++++++++++ pkg/version/version_test.go | 72 +++++++++++++++++++++++++++++++ plugins/ipam/dhcp/main.go | 3 +- plugins/ipam/host-local/main.go | 3 +- plugins/main/bridge/bridge.go | 3 +- plugins/main/ipvlan/ipvlan.go | 3 +- plugins/main/loopback/loopback.go | 3 +- plugins/main/macvlan/macvlan.go | 3 +- plugins/main/ptp/ptp.go | 3 +- plugins/meta/flannel/flannel.go | 3 +- plugins/meta/tuning/tuning.go | 3 +- plugins/test/noop/main.go | 4 +- test | 2 +- 20 files changed, 262 insertions(+), 64 deletions(-) create mode 100644 pkg/version/version_suite_test.go create mode 100644 pkg/version/version_test.go diff --git a/SPEC.md b/SPEC.md index cf752226..91c85d30 100644 --- a/SPEC.md +++ b/SPEC.md @@ -64,8 +64,14 @@ The operations that the CNI plugin needs to support are: - Report version - Parameters: NONE. - - Result: - - The version of the CNI spec implemented by the plugin: `{ "cniVersion": "0.2.0" }` + - Result: information about the CNI spec versions supported by the plugin + + ``` + { + "cniVersion": "0.3.0", // the version of the CNI spec in use for this output + "supportedVersions": [ "0.1.0", "0.2.0", "0.3.0" ] // the list of CNI spec versions that this plugin supports + } + ``` The executable command-line API uses the type of network (see [Network Configuration](#network-configuration) below) as the name of the executable to invoke. It will then look for this executable in a list of predefined directories. Once found, it will invoke the executable using the following environment variables for argument passing: @@ -85,7 +91,7 @@ Success is indicated by a return code of zero and the following JSON printed to ``` { - "cniVersion": "0.2.0", + "cniVersion": "0.3.0", "ip4": { "ip": , "gateway": , (optional) @@ -114,7 +120,7 @@ Examples include generating an `/etc/resolv.conf` file to be injected into the c Errors are indicated by a non-zero return code and the following JSON being printed to stdout: ``` { - "cniVersion": "0.2.0", + "cniVersion": "0.3.0", "code": , "msg": , "details": (optional) @@ -151,7 +157,7 @@ Plugins may define additional fields that they accept and may generate an error ```json { - "cniVersion": "0.2.0", + "cniVersion": "0.3.0", "name": "dbnet", "type": "bridge", // type (plugin) specific @@ -170,7 +176,7 @@ Plugins may define additional fields that they accept and may generate an error ```json { - "cniVersion": "0.2.0", + "cniVersion": "0.3.0", "name": "pci", "type": "ovs", // type (plugin) specific @@ -220,7 +226,7 @@ Success is indicated by a zero return code and the following JSON being printed ``` { - "cniVersion": "0.2.0", + "cniVersion": "0.3.0", "ip4": { "ip": , "gateway": , (optional) diff --git a/libcni/api.go b/libcni/api.go index 340a20cc..6e616da6 100644 --- a/libcni/api.go +++ b/libcni/api.go @@ -19,6 +19,7 @@ import ( "github.com/containernetworking/cni/pkg/invoke" "github.com/containernetworking/cni/pkg/types" + "github.com/containernetworking/cni/pkg/version" ) type RuntimeConf struct { @@ -60,6 +61,17 @@ func (c *CNIConfig) DelNetwork(net *NetworkConfig, rt *RuntimeConf) error { return invoke.ExecPluginWithoutResult(pluginPath, net.Bytes, c.args("DEL", rt)) } +func (c *CNIConfig) GetVersionInfo(pluginType string) (version.PluginInfo, error) { + pluginPath, err := invoke.FindInPath(pluginType, c.Path) + if err != nil { + return nil, err + } + + // TODO: if error is because plugin is old and VERSION command is unrecognized + // then do the right thing and return version.PluginSupports("0.1.0"), nil + return invoke.ExecPluginForVersion(pluginPath) +} + // ===== func (c *CNIConfig) args(action string, rt *RuntimeConf) *invoke.Args { return &invoke.Args{ diff --git a/libcni/api_test.go b/libcni/api_test.go index 2df6b4e3..5e0dd0fe 100644 --- a/libcni/api_test.go +++ b/libcni/api_test.go @@ -155,4 +155,23 @@ var _ = Describe("Invoking the plugin", func() { }) }) }) + + Describe("GetVersionInfo", func() { + It("executes the plugin with the command VERSION", func() { + versionInfo, err := cniConfig.GetVersionInfo("noop") + Expect(err).NotTo(HaveOccurred()) + + Expect(versionInfo).NotTo(BeNil()) + Expect(versionInfo.SupportedVersions()).To(Equal([]string{ + "0.-42.0", "0.1.0", "0.2.0", "0.3.0", + })) + }) + + Context("when finding the plugin fails", func() { + It("returns the error", func() { + _, err := cniConfig.GetVersionInfo("does-not-exist") + Expect(err).To(MatchError(ContainSubstring(`failed to find plugin "does-not-exist"`))) + }) + }) + }) }) diff --git a/pkg/invoke/exec.go b/pkg/invoke/exec.go index d7e38f21..3d32ab7c 100644 --- a/pkg/invoke/exec.go +++ b/pkg/invoke/exec.go @@ -23,6 +23,7 @@ import ( "os/exec" "github.com/containernetworking/cni/pkg/types" + "github.com/containernetworking/cni/pkg/version" ) func pluginErr(err error, output []byte) error { @@ -57,6 +58,15 @@ func ExecPluginWithoutResult(pluginPath string, netconf []byte, args CNIArgs) er return err } +func ExecPluginForVersion(pluginPath string) (version.PluginInfo, error) { + stdoutBytes, err := execPlugin(pluginPath, nil, &Args{Command: "VERSION"}) + if err != nil { + return nil, err + } + + return version.Decode(stdoutBytes) +} + func execPlugin(pluginPath string, netconf []byte, args CNIArgs) ([]byte, error) { return defaultRawExec.ExecPlugin(pluginPath, netconf, args.AsEnv()) } diff --git a/pkg/skel/skel.go b/pkg/skel/skel.go index 19ddf249..de64d7dd 100644 --- a/pkg/skel/skel.go +++ b/pkg/skel/skel.go @@ -39,11 +39,10 @@ type CmdArgs struct { } type dispatcher struct { - Getenv func(string) string - Stdin io.Reader - Stdout io.Writer - Stderr io.Writer - Versioner version.PluginVersioner + Getenv func(string) string + Stdin io.Reader + Stdout io.Writer + Stderr io.Writer } type reqForCmdEntry map[string]bool @@ -144,7 +143,7 @@ func createTypedError(f string, args ...interface{}) *types.Error { } } -func (t *dispatcher) pluginMain(cmdAdd, cmdDel func(_ *CmdArgs) error) *types.Error { +func (t *dispatcher) pluginMain(cmdAdd, cmdDel func(_ *CmdArgs) error, versionInfo version.PluginInfo) *types.Error { cmd, cmdArgs, err := t.getCmdArgsFromEnv() if err != nil { return createTypedError(err.Error()) @@ -158,7 +157,7 @@ func (t *dispatcher) pluginMain(cmdAdd, cmdDel func(_ *CmdArgs) error) *types.Er err = cmdDel(cmdArgs) case "VERSION": - err = t.Versioner.Encode(t.Stdout) + err = versionInfo.Encode(t.Stdout) default: return createTypedError("unknown CNI_COMMAND: %v", cmd) @@ -176,16 +175,15 @@ func (t *dispatcher) pluginMain(cmdAdd, cmdDel func(_ *CmdArgs) error) *types.Er // PluginMain is the "main" for a plugin. It accepts // two callback functions for add and del commands. -func PluginMain(cmdAdd, cmdDel func(_ *CmdArgs) error) { +func PluginMain(cmdAdd, cmdDel func(_ *CmdArgs) error, versionInfo version.PluginInfo) { caller := dispatcher{ - Getenv: os.Getenv, - Stdin: os.Stdin, - Stdout: os.Stdout, - Stderr: os.Stderr, - Versioner: version.DefaultPluginVersioner, + Getenv: os.Getenv, + Stdin: os.Stdin, + Stdout: os.Stdout, + Stderr: os.Stderr, } - err := caller.pluginMain(cmdAdd, cmdDel) + err := caller.pluginMain(cmdAdd, cmdDel, versionInfo) if err != nil { dieErr(err) } diff --git a/pkg/skel/skel_test.go b/pkg/skel/skel_test.go index e6304f5e..0431abbb 100644 --- a/pkg/skel/skel_test.go +++ b/pkg/skel/skel_test.go @@ -53,6 +53,7 @@ var _ = Describe("dispatching to the correct callback", func() { cmdAdd, cmdDel *fakeCmd dispatch *dispatcher expectedCmdArgs *CmdArgs + versionInfo version.PluginInfo ) BeforeEach(func() { @@ -67,13 +68,12 @@ var _ = Describe("dispatching to the correct callback", func() { stdin = strings.NewReader(`{ "some": "config" }`) stdout = &bytes.Buffer{} stderr = &bytes.Buffer{} - versioner := &version.BasicVersioner{CNIVersion: "9.8.7"} + versionInfo = version.PluginSupports("9.8.7") dispatch = &dispatcher{ - Getenv: func(key string) string { return environment[key] }, - Stdin: stdin, - Stdout: stdout, - Stderr: stderr, - Versioner: versioner, + Getenv: func(key string) string { return environment[key] }, + Stdin: stdin, + Stdout: stdout, + Stderr: stderr, } cmdAdd = &fakeCmd{} cmdDel = &fakeCmd{} @@ -90,7 +90,7 @@ var _ = Describe("dispatching to the correct callback", func() { var envVarChecker = func(envVar string, isRequired bool) { delete(environment, envVar) - err := dispatch.pluginMain(cmdAdd.Func, cmdDel.Func) + err := dispatch.pluginMain(cmdAdd.Func, cmdDel.Func, versionInfo) if isRequired { Expect(err).To(Equal(&types.Error{ Code: 100, @@ -104,7 +104,7 @@ var _ = Describe("dispatching to the correct callback", func() { Context("when the CNI_COMMAND is ADD", func() { It("extracts env vars and stdin data and calls cmdAdd", func() { - err := dispatch.pluginMain(cmdAdd.Func, cmdDel.Func) + err := dispatch.pluginMain(cmdAdd.Func, cmdDel.Func, versionInfo) Expect(err).NotTo(HaveOccurred()) Expect(cmdAdd.CallCount).To(Equal(1)) @@ -113,7 +113,7 @@ var _ = Describe("dispatching to the correct callback", func() { }) It("does not call cmdDel", func() { - err := dispatch.pluginMain(cmdAdd.Func, cmdDel.Func) + err := dispatch.pluginMain(cmdAdd.Func, cmdDel.Func, versionInfo) Expect(err).NotTo(HaveOccurred()) Expect(cmdDel.CallCount).To(Equal(0)) @@ -136,7 +136,7 @@ var _ = Describe("dispatching to the correct callback", func() { }) It("reports that all of them are missing, not just the first", func() { - Expect(dispatch.pluginMain(cmdAdd.Func, cmdDel.Func)).NotTo(Succeed()) + Expect(dispatch.pluginMain(cmdAdd.Func, cmdDel.Func, versionInfo)).NotTo(Succeed()) log := stderr.String() Expect(log).To(ContainSubstring("CNI_NETNS env variable missing\n")) Expect(log).To(ContainSubstring("CNI_IFNAME env variable missing\n")) @@ -152,7 +152,7 @@ var _ = Describe("dispatching to the correct callback", func() { }) It("calls cmdDel with the env vars and stdin data", func() { - err := dispatch.pluginMain(cmdAdd.Func, cmdDel.Func) + err := dispatch.pluginMain(cmdAdd.Func, cmdDel.Func, versionInfo) Expect(err).NotTo(HaveOccurred()) Expect(cmdDel.CallCount).To(Equal(1)) @@ -160,7 +160,7 @@ var _ = Describe("dispatching to the correct callback", func() { }) It("does not call cmdAdd", func() { - err := dispatch.pluginMain(cmdAdd.Func, cmdDel.Func) + err := dispatch.pluginMain(cmdAdd.Func, cmdDel.Func, versionInfo) Expect(err).NotTo(HaveOccurred()) Expect(cmdAdd.CallCount).To(Equal(0)) @@ -182,14 +182,17 @@ var _ = Describe("dispatching to the correct callback", func() { }) It("prints the version to stdout", func() { - err := dispatch.pluginMain(cmdAdd.Func, cmdDel.Func) + err := dispatch.pluginMain(cmdAdd.Func, cmdDel.Func, versionInfo) Expect(err).NotTo(HaveOccurred()) - Expect(stdout).To(MatchJSON(`{ "cniVersion": "9.8.7" }`)) + Expect(stdout).To(MatchJSON(`{ + "cniVersion": "0.3.0", + "supportedVersions": ["9.8.7"] + }`)) }) It("does not call cmdAdd or cmdDel", func() { - err := dispatch.pluginMain(cmdAdd.Func, cmdDel.Func) + err := dispatch.pluginMain(cmdAdd.Func, cmdDel.Func, versionInfo) Expect(err).NotTo(HaveOccurred()) Expect(cmdAdd.CallCount).To(Equal(0)) @@ -212,14 +215,14 @@ var _ = Describe("dispatching to the correct callback", func() { }) It("does not call any cmd callback", func() { - dispatch.pluginMain(cmdAdd.Func, cmdDel.Func) + dispatch.pluginMain(cmdAdd.Func, cmdDel.Func, versionInfo) Expect(cmdAdd.CallCount).To(Equal(0)) Expect(cmdDel.CallCount).To(Equal(0)) }) It("returns an error", func() { - err := dispatch.pluginMain(cmdAdd.Func, cmdDel.Func) + err := dispatch.pluginMain(cmdAdd.Func, cmdDel.Func, versionInfo) Expect(err).To(Equal(&types.Error{ Code: 100, @@ -234,14 +237,14 @@ var _ = Describe("dispatching to the correct callback", func() { }) It("does not call any cmd callback", func() { - dispatch.pluginMain(cmdAdd.Func, cmdDel.Func) + dispatch.pluginMain(cmdAdd.Func, cmdDel.Func, versionInfo) Expect(cmdAdd.CallCount).To(Equal(0)) Expect(cmdDel.CallCount).To(Equal(0)) }) It("wraps and returns the error", func() { - err := dispatch.pluginMain(cmdAdd.Func, cmdDel.Func) + err := dispatch.pluginMain(cmdAdd.Func, cmdDel.Func, versionInfo) Expect(err).To(Equal(&types.Error{ Code: 100, @@ -260,7 +263,7 @@ var _ = Describe("dispatching to the correct callback", func() { }) It("returns the error as-is", func() { - err := dispatch.pluginMain(cmdAdd.Func, cmdDel.Func) + err := dispatch.pluginMain(cmdAdd.Func, cmdDel.Func, versionInfo) Expect(err).To(Equal(&types.Error{ Code: 1234, @@ -275,7 +278,7 @@ var _ = Describe("dispatching to the correct callback", func() { }) It("wraps and returns the error", func() { - err := dispatch.pluginMain(cmdAdd.Func, cmdDel.Func) + err := dispatch.pluginMain(cmdAdd.Func, cmdDel.Func, versionInfo) Expect(err).To(Equal(&types.Error{ Code: 100, diff --git a/pkg/version/version.go b/pkg/version/version.go index 2cb075fb..72b6e9b8 100644 --- a/pkg/version/version.go +++ b/pkg/version/version.go @@ -16,27 +16,67 @@ package version import ( "encoding/json" + "fmt" "io" ) -// A PluginVersioner can encode information about its version -type PluginVersioner interface { +// Current reports the version of the CNI spec implemented by this library +func Current() string { + return "0.3.0" +} + +// PluginInfo reports information about CNI versioning +type PluginInfo interface { + // SupportedVersions returns one or more CNI spec versions that the plugin + // supports. If input is provided in one of these versions, then the plugin + // promises to use the same CNI version in its response + SupportedVersions() []string + + // Encode writes this CNI version information as JSON to the given Writer Encode(io.Writer) error } -// BasicVersioner is a PluginVersioner which reports a single cniVersion string -type BasicVersioner struct { - CNIVersion string `json:"cniVersion"` +type simple struct { + CNIVersion_ string `json:"cniVersion"` + SupportedVersions_ []string `json:"supportedVersions,omitempty"` } -func (p *BasicVersioner) Encode(w io.Writer) error { +func (p *simple) Encode(w io.Writer) error { return json.NewEncoder(w).Encode(p) } -// Current reports the version of the CNI spec implemented by this library -func Current() string { - return "0.2.0" +func (p *simple) SupportedVersions() []string { + return p.SupportedVersions_ } -// DefaultPluginVersioner reports the Current library spec version as the cniVersion -var DefaultPluginVersioner = &BasicVersioner{CNIVersion: Current()} +// PluginSupports returns a new PluginInfo that will report the given versions +// as supported +func PluginSupports(supportedVersions ...string) PluginInfo { + if len(supportedVersions) < 1 { + panic("programmer error: you must support at least one version") + } + return &simple{ + CNIVersion_: Current(), + SupportedVersions_: supportedVersions, + } +} + +func Decode(jsonBytes []byte) (PluginInfo, error) { + var info simple + err := json.Unmarshal(jsonBytes, &info) + if err != nil { + return nil, fmt.Errorf("decoding version info: %s", err) + } + if info.CNIVersion_ == "" { + return nil, fmt.Errorf("decoding version info: missing field cniVersion") + } + if len(info.SupportedVersions_) == 0 { + if info.CNIVersion_ == "0.2.0" { + return PluginSupports("0.1.0", "0.2.0"), nil + } + return nil, fmt.Errorf("decoding version info: missing field supportedVersions") + } + return &info, nil +} + +var Legacy = PluginSupports("0.1.0", "0.2.0", "0.3.0") diff --git a/pkg/version/version_suite_test.go b/pkg/version/version_suite_test.go new file mode 100644 index 00000000..25d503c8 --- /dev/null +++ b/pkg/version/version_suite_test.go @@ -0,0 +1,27 @@ +// Copyright 2016 CNI authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package version_test + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + "testing" +) + +func TestVersion(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Version Suite") +} diff --git a/pkg/version/version_test.go b/pkg/version/version_test.go new file mode 100644 index 00000000..08b89aed --- /dev/null +++ b/pkg/version/version_test.go @@ -0,0 +1,72 @@ +// Copyright 2016 CNI authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package version_test + +import ( + "github.com/containernetworking/cni/pkg/version" + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +var _ = Describe("Decode", func() { + It("returns a PluginInfo that represents the given json bytes", func() { + pluginInfo, err := version.Decode([]byte(`{ + "cniVersion": "some-library-version", + "supportedVersions": [ "some-version", "some-other-version" ] + }`)) + Expect(err).NotTo(HaveOccurred()) + Expect(pluginInfo).NotTo(BeNil()) + Expect(pluginInfo.SupportedVersions()).To(Equal([]string{ + "some-version", + "some-other-version", + })) + }) + + Context("when the bytes cannot be decoded as json", func() { + It("returns a meaningful error", func() { + _, err := version.Decode([]byte(`{{{`)) + Expect(err).To(MatchError("decoding version info: invalid character '{' looking for beginning of object key string")) + }) + }) + + Context("when the json bytes are missing the required CNIVersion field", func() { + It("returns a meaningful error", func() { + _, err := version.Decode([]byte(`{ "supportedVersions": [ "foo" ] }`)) + Expect(err).To(MatchError("decoding version info: missing field cniVersion")) + }) + }) + + Context("when there are no supported versions", func() { + Context("when the cniVersion is 0.2.0", func() { + It("infers the supported versions are 0.1.0 and 0.2.0", func() { + pluginInfo, err := version.Decode([]byte(`{ "cniVersion": "0.2.0" }`)) + Expect(err).NotTo(HaveOccurred()) + Expect(pluginInfo).NotTo(BeNil()) + Expect(pluginInfo.SupportedVersions()).To(Equal([]string{ + "0.1.0", + "0.2.0", + })) + }) + }) + + Context("when the cniVersion is >= 0.3.0", func() { + It("returns a meaningful error", func() { + _, err := version.Decode([]byte(`{ "cniVersion": "0.3.0" }`)) + Expect(err).To(MatchError("decoding version info: missing field supportedVersions")) + }) + }) + }) + +}) diff --git a/plugins/ipam/dhcp/main.go b/plugins/ipam/dhcp/main.go index b5378315..0e46af91 100644 --- a/plugins/ipam/dhcp/main.go +++ b/plugins/ipam/dhcp/main.go @@ -22,6 +22,7 @@ import ( "github.com/containernetworking/cni/pkg/skel" "github.com/containernetworking/cni/pkg/types" + "github.com/containernetworking/cni/pkg/version" ) const socketPath = "/run/cni/dhcp.sock" @@ -30,7 +31,7 @@ func main() { if len(os.Args) > 1 && os.Args[1] == "daemon" { runDaemon() } else { - skel.PluginMain(cmdAdd, cmdDel) + skel.PluginMain(cmdAdd, cmdDel, version.Legacy) } } diff --git a/plugins/ipam/host-local/main.go b/plugins/ipam/host-local/main.go index d2f3c305..0e1b0639 100644 --- a/plugins/ipam/host-local/main.go +++ b/plugins/ipam/host-local/main.go @@ -19,10 +19,11 @@ import ( "github.com/containernetworking/cni/pkg/skel" "github.com/containernetworking/cni/pkg/types" + "github.com/containernetworking/cni/pkg/version" ) func main() { - skel.PluginMain(cmdAdd, cmdDel) + skel.PluginMain(cmdAdd, cmdDel, version.Legacy) } func cmdAdd(args *skel.CmdArgs) error { diff --git a/plugins/main/bridge/bridge.go b/plugins/main/bridge/bridge.go index 2fef878d..bc173017 100644 --- a/plugins/main/bridge/bridge.go +++ b/plugins/main/bridge/bridge.go @@ -28,6 +28,7 @@ import ( "github.com/containernetworking/cni/pkg/skel" "github.com/containernetworking/cni/pkg/types" "github.com/containernetworking/cni/pkg/utils" + "github.com/containernetworking/cni/pkg/version" "github.com/vishvananda/netlink" ) @@ -354,5 +355,5 @@ func cmdDel(args *skel.CmdArgs) error { } func main() { - skel.PluginMain(cmdAdd, cmdDel) + skel.PluginMain(cmdAdd, cmdDel, version.Legacy) } diff --git a/plugins/main/ipvlan/ipvlan.go b/plugins/main/ipvlan/ipvlan.go index d7cfc39f..4c58109b 100644 --- a/plugins/main/ipvlan/ipvlan.go +++ b/plugins/main/ipvlan/ipvlan.go @@ -25,6 +25,7 @@ import ( "github.com/containernetworking/cni/pkg/ns" "github.com/containernetworking/cni/pkg/skel" "github.com/containernetworking/cni/pkg/types" + "github.com/containernetworking/cni/pkg/version" "github.com/vishvananda/netlink" ) @@ -171,5 +172,5 @@ func renameLink(curName, newName string) error { } func main() { - skel.PluginMain(cmdAdd, cmdDel) + skel.PluginMain(cmdAdd, cmdDel, version.Legacy) } diff --git a/plugins/main/loopback/loopback.go b/plugins/main/loopback/loopback.go index 186fd54c..1344c130 100644 --- a/plugins/main/loopback/loopback.go +++ b/plugins/main/loopback/loopback.go @@ -18,6 +18,7 @@ import ( "github.com/containernetworking/cni/pkg/ns" "github.com/containernetworking/cni/pkg/skel" "github.com/containernetworking/cni/pkg/types" + "github.com/containernetworking/cni/pkg/version" "github.com/vishvananda/netlink" ) @@ -67,5 +68,5 @@ func cmdDel(args *skel.CmdArgs) error { } func main() { - skel.PluginMain(cmdAdd, cmdDel) + skel.PluginMain(cmdAdd, cmdDel, version.Legacy) } diff --git a/plugins/main/macvlan/macvlan.go b/plugins/main/macvlan/macvlan.go index 63c9f7f0..ef012696 100644 --- a/plugins/main/macvlan/macvlan.go +++ b/plugins/main/macvlan/macvlan.go @@ -26,6 +26,7 @@ import ( "github.com/containernetworking/cni/pkg/skel" "github.com/containernetworking/cni/pkg/types" "github.com/containernetworking/cni/pkg/utils/sysctl" + "github.com/containernetworking/cni/pkg/version" "github.com/vishvananda/netlink" ) @@ -193,5 +194,5 @@ func renameLink(curName, newName string) error { } func main() { - skel.PluginMain(cmdAdd, cmdDel) + skel.PluginMain(cmdAdd, cmdDel, version.Legacy) } diff --git a/plugins/main/ptp/ptp.go b/plugins/main/ptp/ptp.go index 014b472c..a26b09ee 100644 --- a/plugins/main/ptp/ptp.go +++ b/plugins/main/ptp/ptp.go @@ -30,6 +30,7 @@ import ( "github.com/containernetworking/cni/pkg/skel" "github.com/containernetworking/cni/pkg/types" "github.com/containernetworking/cni/pkg/utils" + "github.com/containernetworking/cni/pkg/version" ) func init() { @@ -236,5 +237,5 @@ func cmdDel(args *skel.CmdArgs) error { } func main() { - skel.PluginMain(cmdAdd, cmdDel) + skel.PluginMain(cmdAdd, cmdDel, version.Legacy) } diff --git a/plugins/meta/flannel/flannel.go b/plugins/meta/flannel/flannel.go index 096fe6d6..334bd41b 100644 --- a/plugins/meta/flannel/flannel.go +++ b/plugins/meta/flannel/flannel.go @@ -32,6 +32,7 @@ import ( "github.com/containernetworking/cni/pkg/invoke" "github.com/containernetworking/cni/pkg/skel" "github.com/containernetworking/cni/pkg/types" + "github.com/containernetworking/cni/pkg/version" ) const ( @@ -249,5 +250,5 @@ func cmdDel(args *skel.CmdArgs) error { } func main() { - skel.PluginMain(cmdAdd, cmdDel) + skel.PluginMain(cmdAdd, cmdDel, version.Legacy) } diff --git a/plugins/meta/tuning/tuning.go b/plugins/meta/tuning/tuning.go index 75ba852c..98e92ec9 100644 --- a/plugins/meta/tuning/tuning.go +++ b/plugins/meta/tuning/tuning.go @@ -27,6 +27,7 @@ import ( "github.com/containernetworking/cni/pkg/ns" "github.com/containernetworking/cni/pkg/skel" "github.com/containernetworking/cni/pkg/types" + "github.com/containernetworking/cni/pkg/version" ) // TuningConf represents the network tuning configuration. @@ -78,5 +79,5 @@ func cmdDel(args *skel.CmdArgs) error { } func main() { - skel.PluginMain(cmdAdd, cmdDel) + skel.PluginMain(cmdAdd, cmdDel, version.Legacy) } diff --git a/plugins/test/noop/main.go b/plugins/test/noop/main.go index ddb1acae..1ebc7350 100644 --- a/plugins/test/noop/main.go +++ b/plugins/test/noop/main.go @@ -28,6 +28,7 @@ import ( "strings" "github.com/containernetworking/cni/pkg/skel" + "github.com/containernetworking/cni/pkg/version" "github.com/containernetworking/cni/plugins/test/noop/debug" ) @@ -71,5 +72,6 @@ func cmdDel(args *skel.CmdArgs) error { } func main() { - skel.PluginMain(cmdAdd, cmdDel) + skel.PluginMain(cmdAdd, cmdDel, + version.PluginSupports("0.-42.0", "0.1.0", "0.2.0", "0.3.0")) } diff --git a/test b/test index afb1c94b..7537b40f 100755 --- a/test +++ b/test @@ -11,7 +11,7 @@ set -e source ./build -TESTABLE="libcni plugins/ipam/dhcp plugins/ipam/host-local plugins/main/loopback pkg/invoke pkg/ns pkg/skel pkg/types pkg/utils plugins/main/ipvlan plugins/main/macvlan plugins/main/bridge plugins/main/ptp plugins/test/noop pkg/utils/hwaddr pkg/ip" +TESTABLE="libcni plugins/ipam/dhcp plugins/ipam/host-local plugins/main/loopback pkg/invoke pkg/ns pkg/skel pkg/types pkg/utils plugins/main/ipvlan plugins/main/macvlan plugins/main/bridge plugins/main/ptp plugins/test/noop pkg/utils/hwaddr pkg/ip pkg/version" FORMATTABLE="$TESTABLE pkg/testutils plugins/meta/flannel plugins/meta/tuning" # user has not provided PKG override From adf28a84c62fb0eb7f9949abd1f92dee729a6708 Mon Sep 17 00:00:00 2001 From: Gabe Rosenhouse Date: Mon, 29 Aug 2016 19:20:18 -0400 Subject: [PATCH 2/8] versioning: document meaning of 'Legacy' version support --- pkg/version/version.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pkg/version/version.go b/pkg/version/version.go index 72b6e9b8..62bf8bbb 100644 --- a/pkg/version/version.go +++ b/pkg/version/version.go @@ -79,4 +79,11 @@ func Decode(jsonBytes []byte) (PluginInfo, error) { return &info, nil } +// Legacy PluginInfo describes a plugin that is backwards compatible with the +// CNI spec version 0.1.0. In particular, a runtime compiled against the 0.1.0 +// library ought to work correctly with a plugin that reports support for +// Legacy versions. +// +// Any future CNI spec versions which meet this definition will be added to +// this list. var Legacy = PluginSupports("0.1.0", "0.2.0", "0.3.0") From dea1c6e44d23cbc569eef7e0d6566c01f329e182 Mon Sep 17 00:00:00 2001 From: Gabe Rosenhouse Date: Fri, 2 Sep 2016 13:12:14 -0400 Subject: [PATCH 3/8] pkg/invoke: refactor plugin exec and backfill unit tests --- pkg/invoke/exec.go | 85 ++++++--------- pkg/invoke/exec_test.go | 162 ++++++++++++++-------------- pkg/invoke/fakes/cni_args.go | 27 +++++ pkg/invoke/fakes/raw_exec.go | 36 +++++++ pkg/invoke/fakes/version_decoder.go | 34 ++++++ pkg/invoke/raw_exec.go | 63 +++++++++++ pkg/invoke/raw_exec_test.go | 123 +++++++++++++++++++++ pkg/version/version.go | 4 +- pkg/version/version_test.go | 16 ++- 9 files changed, 413 insertions(+), 137 deletions(-) create mode 100644 pkg/invoke/fakes/cni_args.go create mode 100644 pkg/invoke/fakes/raw_exec.go create mode 100644 pkg/invoke/fakes/version_decoder.go create mode 100644 pkg/invoke/raw_exec.go create mode 100644 pkg/invoke/raw_exec_test.go diff --git a/pkg/invoke/exec.go b/pkg/invoke/exec.go index 3d32ab7c..f95dec11 100644 --- a/pkg/invoke/exec.go +++ b/pkg/invoke/exec.go @@ -15,35 +15,41 @@ package invoke import ( - "bytes" "encoding/json" - "fmt" - "io" "os" - "os/exec" "github.com/containernetworking/cni/pkg/types" "github.com/containernetworking/cni/pkg/version" ) -func pluginErr(err error, output []byte) error { - if _, ok := err.(*exec.ExitError); ok { - emsg := types.Error{} - if perr := json.Unmarshal(output, &emsg); perr != nil { - return fmt.Errorf("netplugin failed but error parsing its diagnostic message %q: %v", string(output), perr) - } - details := "" - if emsg.Details != "" { - details = fmt.Sprintf("; %v", emsg.Details) - } - return fmt.Errorf("%v%v", emsg.Msg, details) - } - - return err +func ExecPluginWithResult(pluginPath string, netconf []byte, args CNIArgs) (*types.Result, error) { + return defaultPluginExec.WithResult(pluginPath, netconf, args) } -func ExecPluginWithResult(pluginPath string, netconf []byte, args CNIArgs) (*types.Result, error) { - stdoutBytes, err := execPlugin(pluginPath, netconf, args) +func ExecPluginWithoutResult(pluginPath string, netconf []byte, args CNIArgs) error { + return defaultPluginExec.WithoutResult(pluginPath, netconf, args) +} + +func ExecPluginForVersion(pluginPath string) (version.PluginInfo, error) { + return defaultPluginExec.GetVersion(pluginPath) +} + +var defaultPluginExec = &PluginExec{ + RawExec: &RawExec{Stderr: os.Stderr}, + VersionDecoder: &version.Decoder{}, +} + +type PluginExec struct { + RawExec interface { + ExecPlugin(pluginPath string, stdinData []byte, environ []string) ([]byte, error) + } + VersionDecoder interface { + Decode(jsonBytes []byte) (version.PluginInfo, error) + } +} + +func (e *PluginExec) WithResult(pluginPath string, netconf []byte, args CNIArgs) (*types.Result, error) { + stdoutBytes, err := e.RawExec.ExecPlugin(pluginPath, netconf, args.AsEnv()) if err != nil { return nil, err } @@ -53,44 +59,17 @@ func ExecPluginWithResult(pluginPath string, netconf []byte, args CNIArgs) (*typ return res, err } -func ExecPluginWithoutResult(pluginPath string, netconf []byte, args CNIArgs) error { - _, err := execPlugin(pluginPath, netconf, args) +func (e *PluginExec) WithoutResult(pluginPath string, netconf []byte, args CNIArgs) error { + _, err := e.RawExec.ExecPlugin(pluginPath, netconf, args.AsEnv()) return err } -func ExecPluginForVersion(pluginPath string) (version.PluginInfo, error) { - stdoutBytes, err := execPlugin(pluginPath, nil, &Args{Command: "VERSION"}) +func (e *PluginExec) GetVersion(pluginPath string) (version.PluginInfo, error) { + args := &Args{Command: "VERSION"} + stdoutBytes, err := e.RawExec.ExecPlugin(pluginPath, nil, args.AsEnv()) if err != nil { return nil, err } - return version.Decode(stdoutBytes) -} - -func execPlugin(pluginPath string, netconf []byte, args CNIArgs) ([]byte, error) { - return defaultRawExec.ExecPlugin(pluginPath, netconf, args.AsEnv()) -} - -var defaultRawExec = &RawExec{Stderr: os.Stderr} - -type RawExec struct { - Stderr io.Writer -} - -func (e *RawExec) ExecPlugin(pluginPath string, stdinData []byte, environ []string) ([]byte, error) { - stdout := &bytes.Buffer{} - - c := exec.Cmd{ - Env: environ, - Path: pluginPath, - Args: []string{pluginPath}, - Stdin: bytes.NewBuffer(stdinData), - Stdout: stdout, - Stderr: e.Stderr, - } - if err := c.Run(); err != nil { - return nil, pluginErr(err, stdout.Bytes()) - } - - return stdout.Bytes(), nil + return e.VersionDecoder.Decode(stdoutBytes) } diff --git a/pkg/invoke/exec_test.go b/pkg/invoke/exec_test.go index 7df60a11..bff3fb73 100644 --- a/pkg/invoke/exec_test.go +++ b/pkg/invoke/exec_test.go @@ -15,109 +15,115 @@ package invoke_test import ( - "bytes" - "io/ioutil" - "os" + "errors" "github.com/containernetworking/cni/pkg/invoke" - - noop_debug "github.com/containernetworking/cni/plugins/test/noop/debug" + "github.com/containernetworking/cni/pkg/invoke/fakes" + "github.com/containernetworking/cni/pkg/version" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" ) -var _ = Describe("RawExec", func() { +var _ = Describe("Executing a plugin", func() { var ( - debugFileName string - debug *noop_debug.Debug - environ []string - stdin []byte - execer *invoke.RawExec + pluginExec *invoke.PluginExec + rawExec *fakes.RawExec + versionDecoder *fakes.VersionDecoder + + pluginPath string + netconf []byte + cniargs *fakes.CNIArgs ) - const reportResult = `{ "some": "result" }` - BeforeEach(func() { - debugFile, err := ioutil.TempFile("", "cni_debug") - Expect(err).NotTo(HaveOccurred()) - Expect(debugFile.Close()).To(Succeed()) - debugFileName = debugFile.Name() + rawExec = &fakes.RawExec{} + rawExec.ExecPluginCall.Returns.ResultBytes = []byte(`{ "ip4": { "ip": "1.2.3.4/24" } }`) - debug = &noop_debug.Debug{ - ReportResult: reportResult, - ReportStderr: "some stderr message", + versionDecoder = &fakes.VersionDecoder{} + versionDecoder.DecodeCall.Returns.PluginInfo = version.PluginSupports("0.42.0") + + pluginExec = &invoke.PluginExec{ + RawExec: rawExec, + VersionDecoder: versionDecoder, } - Expect(debug.WriteDebug(debugFileName)).To(Succeed()) - - environ = []string{ - "CNI_COMMAND=ADD", - "CNI_CONTAINERID=some-container-id", - "CNI_ARGS=DEBUG=" + debugFileName, - "CNI_NETNS=/some/netns/path", - "CNI_PATH=/some/bin/path", - "CNI_IFNAME=some-eth0", - } - stdin = []byte(`{"some":"stdin-json"}`) - execer = &invoke.RawExec{} + pluginPath = "/some/plugin/path" + netconf = []byte(`{ "some": "stdin" }`) + cniargs = &fakes.CNIArgs{} + cniargs.AsEnvCall.Returns.Env = []string{"SOME=ENV"} }) - AfterEach(func() { - Expect(os.Remove(debugFileName)).To(Succeed()) - }) - - It("runs the plugin with the given stdin and environment", func() { - _, err := execer.ExecPlugin(pathToPlugin, stdin, environ) - Expect(err).NotTo(HaveOccurred()) - - debug, err := noop_debug.ReadDebug(debugFileName) - Expect(err).NotTo(HaveOccurred()) - Expect(debug.Command).To(Equal("ADD")) - Expect(debug.CmdArgs.StdinData).To(Equal(stdin)) - Expect(debug.CmdArgs.Netns).To(Equal("/some/netns/path")) - }) - - It("returns the resulting stdout as bytes", func() { - resultBytes, err := execer.ExecPlugin(pathToPlugin, stdin, environ) - Expect(err).NotTo(HaveOccurred()) - - Expect(resultBytes).To(BeEquivalentTo(reportResult)) - }) - - Context("when the Stderr writer is set", func() { - var stderrBuffer *bytes.Buffer - - BeforeEach(func() { - stderrBuffer = &bytes.Buffer{} - execer.Stderr = stderrBuffer - }) - - It("forwards any stderr bytes to the Stderr writer", func() { - _, err := execer.ExecPlugin(pathToPlugin, stdin, environ) + Describe("returning a result", func() { + It("unmarshals the result bytes into the Result type", func() { + result, err := pluginExec.WithResult(pluginPath, netconf, cniargs) Expect(err).NotTo(HaveOccurred()) + Expect(result.IP4.IP.IP.String()).To(Equal("1.2.3.4")) + }) - Expect(stderrBuffer.String()).To(Equal("some stderr message")) + It("passes its arguments through to the rawExec", func() { + pluginExec.WithResult(pluginPath, netconf, cniargs) + Expect(rawExec.ExecPluginCall.Received.PluginPath).To(Equal(pluginPath)) + Expect(rawExec.ExecPluginCall.Received.StdinData).To(Equal(netconf)) + Expect(rawExec.ExecPluginCall.Received.Environ).To(Equal([]string{"SOME=ENV"})) + }) + + Context("when the rawExec fails", func() { + BeforeEach(func() { + rawExec.ExecPluginCall.Returns.Error = errors.New("banana") + }) + It("returns the error", func() { + _, err := pluginExec.WithResult(pluginPath, netconf, cniargs) + Expect(err).To(MatchError("banana")) + }) }) }) - Context("when the plugin errors", func() { + Describe("without returning a result", func() { + It("passes its arguments through to the rawExec", func() { + pluginExec.WithoutResult(pluginPath, netconf, cniargs) + Expect(rawExec.ExecPluginCall.Received.PluginPath).To(Equal(pluginPath)) + Expect(rawExec.ExecPluginCall.Received.StdinData).To(Equal(netconf)) + Expect(rawExec.ExecPluginCall.Received.Environ).To(Equal([]string{"SOME=ENV"})) + }) + + Context("when the rawExec fails", func() { + BeforeEach(func() { + rawExec.ExecPluginCall.Returns.Error = errors.New("banana") + }) + It("returns the error", func() { + err := pluginExec.WithoutResult(pluginPath, netconf, cniargs) + Expect(err).To(MatchError("banana")) + }) + }) + }) + + Describe("discovering the plugin version", func() { BeforeEach(func() { - debug.ReportError = "banana" - Expect(debug.WriteDebug(debugFileName)).To(Succeed()) + rawExec.ExecPluginCall.Returns.ResultBytes = []byte(`{ "some": "version-info" }`) }) - It("wraps and returns the error", func() { - _, err := execer.ExecPlugin(pathToPlugin, stdin, environ) - Expect(err).To(HaveOccurred()) - Expect(err).To(MatchError("banana")) + It("execs the plugin with the command VERSION", func() { + pluginExec.GetVersion(pluginPath) + Expect(rawExec.ExecPluginCall.Received.PluginPath).To(Equal(pluginPath)) + Expect(rawExec.ExecPluginCall.Received.StdinData).To(BeNil()) + Expect(rawExec.ExecPluginCall.Received.Environ).To(ContainElement("CNI_COMMAND=VERSION")) }) - }) - Context("when the system is unable to execute the plugin", func() { - It("returns the error", func() { - _, err := execer.ExecPlugin("/tmp/some/invalid/plugin/path", stdin, environ) - Expect(err).To(HaveOccurred()) - Expect(err).To(MatchError(ContainSubstring("/tmp/some/invalid/plugin/path"))) + It("decodes and returns the version info", func() { + versionInfo, err := pluginExec.GetVersion(pluginPath) + Expect(err).NotTo(HaveOccurred()) + Expect(versionInfo.SupportedVersions()).To(Equal([]string{"0.42.0"})) + Expect(versionDecoder.DecodeCall.Received.JSONBytes).To(MatchJSON(`{ "some": "version-info" }`)) + }) + + Context("when the rawExec fails", func() { + BeforeEach(func() { + rawExec.ExecPluginCall.Returns.Error = errors.New("banana") + }) + It("returns the error", func() { + _, err := pluginExec.GetVersion(pluginPath) + Expect(err).To(MatchError("banana")) + }) }) }) }) diff --git a/pkg/invoke/fakes/cni_args.go b/pkg/invoke/fakes/cni_args.go new file mode 100644 index 00000000..5b1ba29e --- /dev/null +++ b/pkg/invoke/fakes/cni_args.go @@ -0,0 +1,27 @@ +// Copyright 2016 CNI authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package fakes + +type CNIArgs struct { + AsEnvCall struct { + Returns struct { + Env []string + } + } +} + +func (a *CNIArgs) AsEnv() []string { + return a.AsEnvCall.Returns.Env +} diff --git a/pkg/invoke/fakes/raw_exec.go b/pkg/invoke/fakes/raw_exec.go new file mode 100644 index 00000000..5432cdf7 --- /dev/null +++ b/pkg/invoke/fakes/raw_exec.go @@ -0,0 +1,36 @@ +// Copyright 2016 CNI authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package fakes + +type RawExec struct { + ExecPluginCall struct { + Received struct { + PluginPath string + StdinData []byte + Environ []string + } + Returns struct { + ResultBytes []byte + Error error + } + } +} + +func (e *RawExec) ExecPlugin(pluginPath string, stdinData []byte, environ []string) ([]byte, error) { + e.ExecPluginCall.Received.PluginPath = pluginPath + e.ExecPluginCall.Received.StdinData = stdinData + e.ExecPluginCall.Received.Environ = environ + return e.ExecPluginCall.Returns.ResultBytes, e.ExecPluginCall.Returns.Error +} diff --git a/pkg/invoke/fakes/version_decoder.go b/pkg/invoke/fakes/version_decoder.go new file mode 100644 index 00000000..72d29733 --- /dev/null +++ b/pkg/invoke/fakes/version_decoder.go @@ -0,0 +1,34 @@ +// Copyright 2016 CNI authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package fakes + +import "github.com/containernetworking/cni/pkg/version" + +type VersionDecoder struct { + DecodeCall struct { + Received struct { + JSONBytes []byte + } + Returns struct { + PluginInfo version.PluginInfo + Error error + } + } +} + +func (e *VersionDecoder) Decode(jsonData []byte) (version.PluginInfo, error) { + e.DecodeCall.Received.JSONBytes = jsonData + return e.DecodeCall.Returns.PluginInfo, e.DecodeCall.Returns.Error +} diff --git a/pkg/invoke/raw_exec.go b/pkg/invoke/raw_exec.go new file mode 100644 index 00000000..d1bd860d --- /dev/null +++ b/pkg/invoke/raw_exec.go @@ -0,0 +1,63 @@ +// Copyright 2016 CNI authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package invoke + +import ( + "bytes" + "encoding/json" + "fmt" + "io" + "os/exec" + + "github.com/containernetworking/cni/pkg/types" +) + +type RawExec struct { + Stderr io.Writer +} + +func (e *RawExec) ExecPlugin(pluginPath string, stdinData []byte, environ []string) ([]byte, error) { + stdout := &bytes.Buffer{} + + c := exec.Cmd{ + Env: environ, + Path: pluginPath, + Args: []string{pluginPath}, + Stdin: bytes.NewBuffer(stdinData), + Stdout: stdout, + Stderr: e.Stderr, + } + if err := c.Run(); err != nil { + return nil, pluginErr(err, stdout.Bytes()) + } + + return stdout.Bytes(), nil +} + +func pluginErr(err error, output []byte) error { + if _, ok := err.(*exec.ExitError); ok { + emsg := types.Error{} + if perr := json.Unmarshal(output, &emsg); perr != nil { + return fmt.Errorf("netplugin failed but error parsing its diagnostic message %q: %v", string(output), perr) + } + details := "" + if emsg.Details != "" { + details = fmt.Sprintf("; %v", emsg.Details) + } + return fmt.Errorf("%v%v", emsg.Msg, details) + } + + return err +} diff --git a/pkg/invoke/raw_exec_test.go b/pkg/invoke/raw_exec_test.go new file mode 100644 index 00000000..7df60a11 --- /dev/null +++ b/pkg/invoke/raw_exec_test.go @@ -0,0 +1,123 @@ +// Copyright 2016 CNI authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package invoke_test + +import ( + "bytes" + "io/ioutil" + "os" + + "github.com/containernetworking/cni/pkg/invoke" + + noop_debug "github.com/containernetworking/cni/plugins/test/noop/debug" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +var _ = Describe("RawExec", func() { + var ( + debugFileName string + debug *noop_debug.Debug + environ []string + stdin []byte + execer *invoke.RawExec + ) + + const reportResult = `{ "some": "result" }` + + BeforeEach(func() { + debugFile, err := ioutil.TempFile("", "cni_debug") + Expect(err).NotTo(HaveOccurred()) + Expect(debugFile.Close()).To(Succeed()) + debugFileName = debugFile.Name() + + debug = &noop_debug.Debug{ + ReportResult: reportResult, + ReportStderr: "some stderr message", + } + Expect(debug.WriteDebug(debugFileName)).To(Succeed()) + + environ = []string{ + "CNI_COMMAND=ADD", + "CNI_CONTAINERID=some-container-id", + "CNI_ARGS=DEBUG=" + debugFileName, + "CNI_NETNS=/some/netns/path", + "CNI_PATH=/some/bin/path", + "CNI_IFNAME=some-eth0", + } + stdin = []byte(`{"some":"stdin-json"}`) + execer = &invoke.RawExec{} + }) + + AfterEach(func() { + Expect(os.Remove(debugFileName)).To(Succeed()) + }) + + It("runs the plugin with the given stdin and environment", func() { + _, err := execer.ExecPlugin(pathToPlugin, stdin, environ) + Expect(err).NotTo(HaveOccurred()) + + debug, err := noop_debug.ReadDebug(debugFileName) + Expect(err).NotTo(HaveOccurred()) + Expect(debug.Command).To(Equal("ADD")) + Expect(debug.CmdArgs.StdinData).To(Equal(stdin)) + Expect(debug.CmdArgs.Netns).To(Equal("/some/netns/path")) + }) + + It("returns the resulting stdout as bytes", func() { + resultBytes, err := execer.ExecPlugin(pathToPlugin, stdin, environ) + Expect(err).NotTo(HaveOccurred()) + + Expect(resultBytes).To(BeEquivalentTo(reportResult)) + }) + + Context("when the Stderr writer is set", func() { + var stderrBuffer *bytes.Buffer + + BeforeEach(func() { + stderrBuffer = &bytes.Buffer{} + execer.Stderr = stderrBuffer + }) + + It("forwards any stderr bytes to the Stderr writer", func() { + _, err := execer.ExecPlugin(pathToPlugin, stdin, environ) + Expect(err).NotTo(HaveOccurred()) + + Expect(stderrBuffer.String()).To(Equal("some stderr message")) + }) + }) + + Context("when the plugin errors", func() { + BeforeEach(func() { + debug.ReportError = "banana" + Expect(debug.WriteDebug(debugFileName)).To(Succeed()) + }) + + It("wraps and returns the error", func() { + _, err := execer.ExecPlugin(pathToPlugin, stdin, environ) + Expect(err).To(HaveOccurred()) + Expect(err).To(MatchError("banana")) + }) + }) + + Context("when the system is unable to execute the plugin", func() { + It("returns the error", func() { + _, err := execer.ExecPlugin("/tmp/some/invalid/plugin/path", stdin, environ) + Expect(err).To(HaveOccurred()) + Expect(err).To(MatchError(ContainSubstring("/tmp/some/invalid/plugin/path"))) + }) + }) +}) diff --git a/pkg/version/version.go b/pkg/version/version.go index 62bf8bbb..cdb531c0 100644 --- a/pkg/version/version.go +++ b/pkg/version/version.go @@ -61,7 +61,9 @@ func PluginSupports(supportedVersions ...string) PluginInfo { } } -func Decode(jsonBytes []byte) (PluginInfo, error) { +type Decoder struct{} + +func (_ *Decoder) Decode(jsonBytes []byte) (PluginInfo, error) { var info simple err := json.Unmarshal(jsonBytes, &info) if err != nil { diff --git a/pkg/version/version_test.go b/pkg/version/version_test.go index 08b89aed..98a386d7 100644 --- a/pkg/version/version_test.go +++ b/pkg/version/version_test.go @@ -21,8 +21,14 @@ import ( ) var _ = Describe("Decode", func() { + var decoder *version.Decoder + + BeforeEach(func() { + decoder = &version.Decoder{} + }) + It("returns a PluginInfo that represents the given json bytes", func() { - pluginInfo, err := version.Decode([]byte(`{ + pluginInfo, err := decoder.Decode([]byte(`{ "cniVersion": "some-library-version", "supportedVersions": [ "some-version", "some-other-version" ] }`)) @@ -36,14 +42,14 @@ var _ = Describe("Decode", func() { Context("when the bytes cannot be decoded as json", func() { It("returns a meaningful error", func() { - _, err := version.Decode([]byte(`{{{`)) + _, err := decoder.Decode([]byte(`{{{`)) Expect(err).To(MatchError("decoding version info: invalid character '{' looking for beginning of object key string")) }) }) Context("when the json bytes are missing the required CNIVersion field", func() { It("returns a meaningful error", func() { - _, err := version.Decode([]byte(`{ "supportedVersions": [ "foo" ] }`)) + _, err := decoder.Decode([]byte(`{ "supportedVersions": [ "foo" ] }`)) Expect(err).To(MatchError("decoding version info: missing field cniVersion")) }) }) @@ -51,7 +57,7 @@ var _ = Describe("Decode", func() { Context("when there are no supported versions", func() { Context("when the cniVersion is 0.2.0", func() { It("infers the supported versions are 0.1.0 and 0.2.0", func() { - pluginInfo, err := version.Decode([]byte(`{ "cniVersion": "0.2.0" }`)) + pluginInfo, err := decoder.Decode([]byte(`{ "cniVersion": "0.2.0" }`)) Expect(err).NotTo(HaveOccurred()) Expect(pluginInfo).NotTo(BeNil()) Expect(pluginInfo.SupportedVersions()).To(Equal([]string{ @@ -63,7 +69,7 @@ var _ = Describe("Decode", func() { Context("when the cniVersion is >= 0.3.0", func() { It("returns a meaningful error", func() { - _, err := version.Decode([]byte(`{ "cniVersion": "0.3.0" }`)) + _, err := decoder.Decode([]byte(`{ "cniVersion": "0.3.0" }`)) Expect(err).To(MatchError("decoding version info: missing field supportedVersions")) }) }) From deb44660413d94ac5c40a01f6cfcd4632dab0d5d Mon Sep 17 00:00:00 2001 From: Gabe Rosenhouse Date: Tue, 23 Aug 2016 22:57:00 -0700 Subject: [PATCH 4/8] versioning: adds tooling to compile a program against a given old CNI version Allows us to write tests that cover interactions between components of differing versions --- pkg/version/testhelpers/testhelpers.go | 156 ++++++++++++++++++ .../testhelpers/testhelpers_suite_test.go | 27 +++ pkg/version/testhelpers/testhelpers_test.go | 106 ++++++++++++ test | 2 +- 4 files changed, 290 insertions(+), 1 deletion(-) create mode 100644 pkg/version/testhelpers/testhelpers.go create mode 100644 pkg/version/testhelpers/testhelpers_suite_test.go create mode 100644 pkg/version/testhelpers/testhelpers_test.go diff --git a/pkg/version/testhelpers/testhelpers.go b/pkg/version/testhelpers/testhelpers.go new file mode 100644 index 00000000..773d0120 --- /dev/null +++ b/pkg/version/testhelpers/testhelpers.go @@ -0,0 +1,156 @@ +// Copyright 2016 CNI authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package testhelpers supports testing of CNI components of different versions +// +// For example, to build a plugin against an old version of the CNI library, +// we can pass the plugin's source and the old git commit reference to BuildAt. +// We could then test how the built binary responds when called by the latest +// version of this library. +package testhelpers + +import ( + "fmt" + "io/ioutil" + "math/rand" + "os" + "os/exec" + "path/filepath" + "strings" + "time" +) + +const packageBaseName = "github.com/containernetworking/cni" + +func run(cmd *exec.Cmd) error { + out, err := cmd.CombinedOutput() + if err != nil { + command := strings.Join(cmd.Args, " ") + return fmt.Errorf("running %q: %s", command, out) + } + return nil +} + +func goBuildEnviron(gopath string) []string { + environ := os.Environ() + for i, kvp := range environ { + if strings.HasPrefix(kvp, "GOPATH=") { + environ[i] = "GOPATH=" + gopath + return environ + } + } + environ = append(environ, "GOPATH="+gopath) + return environ +} + +func buildGoProgram(gopath, packageName, outputFilePath string) error { + cmd := exec.Command("go", "build", "-o", outputFilePath, packageName) + cmd.Env = goBuildEnviron(gopath) + return run(cmd) +} + +func createSingleFilePackage(gopath, packageName string, fileContents []byte) error { + dirName := filepath.Join(gopath, "src", packageName) + err := os.MkdirAll(dirName, 0700) + if err != nil { + return err + } + + return ioutil.WriteFile(filepath.Join(dirName, "main.go"), fileContents, 0600) +} + +func removePackage(gopath, packageName string) error { + dirName := filepath.Join(gopath, "src", packageName) + return os.RemoveAll(dirName) +} + +func isRepoRoot(path string) bool { + _, err := ioutil.ReadDir(filepath.Join(path, ".git")) + return (err == nil) && (filepath.Base(path) == "cni") +} + +func LocateCurrentGitRepo() (string, error) { + dir, err := os.Getwd() + if err != nil { + return "", err + } + + for i := 0; i < 5; i++ { + if isRepoRoot(dir) { + return dir, nil + } + + dir, err = filepath.Abs(filepath.Dir(dir)) + if err != nil { + return "", fmt.Errorf("abs(dir(%q)): %s", dir, err) + } + } + + return "", fmt.Errorf("unable to find cni repo root, landed at %q", dir) +} + +func gitCloneThisRepo(cloneDestination string) error { + err := os.MkdirAll(cloneDestination, 0700) + if err != nil { + return err + } + + currentGitRepo, err := LocateCurrentGitRepo() + if err != nil { + return err + } + + return run(exec.Command("git", "clone", currentGitRepo, cloneDestination)) +} + +func gitCheckout(localRepo string, gitRef string) error { + return run(exec.Command("git", "-C", localRepo, "checkout", gitRef)) +} + +// BuildAt builds the go programSource using the version of the CNI library +// at gitRef, and saves the resulting binary file at outputFilePath +func BuildAt(programSource []byte, gitRef string, outputFilePath string) error { + tempGoPath, err := ioutil.TempDir("", "cni-git-") + if err != nil { + return err + } + defer os.RemoveAll(tempGoPath) + + cloneDestination := filepath.Join(tempGoPath, "src", packageBaseName) + err = gitCloneThisRepo(cloneDestination) + if err != nil { + return err + } + + err = gitCheckout(cloneDestination, gitRef) + if err != nil { + return err + } + + rand.Seed(time.Now().UnixNano()) + testPackageName := fmt.Sprintf("test-package-%x", rand.Int31()) + + err = createSingleFilePackage(tempGoPath, testPackageName, programSource) + if err != nil { + return err + } + defer removePackage(tempGoPath, testPackageName) + + err = buildGoProgram(tempGoPath, testPackageName, outputFilePath) + if err != nil { + return err + } + + return nil +} diff --git a/pkg/version/testhelpers/testhelpers_suite_test.go b/pkg/version/testhelpers/testhelpers_suite_test.go new file mode 100644 index 00000000..72f65f9c --- /dev/null +++ b/pkg/version/testhelpers/testhelpers_suite_test.go @@ -0,0 +1,27 @@ +// Copyright 2016 CNI authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package testhelpers_test + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + "testing" +) + +func TestTesthelpers(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Testhelpers Suite") +} diff --git a/pkg/version/testhelpers/testhelpers_test.go b/pkg/version/testhelpers/testhelpers_test.go new file mode 100644 index 00000000..3473cd59 --- /dev/null +++ b/pkg/version/testhelpers/testhelpers_test.go @@ -0,0 +1,106 @@ +// Copyright 2016 CNI authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package testhelpers_test + +import ( + "io/ioutil" + "os" + "os/exec" + "path/filepath" + + "github.com/containernetworking/cni/pkg/version/testhelpers" + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +var _ = Describe("BuildAt", func() { + var ( + gitRef string + outputFilePath string + outputDir string + programSource []byte + ) + BeforeEach(func() { + programSource = []byte(`package main + +import "github.com/containernetworking/cni/pkg/skel" + +func c(_ *skel.CmdArgs) error { return nil } + +func main() { skel.PluginMain(c, c) } +`) + gitRef = "f4364185253" + + var err error + outputDir, err = ioutil.TempDir("", "bin") + Expect(err).NotTo(HaveOccurred()) + outputFilePath = filepath.Join(outputDir, "some-binary") + }) + + AfterEach(func() { + Expect(os.RemoveAll(outputDir)).To(Succeed()) + }) + + It("builds the provided source code using the CNI library at the given git ref", func() { + Expect(outputFilePath).NotTo(BeAnExistingFile()) + + err := testhelpers.BuildAt(programSource, gitRef, outputFilePath) + Expect(err).NotTo(HaveOccurred()) + + Expect(outputFilePath).To(BeAnExistingFile()) + + cmd := exec.Command(outputFilePath) + cmd.Env = []string{"CNI_COMMAND=VERSION"} + output, err := cmd.CombinedOutput() + Expect(err).To(BeAssignableToTypeOf(&exec.ExitError{})) + Expect(output).To(ContainSubstring("unknown CNI_COMMAND: VERSION")) + }) +}) + +var _ = Describe("LocateCurrentGitRepo", func() { + It("returns the path to the root of the CNI git repo", func() { + path, err := testhelpers.LocateCurrentGitRepo() + Expect(err).NotTo(HaveOccurred()) + + AssertItIsTheCNIRepoRoot(path) + }) + + Context("when run from a different directory", func() { + BeforeEach(func() { + os.Chdir("..") + }) + + It("still finds the CNI repo root", func() { + path, err := testhelpers.LocateCurrentGitRepo() + Expect(err).NotTo(HaveOccurred()) + + AssertItIsTheCNIRepoRoot(path) + }) + }) +}) + +func AssertItIsTheCNIRepoRoot(path string) { + Expect(path).To(BeADirectory()) + files, err := ioutil.ReadDir(path) + Expect(err).NotTo(HaveOccurred()) + + names := []string{} + for _, file := range files { + names = append(names, file.Name()) + } + + Expect(names).To(ContainElement("SPEC.md")) + Expect(names).To(ContainElement("libcni")) + Expect(names).To(ContainElement("cnitool")) +} diff --git a/test b/test index 7537b40f..8c2b3ce5 100755 --- a/test +++ b/test @@ -11,7 +11,7 @@ set -e source ./build -TESTABLE="libcni plugins/ipam/dhcp plugins/ipam/host-local plugins/main/loopback pkg/invoke pkg/ns pkg/skel pkg/types pkg/utils plugins/main/ipvlan plugins/main/macvlan plugins/main/bridge plugins/main/ptp plugins/test/noop pkg/utils/hwaddr pkg/ip pkg/version" +TESTABLE="libcni plugins/ipam/dhcp plugins/ipam/host-local plugins/main/loopback pkg/invoke pkg/ns pkg/skel pkg/types pkg/utils plugins/main/ipvlan plugins/main/macvlan plugins/main/bridge plugins/main/ptp plugins/test/noop pkg/utils/hwaddr pkg/ip pkg/version pkg/version/testhelpers" FORMATTABLE="$TESTABLE pkg/testutils plugins/meta/flannel plugins/meta/tuning" # user has not provided PKG override From 97192fc979312e9d5cb419c07238d9b327279702 Mon Sep 17 00:00:00 2001 From: Gabe Rosenhouse Date: Fri, 2 Sep 2016 16:01:22 -0400 Subject: [PATCH 5/8] invoke: correctly infer version for 0.1.0-vintage plugins Older plugins return a known error when issued the VERSION command. Capture this error and report it as a 0.1.0 version plugin. --- libcni/api.go | 2 - pkg/invoke/exec.go | 12 ++++- pkg/invoke/exec_integration_test.go | 72 +++++++++++++++++++++++++++++ pkg/invoke/exec_test.go | 2 +- 4 files changed, 84 insertions(+), 4 deletions(-) create mode 100644 pkg/invoke/exec_integration_test.go diff --git a/libcni/api.go b/libcni/api.go index 6e616da6..08883250 100644 --- a/libcni/api.go +++ b/libcni/api.go @@ -67,8 +67,6 @@ func (c *CNIConfig) GetVersionInfo(pluginType string) (version.PluginInfo, error return nil, err } - // TODO: if error is because plugin is old and VERSION command is unrecognized - // then do the right thing and return version.PluginSupports("0.1.0"), nil return invoke.ExecPluginForVersion(pluginPath) } diff --git a/pkg/invoke/exec.go b/pkg/invoke/exec.go index f95dec11..fe792b62 100644 --- a/pkg/invoke/exec.go +++ b/pkg/invoke/exec.go @@ -65,9 +65,19 @@ func (e *PluginExec) WithoutResult(pluginPath string, netconf []byte, args CNIAr } func (e *PluginExec) GetVersion(pluginPath string) (version.PluginInfo, error) { - args := &Args{Command: "VERSION"} + args := &Args{ + Command: "VERSION", + + // set fake values required by plugins built against an older version of skel + NetNS: "/tmp/not/a/container", + IfName: "not-an-interface", + Path: "/tmp/not/a/path", + } stdoutBytes, err := e.RawExec.ExecPlugin(pluginPath, nil, args.AsEnv()) if err != nil { + if err.Error() == "unknown CNI_COMMAND: VERSION" { + return version.PluginSupports("0.1.0"), nil + } return nil, err } diff --git a/pkg/invoke/exec_integration_test.go b/pkg/invoke/exec_integration_test.go new file mode 100644 index 00000000..c16090ad --- /dev/null +++ b/pkg/invoke/exec_integration_test.go @@ -0,0 +1,72 @@ +// Copyright 2016 CNI authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package invoke_test + +import ( + "io/ioutil" + "os" + "path/filepath" + + "github.com/containernetworking/cni/pkg/invoke" + "github.com/containernetworking/cni/pkg/version" + "github.com/containernetworking/cni/pkg/version/testhelpers" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/extensions/table" + . "github.com/onsi/gomega" +) + +var _ = Describe("GetVersion, integration tests", func() { + var ( + pluginDir string + pluginPath string + ) + + BeforeEach(func() { + pluginDir, err := ioutil.TempDir("", "plugins") + Expect(err).NotTo(HaveOccurred()) + pluginPath = filepath.Join(pluginDir, "test-plugin") + }) + + AfterEach(func() { + Expect(os.RemoveAll(pluginDir)).To(Succeed()) + }) + + DescribeTable("correctly reporting plugin versions", + func(gitRef string, pluginSource string, expectedVersions version.PluginInfo) { + Expect(testhelpers.BuildAt([]byte(pluginSource), gitRef, pluginPath)).To(Succeed()) + versionInfo, err := invoke.ExecPluginForVersion(pluginPath) + Expect(err).NotTo(HaveOccurred()) + + Expect(versionInfo.SupportedVersions()).To(ConsistOf(expectedVersions.SupportedVersions())) + }, + Entry("old plugin, before VERSION was introduced", git_ref_v010, plugin_source_v010, version.PluginSupports("0.1.0")), + Entry("when VERSION was introduced", git_ref_v020, plugin_source_v010, version.PluginSupports("0.1.0", "0.2.0")), + ) +}) + +// a minimal 0.1.0 / 0.2.0 plugin +const plugin_source_v010 = `package main + +import "github.com/containernetworking/cni/pkg/skel" +import "fmt" + +func c(_ *skel.CmdArgs) error { fmt.Println("{}"); return nil } + +func main() { skel.PluginMain(c, c) } +` + +const git_ref_v010 = "2c482f4" +const git_ref_v020 = "349d66d" diff --git a/pkg/invoke/exec_test.go b/pkg/invoke/exec_test.go index bff3fb73..7b591f17 100644 --- a/pkg/invoke/exec_test.go +++ b/pkg/invoke/exec_test.go @@ -25,7 +25,7 @@ import ( . "github.com/onsi/gomega" ) -var _ = Describe("Executing a plugin", func() { +var _ = Describe("Executing a plugin, unit tests", func() { var ( pluginExec *invoke.PluginExec rawExec *fakes.RawExec From bf31ed1591effba2a9c5d8395a7c580c0738f7de Mon Sep 17 00:00:00 2001 From: Gabe Rosenhouse Date: Fri, 2 Sep 2016 16:39:01 -0400 Subject: [PATCH 6/8] invoke: better name and unit test coverage for GetVersionInfo --- libcni/api.go | 2 +- pkg/invoke/exec.go | 12 ++++++------ pkg/invoke/exec_integration_test.go | 2 +- pkg/invoke/exec_test.go | 28 +++++++++++++++++++++++++--- 4 files changed, 33 insertions(+), 11 deletions(-) diff --git a/libcni/api.go b/libcni/api.go index 08883250..273123d8 100644 --- a/libcni/api.go +++ b/libcni/api.go @@ -67,7 +67,7 @@ func (c *CNIConfig) GetVersionInfo(pluginType string) (version.PluginInfo, error return nil, err } - return invoke.ExecPluginForVersion(pluginPath) + return invoke.GetVersionInfo(pluginPath) } // ===== diff --git a/pkg/invoke/exec.go b/pkg/invoke/exec.go index fe792b62..68efbb9b 100644 --- a/pkg/invoke/exec.go +++ b/pkg/invoke/exec.go @@ -30,8 +30,8 @@ func ExecPluginWithoutResult(pluginPath string, netconf []byte, args CNIArgs) er return defaultPluginExec.WithoutResult(pluginPath, netconf, args) } -func ExecPluginForVersion(pluginPath string) (version.PluginInfo, error) { - return defaultPluginExec.GetVersion(pluginPath) +func GetVersionInfo(pluginPath string) (version.PluginInfo, error) { + return defaultPluginExec.GetVersionInfo(pluginPath) } var defaultPluginExec = &PluginExec{ @@ -64,14 +64,14 @@ func (e *PluginExec) WithoutResult(pluginPath string, netconf []byte, args CNIAr return err } -func (e *PluginExec) GetVersion(pluginPath string) (version.PluginInfo, error) { +func (e *PluginExec) GetVersionInfo(pluginPath string) (version.PluginInfo, error) { args := &Args{ Command: "VERSION", // set fake values required by plugins built against an older version of skel - NetNS: "/tmp/not/a/container", - IfName: "not-an-interface", - Path: "/tmp/not/a/path", + NetNS: "dummy", + IfName: "dummy", + Path: "dummy", } stdoutBytes, err := e.RawExec.ExecPlugin(pluginPath, nil, args.AsEnv()) if err != nil { diff --git a/pkg/invoke/exec_integration_test.go b/pkg/invoke/exec_integration_test.go index c16090ad..f02374c0 100644 --- a/pkg/invoke/exec_integration_test.go +++ b/pkg/invoke/exec_integration_test.go @@ -47,7 +47,7 @@ var _ = Describe("GetVersion, integration tests", func() { DescribeTable("correctly reporting plugin versions", func(gitRef string, pluginSource string, expectedVersions version.PluginInfo) { Expect(testhelpers.BuildAt([]byte(pluginSource), gitRef, pluginPath)).To(Succeed()) - versionInfo, err := invoke.ExecPluginForVersion(pluginPath) + versionInfo, err := invoke.GetVersionInfo(pluginPath) Expect(err).NotTo(HaveOccurred()) Expect(versionInfo.SupportedVersions()).To(ConsistOf(expectedVersions.SupportedVersions())) diff --git a/pkg/invoke/exec_test.go b/pkg/invoke/exec_test.go index 7b591f17..94007d7c 100644 --- a/pkg/invoke/exec_test.go +++ b/pkg/invoke/exec_test.go @@ -103,14 +103,14 @@ var _ = Describe("Executing a plugin, unit tests", func() { }) It("execs the plugin with the command VERSION", func() { - pluginExec.GetVersion(pluginPath) + pluginExec.GetVersionInfo(pluginPath) Expect(rawExec.ExecPluginCall.Received.PluginPath).To(Equal(pluginPath)) Expect(rawExec.ExecPluginCall.Received.StdinData).To(BeNil()) Expect(rawExec.ExecPluginCall.Received.Environ).To(ContainElement("CNI_COMMAND=VERSION")) }) It("decodes and returns the version info", func() { - versionInfo, err := pluginExec.GetVersion(pluginPath) + versionInfo, err := pluginExec.GetVersionInfo(pluginPath) Expect(err).NotTo(HaveOccurred()) Expect(versionInfo.SupportedVersions()).To(Equal([]string{"0.42.0"})) Expect(versionDecoder.DecodeCall.Received.JSONBytes).To(MatchJSON(`{ "some": "version-info" }`)) @@ -121,9 +121,31 @@ var _ = Describe("Executing a plugin, unit tests", func() { rawExec.ExecPluginCall.Returns.Error = errors.New("banana") }) It("returns the error", func() { - _, err := pluginExec.GetVersion(pluginPath) + _, err := pluginExec.GetVersionInfo(pluginPath) Expect(err).To(MatchError("banana")) }) }) + + Context("when the plugin is too old to recognize the VERSION command", func() { + BeforeEach(func() { + rawExec.ExecPluginCall.Returns.Error = errors.New("unknown CNI_COMMAND: VERSION") + }) + + It("interprets the error as a 0.1.0 version", func() { + versionInfo, err := pluginExec.GetVersionInfo(pluginPath) + Expect(err).NotTo(HaveOccurred()) + Expect(versionInfo.SupportedVersions()).To(ConsistOf("0.1.0")) + }) + + It("sets dummy values for env vars required by very old plugins", func() { + pluginExec.GetVersionInfo(pluginPath) + + env := rawExec.ExecPluginCall.Received.Environ + Expect(env).To(ContainElement("CNI_NETNS=dummy")) + Expect(env).To(ContainElement("CNI_IFNAME=dummy")) + Expect(env).To(ContainElement("CNI_PATH=dummy")) + }) + }) + }) }) From d5e2e375d47a409347ccf311c734e935f59394e6 Mon Sep 17 00:00:00 2001 From: Gabe Rosenhouse Date: Tue, 6 Sep 2016 11:22:27 -0400 Subject: [PATCH 7/8] versioning: misc cleanups highlights: - NetConf struct finally includes cniVersion field - improve test coverage of current version report behavior - godoc a few key functions - allow tests to control version list reported by no-op plugin --- libcni/api.go | 4 + pkg/invoke/exec.go | 6 +- ...est.go => get_version_integration_test.go} | 17 ++++ pkg/types/types.go | 2 + pkg/version/plugin.go | 77 +++++++++++++++++++ .../{version_test.go => plugin_test.go} | 6 +- pkg/version/version.go | 62 --------------- plugins/test/noop/debug/debug.go | 16 ++-- plugins/test/noop/main.go | 21 ++++- plugins/test/noop/noop_test.go | 25 +++++- 10 files changed, 162 insertions(+), 74 deletions(-) rename pkg/invoke/{exec_integration_test.go => get_version_integration_test.go} (78%) create mode 100644 pkg/version/plugin.go rename pkg/version/{version_test.go => plugin_test.go} (94%) diff --git a/libcni/api.go b/libcni/api.go index 273123d8..8ba78b26 100644 --- a/libcni/api.go +++ b/libcni/api.go @@ -43,6 +43,7 @@ type CNIConfig struct { Path []string } +// AddNetwork executes the plugin with the ADD command func (c *CNIConfig) AddNetwork(net *NetworkConfig, rt *RuntimeConf) (*types.Result, error) { pluginPath, err := invoke.FindInPath(net.Network.Type, c.Path) if err != nil { @@ -52,6 +53,7 @@ func (c *CNIConfig) AddNetwork(net *NetworkConfig, rt *RuntimeConf) (*types.Resu return invoke.ExecPluginWithResult(pluginPath, net.Bytes, c.args("ADD", rt)) } +// DelNetwork executes the plugin with the DEL command func (c *CNIConfig) DelNetwork(net *NetworkConfig, rt *RuntimeConf) error { pluginPath, err := invoke.FindInPath(net.Network.Type, c.Path) if err != nil { @@ -61,6 +63,8 @@ func (c *CNIConfig) DelNetwork(net *NetworkConfig, rt *RuntimeConf) error { return invoke.ExecPluginWithoutResult(pluginPath, net.Bytes, c.args("DEL", rt)) } +// GetVersionInfo reports which versions of the CNI spec are supported by +// the given plugin. func (c *CNIConfig) GetVersionInfo(pluginType string) (version.PluginInfo, error) { pluginPath, err := invoke.FindInPath(pluginType, c.Path) if err != nil { diff --git a/pkg/invoke/exec.go b/pkg/invoke/exec.go index 68efbb9b..7eb06156 100644 --- a/pkg/invoke/exec.go +++ b/pkg/invoke/exec.go @@ -36,7 +36,7 @@ func GetVersionInfo(pluginPath string) (version.PluginInfo, error) { var defaultPluginExec = &PluginExec{ RawExec: &RawExec{Stderr: os.Stderr}, - VersionDecoder: &version.Decoder{}, + VersionDecoder: &version.PluginDecoder{}, } type PluginExec struct { @@ -64,6 +64,10 @@ func (e *PluginExec) WithoutResult(pluginPath string, netconf []byte, args CNIAr return err } +// GetVersionInfo returns the version information available about the plugin. +// For recent-enough plugins, it uses the information returned by the VERSION +// command. For older plugins which do not recognize that command, it reports +// version 0.1.0 func (e *PluginExec) GetVersionInfo(pluginPath string) (version.PluginInfo, error) { args := &Args{ Command: "VERSION", diff --git a/pkg/invoke/exec_integration_test.go b/pkg/invoke/get_version_integration_test.go similarity index 78% rename from pkg/invoke/exec_integration_test.go rename to pkg/invoke/get_version_integration_test.go index f02374c0..d10826dd 100644 --- a/pkg/invoke/exec_integration_test.go +++ b/pkg/invoke/get_version_integration_test.go @@ -54,9 +54,26 @@ var _ = Describe("GetVersion, integration tests", func() { }, Entry("old plugin, before VERSION was introduced", git_ref_v010, plugin_source_v010, version.PluginSupports("0.1.0")), Entry("when VERSION was introduced", git_ref_v020, plugin_source_v010, version.PluginSupports("0.1.0", "0.2.0")), + Entry("when plugins report their own version support", git_ref_v030, plugin_source_v030, version.PluginSupports("0.3.0", "0.999.0")), + Entry("HEAD", "HEAD", plugin_source_v030, version.PluginSupports("0.3.0", "0.999.0")), ) }) +// a 0.3.0 plugin that can report its own versions +const plugin_source_v030 = `package main + +import ( + "github.com/containernetworking/cni/pkg/skel" + "github.com/containernetworking/cni/pkg/version" + "fmt" +) + +func c(_ *skel.CmdArgs) error { fmt.Println("{}"); return nil } + +func main() { skel.PluginMain(c, c, version.PluginSupports("0.3.0", "0.999.0")) } +` +const git_ref_v030 = "bf31ed15" + // a minimal 0.1.0 / 0.2.0 plugin const plugin_source_v010 = `package main diff --git a/pkg/types/types.go b/pkg/types/types.go index 6948dcb1..ba02580a 100644 --- a/pkg/types/types.go +++ b/pkg/types/types.go @@ -57,6 +57,8 @@ func (n *IPNet) UnmarshalJSON(data []byte) error { // NetConf describes a network. type NetConf struct { + CNIVersion string `json:"cniVersion,omitempty"` + Name string `json:"name,omitempty"` Type string `json:"type,omitempty"` IPAM struct { diff --git a/pkg/version/plugin.go b/pkg/version/plugin.go new file mode 100644 index 00000000..9bd7dc83 --- /dev/null +++ b/pkg/version/plugin.go @@ -0,0 +1,77 @@ +// Copyright 2016 CNI authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package version + +import ( + "encoding/json" + "fmt" + "io" +) + +// PluginInfo reports information about CNI versioning +type PluginInfo interface { + // SupportedVersions returns one or more CNI spec versions that the plugin + // supports. If input is provided in one of these versions, then the plugin + // promises to use the same CNI version in its response + SupportedVersions() []string + + // Encode writes this CNI version information as JSON to the given Writer + Encode(io.Writer) error +} + +type pluginInfo struct { + CNIVersion_ string `json:"cniVersion"` + SupportedVersions_ []string `json:"supportedVersions,omitempty"` +} + +func (p *pluginInfo) Encode(w io.Writer) error { + return json.NewEncoder(w).Encode(p) +} + +func (p *pluginInfo) SupportedVersions() []string { + return p.SupportedVersions_ +} + +// PluginSupports returns a new PluginInfo that will report the given versions +// as supported +func PluginSupports(supportedVersions ...string) PluginInfo { + if len(supportedVersions) < 1 { + panic("programmer error: you must support at least one version") + } + return &pluginInfo{ + CNIVersion_: Current(), + SupportedVersions_: supportedVersions, + } +} + +type PluginDecoder struct{} + +func (_ *PluginDecoder) Decode(jsonBytes []byte) (PluginInfo, error) { + var info pluginInfo + err := json.Unmarshal(jsonBytes, &info) + if err != nil { + return nil, fmt.Errorf("decoding version info: %s", err) + } + if info.CNIVersion_ == "" { + return nil, fmt.Errorf("decoding version info: missing field cniVersion") + } + if len(info.SupportedVersions_) == 0 { + if info.CNIVersion_ == "0.2.0" { + return PluginSupports("0.1.0", "0.2.0"), nil + } + return nil, fmt.Errorf("decoding version info: missing field supportedVersions") + } + return &info, nil +} diff --git a/pkg/version/version_test.go b/pkg/version/plugin_test.go similarity index 94% rename from pkg/version/version_test.go rename to pkg/version/plugin_test.go index 98a386d7..a58bd35a 100644 --- a/pkg/version/version_test.go +++ b/pkg/version/plugin_test.go @@ -20,11 +20,11 @@ import ( . "github.com/onsi/gomega" ) -var _ = Describe("Decode", func() { - var decoder *version.Decoder +var _ = Describe("Decoding versions reported by a plugin", func() { + var decoder *version.PluginDecoder BeforeEach(func() { - decoder = &version.Decoder{} + decoder = &version.PluginDecoder{} }) It("returns a PluginInfo that represents the given json bytes", func() { diff --git a/pkg/version/version.go b/pkg/version/version.go index cdb531c0..5f937f7e 100644 --- a/pkg/version/version.go +++ b/pkg/version/version.go @@ -14,73 +14,11 @@ package version -import ( - "encoding/json" - "fmt" - "io" -) - // Current reports the version of the CNI spec implemented by this library func Current() string { return "0.3.0" } -// PluginInfo reports information about CNI versioning -type PluginInfo interface { - // SupportedVersions returns one or more CNI spec versions that the plugin - // supports. If input is provided in one of these versions, then the plugin - // promises to use the same CNI version in its response - SupportedVersions() []string - - // Encode writes this CNI version information as JSON to the given Writer - Encode(io.Writer) error -} - -type simple struct { - CNIVersion_ string `json:"cniVersion"` - SupportedVersions_ []string `json:"supportedVersions,omitempty"` -} - -func (p *simple) Encode(w io.Writer) error { - return json.NewEncoder(w).Encode(p) -} - -func (p *simple) SupportedVersions() []string { - return p.SupportedVersions_ -} - -// PluginSupports returns a new PluginInfo that will report the given versions -// as supported -func PluginSupports(supportedVersions ...string) PluginInfo { - if len(supportedVersions) < 1 { - panic("programmer error: you must support at least one version") - } - return &simple{ - CNIVersion_: Current(), - SupportedVersions_: supportedVersions, - } -} - -type Decoder struct{} - -func (_ *Decoder) Decode(jsonBytes []byte) (PluginInfo, error) { - var info simple - err := json.Unmarshal(jsonBytes, &info) - if err != nil { - return nil, fmt.Errorf("decoding version info: %s", err) - } - if info.CNIVersion_ == "" { - return nil, fmt.Errorf("decoding version info: missing field cniVersion") - } - if len(info.SupportedVersions_) == 0 { - if info.CNIVersion_ == "0.2.0" { - return PluginSupports("0.1.0", "0.2.0"), nil - } - return nil, fmt.Errorf("decoding version info: missing field supportedVersions") - } - return &info, nil -} - // Legacy PluginInfo describes a plugin that is backwards compatible with the // CNI spec version 0.1.0. In particular, a runtime compiled against the 0.1.0 // library ought to work correctly with a plugin that reports support for diff --git a/plugins/test/noop/debug/debug.go b/plugins/test/noop/debug/debug.go index 71770bd3..5bc6e4f0 100644 --- a/plugins/test/noop/debug/debug.go +++ b/plugins/test/noop/debug/debug.go @@ -24,11 +24,17 @@ import ( // Debug is used to control and record the behavior of the noop plugin type Debug struct { - ReportResult string - ReportError string - ReportStderr string - Command string - CmdArgs skel.CmdArgs + // Report* fields allow the test to control the behavior of the no-op plugin + ReportResult string + ReportError string + ReportStderr string + ReportVersionSupport []string + + // Command stores the CNI command that the plugin received + Command string + + // CmdArgs stores the CNI Args and Env Vars that the plugin recieved + CmdArgs skel.CmdArgs } // ReadDebug will return a debug file recorded by the noop plugin diff --git a/plugins/test/noop/main.go b/plugins/test/noop/main.go index 1ebc7350..6d6237df 100644 --- a/plugins/test/noop/main.go +++ b/plugins/test/noop/main.go @@ -63,6 +63,23 @@ func debugBehavior(args *skel.CmdArgs, command string) error { return nil } +func debugGetSupportedVersions() []string { + vers := []string{"0.-42.0", "0.1.0", "0.2.0", "0.3.0"} + cniArgs := os.Getenv("CNI_ARGS") + if cniArgs == "" { + return vers + } + debugFilePath := strings.TrimPrefix(cniArgs, "DEBUG=") + debug, err := debug.ReadDebug(debugFilePath) + if err != nil { + panic("test setup error: unable to read debug file: " + err.Error()) + } + if debug.ReportVersionSupport == nil { + return vers + } + return debug.ReportVersionSupport +} + func cmdAdd(args *skel.CmdArgs) error { return debugBehavior(args, "ADD") } @@ -72,6 +89,6 @@ func cmdDel(args *skel.CmdArgs) error { } func main() { - skel.PluginMain(cmdAdd, cmdDel, - version.PluginSupports("0.-42.0", "0.1.0", "0.2.0", "0.3.0")) + supportedVersions := debugGetSupportedVersions() + skel.PluginMain(cmdAdd, cmdDel, version.PluginSupports(supportedVersions...)) } diff --git a/plugins/test/noop/noop_test.go b/plugins/test/noop/noop_test.go index c6ce3dd4..968cc838 100644 --- a/plugins/test/noop/noop_test.go +++ b/plugins/test/noop/noop_test.go @@ -21,6 +21,7 @@ import ( "strings" "github.com/containernetworking/cni/pkg/skel" + "github.com/containernetworking/cni/pkg/version" noop_debug "github.com/containernetworking/cni/plugins/test/noop/debug" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -38,7 +39,10 @@ var _ = Describe("No-op plugin", func() { const reportResult = `{ "ip4": { "ip": "10.1.2.3/24" }, "dns": {} }` BeforeEach(func() { - debug = &noop_debug.Debug{ReportResult: reportResult} + debug = &noop_debug.Debug{ + ReportResult: reportResult, + ReportVersionSupport: []string{"0.1.0", "0.2.0", "0.3.0"}, + } debugFile, err := ioutil.TempFile("", "cni_debug") Expect(err).NotTo(HaveOccurred()) @@ -122,6 +126,25 @@ var _ = Describe("No-op plugin", func() { Expect(debug.Command).To(Equal("DEL")) Expect(debug.CmdArgs).To(Equal(expectedCmdArgs)) }) + }) + Context("when the CNI_COMMAND is VERSION", func() { + BeforeEach(func() { + cmd.Env[0] = "CNI_COMMAND=VERSION" + debug.ReportVersionSupport = []string{"0.123.0", "0.3.0"} + + Expect(debug.WriteDebug(debugFileName)).To(Succeed()) + }) + + It("claims to support the specified versions", func() { + session, err := gexec.Start(cmd, GinkgoWriter, GinkgoWriter) + Expect(err).NotTo(HaveOccurred()) + Eventually(session).Should(gexec.Exit(0)) + decoder := &version.PluginDecoder{} + pluginInfo, err := decoder.Decode(session.Out.Contents()) + Expect(err).NotTo(HaveOccurred()) + Expect(pluginInfo.SupportedVersions()).To(ConsistOf( + "0.123.0", "0.3.0")) + }) }) }) From 7958b9f0ccbbc1be2028d7cf7ed8971c4832f259 Mon Sep 17 00:00:00 2001 From: Gabe Rosenhouse Date: Tue, 6 Sep 2016 15:24:12 -0400 Subject: [PATCH 8/8] versioning: revert spec version to 0.2.0 --- SPEC.md | 14 +++--- libcni/api_test.go | 2 +- pkg/invoke/get_version_integration_test.go | 40 ++++++++++++----- pkg/skel/skel_test.go | 2 +- pkg/version/plugin_test.go | 51 ++++++++++++---------- pkg/version/version.go | 6 +-- plugins/test/noop/main.go | 2 +- plugins/test/noop/noop_test.go | 4 +- 8 files changed, 73 insertions(+), 48 deletions(-) diff --git a/SPEC.md b/SPEC.md index 91c85d30..82966add 100644 --- a/SPEC.md +++ b/SPEC.md @@ -68,8 +68,8 @@ The operations that the CNI plugin needs to support are: ``` { - "cniVersion": "0.3.0", // the version of the CNI spec in use for this output - "supportedVersions": [ "0.1.0", "0.2.0", "0.3.0" ] // the list of CNI spec versions that this plugin supports + "cniVersion": "0.2.0", // the version of the CNI spec in use for this output + "supportedVersions": [ "0.1.0", "0.2.0" ] // the list of CNI spec versions that this plugin supports } ``` @@ -91,7 +91,7 @@ Success is indicated by a return code of zero and the following JSON printed to ``` { - "cniVersion": "0.3.0", + "cniVersion": "0.2.0", "ip4": { "ip": , "gateway": , (optional) @@ -120,7 +120,7 @@ Examples include generating an `/etc/resolv.conf` file to be injected into the c Errors are indicated by a non-zero return code and the following JSON being printed to stdout: ``` { - "cniVersion": "0.3.0", + "cniVersion": "0.2.0", "code": , "msg": , "details": (optional) @@ -157,7 +157,7 @@ Plugins may define additional fields that they accept and may generate an error ```json { - "cniVersion": "0.3.0", + "cniVersion": "0.2.0", "name": "dbnet", "type": "bridge", // type (plugin) specific @@ -176,7 +176,7 @@ Plugins may define additional fields that they accept and may generate an error ```json { - "cniVersion": "0.3.0", + "cniVersion": "0.2.0", "name": "pci", "type": "ovs", // type (plugin) specific @@ -226,7 +226,7 @@ Success is indicated by a zero return code and the following JSON being printed ``` { - "cniVersion": "0.3.0", + "cniVersion": "0.2.0", "ip4": { "ip": , "gateway": , (optional) diff --git a/libcni/api_test.go b/libcni/api_test.go index 5e0dd0fe..6ac88e89 100644 --- a/libcni/api_test.go +++ b/libcni/api_test.go @@ -163,7 +163,7 @@ var _ = Describe("Invoking the plugin", func() { Expect(versionInfo).NotTo(BeNil()) Expect(versionInfo.SupportedVersions()).To(Equal([]string{ - "0.-42.0", "0.1.0", "0.2.0", "0.3.0", + "0.-42.0", "0.1.0", "0.2.0", })) }) diff --git a/pkg/invoke/get_version_integration_test.go b/pkg/invoke/get_version_integration_test.go index d10826dd..7e58a9be 100644 --- a/pkg/invoke/get_version_integration_test.go +++ b/pkg/invoke/get_version_integration_test.go @@ -52,15 +52,33 @@ var _ = Describe("GetVersion, integration tests", func() { Expect(versionInfo.SupportedVersions()).To(ConsistOf(expectedVersions.SupportedVersions())) }, - Entry("old plugin, before VERSION was introduced", git_ref_v010, plugin_source_v010, version.PluginSupports("0.1.0")), - Entry("when VERSION was introduced", git_ref_v020, plugin_source_v010, version.PluginSupports("0.1.0", "0.2.0")), - Entry("when plugins report their own version support", git_ref_v030, plugin_source_v030, version.PluginSupports("0.3.0", "0.999.0")), - Entry("HEAD", "HEAD", plugin_source_v030, version.PluginSupports("0.3.0", "0.999.0")), + + Entry("historical: before VERSION was introduced", + git_ref_v010, plugin_source_no_custom_versions, + version.PluginSupports("0.1.0"), + ), + + Entry("historical: when VERSION was introduced but plugins couldn't customize it", + git_ref_v020_no_custom_versions, plugin_source_no_custom_versions, + version.PluginSupports("0.1.0", "0.2.0"), + ), + + Entry("historical: when plugins started reporting their own version list", + git_ref_v020_custom_versions, plugin_source_v020_custom_versions, + version.PluginSupports("0.2.0", "0.999.0"), + ), + + // this entry tracks the current behavior. Before you change it, ensure + // that its previous behavior is captured in the most recent "historical" entry + Entry("current", + "HEAD", plugin_source_v020_custom_versions, + version.PluginSupports("0.2.0", "0.999.0"), + ), ) }) -// a 0.3.0 plugin that can report its own versions -const plugin_source_v030 = `package main +// a 0.2.0 plugin that can report its own versions +const plugin_source_v020_custom_versions = `package main import ( "github.com/containernetworking/cni/pkg/skel" @@ -70,12 +88,12 @@ import ( func c(_ *skel.CmdArgs) error { fmt.Println("{}"); return nil } -func main() { skel.PluginMain(c, c, version.PluginSupports("0.3.0", "0.999.0")) } +func main() { skel.PluginMain(c, c, version.PluginSupports("0.2.0", "0.999.0")) } ` -const git_ref_v030 = "bf31ed15" +const git_ref_v020_custom_versions = "bf31ed15" -// a minimal 0.1.0 / 0.2.0 plugin -const plugin_source_v010 = `package main +// a minimal 0.1.0 / 0.2.0 plugin that cannot report it's own version support +const plugin_source_no_custom_versions = `package main import "github.com/containernetworking/cni/pkg/skel" import "fmt" @@ -86,4 +104,4 @@ func main() { skel.PluginMain(c, c) } ` const git_ref_v010 = "2c482f4" -const git_ref_v020 = "349d66d" +const git_ref_v020_no_custom_versions = "349d66d" diff --git a/pkg/skel/skel_test.go b/pkg/skel/skel_test.go index 0431abbb..1cc533bd 100644 --- a/pkg/skel/skel_test.go +++ b/pkg/skel/skel_test.go @@ -186,7 +186,7 @@ var _ = Describe("dispatching to the correct callback", func() { Expect(err).NotTo(HaveOccurred()) Expect(stdout).To(MatchJSON(`{ - "cniVersion": "0.3.0", + "cniVersion": "0.2.0", "supportedVersions": ["9.8.7"] }`)) }) diff --git a/pkg/version/plugin_test.go b/pkg/version/plugin_test.go index a58bd35a..124288fd 100644 --- a/pkg/version/plugin_test.go +++ b/pkg/version/plugin_test.go @@ -21,17 +21,21 @@ import ( ) var _ = Describe("Decoding versions reported by a plugin", func() { - var decoder *version.PluginDecoder + var ( + decoder *version.PluginDecoder + versionStdout []byte + ) BeforeEach(func() { decoder = &version.PluginDecoder{} + versionStdout = []byte(`{ + "cniVersion": "some-library-version", + "supportedVersions": [ "some-version", "some-other-version" ] + }`) }) It("returns a PluginInfo that represents the given json bytes", func() { - pluginInfo, err := decoder.Decode([]byte(`{ - "cniVersion": "some-library-version", - "supportedVersions": [ "some-version", "some-other-version" ] - }`)) + pluginInfo, err := decoder.Decode(versionStdout) Expect(err).NotTo(HaveOccurred()) Expect(pluginInfo).NotTo(BeNil()) Expect(pluginInfo.SupportedVersions()).To(Equal([]string{ @@ -41,37 +45,40 @@ var _ = Describe("Decoding versions reported by a plugin", func() { }) Context("when the bytes cannot be decoded as json", func() { + BeforeEach(func() { + versionStdout = []byte(`{{{`) + }) + It("returns a meaningful error", func() { - _, err := decoder.Decode([]byte(`{{{`)) + _, err := decoder.Decode(versionStdout) Expect(err).To(MatchError("decoding version info: invalid character '{' looking for beginning of object key string")) }) }) Context("when the json bytes are missing the required CNIVersion field", func() { + BeforeEach(func() { + versionStdout = []byte(`{ "supportedVersions": [ "foo" ] }`) + }) + It("returns a meaningful error", func() { - _, err := decoder.Decode([]byte(`{ "supportedVersions": [ "foo" ] }`)) + _, err := decoder.Decode(versionStdout) Expect(err).To(MatchError("decoding version info: missing field cniVersion")) }) }) Context("when there are no supported versions", func() { - Context("when the cniVersion is 0.2.0", func() { - It("infers the supported versions are 0.1.0 and 0.2.0", func() { - pluginInfo, err := decoder.Decode([]byte(`{ "cniVersion": "0.2.0" }`)) - Expect(err).NotTo(HaveOccurred()) - Expect(pluginInfo).NotTo(BeNil()) - Expect(pluginInfo.SupportedVersions()).To(Equal([]string{ - "0.1.0", - "0.2.0", - })) - }) + BeforeEach(func() { + versionStdout = []byte(`{ "cniVersion": "0.2.0" }`) }) - Context("when the cniVersion is >= 0.3.0", func() { - It("returns a meaningful error", func() { - _, err := decoder.Decode([]byte(`{ "cniVersion": "0.3.0" }`)) - Expect(err).To(MatchError("decoding version info: missing field supportedVersions")) - }) + It("assumes that the supported versions are 0.1.0 and 0.2.0", func() { + pluginInfo, err := decoder.Decode(versionStdout) + Expect(err).NotTo(HaveOccurred()) + Expect(pluginInfo).NotTo(BeNil()) + Expect(pluginInfo.SupportedVersions()).To(Equal([]string{ + "0.1.0", + "0.2.0", + })) }) }) diff --git a/pkg/version/version.go b/pkg/version/version.go index 5f937f7e..e39c3b55 100644 --- a/pkg/version/version.go +++ b/pkg/version/version.go @@ -16,7 +16,7 @@ package version // Current reports the version of the CNI spec implemented by this library func Current() string { - return "0.3.0" + return "0.2.0" } // Legacy PluginInfo describes a plugin that is backwards compatible with the @@ -24,6 +24,6 @@ func Current() string { // library ought to work correctly with a plugin that reports support for // Legacy versions. // -// Any future CNI spec versions which meet this definition will be added to +// Any future CNI spec versions which meet this definition should be added to // this list. -var Legacy = PluginSupports("0.1.0", "0.2.0", "0.3.0") +var Legacy = PluginSupports("0.1.0", "0.2.0") diff --git a/plugins/test/noop/main.go b/plugins/test/noop/main.go index 6d6237df..d3c5698f 100644 --- a/plugins/test/noop/main.go +++ b/plugins/test/noop/main.go @@ -64,7 +64,7 @@ func debugBehavior(args *skel.CmdArgs, command string) error { } func debugGetSupportedVersions() []string { - vers := []string{"0.-42.0", "0.1.0", "0.2.0", "0.3.0"} + vers := []string{"0.-42.0", "0.1.0", "0.2.0"} cniArgs := os.Getenv("CNI_ARGS") if cniArgs == "" { return vers diff --git a/plugins/test/noop/noop_test.go b/plugins/test/noop/noop_test.go index 968cc838..cb44817f 100644 --- a/plugins/test/noop/noop_test.go +++ b/plugins/test/noop/noop_test.go @@ -131,7 +131,7 @@ var _ = Describe("No-op plugin", func() { Context("when the CNI_COMMAND is VERSION", func() { BeforeEach(func() { cmd.Env[0] = "CNI_COMMAND=VERSION" - debug.ReportVersionSupport = []string{"0.123.0", "0.3.0"} + debug.ReportVersionSupport = []string{"0.123.0", "0.2.0"} Expect(debug.WriteDebug(debugFileName)).To(Succeed()) }) @@ -144,7 +144,7 @@ var _ = Describe("No-op plugin", func() { pluginInfo, err := decoder.Decode(session.Out.Contents()) Expect(err).NotTo(HaveOccurred()) Expect(pluginInfo.SupportedVersions()).To(ConsistOf( - "0.123.0", "0.3.0")) + "0.123.0", "0.2.0")) }) }) })