From 2a120531223361f33b2c4d2387b965cbe87a4527 Mon Sep 17 00:00:00 2001 From: Martynas Pumputis Date: Sun, 17 Dec 2017 18:51:05 +0100 Subject: [PATCH] Do not skip bridge initialization if such exists EnsureBridge used to skip creation and initialization of the weave bridge and friends if such existed. This caused problems when EnsureBridge failed before completing all init steps and so, subsequent weaver could not work without "weave reset". This change makes EnsureBridge idempotent. --- common/odp/odp.go | 12 ++++++++++++ net/bridge.go | 35 +++++++++++++++++++++++++---------- net/netlink.go | 16 ++++++++++++++++ net/veth.go | 11 ++++++++--- plugin/net/driver.go | 2 +- 5 files changed, 62 insertions(+), 14 deletions(-) create mode 100644 net/netlink.go diff --git a/common/odp/odp.go b/common/odp/odp.go index 0ea0b8b1..de9a527d 100644 --- a/common/odp/odp.go +++ b/common/odp/odp.go @@ -75,6 +75,14 @@ func DeleteDatapath(dpname string) error { } func AddDatapathInterface(dpname string, ifname string) error { + return addDatapathInterface(dpname, ifname, false) +} + +func AddDatapathInterfaceIfNotExist(dpname string, ifname string) error { + return addDatapathInterface(dpname, ifname, true) +} + +func addDatapathInterface(dpname, ifname string, ignoreIfExist bool) error { dpif, err := odp.NewDpif() if err != nil { return err @@ -87,5 +95,9 @@ func AddDatapathInterface(dpname string, ifname string) error { } _, err = dp.CreateVport(odp.NewNetdevVportSpec(ifname)) + if ignoreIfExist && err == odp.NetlinkError(syscall.EEXIST) { + return nil + } + return err } diff --git a/net/bridge.go b/net/bridge.go index 9359fa86..0c87bcbe 100644 --- a/net/bridge.go +++ b/net/bridge.go @@ -227,12 +227,19 @@ func (config *BridgeConfig) configuredBridgeType() Bridge { } func EnsureBridge(procPath string, config *BridgeConfig, log *logrus.Logger) (Bridge, error) { - bridgeType, err := ExistingBridgeType(config.WeaveBridgeName, config.DatapathName) - if bridgeType != nil || err != nil { - return bridgeType, err + existingBridgeType, err := ExistingBridgeType(config.WeaveBridgeName, config.DatapathName) + if err != nil { + return nil, err + } + + bridgeType := config.configuredBridgeType() + + if existingBridgeType != nil && bridgeType.String() != existingBridgeType.String() { + return nil, + fmt.Errorf("Existing bridge type %q is different than requested %q. Please do 'weave reset' and try again", + existingBridgeType, bridgeType) } - bridgeType = config.configuredBridgeType() for { if err := bridgeType.init(config); err != nil { if errors.Cause(err) == errBridgeNotSupported { @@ -288,7 +295,7 @@ func (b bridgeImpl) initPrep(config *BridgeConfig) error { config.MTU = 65535 } b.bridge = &netlink.Bridge{LinkAttrs: linkAttrs} - if err = netlink.LinkAdd(b.bridge); err != nil { + if err := LinkAddIfNotExist(b.bridge); err != nil { return errors.Wrapf(err, "creating bridge %q", config.WeaveBridgeName) } if err := netlink.LinkSetHardwareAddr(b.bridge, mac); err != nil { @@ -320,7 +327,7 @@ func (b bridgeImpl) init(config *BridgeConfig) error { if err := b.initPrep(config); err != nil { return err } - if _, err := CreateAndAttachVeth(BridgeIfName, PcapIfName, config.WeaveBridgeName, config.MTU, true, func(veth netlink.Link) error { + if _, err := CreateAndAttachVeth(BridgeIfName, PcapIfName, config.WeaveBridgeName, config.MTU, true, false, func(veth netlink.Link) error { return netlink.LinkSetUp(veth) }); err != nil { return errors.Wrap(err, "creating pcap veth pair") @@ -370,11 +377,11 @@ func (bf bridgedFastdpImpl) init(config *BridgeConfig) error { if err := bf.bridgeImpl.initPrep(config); err != nil { return err } - if _, err := CreateAndAttachVeth(BridgeIfName, DatapathIfName, config.WeaveBridgeName, config.MTU, true, func(veth netlink.Link) error { + if _, err := CreateAndAttachVeth(BridgeIfName, DatapathIfName, config.WeaveBridgeName, config.MTU, true, false, func(veth netlink.Link) error { if err := netlink.LinkSetUp(veth); err != nil { return errors.Wrapf(err, "setting link up on %q", veth.Attrs().Name) } - if err := odp.AddDatapathInterface(bf.datapathName, veth.Attrs().Name); err != nil { + if err := odp.AddDatapathInterfaceIfNotExist(bf.datapathName, veth.Attrs().Name); err != nil { return errors.Wrapf(err, "adding interface %q to datapath %q", veth.Attrs().Name, bf.datapathName) } return nil @@ -394,7 +401,7 @@ func (bf bridgedFastdpImpl) attach(veth *netlink.Veth) error { } func (f fastdpImpl) attach(veth *netlink.Veth) error { - return odp.AddDatapathInterface(f.datapathName, veth.Attrs().Name) + return odp.AddDatapathInterfaceIfNotExist(f.datapathName, veth.Attrs().Name) } func configureIPTables(config *BridgeConfig) error { @@ -404,10 +411,18 @@ func configureIPTables(config *BridgeConfig) error { } if config.DockerBridgeName != "" { if config.WeaveBridgeName != config.DockerBridgeName { - err := ipt.Insert("filter", "FORWARD", 1, "-i", config.DockerBridgeName, "-o", config.WeaveBridgeName, "-j", "DROP") + // This check is not ideal, as the rule should be at the very top + // of the chain. + found, err := ipt.Exists("filter", "FORWARD", "-i", config.DockerBridgeName, "-o", config.WeaveBridgeName, "-j", "DROP") if err != nil { return err } + if !found { + err := ipt.Insert("filter", "FORWARD", 1, "-i", config.DockerBridgeName, "-o", config.WeaveBridgeName, "-j", "DROP") + if err != nil { + return err + } + } } dockerBridgeIP, err := FindBridgeIP(config.DockerBridgeName, nil) diff --git a/net/netlink.go b/net/netlink.go new file mode 100644 index 00000000..a6b45d2f --- /dev/null +++ b/net/netlink.go @@ -0,0 +1,16 @@ +package net + +import ( + "syscall" + + "github.com/pkg/errors" + "github.com/vishvananda/netlink" +) + +func LinkAddIfNotExist(link netlink.Link) error { + err := netlink.LinkAdd(link) + if err != nil && err == syscall.EEXIST { + return nil + } + return errors.Wrapf(err, "creating link %q", link) +} diff --git a/net/veth.go b/net/veth.go index f7f44c2a..6c188fb5 100644 --- a/net/veth.go +++ b/net/veth.go @@ -13,7 +13,7 @@ import ( ) // create and attach a veth to the Weave bridge -func CreateAndAttachVeth(name, peerName, bridgeName string, mtu int, keepTXOn bool, init func(peer netlink.Link) error) (*netlink.Veth, error) { +func CreateAndAttachVeth(name, peerName, bridgeName string, mtu int, keepTXOn bool, errIfLinkExist bool, init func(peer netlink.Link) error) (*netlink.Veth, error) { bridge, err := netlink.LinkByName(bridgeName) if err != nil { return nil, fmt.Errorf(`bridge "%s" not present; did you launch weave?`, bridgeName) @@ -28,7 +28,12 @@ func CreateAndAttachVeth(name, peerName, bridgeName string, mtu int, keepTXOn bo MTU: mtu}, PeerName: peerName, } - if err := netlink.LinkAdd(veth); err != nil { + + linkAdd := LinkAddIfNotExist + if errIfLinkExist { + linkAdd = netlink.LinkAdd + } + if err := linkAdd(veth); err != nil { return nil, fmt.Errorf(`could not create veth pair %s-%s: %s`, name, peerName, err) } @@ -116,7 +121,7 @@ func AttachContainer(netNSPath, id, ifName, bridgeName string, mtu int, withMult id = id[:maxIDLen] // trim passed ID if too long } name, peerName := vethPrefix+"pl"+id, vethPrefix+"pg"+id - veth, err := CreateAndAttachVeth(name, peerName, bridgeName, mtu, keepTXOn, func(veth netlink.Link) error { + veth, err := CreateAndAttachVeth(name, peerName, bridgeName, mtu, keepTXOn, true, func(veth netlink.Link) error { if err := netlink.LinkSetNsFd(veth, int(ns)); err != nil { return fmt.Errorf("failed to move veth to container netns: %s", err) } diff --git a/plugin/net/driver.go b/plugin/net/driver.go index e5f78124..70de3efa 100644 --- a/plugin/net/driver.go +++ b/plugin/net/driver.go @@ -134,7 +134,7 @@ func (driver *driver) CreateEndpoint(create *api.CreateEndpointRequest) (*api.Cr // create veths. note we assume endpoint IDs are unique in the first 9 chars name, peerName := vethPair(create.EndpointID) - if _, err := weavenet.CreateAndAttachVeth(name, peerName, weavenet.WeaveBridgeName, 0, false, nil); err != nil { + if _, err := weavenet.CreateAndAttachVeth(name, peerName, weavenet.WeaveBridgeName, 0, false, true, nil); err != nil { return nil, driver.error("JoinEndpoint", "%s", err) }