From 0cfaa590de9e128ab9d63de6783235d4e55fa4ad Mon Sep 17 00:00:00 2001 From: Alessandro Boch Date: Tue, 6 Oct 2015 20:29:30 -0700 Subject: [PATCH] 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 --- libnetwork/ipam/allocator.go | 7 +++- libnetwork/libnetwork_internal_test.go | 37 ++++++++++++++++++++++ libnetwork/network.go | 44 ++++++++++++++++++++------ 3 files changed, 77 insertions(+), 11 deletions(-) diff --git a/libnetwork/ipam/allocator.go b/libnetwork/ipam/allocator.go index eb325721ac..61a5cdc398 100644 --- a/libnetwork/ipam/allocator.go +++ b/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 diff --git a/libnetwork/libnetwork_internal_test.go b/libnetwork/libnetwork_internal_test.go index f59b41a79f..0e8241f6b5 100644 --- a/libnetwork/libnetwork_internal_test.go +++ b/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() + } +} diff --git a/libnetwork/network.go b/libnetwork/network.go index f659259271..be5577de9b 100644 --- a/libnetwork/network.go +++ b/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) + } } } }