From 35fab0aa6fa8530fa4be13d6a9fdd4e6313511a5 Mon Sep 17 00:00:00 2001 From: Alessandro Boch Date: Thu, 21 May 2015 13:13:19 -0700 Subject: [PATCH] Bridge driver to support multiple networks Signed-off-by: Alessandro Boch --- libnetwork/client/network.go | 4 +- libnetwork/drivers/bridge/bridge.go | 129 ++++++++++++++---- libnetwork/drivers/bridge/bridge_test.go | 8 +- libnetwork/drivers/bridge/network_test.go | 5 +- .../drivers/bridge/port_mapping_test.go | 6 +- libnetwork/drivers/bridge/setup_ipv4.go | 36 ++--- 6 files changed, 137 insertions(+), 51 deletions(-) diff --git a/libnetwork/client/network.go b/libnetwork/client/network.go index 5424e027b2..7e54563cff 100644 --- a/libnetwork/client/network.go +++ b/libnetwork/client/network.go @@ -13,7 +13,7 @@ import ( ) const ( - nullNetType = "null" + defaultDriverType = "bridge" ) type command struct { @@ -52,7 +52,7 @@ func (cli *NetworkCli) CmdNetworkCreate(chain string, args ...string) error { return err } if *flDriver == "" { - *flDriver = nullNetType + *flDriver = defaultDriverType } // Construct network create request body diff --git a/libnetwork/drivers/bridge/bridge.go b/libnetwork/drivers/bridge/bridge.go index dd263ef79d..54be3d56a7 100644 --- a/libnetwork/drivers/bridge/bridge.go +++ b/libnetwork/drivers/bridge/bridge.go @@ -86,8 +86,9 @@ type bridgeNetwork struct { } type driver struct { - config *configuration - network *bridgeNetwork + config *configuration + network *bridgeNetwork + networks map[types.UUID]*bridgeNetwork sync.Mutex } @@ -98,7 +99,7 @@ func init() { // New constructs a new bridge driver func newDriver() driverapi.Driver { - return &driver{} + return &driver{networks: map[types.UUID]*bridgeNetwork{}} } // Init registers a new instance of bridge driver @@ -146,6 +147,27 @@ func (c *networkConfiguration) Validate() error { return nil } +// Conflict check if two NetworkConfiguration objects overlap in the multinetwork +func (c *networkConfiguration) Conflict(o *networkConfiguration) bool { + if o == nil { + return false + } + + // Also empty, becasue only one network with empty name is allowed + if c.BridgeName == o.BridgeName { + return true + } + + // They must be in different subnets + + if (c.AddressIPv4 != nil && o.AddressIPv4.IP != nil) && + (c.AddressIPv4.Contains(o.AddressIPv4.IP) || o.AddressIPv4.Contains(c.AddressIPv4.IP)) { + return true + } + + return false +} + // FromMap retrieve the configuration data from the map form. func (c *networkConfiguration) FromMap(data map[string]interface{}) error { if i, ok := data["BridgeName"]; ok && i != nil { @@ -340,11 +362,18 @@ func (d *driver) Config(option map[string]interface{}) error { } func (d *driver) getNetwork(id types.UUID) (*bridgeNetwork, error) { - // Just a dummy function to return the only network managed by Bridge driver. - // But this API makes the caller code unchanged when we move to support multiple networks. d.Lock() defer d.Unlock() - return d.network, nil + + if id == "" { + return nil, types.BadRequestErrorf("invalid network id: %s", id) + } + + if nw, ok := d.networks[id]; ok { + return nw, nil + } + + return nil, nil } func parseNetworkOptions(option options.Generic) (*networkConfiguration, error) { @@ -387,35 +416,71 @@ func (d *driver) CreateNetwork(id types.UUID, option map[string]interface{}) err d.Lock() // Sanity checks - if d.network != nil { + if _, ok := d.networks[id]; ok { d.Unlock() - return &ErrNetworkExists{} + return types.ForbiddenErrorf("network %s exists", id) + } + + // Parse and validate the config. It should not conflict with existing networks' config + config, err := parseNetworkOptions(option) + if err != nil { + d.Unlock() + return err + } + for _, nw := range d.networks { + if nw.config.Conflict(config) { + d.Unlock() + return types.ForbiddenErrorf("conflicts with network %s (%s)", nw.id, nw.config.BridgeName) + } } // Create and set network handler in driver - d.network = &bridgeNetwork{id: id, endpoints: make(map[types.UUID]*bridgeEndpoint)} - network := d.network + network := &bridgeNetwork{id: id, endpoints: make(map[types.UUID]*bridgeEndpoint), config: config} + d.networks[id] = network d.Unlock() // On failure make sure to reset driver network handler to nil defer func() { if err != nil { d.Lock() - d.network = nil + delete(d.networks, id) d.Unlock() } }() - config, err := parseNetworkOptions(option) - if err != nil { - return err - } - network.config = config - // Create or retrieve the bridge L3 interface bridgeIface := newInterface(config) network.bridge = bridgeIface + // Verify network does not conflict with previously configured networks + // on parameters that were chosen by the driver. + d.Lock() + for _, nw := range d.networks { + if nw.id == id { + continue + } + // Verify the name (which may have been set by newInterface()) does not conflict with + // existing bridge interfaces. Ironically the system chosen name gets stored in the config... + // Basically we are checking if the two original configs were both empty. + if nw.config.BridgeName == config.BridgeName { + d.Unlock() + return types.ForbiddenErrorf("conflicts with network %s (%s)", nw.id, nw.config.BridgeName) + } + // If this network config specifies the AddressIPv4, we need + // to make sure it does not conflict with any previously allocated + // bridges. This could not be completely caught by the config conflict + // check, because networks which config does not specify the AddressIPv4 + // get their address and subnet selected by the driver (see electBridgeIPv4()) + if config.AddressIPv4 != nil { + if nw.bridge.bridgeIPv4.Contains(config.AddressIPv4.IP) || + config.AddressIPv4.Contains(nw.bridge.bridgeIPv4.IP) { + d.Unlock() + return types.ForbiddenErrorf("conflicts with network %s (%s)", nw.id, nw.config.BridgeName) + } + } + } + d.Unlock() + // Prepare the bridge setup configuration bridgeSetup := newBridgeSetup(config, bridgeIface) @@ -485,8 +550,12 @@ func (d *driver) DeleteNetwork(nid types.UUID) error { // Get network handler and remove it from driver d.Lock() - n := d.network - d.network = nil + n, ok := d.networks[nid] + if !ok { + d.Unlock() + return types.InternalMaskableErrorf("network %s does not exist", nid) + } + delete(d.networks, nid) d.Unlock() // On failure set network handler back in driver, but @@ -494,8 +563,8 @@ func (d *driver) DeleteNetwork(nid types.UUID) error { defer func() { if err != nil { d.Lock() - if d.network == nil { - d.network = n + if _, ok := d.networks[nid]; !ok { + d.networks[nid] = n } d.Unlock() } @@ -535,7 +604,11 @@ func (d *driver) CreateEndpoint(nid, eid types.UUID, epInfo driverapi.EndpointIn // Get the network handler and make sure it exists d.Lock() - n := d.network + n, ok := d.networks[nid] + if !ok { + d.Unlock() + return types.NotFoundErrorf("network %s does not exist", nid) + } config := n.config d.Unlock() if n == nil { @@ -716,7 +789,11 @@ func (d *driver) DeleteEndpoint(nid, eid types.UUID) error { // Get the network handler and make sure it exists d.Lock() - n := d.network + n, ok := d.networks[nid] + if !ok { + d.Unlock() + return types.NotFoundErrorf("network %s does not exist", nid) + } config := n.config d.Unlock() if n == nil { @@ -787,7 +864,11 @@ func (d *driver) DeleteEndpoint(nid, eid types.UUID) error { func (d *driver) EndpointOperInfo(nid, eid types.UUID) (map[string]interface{}, error) { // Get the network handler and make sure it exists d.Lock() - n := d.network + n, ok := d.networks[nid] + if !ok { + d.Unlock() + return nil, types.NotFoundErrorf("network %s does not exist", nid) + } d.Unlock() if n == nil { return nil, driverapi.ErrNoNetwork(nid) diff --git a/libnetwork/drivers/bridge/bridge_test.go b/libnetwork/drivers/bridge/bridge_test.go index 7ad5bafb10..28aee2e5e8 100644 --- a/libnetwork/drivers/bridge/bridge_test.go +++ b/libnetwork/drivers/bridge/bridge_test.go @@ -194,8 +194,12 @@ func testQueryEndpointInfo(t *testing.T, ulPxyEnabled bool) { t.Fatalf("Failed to create an endpoint : %s", err.Error()) } - ep, _ := dd.network.endpoints["ep1"] - data, err := d.EndpointOperInfo(dd.network.id, ep.id) + network, ok := dd.networks["net1"] + if !ok { + t.Fatalf("Cannot find network %s inside driver", "net1") + } + ep, _ := network.endpoints["ep1"] + data, err := d.EndpointOperInfo(network.id, ep.id) if err != nil { t.Fatalf("Failed to ask for endpoint operational data: %v", err) } diff --git a/libnetwork/drivers/bridge/network_test.go b/libnetwork/drivers/bridge/network_test.go index 3fb74c633a..2a7a4690f7 100644 --- a/libnetwork/drivers/bridge/network_test.go +++ b/libnetwork/drivers/bridge/network_test.go @@ -79,7 +79,10 @@ func TestLinkCreate(t *testing.T) { t.Fatalf("Could not find source link %s: %v", te.ifaces[0].srcName, err) } - n := dr.network + n, ok := dr.networks["dummy"] + if !ok { + t.Fatalf("Cannot find network %s inside driver", "dummy") + } ip := te.ifaces[0].addr.IP if !n.bridge.bridgeIPv4.Contains(ip) { t.Fatalf("IP %s is not a valid ip in the subnet %s", ip.String(), n.bridge.bridgeIPv4.String()) diff --git a/libnetwork/drivers/bridge/port_mapping_test.go b/libnetwork/drivers/bridge/port_mapping_test.go index c5c1c5ee93..3a7a30796c 100644 --- a/libnetwork/drivers/bridge/port_mapping_test.go +++ b/libnetwork/drivers/bridge/port_mapping_test.go @@ -47,7 +47,11 @@ func TestPortMappingConfig(t *testing.T) { } dd := d.(*driver) - ep, _ := dd.network.endpoints["ep1"] + network, ok := dd.networks["dummy"] + if !ok { + t.Fatalf("Cannot find network %s inside driver", "dummy") + } + ep, _ := network.endpoints["ep1"] if len(ep.portMapping) != 2 { t.Fatalf("Failed to store the port bindings into the sandbox info. Found: %v", ep.portMapping) } diff --git a/libnetwork/drivers/bridge/setup_ipv4.go b/libnetwork/drivers/bridge/setup_ipv4.go index a16ff60fbf..d05a47aea0 100644 --- a/libnetwork/drivers/bridge/setup_ipv4.go +++ b/libnetwork/drivers/bridge/setup_ipv4.go @@ -19,27 +19,21 @@ func init() { // In theory this shouldn't matter - in practice there's bound to be a few scripts relying // on the internal addressing or other stupid things like that. // They shouldn't, but hey, let's not break them unless we really have to. - for _, addr := range []string{ - "172.17.42.1/16", // Don't use 172.16.0.0/16, it conflicts with EC2 DNS 172.16.0.23 - "10.0.42.1/16", // Don't even try using the entire /8, that's too intrusive - "10.1.42.1/16", - "10.42.42.1/16", - "172.16.42.1/24", - "172.16.43.1/24", - "172.16.44.1/24", - "10.0.42.1/24", - "10.0.43.1/24", - "192.168.42.1/24", - "192.168.43.1/24", - "192.168.44.1/24", - } { - ip, net, err := net.ParseCIDR(addr) - if err != nil { - log.Errorf("Failed to parse address %s", addr) - continue - } - net.IP = ip.To4() - bridgeNetworks = append(bridgeNetworks, net) + // Don't use 172.16.0.0/16, it conflicts with EC2 DNS 172.16.0.23 + + // 172.[17-31].42.1/16 + mask := []byte{255, 255, 0, 0} + for i := 17; i < 32; i++ { + bridgeNetworks = append(bridgeNetworks, &net.IPNet{IP: []byte{172, byte(i), 42, 1}, Mask: mask}) + } + // 10.[0-255].42.1/16 + for i := 0; i < 256; i++ { + bridgeNetworks = append(bridgeNetworks, &net.IPNet{IP: []byte{10, byte(i), 42, 1}, Mask: mask}) + } + // 192.168.[42-44].1/24 + mask[2] = 255 + for i := 42; i < 45; i++ { + bridgeNetworks = append(bridgeNetworks, &net.IPNet{IP: []byte{192, 168, byte(i), 1}, Mask: mask}) } }