reweave/weave
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #3204 from weaveworks/issues/3133-idempotent-bridge

Browse Source 
Do not skip bridge creation if bridge exists
This commit is contained in:
Bryan Boreham authored and GitHub committed 2018-01-09 17:20:06 +00:00
2 parents 3074892f1b 1d67e2f955
commit 6cfb208f3f
12 files changed
+188 -111
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

common/odp/odp.go
+12
View File
@@ -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
}
net/bridge.go
+83 -33
View File
@@ -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 is not ideal, as it does not check whether the rule is at the 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)
@@ -435,48 +450,46 @@ func configureIPTables(config *BridgeConfig) error {
}
}
// The order among weave filter/FORWARD rules is important!
fwdRules := make([][]string, 0)
if config.NPC {
// Steer traffic via the NPC
_ = ipt.NewChain("filter", "WEAVE-NPC") // ignore error because it might already exist
// If WEAVE-NPC chain doesn't exist then next line will fail
if err = ipt.AppendUnique("filter", "FORWARD", "-o", config.WeaveBridgeName, "-j", "WEAVE-NPC"); err != nil {
return err
}
if err = ipt.AppendUnique("filter", "FORWARD", "-o", config.WeaveBridgeName, "-m", "state", "--state", "NEW", "-j", "NFLOG", "--nflog-group", "86"); err != nil {
return err
}
if err = ipt.AppendUnique("filter", "FORWARD", "-o", config.WeaveBridgeName, "-j", "DROP"); err != nil {
return err
}
// If WEAVE-NPC chain doesn't exist then creating a rule in the chain will fail
fwdRules = append(fwdRules,
[][]string{
{"-o", config.WeaveBridgeName, "-j", "WEAVE-NPC"},
{"-o", config.WeaveBridgeName, "-m", "state", "--state", "NEW", "-j", "NFLOG", "--nflog-group", "86"},
{"-o", config.WeaveBridgeName, "-j", "DROP"},
}...)
} else {
// Work around the situation where there are no rules allowing traffic
// across our bridge. E.g. ufw
if err = ipt.AppendUnique("filter", "FORWARD", "-i", config.WeaveBridgeName, "-o", config.WeaveBridgeName, "-j", "ACCEPT"); err != nil {
return err
}
fwdRules = append(fwdRules, []string{"-i", config.WeaveBridgeName, "-o", config.WeaveBridgeName, "-j", "ACCEPT"})
}
if !config.NPC {
// Create a chain for allowing ingress traffic when the bridge is exposed
_ = ipt.NewChain("filter", "WEAVE-EXPOSE")
if err = ipt.AppendUnique("filter", "FORWARD", "-o", config.WeaveBridgeName, "-j", "WEAVE-EXPOSE"); err != nil {
return err
}
fwdRules = append(fwdRules, []string{"-o", config.WeaveBridgeName, "-j", "WEAVE-EXPOSE"})
}
// Forward from weave to the rest of the world
if err = ipt.AppendUnique("filter", "FORWARD", "-i", config.WeaveBridgeName, "!", "-o", config.WeaveBridgeName, "-j", "ACCEPT"); err != nil {
return err
}
fwdRules = append(fwdRules, []string{"-i", config.WeaveBridgeName, "!", "-o", config.WeaveBridgeName, "-j", "ACCEPT"})
// and allow replies back
if err = ipt.AppendUnique("filter", "FORWARD", "-o", config.WeaveBridgeName, "-m", "conntrack", "--ctstate", "RELATED,ESTABLISHED", "-j", "ACCEPT"); err != nil {
fwdRules = append(fwdRules, []string{"-o", config.WeaveBridgeName, "-m", "conntrack", "--ctstate", "RELATED,ESTABLISHED", "-j", "ACCEPT"})
if err := ensureRules("filter", "FORWARD", fwdRules, ipt); err != nil {
return err
}
// create a chain for masquerading
if err = ipt.ClearChain("nat", "WEAVE"); err != nil {
return errors.Wrap(err, "clearing WEAVE chain")
}
//
// NB: we do not clear the chain to preserve existing rules
// inserted by "weave expose".
_ = ipt.NewChain("nat", "WEAVE")
return ipt.AppendUnique("nat", "POSTROUTING", "-j", "WEAVE")
}
@@ -487,3 +500,40 @@ func linkSetUpByName(linkName string) error {
}
return netlink.LinkSetUp(link)
}
// ensureRules ensures the presence of given iptables rules.
//
// If any rule from the list is missing, the function deletes all given
// rules and re-appends them to ensure the order of the rules.
func ensureRules(table, chain string, rulespecs [][]string, ipt *iptables.IPTables) error {
allFound := true
for _, rs := range rulespecs {
found, err := ipt.Exists(table, chain, rs...)
if err != nil {
return errors.Wrapf(err, "ipt.Exists(%s, %s, %s)", table, chain, rs)
}
if !found {
allFound = false
break
}
}
// All rules exist, do nothing.
if allFound {
return nil
}
for _, rs := range rulespecs {
// If any is missing, then delete all, as we need to preserve the order of
// given rules. Ignore errors, as rule might not exist.
if !allFound {
ipt.Delete(table, chain, rs...)
}
if err := ipt.Append(table, chain, rs...); err != nil {
return errors.Wrapf(err, "ipt.Append(%s, %s, %s)", table, chain, rs)
}
}
return nil
}
net/netlink.go
+16
View File
@@ -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)
}
net/veth.go
+8 -3
View File
@@ -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)
}
plugin/net/driver.go
+1 -1
View File
@@ -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)
}
prog/weaver/main.go
+1
View File
@@ -197,6 +197,7 @@ func main() {
mflag.StringVar(&dnsConfig.ResolvConf, []string{"-resolv-conf"}, "", "path to resolver configuration for fallback DNS lookups")
mflag.StringVar(&bridgeConfig.DatapathName, []string{"-datapath"}, "", "ODP datapath name")
mflag.BoolVar(&bridgeConfig.NoFastdp, []string{"-no-fastdp"}, false, "Disable Fast Datapath")
mflag.BoolVar(&bridgeConfig.NoBridgedFastdp, []string{"-no-bridged-fastdp"}, false, "Disable Bridged Fast Datapath")
mflag.StringVar(&trustedSubnetStr, []string{"-trusted-subnets"}, "", "comma-separated list of trusted subnets in CIDR notation")
mflag.StringVar(&dbPrefix, []string{"-db-prefix"}, "/weavedb/weave", "pathname/prefix of filename to store data")
mflag.StringVar(&procPath, []string{"-proc-path"}, "/proc", "path to reach host /proc filesystem")
prog/weaveutil/attach.go
+4 -9
View File
@@ -4,7 +4,6 @@ import (
"fmt"
"net"
"os"
"strconv"
"strings"
"syscall"
@@ -15,8 +14,8 @@ import (
)
func attach(args []string) error {
if len(args) < 4 {
cmdUsage("attach-container", "[--no-multicast-route] [--keep-tx-on] [--hairpin-mode=true|false] <container-id> <bridge-name> <mtu> <cidr>...")
if len(args) < 3 {
cmdUsage("attach-container", "[--no-multicast-route] [--keep-tx-on] [--hairpin-mode=true|false] <container-id> <bridge-name> <cidr>...")
}
keepTXOn := false
@@ -52,16 +51,12 @@ func attach(args []string) error {
} else if nsHost.Equal(nsContainer) {
return fmt.Errorf("Container is running in the host network namespace, and therefore cannot be\nconnected to weave - perhaps the container was started with --net=host")
}
mtu, err := strconv.Atoi(args[2])
if err != nil && args[3] != "" {
return fmt.Errorf("unable to parse mtu %q: %s", args[2], err)
}
cidrs, err := parseCIDRs(args[3:])
cidrs, err := parseCIDRs(args[2:])
if err != nil {
return err
}
err = weavenet.AttachContainer(weavenet.NSPathByPid(pid), fmt.Sprint(pid), weavenet.VethName, args[1], mtu, withMulticastRoute, cidrs, keepTXOn, hairpinMode)
err = weavenet.AttachContainer(weavenet.NSPathByPid(pid), fmt.Sprint(pid), weavenet.VethName, args[1], 0, withMulticastRoute, cidrs, keepTXOn, hairpinMode)
// If we detected an error but the container has died, tell the user that instead.
if err != nil && !processExists(pid) {
err = fmt.Errorf("Container %s died", args[0])
prog/weaveutil/bridge.go
-35
View File
@@ -2,9 +2,7 @@ package main
import (
"fmt"
"strconv"
"github.com/weaveworks/weave/common"
weavenet "github.com/weaveworks/weave/net"
)
@@ -22,36 +20,3 @@ func detectBridgeType(args []string) error {
}
return nil
}
func createBridge(args []string) error {
if len(args) != 10 {
cmdUsage("create-bridge", "<docker-bridge-name> <weave-bridge-name> <datapath-name> <mtu> <port> <mac> <no-fastdp> <no-bridged-fastdp> <proc-path> <expect-npc>")
}
mtu, err := strconv.Atoi(args[3])
if err != nil && args[3] != "" {
return err
}
port, err := strconv.Atoi(args[4])
if err != nil {
return err
}
config := weavenet.BridgeConfig{
DockerBridgeName: args[0],
WeaveBridgeName: args[1],
DatapathName: args[2],
MTU: mtu,
Port: port,
Mac: args[5],
NoFastdp: args[6] != "",
NoBridgedFastdp: args[7] != "",
NPC: args[9] == "--expect-npc",
}
procPath := args[8]
bridgeType, err := weavenet.EnsureBridge(procPath, &config, common.Log)
if err != nil {
return err
}
fmt.Println(bridgeType.String())
return nil
}
prog/weaveutil/main.go
-1
View File
@@ -17,7 +17,6 @@ func init() {
"help": help,
"netcheck": netcheck,
"docker-tls-args": dockerTLSArgs,
"create-bridge": createBridge,
"detect-bridge-type": detectBridgeType,
"create-datapath": createDatapath,
"delete-datapath": deleteDatapath,
test/191_create_bridge_2_test.sh
Executable
+59
View File
@@ -0,0 +1,59 @@
#! /bin/bash
. "$(dirname "$0")/config.sh"
C1=10.32.0.2
C2=10.32.0.3
kill_weaver() {
run_on $HOST1 sudo ip link set weave down
WEAVER_PID=$(container_pid $HOST1 weave)
run_on $HOST1 sudo kill -9 $WEAVER_PID
sleep 3
}
start_suite "Re-create bridge after restart"
# Should create a bridge of the "bridge" type.
WEAVE_NO_FASTDP=1 weave_on $HOST1 launch
WEAVE_NO_FASTDP=1 weave_on $HOST2 launch $HOST1
start_container $HOST1 $C1/24 --name=c1
start_container $HOST2 $C2/24 --name=c2
assert_raises "exec_on $HOST1 c1 $PING $C2"
kill_weaver # should re-create the bridge
assert_raises "exec_on $HOST1 c1 $PING $C2"
# Should create a bridge of the "bridged_fastdp" type
weave_on $HOST1 reset
weave_on $HOST1 launch $HOST2
weave_on $HOST1 attach $C1/24 c1
assert_raises "exec_on $HOST1 c1 $PING $C2"
kill_weaver # should re-create the bridge
assert_raises "exec_on $HOST1 c1 $PING $C2"
# test restore of iptables
weave_on $HOST1 reset
# `--expect-npc` to trigger creation of WEAVE-NPC iptables chain.
weave_on $HOST1 launch --expect-npc
# To create POSTROUTING rules.
weave_on $HOST1 expose
IPT_BEFORE=$(mktemp)
IPT_AFTER=$(mktemp)
run_on $HOST1 "sudo iptables-save | grep -i weave | grep -v '\[.*:.*\]' > $IPT_BEFORE"
run_on $HOST1 sudo iptables -t filter -D FORWARD -o weave -j WEAVE-NPC
run_on $HOST1 sudo iptables -t nat -D POSTROUTING -j WEAVE
kill_weaver # should re-create the bridge and iptables friends
# Rudimentary check that weave related iptables rules has been restored
run_on $HOST1 "sudo iptables-save | grep -i weave | grep -v '\[.*:.*\]' > $IPT_AFTER"
assert_raises "run_on $HOST1 diff $IPT_BEFORE $IPT_AFTER"
end_suite
vendor/github.com/vishvananda/netlink
Generated Vendored
+1 -1
Submodule vendor/github.com/vishvananda/netlink updated: c19091b1c6...f67b75edbf.
weave
+3 -28
View File
@@ -405,27 +405,6 @@ detect_bridge_type() {
MTU=${WEAVE_MTU:-$(cat /sys/class/net/$BRIDGE/mtu)}
}
try_create_bridge() {
# Pass /dev/null because we do not have access to the weavedb files when this runs; BoltDB
# cannot open the file anyway if weaver is running, which is likely for attach|expose|hide.
MAC=$(util_op unique-id "/dev/null" "$HOST_ROOT")
BRIDGE_TYPE=$(util_op create-bridge "$DOCKER_BRIDGE" "$BRIDGE" "$DATAPATH" "$WEAVE_MTU" "$PORT" "$MAC" "$WEAVE_NO_FASTDP" "$WEAVE_NO_BRIDGED_FASTDP" "$PROC_PATH" "$1")
# Set some variables that are expected by code that comes later
[ "$BRIDGE_TYPE" != "fastdp" ] || DATAPATH="$BRIDGE"
MTU=${WEAVE_MTU:-$(cat /sys/class/net/$BRIDGE/mtu)}
}
create_bridge() {
validate_bridge_type
if ! try_create_bridge "$@" ; then
echo "Creating bridge '$BRIDGE' failed" >&2
# reset to original value so we destroy both kinds
DATAPATH=datapath
destroy_bridge
exit 1
fi
}
validate_bridge_type() {
detect_bridge_type || return 0
@@ -557,6 +536,7 @@ router_bridge_opts() {
echo --docker-bridge "$DOCKER_BRIDGE" --weave-bridge "$BRIDGE" --datapath "$DATAPATH"
[ -z "$WEAVE_MTU" ] || echo --mtu "$WEAVE_MTU"
[ -z $WEAVE_NO_FASTDP ] || echo --no-fastdp
[ -z $WEAVE_NO_BRIDGED_FASTDP ] || echo --no-bridged-fastdp
}
######################################################################
@@ -918,9 +898,7 @@ check_overlap() {
}
detect_awsvpc() {
# Ignoring errors here: if we cannot detect AWSVPC we will skip the relevant
# steps, because "attach" should work without the weave router running.
[ "$(call_weave GET /ipinfo/tracker)" != "awsvpc" ] || AWSVPC=1
[ "$(call_weave GET /ipinfo/tracker 2>/dev/null)" != "awsvpc" ] || AWSVPC=1
}
# Call IPAM as necessary to lookup or allocate addresses
@@ -1436,7 +1414,6 @@ case "$COMMAND" in
done
[ $# -eq 1 ] || usage
CONTAINER=$(util_op container-id $1)
create_bridge
ipam_cidrs allocate $CONTAINER $CIDR_ARGS
if [ -n "$REWRITE_HOSTS" ] ; then
# rewrite /etc/hosts, unlinking the file (so Docker does
@@ -1444,7 +1421,7 @@ case "$COMMAND" in
util_op rewrite-etc-hosts "$CONTAINER" "$EXEC_IMAGE" "$ALL_CIDRS" $DNS_EXTRA_HOSTS
fi
[ -n "$AWSVPC" ] && ATTACH_ARGS="--no-multicast-route --keep-tx-on"
util_op attach-container $ATTACH_ARGS $CONTAINER $BRIDGE $MTU $ALL_CIDRS >/dev/null
util_op attach-container $ATTACH_ARGS $CONTAINER $BRIDGE $ALL_CIDRS >/dev/null
[ -n "$WITHOUT_DNS" ] || when_weave_running with_container_fqdn $CONTAINER put_dns_fqdn $ALL_CIDRS
show_addrs $ALL_CIDRS
;;
@@ -1494,7 +1471,6 @@ case "$COMMAND" in
[ $# -eq 2 -a "$1" = "-h" ] || usage
FQDN="$2"
fi
create_bridge
expose_ip
show_addrs $ALL_CIDRS
;;
@@ -1502,7 +1478,6 @@ case "$COMMAND" in
collect_cidr_args "$@"
shift $CIDR_ARG_COUNT
ipam_cidrs lookup weave:expose $CIDR_ARGS
create_bridge
for CIDR in $ALL_CIDRS ; do
if ip addr show dev $BRIDGE | grep -qF $CIDR ; then
ip addr del dev $BRIDGE $CIDR