From 7f90f9d5593617876546ce2b57b4446ddc33292a Mon Sep 17 00:00:00 2001 From: Stefan Junker Date: Fri, 27 May 2016 10:56:24 +0200 Subject: [PATCH 1/3] pkg/skel: allow arg requriements specified by CMD --- pkg/skel/skel.go | 64 ++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 54 insertions(+), 10 deletions(-) diff --git a/pkg/skel/skel.go b/pkg/skel/skel.go index 7347b078..1f9438c1 100644 --- a/pkg/skel/skel.go +++ b/pkg/skel/skel.go @@ -36,28 +36,72 @@ type CmdArgs struct { StdinData []byte } +type reqForCmdEntry map[string]bool + // PluginMain is the "main" for a plugin. It accepts // two callback functions for add and del commands. func PluginMain(cmdAdd, cmdDel func(_ *CmdArgs) error) { var cmd, contID, netns, ifName, args, path string vars := []struct { - name string - val *string - req bool + name string + val *string + reqForCmd reqForCmdEntry }{ - {"CNI_COMMAND", &cmd, true}, - {"CNI_CONTAINERID", &contID, false}, - {"CNI_NETNS", &netns, true}, - {"CNI_IFNAME", &ifName, true}, - {"CNI_ARGS", &args, false}, - {"CNI_PATH", &path, true}, + { + "CNI_COMMAND", + &cmd, + reqForCmdEntry{ + "ADD": true, + "DEL": true, + }, + }, + { + "CNI_CONTAINERID", + &contID, + reqForCmdEntry{ + "ADD": false, + "DEL": false, + }, + }, + { + "CNI_NETNS", + &netns, + reqForCmdEntry{ + "ADD": true, + "DEL": true, + }, + }, + { + "CNI_IFNAME", + &ifName, + reqForCmdEntry{ + "ADD": true, + "DEL": true, + }, + }, + { + "CNI_ARGS", + &args, + reqForCmdEntry{ + "ADD": false, + "DEL": false, + }, + }, + { + "CNI_PATH", + &path, + reqForCmdEntry{ + "ADD": true, + "DEL": true, + }, + }, } argsMissing := false for _, v := range vars { *v.val = os.Getenv(v.name) - if v.req && *v.val == "" { + if v.reqForCmd[cmd] && *v.val == "" { log.Printf("%v env variable missing", v.name) argsMissing = true } From 72337159c1c28772f752e9abedf74f7b24fce11a Mon Sep 17 00:00:00 2001 From: Stefan Junker Date: Fri, 27 May 2016 10:57:39 +0200 Subject: [PATCH 2/3] plugins: don't require CNI_NETNS for DEL command This will allow to free up the IPAM allocations when the caller doesn't have access to the network namespace anymore, e.g. due to a reboot. --- pkg/skel/skel.go | 2 +- plugins/main/bridge/bridge.go | 4 ++++ plugins/main/ipvlan/ipvlan.go | 4 ++++ plugins/main/macvlan/macvlan.go | 4 ++++ plugins/main/ptp/ptp.go | 4 ++++ 5 files changed, 17 insertions(+), 1 deletion(-) diff --git a/pkg/skel/skel.go b/pkg/skel/skel.go index 1f9438c1..9cf03917 100644 --- a/pkg/skel/skel.go +++ b/pkg/skel/skel.go @@ -69,7 +69,7 @@ func PluginMain(cmdAdd, cmdDel func(_ *CmdArgs) error) { &netns, reqForCmdEntry{ "ADD": true, - "DEL": true, + "DEL": false, }, }, { diff --git a/plugins/main/bridge/bridge.go b/plugins/main/bridge/bridge.go index 6a90a4a6..d4fc89c3 100644 --- a/plugins/main/bridge/bridge.go +++ b/plugins/main/bridge/bridge.go @@ -289,6 +289,10 @@ func cmdDel(args *skel.CmdArgs) error { return err } + if args.Netns == "" { + return nil + } + var ipn *net.IPNet err = ns.WithNetNSPath(args.Netns, func(_ ns.NetNS) error { var err error diff --git a/plugins/main/ipvlan/ipvlan.go b/plugins/main/ipvlan/ipvlan.go index 84f9c77f..d7cfc39f 100644 --- a/plugins/main/ipvlan/ipvlan.go +++ b/plugins/main/ipvlan/ipvlan.go @@ -152,6 +152,10 @@ func cmdDel(args *skel.CmdArgs) error { return err } + if args.Netns == "" { + return nil + } + return ns.WithNetNSPath(args.Netns, func(_ ns.NetNS) error { return ip.DelLinkByName(args.IfName) }) diff --git a/plugins/main/macvlan/macvlan.go b/plugins/main/macvlan/macvlan.go index f7eb6569..7739d7b8 100644 --- a/plugins/main/macvlan/macvlan.go +++ b/plugins/main/macvlan/macvlan.go @@ -170,6 +170,10 @@ func cmdDel(args *skel.CmdArgs) error { return err } + if args.Netns == "" { + return nil + } + return ns.WithNetNSPath(args.Netns, func(_ ns.NetNS) error { return ip.DelLinkByName(args.IfName) }) diff --git a/plugins/main/ptp/ptp.go b/plugins/main/ptp/ptp.go index ec6e23ef..aa695e39 100644 --- a/plugins/main/ptp/ptp.go +++ b/plugins/main/ptp/ptp.go @@ -199,6 +199,10 @@ func cmdDel(args *skel.CmdArgs) error { return err } + if args.Netns == "" { + return nil + } + var ipn *net.IPNet err := ns.WithNetNSPath(args.Netns, func(_ ns.NetNS) error { var err error From d582c9ce8f94536c5c8fe2bc3b01304d82871047 Mon Sep 17 00:00:00 2001 From: Stefan Junker Date: Fri, 27 May 2016 12:26:42 +0200 Subject: [PATCH 3/3] skel/test: add case for empty NETNS --- pkg/skel/skel_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pkg/skel/skel_test.go b/pkg/skel/skel_test.go index 44695648..a52e0145 100644 --- a/pkg/skel/skel_test.go +++ b/pkg/skel/skel_test.go @@ -71,5 +71,14 @@ var _ = Describe("Skel", func() { // Expect(err).NotTo(HaveOccurred()) // PluginMain(fErr, nil) // }) + + It("should not fail with DEL and no NETNS and noop callback", func() { + err := os.Setenv("CNI_COMMAND", "DEL") + Expect(err).NotTo(HaveOccurred()) + err = os.Unsetenv("CNI_NETNS") + Expect(err).NotTo(HaveOccurred()) + PluginMain(nil, fNoop) + }) + }) })