From 30213bd71e4cf7684078614e2253c186cd956f0c Mon Sep 17 00:00:00 2001 From: Marc CARRE Date: Thu, 4 May 2017 15:44:37 +0100 Subject: [PATCH 1/2] Comply to CNI 0.5 spec by always returning nil on DEL. See also: "Plugins should generally complete a DEL action without error even if some resources are missing." in: https://github.com/containernetworking/cni/blob/v0.5.0/SPEC.md --- plugin/net/cni.go | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/plugin/net/cni.go b/plugin/net/cni.go index 0af6e108..b8439584 100644 --- a/plugin/net/cni.go +++ b/plugin/net/cni.go @@ -6,7 +6,6 @@ import ( "fmt" "net" "os" - "strings" "syscall" "github.com/containernetworking/cni/pkg/ipam" @@ -183,16 +182,21 @@ func assignBridgeIP(bridgeName string, ipnet net.IPNet) error { return nil } +// As of CNI 0.5 spec: +// "Plugins should generally complete a DEL action without error even if some resources are missing" +// this method should therefore return nil in most, if not all, cases. func (c *CNIPlugin) CmdDel(args *skel.CmdArgs) error { conf, err := loadNetConf(args.StdinData) if err != nil { - return err + logOnStderr(err) + return nil } // As of CNI 0.3 spec, runtimes can send blank if they just want the address deallocated if args.Netns != "" { if _, err = weavenet.WithNetNS(args.Netns, "del-iface", args.IfName); err != nil { - return fmt.Errorf("error removing interface %q: %s", args.IfName, err) + logOnStderr(fmt.Errorf("error removing interface %q: %s", args.IfName, err)) + return nil } } @@ -202,16 +206,16 @@ func (c *CNIPlugin) CmdDel(args *skel.CmdArgs) error { } else { err = ipam.ExecDel(conf.IPAM.Type, args.StdinData) } - // Hack - don't know how we should detect this situation properly - if args.Netns == "" && strings.Contains(err.Error(), "no addresses") { - err = nil - } - if err != nil { - return fmt.Errorf("unable to release IP address: %s", err) - } + logOnStderr(fmt.Errorf("unable to release IP address: %s", err)) return nil } +// Standard error is inherited from the caller (e.g. Kubelet, when running Weave Net's CNI plugin +// under Kubernetes) hence provided errors should eventually appear in the caller's log files. +func logOnStderr(err error) { + fmt.Fprintln(os.Stderr, "weave-cni:", err) +} + type NetConf struct { types.NetConf BrName string `json:"bridge"` From be11db45d8b45b98301dfa152dbf86617c94a919 Mon Sep 17 00:00:00 2001 From: Marc CARRE Date: Thu, 4 May 2017 15:49:56 +0100 Subject: [PATCH 2/2] Ignore error when deleting a container's interface. Fixes #2921, as all errors are now ignored, including `ENOENT`. This command typically attempts to delete the network interface `eth0` of a container being destroyed by Kubernetes. If this interface has already been destroyed or is missing for whatever reason, we still want to remove this container's entry from IPAM in order to free associated resources. --- plugin/net/cni.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin/net/cni.go b/plugin/net/cni.go index b8439584..8265bf45 100644 --- a/plugin/net/cni.go +++ b/plugin/net/cni.go @@ -195,8 +195,8 @@ func (c *CNIPlugin) CmdDel(args *skel.CmdArgs) error { // As of CNI 0.3 spec, runtimes can send blank if they just want the address deallocated if args.Netns != "" { if _, err = weavenet.WithNetNS(args.Netns, "del-iface", args.IfName); err != nil { + // We log the error and carry on instead of returning nil, as there may still be resources to free in IPAM. logOnStderr(fmt.Errorf("error removing interface %q: %s", args.IfName, err)) - return nil } }