Merge pull request #599 from aboch/il

Fix in handling aux addresses

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,11 +59,19 @@ 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
+	// 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
 }
 
@@ -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 {
+				if !d.Pool.Contains(ip) {
-					return nil, types.InternalErrorf("failed to allocate secondary ip address %s(%s): %v", k, v, err)
+					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,11 +902,13 @@ 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 {
+				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)
 					}
 				}
 			}
+		}
 		if err := ipam.ReleasePool(d.PoolID); err != nil {
 			log.Warnf("Failed to release address pool %s on delete of network %s (%s): %v", d.PoolID, n.Name(), n.ID(), err)
 		}