From 673d0af97d12f64a841c5a6ec24dca53e46784f6 Mon Sep 17 00:00:00 2001 From: Marc CARRE Date: Mon, 13 Mar 2017 13:16:18 +0000 Subject: [PATCH 01/11] Add failing test catching defunct launch.sh processses. Reproduces issue #2836, i.e.: - during initialisation: ``` $ ps auxf [...] root 1380 0.4 1.6 879660 60732 ? Ssl Mar13 5:54 /usr/bin/docker daemon -H fd:// -H unix:/// root 1664 0.0 0.4 502116 18540 ? Ssl Mar13 0:06 \_ docker-containerd -l /var/run/docker/li root 12615 0.0 0.1 200496 5424 ? Sl 14:38 0:00 \_ docker-containerd-shim c5637e5bbdcb root 12629 6.2 1.6 296400 62312 ? Ssl 14:38 0:00 | \_ /home/weave/weaver --port=6783 root 12910 0.0 0.0 1524 68 ? S 14:38 0:00 | \_ /bin/sh /home/weave/launch. root 13002 0.0 0.0 1772 1232 ? S 14:38 0:00 | \_ /bin/sh /home/weave/wea root 13027 0.0 0.0 1772 320 ? S 14:38 0:00 | \_ /bin/sh /home/weave root 13028 0.0 0.0 14656 2708 ? S 14:38 0:00 | \_ curl -o /tmp/we [...] ``` - after initialisation: ``` $ ps auxf [...] root 1380 0.4 1.6 879660 60732 ? Ssl Mar13 5:54 /usr/bin/docker daemon -H fd:// -H unix:/// root 1664 0.0 0.4 502116 18540 ? Ssl Mar13 0:06 \_ docker-containerd -l /var/run/docker/li root 12615 0.0 0.1 200496 5424 ? Sl 14:38 0:00 \_ docker-containerd-shim c5637e5bbdcb root 12629 3.5 1.6 297460 63340 ? Ssl 14:38 0:00 | \_ /home/weave/weaver --port=6783 root 12910 0.0 0.0 0 0 ? Z 14:38 0:00 | \_ [launch.sh] [...] ``` Sample output: ``` test #5 "run_on mct-0.us-central1-a.weave-net ps aux | grep -c '[d]efunct'" failed: expected "0" got "1" test #6 "run_on mct-1.us-central1-a.weave-net ps aux | grep -c '[d]efunct'" failed: expected "0" got "1" test #7 "run_on mct-2.us-central1-a.weave-net ps aux | grep -c '[d]efunct'" failed: expected "0" got "1" 3 of 8 tests failed in 62.012s. ``` --- test/840_weave_kube_3_test.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/840_weave_kube_3_test.sh b/test/840_weave_kube_3_test.sh index 7823f929..6a9e8db6 100755 --- a/test/840_weave_kube_3_test.sh +++ b/test/840_weave_kube_3_test.sh @@ -52,6 +52,11 @@ assert_raises "run_on $HOST1 $PING $HOST2EXPIP" assert_raises "run_on $HOST2 $PING $HOST1EXPIP" assert_raises "run_on $HOST3 $PING $HOST2EXPIP" +# Ensure we do not generate any defunct process (e.g. launch.sh) after starting weaver: +assert "run_on $HOST1 ps aux | grep -c '[d]efunct'" "0" +assert "run_on $HOST2 ps aux | grep -c '[d]efunct'" "0" +assert "run_on $HOST3 ps aux | grep -c '[d]efunct'" "0" + # See if we can get some pods running that connect to the network run_on $HOST1 "kubectl run hello --image=weaveworks/hello-world --replicas=3" From 21a0fc30e81ae824ddca607f4fd46e85047c0a7d Mon Sep 17 00:00:00 2001 From: Marc CARRE Date: Mon, 13 Mar 2017 14:49:40 +0000 Subject: [PATCH 02/11] Run weaver without exec. This: 1. prevents from generating defunct (a.k.a. zombie) processes, and therefore fixes #2836; but also 2. reintroduces #2684/#2688 as shells do not forward signals and we are still running more than one process. A proper fix would leverage something like tini. See also: - github.com/krallin/tini - github.com/krallin/tini/issues/8 - github.com/docker-library/official-images#init Sample output: - during initialisation: ``` $ ps auxf [...] root 1380 0.4 1.9 879660 74660 ? Ssl Mar13 6:02 /usr/bin/docker daemon -H fd:// -H unix:/// root 1664 0.0 0.4 502116 18528 ? Ssl Mar13 0:06 \_ docker-containerd -l /var/run/docker/li root 15482 0.0 0.0 200496 3364 ? Sl 15:04 0:00 \_ docker-containerd-shim 3d1f5eb6e090 root 15496 0.0 0.0 1524 996 ? Ss 15:04 0:00 | \_ /bin/sh /home/weave/launch.sh root 15780 0.0 0.0 1524 64 ? S 15:04 0:00 | \_ /bin/sh /home/weave/launch. root 15872 0.0 0.0 1772 1272 ? S 15:04 0:00 | | \_ /bin/sh /home/weave/wea root 15897 0.0 0.0 1772 324 ? S 15:04 0:00 | | \_ /bin/sh /home/weave root 15898 0.0 0.0 14656 2948 ? S 15:04 0:00 | | \_ curl -o /tmp/we root 15781 10.0 1.5 484556 60296 ? Sl 15:04 0:00 | \_ /home/weave/weaver --port=6 [...] ``` - after initialisation: ``` $ ps auxf [...] root 1380 0.4 1.9 879660 74660 ? Ssl Mar13 6:02 /usr/bin/docker daemon -H fd:// -H unix:/// root 1664 0.0 0.4 502116 18592 ? Ssl Mar13 0:06 \_ docker-containerd -l /var/run/docker/li root 15482 0.0 0.0 200496 3364 ? Sl 15:04 0:00 \_ docker-containerd-shim 3d1f5eb6e090 root 15496 0.0 0.0 1524 996 ? Ss 15:04 0:00 | \_ /bin/sh /home/weave/launch.sh root 15781 2.3 1.6 503064 64320 ? Sl 15:04 0:00 | \_ /home/weave/weaver --port=6[...] ``` --- prog/weave-kube/launch.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prog/weave-kube/launch.sh b/prog/weave-kube/launch.sh index e7f79331..1b851d49 100755 --- a/prog/weave-kube/launch.sh +++ b/prog/weave-kube/launch.sh @@ -117,7 +117,7 @@ post_start_actions() { post_start_actions & -exec /home/weave/weaver $EXTRA_ARGS --port=6783 $BRIDGE_OPTIONS \ +/home/weave/weaver $EXTRA_ARGS --port=6783 $BRIDGE_OPTIONS \ --http-addr=$HTTP_ADDR --status-addr=$STATUS_ADDR --docker-api='' --no-dns \ --ipalloc-range=$IPALLOC_RANGE $NICKNAME_ARG \ --ipalloc-init $IPALLOC_INIT \ From dc2925263c5920dd501ad73e57a2512185bc1009 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stephan=20Austerm=C3=BChle?= Date: Thu, 23 Feb 2017 17:37:31 +0100 Subject: [PATCH 03/11] Destroy ipsets prefixed `weave-` only (issue #2751) --- npc/constants.go | 2 ++ npc/ipset/ipset.go | 55 +++++++++++++++++++++++++++++++++--------- npc/selector.go | 2 +- prog/weave-npc/main.go | 16 +++++++++--- 4 files changed, 59 insertions(+), 16 deletions(-) diff --git a/npc/constants.go b/npc/constants.go index 64565484..7ab01f7b 100644 --- a/npc/constants.go +++ b/npc/constants.go @@ -6,4 +6,6 @@ const ( MainChain = "WEAVE-NPC" DefaultChain = "WEAVE-NPC-DEFAULT" IngressChain = "WEAVE-NPC-INGRESS" + + IpsetNamePrefix = "weave-" ) diff --git a/npc/ipset/ipset.go b/npc/ipset/ipset.go index 3a27327b..1a197bb1 100644 --- a/npc/ipset/ipset.go +++ b/npc/ipset/ipset.go @@ -1,7 +1,9 @@ package ipset import ( + "bytes" "os/exec" + "strings" "github.com/pkg/errors" ) @@ -22,6 +24,8 @@ type Interface interface { Flush(ipsetName Name) error Destroy(ipsetName Name) error + List(prefix string) ([]Name, error) + FlushAll() error DestroyAll() error } @@ -35,48 +39,77 @@ func New() Interface { } func (i *ipset) Create(ipsetName Name, ipsetType Type) error { - return doExec("create", string(ipsetName), string(ipsetType)) + err, _ := doExec("create", string(ipsetName), string(ipsetType)) + return err } func (i *ipset) AddEntry(ipsetName Name, entry string) error { if i.inc(ipsetName, entry) > 1 { // already in the set return nil } - return doExec("add", string(ipsetName), entry) + err, _ := doExec("add", string(ipsetName), entry) + return err } func (i *ipset) DelEntry(ipsetName Name, entry string) error { if i.dec(ipsetName, entry) > 0 { // still needed return nil } - return doExec("del", string(ipsetName), entry) + err, _ := doExec("del", string(ipsetName), entry) + return err } func (i *ipset) Flush(ipsetName Name) error { i.removeSet(ipsetName) - return doExec("flush", string(ipsetName)) + err, _ := doExec("flush", string(ipsetName)) + return err } func (i *ipset) FlushAll() error { i.refCount = newRefCount() - return doExec("flush") + err, _ := doExec("flush") + return err } func (i *ipset) Destroy(ipsetName Name) error { i.removeSet(ipsetName) - return doExec("destroy", string(ipsetName)) + err, _ := doExec("destroy", string(ipsetName)) + return err } func (i *ipset) DestroyAll() error { i.refCount = newRefCount() - return doExec("destroy") + err, _ := doExec("destroy") + return err } -func doExec(args ...string) error { - if output, err := exec.Command("ipset", args...).CombinedOutput(); err != nil { - return errors.Wrapf(err, "ipset %v failed: %s", args, output) +// Fetch a list of all existing sets +func (i *ipset) List(prefix string) ([]Name, error) { + err, output := doExec("list","-name","-output","plain") + + var selected []Name + if err == nil && len(output) > 0 { + output = bytes.TrimRight(output, "\n") + sets := strings.Split(string(output[:]), "\n") + + plen := len(prefix) + for _, v := range sets { + if (plen <= len(v)) && (prefix == v[:len(prefix)]) { + selected = append(selected, Name(v)) + } + } + } - return nil + + return selected, err +} + +func doExec(args ...string) (error, []byte) { + output, err := exec.Command("ipset", args...).CombinedOutput() + if err != nil { + return errors.Wrapf(err, "ipset %v failed: %s", args, output), output + } + return nil, output } // Reference-counting diff --git a/npc/selector.go b/npc/selector.go index b03c97b1..1f34fa3b 100644 --- a/npc/selector.go +++ b/npc/selector.go @@ -29,7 +29,7 @@ func newSelectorSpec(json *unversioned.LabelSelector, nsName string, ipsetType i // We prefix the selector string with the namespace name when generating // the shortname because you can specify the same selector in multiple // namespaces - we need those to map to distinct ipsets - ipsetName: ipset.Name("weave-" + shortName(nsName+":"+key)), + ipsetName: ipset.Name(IpsetNamePrefix + shortName(nsName+":"+key)), ipsetType: ipsetType}, nil } diff --git a/prog/weave-npc/main.go b/prog/weave-npc/main.go index 2c0623a2..9908ac22 100644 --- a/prog/weave-npc/main.go +++ b/prog/weave-npc/main.go @@ -80,14 +80,22 @@ func resetIPTables(ipt *iptables.IPTables) error { } func resetIPSets(ips ipset.Interface) error { - // TODO should restrict ipset operations to the `weave-` prefix: + // Remove ipsets prefixed `weave-` only - if err := ips.FlushAll(); err != nil { + sets, err := ips.List(npc.IpsetNamePrefix) + if (err != nil) { + common.Log.Errorf("Failed to retrieve list of ipsets") return err } - if err := ips.DestroyAll(); err != nil { - return err + common.Log.Debugf("Got list of ipsets: %v", sets) + + for _, s := range sets { + common.Log.Debugf("Destroying ipsets '%s'", string(s)) + if err := ips.Destroy(s); err != nil { + common.Log.Errorf("Failed to destroy ipset '%s'", string(s)) + return err + } } return nil From f9f3b18ce4376be013e2ef08d931531ba44e0c90 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Mon, 6 Mar 2017 17:30:13 +0000 Subject: [PATCH 04/11] Test whether non-weave ipsets get blown away --- test/840_weave_kube_3_test.sh | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/840_weave_kube_3_test.sh b/test/840_weave_kube_3_test.sh index 6a9e8db6..d206524e 100755 --- a/test/840_weave_kube_3_test.sh +++ b/test/840_weave_kube_3_test.sh @@ -20,6 +20,10 @@ SUCCESS="6 established" tear_down_kubeadm +# Make an ipset, so we can check it doesn't get wiped out by Weave Net +docker_on $HOST1 run --rm --privileged --net=host --entrypoint=/usr/sbin/ipset weaveworks/weave-npc create test_840_ipset bitmap:ip range 192.168.1.0/24 || true +docker_on $HOST1 run --rm --privileged --net=host --entrypoint=/usr/sbin/ipset weaveworks/weave-npc add test_840_ipset 192.168.1.11 + run_on $HOST1 "sudo systemctl start kubelet && sudo kubeadm init --token=$TOKEN" run_on $HOST2 "sudo systemctl start kubelet && sudo kubeadm join --token=$TOKEN $HOST1IP" run_on $HOST3 "sudo systemctl start kubelet && sudo kubeadm join --token=$TOKEN $HOST1IP" @@ -76,4 +80,7 @@ assert_raises wait_for_pods tear_down_kubeadm +# Destroy our test ipset, and implicitly check it is still there +assert_raises "docker_on $HOST1 run --rm --privileged --net=host --entrypoint=/usr/sbin/ipset weaveworks/weave-npc destroy test_840_ipset" + end_suite From 6ddbc5cb572ca4424d7be337714f2b3270b906d5 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Mon, 6 Mar 2017 17:31:09 +0000 Subject: [PATCH 05/11] Simplify List() --- npc/ipset/ipset.go | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/npc/ipset/ipset.go b/npc/ipset/ipset.go index 1a197bb1..a30f799c 100644 --- a/npc/ipset/ipset.go +++ b/npc/ipset/ipset.go @@ -1,7 +1,6 @@ package ipset import ( - "bytes" "os/exec" "strings" @@ -83,22 +82,19 @@ func (i *ipset) DestroyAll() error { return err } -// Fetch a list of all existing sets +// Fetch a list of all existing sets with a given prefix func (i *ipset) List(prefix string) ([]Name, error) { err, output := doExec("list","-name","-output","plain") + if err != nil { + return nil, err + } var selected []Name - if err == nil && len(output) > 0 { - output = bytes.TrimRight(output, "\n") - sets := strings.Split(string(output[:]), "\n") - - plen := len(prefix) - for _, v := range sets { - if (plen <= len(v)) && (prefix == v[:len(prefix)]) { - selected = append(selected, Name(v)) - } + sets := strings.Split(string(output), "\n") + for _, v := range sets { + if strings.HasPrefix(v, prefix) { + selected = append(selected, Name(v)) } - } return selected, err From 5f30846092ae25eaa0d1392127bf333fda8c744e Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Mon, 6 Mar 2017 17:46:28 +0000 Subject: [PATCH 06/11] Unwind changes to doExec and handle List() error differently exec.Output() already wraps the stderr output, so we don't need to do that. --- npc/ipset/ipset.go | 32 ++++++++++++-------------------- 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/npc/ipset/ipset.go b/npc/ipset/ipset.go index a30f799c..d4dd5160 100644 --- a/npc/ipset/ipset.go +++ b/npc/ipset/ipset.go @@ -38,53 +38,46 @@ func New() Interface { } func (i *ipset) Create(ipsetName Name, ipsetType Type) error { - err, _ := doExec("create", string(ipsetName), string(ipsetType)) - return err + return doExec("create", string(ipsetName), string(ipsetType)) } func (i *ipset) AddEntry(ipsetName Name, entry string) error { if i.inc(ipsetName, entry) > 1 { // already in the set return nil } - err, _ := doExec("add", string(ipsetName), entry) - return err + return doExec("add", string(ipsetName), entry) } func (i *ipset) DelEntry(ipsetName Name, entry string) error { if i.dec(ipsetName, entry) > 0 { // still needed return nil } - err, _ := doExec("del", string(ipsetName), entry) - return err + return doExec("del", string(ipsetName), entry) } func (i *ipset) Flush(ipsetName Name) error { i.removeSet(ipsetName) - err, _ := doExec("flush", string(ipsetName)) - return err + return doExec("flush", string(ipsetName)) } func (i *ipset) FlushAll() error { i.refCount = newRefCount() - err, _ := doExec("flush") - return err + return doExec("flush") } func (i *ipset) Destroy(ipsetName Name) error { i.removeSet(ipsetName) - err, _ := doExec("destroy", string(ipsetName)) - return err + return doExec("destroy", string(ipsetName)) } func (i *ipset) DestroyAll() error { i.refCount = newRefCount() - err, _ := doExec("destroy") - return err + return doExec("destroy") } // Fetch a list of all existing sets with a given prefix func (i *ipset) List(prefix string) ([]Name, error) { - err, output := doExec("list","-name","-output","plain") + output, err := exec.Command("ipset", "list", "-name", "-output", "plain").Output() if err != nil { return nil, err } @@ -100,12 +93,11 @@ func (i *ipset) List(prefix string) ([]Name, error) { return selected, err } -func doExec(args ...string) (error, []byte) { - output, err := exec.Command("ipset", args...).CombinedOutput() - if err != nil { - return errors.Wrapf(err, "ipset %v failed: %s", args, output), output +func doExec(args ...string) error { + if output, err := exec.Command("ipset", args...).CombinedOutput(); err != nil { + return errors.Wrapf(err, "ipset %v failed: %s", args, output) } - return nil, output + return nil } // Reference-counting From 623abeef4c764bcae1547bfa67a22bb43820f367 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Mon, 6 Mar 2017 18:06:09 +0000 Subject: [PATCH 07/11] gofmt --- prog/weave-npc/main.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/prog/weave-npc/main.go b/prog/weave-npc/main.go index 9908ac22..4a2f205f 100644 --- a/prog/weave-npc/main.go +++ b/prog/weave-npc/main.go @@ -83,12 +83,12 @@ func resetIPSets(ips ipset.Interface) error { // Remove ipsets prefixed `weave-` only sets, err := ips.List(npc.IpsetNamePrefix) - if (err != nil) { + if err != nil { common.Log.Errorf("Failed to retrieve list of ipsets") return err } - common.Log.Debugf("Got list of ipsets: %v", sets) + common.Log.Debugf("Got list of ipsets: %v", sets) for _, s := range sets { common.Log.Debugf("Destroying ipsets '%s'", string(s)) From c3c28ea27e5fda4220e65fd1d02e23a67a04c610 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 15 Mar 2017 13:49:24 +0000 Subject: [PATCH 08/11] Update CNI library to version 0.5.0 --- .gitmodules | 6 ++-- Makefile | 2 ++ plugin/ipam/cni.go | 24 +++++++------ plugin/net/cni.go | 42 ++++++++++++++--------- prog/weaveutil/cni.go | 7 ++-- vendor/github.com/appc/cni | 1 - vendor/github.com/containernetworking/cni | 1 + 7 files changed, 49 insertions(+), 34 deletions(-) delete mode 160000 vendor/github.com/appc/cni create mode 160000 vendor/github.com/containernetworking/cni diff --git a/.gitmodules b/.gitmodules index d653c81a..918c7914 100644 --- a/.gitmodules +++ b/.gitmodules @@ -61,9 +61,6 @@ [submodule "vendor/github.com/hashicorp/go-cleanhttp"] path = vendor/github.com/hashicorp/go-cleanhttp url = http://github.com/hashicorp/go-cleanhttp -[submodule "vendor/github.com/appc/cni"] - path = vendor/github.com/appc/cni - url = https://github.com/appc/cni [submodule "vendor/github.com/coreos/go-iptables"] path = vendor/github.com/coreos/go-iptables url = https://github.com/coreos/go-iptables @@ -124,3 +121,6 @@ [submodule "vendor/github.com/weaveworks/common"] path = vendor/github.com/weaveworks/common url = https://github.com/weaveworks/common +[submodule "vendor/github.com/containernetworking/cni"] + path = vendor/github.com/containernetworking/cni + url = https://github.com/containernetworking/cni diff --git a/Makefile b/Makefile index 5fc050a5..63506a8a 100644 --- a/Makefile +++ b/Makefile @@ -173,6 +173,8 @@ ifeq ($(BUILD_IN_CONTAINER),true) # It bind-mounts the source into the container and passes all important variables exes $(EXES) tests lint: $(BUILD_UPTODATE) git submodule update --init +# Containernetworking has another copy of vishvananda/netlink which leads to duplicate definitions + -@rm -r vendor/github.com/containernetworking/cni/vendor @mkdir -p $(shell pwd)/.pkg $(SUDO) docker run $(RM) $(RUN_FLAGS) \ -v $(shell pwd):/go/src/github.com/weaveworks/weave \ diff --git a/plugin/ipam/cni.go b/plugin/ipam/cni.go index 4c9a45a8..ac371f6d 100644 --- a/plugin/ipam/cni.go +++ b/plugin/ipam/cni.go @@ -5,8 +5,9 @@ import ( "fmt" "net" - "github.com/appc/cni/pkg/skel" - "github.com/appc/cni/pkg/types" + "github.com/containernetworking/cni/pkg/skel" + "github.com/containernetworking/cni/pkg/types" + "github.com/containernetworking/cni/pkg/types/current" ) func (i *Ipam) CmdAdd(args *skel.CmdArgs) error { @@ -17,7 +18,7 @@ func (i *Ipam) CmdAdd(args *skel.CmdArgs) error { return result.Print() } -func (i *Ipam) Allocate(args *skel.CmdArgs) (*types.Result, error) { +func (i *Ipam) Allocate(args *skel.CmdArgs) (types.Result, error) { // extract the things we care about conf, err := loadIPAMConf(args.StdinData) if err != nil { @@ -46,12 +47,13 @@ func (i *Ipam) Allocate(args *skel.CmdArgs) (*types.Result, error) { if err != nil { return nil, err } - result := &types.Result{ - IP4: &types.IPConfig{ - IP: *ipnet, + result := ¤t.Result{ + IPs: []*current.IPConfig{{ + Version: "4", + Address: *ipnet, Gateway: conf.Gateway, - Routes: conf.Routes, - }, + }}, + Routes: conf.Routes, } return result, nil } @@ -65,9 +67,9 @@ func (i *Ipam) Release(args *skel.CmdArgs) error { } type ipamConf struct { - Subnet string `json:"subnet,omitempty"` - Gateway net.IP `json:"gateway,omitempty"` - Routes []types.Route `json:"routes"` + Subnet string `json:"subnet,omitempty"` + Gateway net.IP `json:"gateway,omitempty"` + Routes []*types.Route `json:"routes"` } type netConf struct { diff --git a/plugin/net/cni.go b/plugin/net/cni.go index 5ed1c35a..5cb47e91 100644 --- a/plugin/net/cni.go +++ b/plugin/net/cni.go @@ -7,9 +7,10 @@ import ( "net" "syscall" - "github.com/appc/cni/pkg/ipam" - "github.com/appc/cni/pkg/skel" - "github.com/appc/cni/pkg/types" + "github.com/containernetworking/cni/pkg/ipam" + "github.com/containernetworking/cni/pkg/skel" + "github.com/containernetworking/cni/pkg/types" + "github.com/containernetworking/cni/pkg/types/current" "github.com/vishvananda/netlink" "github.com/vishvananda/netns" weaveapi "github.com/weaveworks/weave/api" @@ -40,17 +41,23 @@ func loadNetConf(bytes []byte) (*NetConf, error) { return n, nil } -func (c *CNIPlugin) getIP(ipamType string, args *skel.CmdArgs) (result *types.Result, err error) { +func (c *CNIPlugin) getIP(ipamType string, args *skel.CmdArgs) (newResult *current.Result, err error) { + var result types.Result // Default IPAM is Weave's own if ipamType == "" { result, err = ipamplugin.NewIpam(c.weave).Allocate(args) } else { result, err = ipam.ExecAdd(ipamType, args.StdinData) } - if err == nil && result.IP4 == nil { + if err != nil { + return nil, err + } + newResult, err = current.NewResultFromResult(result) + // Check if ipam returned no results without error + if err == nil && len(newResult.IPs) == 0 { return nil, fmt.Errorf("IPAM plugin failed to allocate IP address") } - return result, err + return newResult, err } func (c *CNIPlugin) CmdAdd(args *skel.CmdArgs) error { @@ -70,10 +77,12 @@ func (c *CNIPlugin) CmdAdd(args *skel.CmdArgs) error { if err != nil { return fmt.Errorf("unable to allocate IP address: %s", err) } + // Only expecting one address + ip := result.IPs[0] // If config says nothing about routes or gateway, default one will be via the bridge - if result.IP4.Routes == nil && result.IP4.Gateway == nil { - bridgeIP, err := weavenet.FindBridgeIP(conf.BrName, &result.IP4.IP) + if len(result.Routes) == 0 && ip.Gateway == nil { + bridgeIP, err := weavenet.FindBridgeIP(conf.BrName, &ip.Address) if err == weavenet.ErrBridgeNoIP { bridgeArgs := *args bridgeArgs.ContainerID = "weave:expose" @@ -81,17 +90,18 @@ func (c *CNIPlugin) CmdAdd(args *skel.CmdArgs) error { if err != nil { return fmt.Errorf("unable to allocate IP address for bridge: %s", err) } - if err := assignBridgeIP(conf.BrName, bridgeIPResult.IP4.IP); err != nil { + bridgeCIDR := bridgeIPResult.IPs[0].Address + if err := assignBridgeIP(conf.BrName, bridgeCIDR); err != nil { return fmt.Errorf("unable to assign IP address to bridge: %s", err) } - if err := weavenet.ExposeNAT(bridgeIPResult.IP4.IP); err != nil { + if err := weavenet.ExposeNAT(bridgeCIDR); err != nil { return fmt.Errorf("unable to create NAT rules: %s", err) } - bridgeIP = bridgeIPResult.IP4.IP.IP + bridgeIP = bridgeCIDR.IP } else if err != nil { return err } - result.IP4.Gateway = bridgeIP + result.IPs[0].Gateway = bridgeIP } ns, err := netns.GetFromPath(args.Netns) @@ -110,11 +120,11 @@ func (c *CNIPlugin) CmdAdd(args *skel.CmdArgs) error { id = fmt.Sprintf("%x", data) } - if err := weavenet.AttachContainer(args.Netns, id, args.IfName, conf.BrName, conf.MTU, false, []*net.IPNet{&result.IP4.IP}, false); err != nil { + if err := weavenet.AttachContainer(args.Netns, id, args.IfName, conf.BrName, conf.MTU, false, []*net.IPNet{&ip.Address}, false); err != nil { return err } if err := weavenet.WithNetNSLinkUnsafe(ns, args.IfName, func(link netlink.Link) error { - return setupRoutes(link, args.IfName, result.IP4.IP, result.IP4.Gateway, result.IP4.Routes) + return setupRoutes(link, args.IfName, ip.Address, ip.Gateway, result.Routes) }); err != nil { return fmt.Errorf("error setting up routes: %s", err) } @@ -123,7 +133,7 @@ func (c *CNIPlugin) CmdAdd(args *skel.CmdArgs) error { return result.Print() } -func setupRoutes(link netlink.Link, name string, ipnet net.IPNet, gw net.IP, routes []types.Route) error { +func setupRoutes(link netlink.Link, name string, ipnet net.IPNet, gw net.IP, routes []*types.Route) error { var err error if routes == nil { // If config says nothing about routes, add a default one if !ipnet.Contains(gw) { @@ -133,7 +143,7 @@ func setupRoutes(link netlink.Link, name string, ipnet net.IPNet, gw net.IP, rou return err } } - routes = []types.Route{{Dst: zeroNetwork}} + routes = []*types.Route{{Dst: zeroNetwork}} } for _, r := range routes { if r.GW != nil { diff --git a/prog/weaveutil/cni.go b/prog/weaveutil/cni.go index 3f5dba4d..e6157261 100644 --- a/prog/weaveutil/cni.go +++ b/prog/weaveutil/cni.go @@ -3,7 +3,8 @@ package main import ( "os" - cni "github.com/appc/cni/pkg/skel" + cni "github.com/containernetworking/cni/pkg/skel" + "github.com/containernetworking/cni/pkg/version" weaveapi "github.com/weaveworks/weave/api" "github.com/weaveworks/weave/common" @@ -14,13 +15,13 @@ import ( func cniNet(args []string) error { weave := weaveapi.NewClient(os.Getenv("WEAVE_HTTP_ADDR"), common.Log) n := netplugin.NewCNIPlugin(weave) - cni.PluginMain(n.CmdAdd, n.CmdDel) + cni.PluginMain(n.CmdAdd, n.CmdDel, version.PluginSupports("0.1.0", "0.2.0", "0.3.0")) return nil } func cniIPAM(args []string) error { weave := weaveapi.NewClient(os.Getenv("WEAVE_HTTP_ADDR"), common.Log) i := ipamplugin.NewIpam(weave) - cni.PluginMain(i.CmdAdd, i.CmdDel) + cni.PluginMain(i.CmdAdd, i.CmdDel, version.PluginSupports("0.1.0", "0.2.0", "0.3.0")) return nil } diff --git a/vendor/github.com/appc/cni b/vendor/github.com/appc/cni deleted file mode 160000 index 8bb41b9d..00000000 --- a/vendor/github.com/appc/cni +++ /dev/null @@ -1 +0,0 @@ -Subproject commit 8bb41b9d6f5029dd7bbdb17045d561736f9fb61a diff --git a/vendor/github.com/containernetworking/cni b/vendor/github.com/containernetworking/cni new file mode 160000 index 00000000..4ce9b019 --- /dev/null +++ b/vendor/github.com/containernetworking/cni @@ -0,0 +1 @@ +Subproject commit 4ce9b019aab51b28a32ff6549784a69f9b209fe4 From 5a6cb8e7080350e06ffa297fb3e3d4fc5aa72381 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 15 Mar 2017 16:20:41 +0000 Subject: [PATCH 09/11] Allow CNI netns to be blank on DEL Also don't error on DEL with no namespace and no addresses --- plugin/net/cni.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/plugin/net/cni.go b/plugin/net/cni.go index 5cb47e91..452b1b83 100644 --- a/plugin/net/cni.go +++ b/plugin/net/cni.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "net" + "strings" "syscall" "github.com/containernetworking/cni/pkg/ipam" @@ -180,8 +181,11 @@ func (c *CNIPlugin) CmdDel(args *skel.CmdArgs) error { return err } - if _, err = weavenet.WithNetNS(args.Netns, "del-iface", args.IfName); err != nil { - return fmt.Errorf("error removing interface %q: %s", args.IfName, err) + // 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) + } } // Default IPAM is Weave's own @@ -190,6 +194,10 @@ 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) } From b5172f46c162b573a14b33dc0fd26b9937439953 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 15 Mar 2017 17:48:12 +0000 Subject: [PATCH 10/11] Honour the input version number --- plugin/ipam/cni.go | 6 +++++- plugin/net/cni.go | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/plugin/ipam/cni.go b/plugin/ipam/cni.go index ac371f6d..628cb1db 100644 --- a/plugin/ipam/cni.go +++ b/plugin/ipam/cni.go @@ -11,11 +11,15 @@ import ( ) func (i *Ipam) CmdAdd(args *skel.CmdArgs) error { + var conf types.NetConf + if err := json.Unmarshal(args.StdinData, &conf); err != nil { + return fmt.Errorf("failed to load netconf: %v", err) + } result, err := i.Allocate(args) if err != nil { return err } - return result.Print() + return types.PrintResult(result, conf.CNIVersion) } func (i *Ipam) Allocate(args *skel.CmdArgs) (types.Result, error) { diff --git a/plugin/net/cni.go b/plugin/net/cni.go index 452b1b83..fcf208fc 100644 --- a/plugin/net/cni.go +++ b/plugin/net/cni.go @@ -131,7 +131,7 @@ func (c *CNIPlugin) CmdAdd(args *skel.CmdArgs) error { } result.DNS = conf.DNS - return result.Print() + return types.PrintResult(result, conf.CNIVersion) } func setupRoutes(link netlink.Link, name string, ipnet net.IPNet, gw net.IP, routes []*types.Route) error { From 31b7e063e7da3af12b8d3f7a61e2820273dc1511 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 15 Mar 2017 17:56:26 +0000 Subject: [PATCH 11/11] Use version 3 in the test CNI_VERSION env var was never used, so removed from the test script. The second test is crashing on v1, so bumped to v3. --- test/830_cni_plugin_test.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/830_cni_plugin_test.sh b/test/830_cni_plugin_test.sh index b5d2cd4a..f33485fe 100755 --- a/test/830_cni_plugin_test.sh +++ b/test/830_cni_plugin_test.sh @@ -7,7 +7,7 @@ start_suite "Test CNI plugin" cni_connect() { pid=$(container_pid $1 $2) id=$(docker_on $1 inspect -f '{{.Id}}' $2) - run_on $1 sudo CNI_VERSION=1 CNI_COMMAND=ADD CNI_CONTAINERID=$id CNI_IFNAME=eth0 \ + run_on $1 sudo CNI_COMMAND=ADD CNI_CONTAINERID=$id CNI_IFNAME=eth0 \ CNI_NETNS=/proc/$pid/ns/net CNI_PATH=/opt/cni/bin /opt/cni/bin/weave-net } @@ -30,6 +30,7 @@ EOF cni_connect $HOST1 c2 <