Skip to content
This repository has been archived by the owner on Jun 20, 2024. It is now read-only.

Commit

Permalink
Do not skip bridge initialization if such exists
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
brb committed Dec 17, 2017
1 parent 1860f21 commit 2a12053
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 14 deletions.
12 changes: 12 additions & 0 deletions common/odp/odp.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
35 changes: 25 additions & 10 deletions net/bridge.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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)
Expand Down
16 changes: 16 additions & 0 deletions net/netlink.go
Original file line number Diff line number Diff line change
@@ -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)
}
11 changes: 8 additions & 3 deletions net/veth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
}

Expand Down Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion plugin/net/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down

0 comments on commit 2a12053

Please sign in to comment.