Fix in handling aux addresses

- libnetwork should reserve only the auxiliary
  addresses which belong to the container
  addresable pool. And should fail the network
  creation if the aux addr does not belong to
  the master pool.

Signed-off-by: Alessandro Boch <aboch@docker.com>

libnetwork/ipam/allocator.go

@@ -412,11 +412,16 @@ func (a *Allocator) ReleaseAddress(poolID string, address net.IP) error {
 		return ipamapi.ErrBadPool
 	}
 
-	if address == nil || !p.Pool.Contains(address) {
+	if address == nil {
 		aSpace.Unlock()
 		return ipamapi.ErrInvalidRequest
 	}
 
+	if !p.Pool.Contains(address) {
+		aSpace.Unlock()
+		return ipamapi.ErrIPOutOfRange
+	}
+
 	c := p
 	for c.Range != nil {
 		k = c.ParentKey

libnetwork/libnetwork_internal_test.go

@@ -7,6 +7,7 @@ import (
 	"testing"
 
 	"github.com/docker/libnetwork/driverapi"
+	"github.com/docker/libnetwork/ipamapi"
 	"github.com/docker/libnetwork/netlabel"
 	"github.com/docker/libnetwork/types"
 )
@@ -291,3 +292,39 @@ func compareAddresses(a, b map[string]*net.IPNet) bool {
 	}
 	return true
 }
+
+func TestAuxAddresses(t *testing.T) {
+	c, err := New()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer c.Stop()
+
+	n := &network{ipamType: ipamapi.DefaultIPAM, ctrlr: c.(*controller)}
+
+	input := []struct {
+		masterPool   string
+		subPool      string
+		auxAddresses map[string]string
+		good         bool
+	}{
+		{"192.168.0.0/16", "", map[string]string{"goodOne": "192.168.2.2"}, true},
+		{"192.168.0.0/16", "", map[string]string{"badOne": "192.169.2.3"}, false},
+		{"192.168.0.0/16", "192.168.1.0/24", map[string]string{"goodOne": "192.168.1.2"}, true},
+		{"192.168.0.0/16", "192.168.1.0/24", map[string]string{"stillGood": "192.168.2.4"}, true},
+		{"192.168.0.0/16", "192.168.1.0/24", map[string]string{"badOne": "192.169.2.4"}, false},
+	}
+
+	for _, i := range input {
+
+		n.ipamV4Config = []*IpamConf{&IpamConf{PreferredPool: i.masterPool, SubPool: i.subPool, AuxAddresses: i.auxAddresses}}
+
+		_, err := n.ipamAllocate()
+
+		if i.good != (err == nil) {
+			t.Fatalf("Unexpected result for %v: %v", i, err)
+		}
+
+		n.ipamRelease()
+	}
+}

libnetwork/network.go

@@ -12,6 +12,7 @@ import (
 	"github.com/docker/libnetwork/datastore"
 	"github.com/docker/libnetwork/driverapi"
 	"github.com/docker/libnetwork/etchosts"
+	"github.com/docker/libnetwork/ipamapi"
 	"github.com/docker/libnetwork/netlabel"
 	"github.com/docker/libnetwork/options"
 	"github.com/docker/libnetwork/types"
@@ -58,12 +59,20 @@ type svcMap map[string]net.IP
 
 // IpamConf contains all the ipam related configurations for a network
 type IpamConf struct {
+	// The master address pool for containers and network iterfaces
 	PreferredPool string
-	SubPool       string
-	Options       map[string]string // IPAM input options
-	IsV6          bool
-	Gateway       string
-	AuxAddresses  map[string]string
+	// A subset of the master pool. If specified,
+	// this becomes the container pool
+	SubPool string
+	// Input options for IPAM Driver (optional)
+	Options map[string]string // IPAM input options
+	// IPv6 flag, Needed when no preferred pool is specified
+	IsV6 bool
+	// Preferred Network Gateway address (optional)
+	Gateway string
+	// Auxiliary addresses for network driver. Must be within the master pool.
+	// libnetwork will reserve them if they fall into the container pool
+	AuxAddresses map[string]string
 }
 
 // Validate checks whether the configuration is valid
@@ -845,15 +854,28 @@ func (n *network) ipamAllocate() ([]func(), error) {
 				log.Warnf("Failed to release gw address %s after failure to create network %s (%s)", d.Gateway, n.Name(), n.ID())
 			}
 		})
+		// Auxiliary addresses must be part of the master address pool
+		// If they fall into the container addressable pool, libnetwork will reserve them
 		if cfg.AuxAddresses != nil {
 			var ip net.IP
 			d.IPAMData.AuxAddresses = make(map[string]*net.IPNet, len(cfg.AuxAddresses))
 			for k, v := range cfg.AuxAddresses {
 				if ip = net.ParseIP(v); ip == nil {
-					return nil, types.BadRequestErrorf("non parsable secondary ip address %s (%s) passed for network %s", k, v, n.Name())
+					return nil, types.BadRequestErrorf("non parsable secondary ip address (%s:%s) passed for network %s", k, v, n.Name())
 				}
-				if d.IPAMData.AuxAddresses[k], _, err = ipam.RequestAddress(d.PoolID, ip, nil); err != nil {
-					return nil, types.InternalErrorf("failed to allocate secondary ip address %s(%s): %v", k, v, err)
+				if !d.Pool.Contains(ip) {
+					return cnl, types.ForbiddenErrorf("auxilairy address: (%s:%s) must belong to the master pool: %s", k, v, d.Pool)
+				}
+				// Attempt reservation in the container addressable pool, silent the error if address does not belong to that pool
+				if d.IPAMData.AuxAddresses[k], _, err = ipam.RequestAddress(d.PoolID, ip, nil); err != nil && err != ipamapi.ErrIPOutOfRange {
+					return nil, types.InternalErrorf("failed to allocate secondary ip address (%s:%s): %v", k, v, err)
+				}
+				if err == nil {
+					cnl = append(cnl, func() {
+						if err := ipam.ReleaseAddress(d.PoolID, ip); err != nil {
+							log.Warnf("Failed to release secondary ip address %s(%s) after failure to create network %s (%s)", k, v, ip, n.Name(), n.ID())
+						}
+					})
 				}
 			}
 		}
@@ -880,8 +902,10 @@ func (n *network) ipamRelease() {
 		}
 		if d.IPAMData.AuxAddresses != nil {
 			for k, nw := range d.IPAMData.AuxAddresses {
-				if err := ipam.ReleaseAddress(d.PoolID, nw.IP); err != nil {
-					log.Warnf("Failed to release secondary ip address %s (%v) on delete of network %s (%s): %v", k, nw.IP, n.Name(), n.ID(), err)
+				if d.Pool.Contains(nw.IP) {
+					if err := ipam.ReleaseAddress(d.PoolID, nw.IP); err != nil && err != ipamapi.ErrIPOutOfRange {
+						log.Warnf("Failed to release secondary ip address %s (%v) on delete of network %s (%s): %v", k, nw.IP, n.Name(), n.ID(), err)
+					}
 				}
 			}
 		}