From c0df562fc09c2f060b28452af48ffa5d9685d6d1 Mon Sep 17 00:00:00 2001 From: Abhinandan Prativadi Date: Thu, 20 Jul 2017 15:13:24 -0700 Subject: [PATCH 1/2] Adding ipam options to ipam driver requests Currently ipam options doesnt seem to be passed to remote ipam drivers. Remote ipam drivers that depend on any driver specific options will not work if ipam options are not included in the RequestAddress fromthe swarm manager to allocate IPs. This is minor fix to pass the ipam options to ipam drivers. Signed-off-by: Abhinandan Prativadi (cherry picked from commit ed271d4ae37d782f53d70f04535850239aff2722) Signed-off-by: Sebastiaan van Stijn --- .../cnmallocator/networkallocator.go | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/manager/allocator/cnmallocator/networkallocator.go b/manager/allocator/cnmallocator/networkallocator.go index 2cfc95130d..f2624688e9 100644 --- a/manager/allocator/cnmallocator/networkallocator.go +++ b/manager/allocator/cnmallocator/networkallocator.go @@ -553,6 +553,7 @@ func (na *cnmNetworkAllocator) releaseEndpoints(networks []*api.NetworkAttachmen // allocate virtual IP for a single endpoint attachment of the service. func (na *cnmNetworkAllocator) allocateVIP(vip *api.Endpoint_VirtualIP) error { + var opts map[string]string localNet := na.getNetwork(vip.NetworkID) if localNet == nil { return errors.New("networkallocator: could not find local network state") @@ -582,9 +583,12 @@ func (na *cnmNetworkAllocator) allocateVIP(vip *api.Endpoint_VirtualIP) error { return err } } + if localNet.nw.IPAM != nil && localNet.nw.IPAM.Driver != nil { + opts = localNet.nw.IPAM.Driver.Options + } for _, poolID := range localNet.pools { - ip, _, err := ipam.RequestAddress(poolID, addr, nil) + ip, _, err := ipam.RequestAddress(poolID, addr, opts) if err != nil && err != ipamapi.ErrNoAvailableIPs && err != ipamapi.ErrIPOutOfRange { return errors.Wrap(err, "could not allocate VIP from IPAM") } @@ -636,6 +640,7 @@ func (na *cnmNetworkAllocator) deallocateVIP(vip *api.Endpoint_VirtualIP) error // allocate the IP addresses for a single network attachment of the task. func (na *cnmNetworkAllocator) allocateNetworkIPs(nAttach *api.NetworkAttachment) error { var ip *net.IPNet + var opts map[string]string ipam, _, _, err := na.resolveIPAM(nAttach.Network) if err != nil { @@ -666,10 +671,14 @@ func (na *cnmNetworkAllocator) allocateNetworkIPs(nAttach *api.NetworkAttachment } } + if localNet.nw.IPAM != nil && localNet.nw.IPAM.Driver != nil { + opts = localNet.nw.IPAM.Driver.Options + } + for _, poolID := range localNet.pools { var err error - ip, _, err = ipam.RequestAddress(poolID, addr, nil) + ip, _, err = ipam.RequestAddress(poolID, addr, opts) if err != nil && err != ipamapi.ErrNoAvailableIPs && err != ipamapi.ErrIPOutOfRange { return errors.Wrap(err, "could not allocate IP from IPAM") } @@ -897,8 +906,14 @@ func (na *cnmNetworkAllocator) allocatePools(n *api.Network) (map[string]string, } gwIP.IP = ip } + if dOptions == nil { + dOptions = make(map[string]string) + } + dOptions[ipamapi.RequestAddressType] = netlabel.Gateway + defer delete(dOptions, ipamapi.RequestAddressType) + if ic.Gateway != "" || gwIP == nil { - gwIP, _, err = ipam.RequestAddress(poolID, net.ParseIP(ic.Gateway), map[string]string{ipamapi.RequestAddressType: netlabel.Gateway}) + gwIP, _, err = ipam.RequestAddress(poolID, net.ParseIP(ic.Gateway), dOptions) if err != nil { // Rollback by releasing all the resources allocated so far. releasePools(ipam, ipamConfigs[:i], pools) From 4b7b299249b41adfeb41e2d57476aee8f8be1c10 Mon Sep 17 00:00:00 2001 From: Abhinandan Prativadi Date: Tue, 3 Oct 2017 12:30:31 -0700 Subject: [PATCH 2/2] Serializing IP allocation This commit contains fix to serialize IPAM and Port allocation. This would fix transient issues seen due to immediate resource release. Signed-off-by: Abhinandan Prativadi (cherry picked from commit fe0d7f63cd93f9cb65c8533393a1447f4f55c63c) Signed-off-by: Sebastiaan van Stijn --- .../cnmallocator/networkallocator.go | 21 ++++++-- .../cnmallocator/networkallocator_test.go | 6 +-- .../allocator/cnmallocator/portallocator.go | 2 +- vendor.conf | 2 +- .../docker/libnetwork/bitseq/sequence.go | 49 +++++++++++++++---- .../docker/libnetwork/bitseq/store.go | 1 + .../drivers/overlay/ovmanager/ovmanager.go | 2 +- .../github.com/docker/libnetwork/idm/idm.go | 8 +-- .../docker/libnetwork/ipam/allocator.go | 20 +++++--- .../docker/libnetwork/ipamapi/labels.go | 10 ++++ .../docker/libnetwork/types/types.go | 4 +- .../github.com/docker/libnetwork/vendor.conf | 2 +- 12 files changed, 95 insertions(+), 32 deletions(-) create mode 100644 vendor/github.com/docker/libnetwork/ipamapi/labels.go diff --git a/manager/allocator/cnmallocator/networkallocator.go b/manager/allocator/cnmallocator/networkallocator.go index f2624688e9..75dc2b27a3 100644 --- a/manager/allocator/cnmallocator/networkallocator.go +++ b/manager/allocator/cnmallocator/networkallocator.go @@ -584,7 +584,8 @@ func (na *cnmNetworkAllocator) allocateVIP(vip *api.Endpoint_VirtualIP) error { } } if localNet.nw.IPAM != nil && localNet.nw.IPAM.Driver != nil { - opts = localNet.nw.IPAM.Driver.Options + // set ipam allocation method to serial + opts = setIPAMSerialAlloc(localNet.nw.IPAM.Driver.Options) } for _, poolID := range localNet.pools { @@ -670,9 +671,10 @@ func (na *cnmNetworkAllocator) allocateNetworkIPs(nAttach *api.NetworkAttachment } } } - + // Set the ipam options if the network has an ipam driver. if localNet.nw.IPAM != nil && localNet.nw.IPAM.Driver != nil { - opts = localNet.nw.IPAM.Driver.Options + // set ipam allocation method to serial + opts = setIPAMSerialAlloc(localNet.nw.IPAM.Driver.Options) } for _, poolID := range localNet.pools { @@ -910,6 +912,8 @@ func (na *cnmNetworkAllocator) allocatePools(n *api.Network) (map[string]string, dOptions = make(map[string]string) } dOptions[ipamapi.RequestAddressType] = netlabel.Gateway + // set ipam allocation method to serial + dOptions = setIPAMSerialAlloc(dOptions) defer delete(dOptions, ipamapi.RequestAddressType) if ic.Gateway != "" || gwIP == nil { @@ -974,3 +978,14 @@ func IsBuiltInDriver(name string) bool { } return false } + +// setIPAMSerialAlloc sets the ipam allocation method to serial +func setIPAMSerialAlloc(opts map[string]string) map[string]string { + if opts == nil { + opts = make(map[string]string) + } + if _, ok := opts[ipamapi.AllocSerialPrefix]; !ok { + opts[ipamapi.AllocSerialPrefix] = "true" + } + return opts +} diff --git a/manager/allocator/cnmallocator/networkallocator_test.go b/manager/allocator/cnmallocator/networkallocator_test.go index 209ad24920..025adb441c 100644 --- a/manager/allocator/cnmallocator/networkallocator_test.go +++ b/manager/allocator/cnmallocator/networkallocator_test.go @@ -772,8 +772,8 @@ func TestServiceAddRemovePortsIngressMode(t *testing.T) { assert.Len(t, s.Endpoint.Ports, 0) assert.Len(t, s.Endpoint.VirtualIPs, 0) - // Publish port again and ensure VIP is the same that was deallocated - // and there is no leak. + // Publish port again and ensure VIP is not the same that was deallocated. + // Since IP allocation is serial we should receive the next available IP. s.Spec.Endpoint.Ports = append(s.Spec.Endpoint.Ports, &api.PortConfig{Name: "some_tcp", TargetPort: 1234, PublishedPort: 1234, @@ -786,7 +786,7 @@ func TestServiceAddRemovePortsIngressMode(t *testing.T) { assert.Len(t, s.Endpoint.Ports, 1) assert.Equal(t, uint32(1234), s.Endpoint.Ports[0].PublishedPort) assert.Len(t, s.Endpoint.VirtualIPs, 1) - assert.Equal(t, allocatedVIP, s.Endpoint.VirtualIPs[0].Addr) + assert.NotEqual(t, allocatedVIP, s.Endpoint.VirtualIPs[0].Addr) } func TestServiceUpdate(t *testing.T) { diff --git a/manager/allocator/cnmallocator/portallocator.go b/manager/allocator/cnmallocator/portallocator.go index b09ac47c79..19dcbec772 100644 --- a/manager/allocator/cnmallocator/portallocator.go +++ b/manager/allocator/cnmallocator/portallocator.go @@ -382,7 +382,7 @@ func (ps *portSpace) allocate(p *api.PortConfig) (err error) { } // Check out an arbitrary port from dynamic port space. - swarmPort, err := ps.dynamicPortSpace.GetID() + swarmPort, err := ps.dynamicPortSpace.GetID(true) if err != nil { return } diff --git a/vendor.conf b/vendor.conf index abb1a6c247..8949ea01fc 100644 --- a/vendor.conf +++ b/vendor.conf @@ -24,7 +24,7 @@ github.com/docker/go-connections 3ede32e2033de7505e6500d6c868c2b9ed9f169d github.com/docker/go-events 9461782956ad83b30282bf90e31fa6a70c255ba9 github.com/docker/go-units 954fed01cc617c55d838fa2230073f2cb17386c8 github.com/docker/libkv 9fd56606e928ff1f309808f5d5a0b7a2ef73f9a8 -github.com/docker/libnetwork 19ac3ea7f52bb46e0eb10669756cdae0c441a5b1 +github.com/docker/libnetwork 21544598c53fa36a3c771a8725c643dd2340f845 github.com/docker/libtrust 9cbd2a1374f46905c68a4eb3694a130610adc62a github.com/opencontainers/runc d40db12e72a40109dfcf28539f5ee0930d2f0277 github.com/opencontainers/go-digest 21dfd564fd89c944783d00d069f33e3e7123c448 diff --git a/vendor/github.com/docker/libnetwork/bitseq/sequence.go b/vendor/github.com/docker/libnetwork/bitseq/sequence.go index 3946473d8b..a1a9810dc5 100644 --- a/vendor/github.com/docker/libnetwork/bitseq/sequence.go +++ b/vendor/github.com/docker/libnetwork/bitseq/sequence.go @@ -41,6 +41,7 @@ type Handle struct { id string dbIndex uint64 dbExists bool + curr uint64 store datastore.DataStore sync.Mutex } @@ -193,26 +194,27 @@ func (h *Handle) getCopy() *Handle { dbIndex: h.dbIndex, dbExists: h.dbExists, store: h.store, + curr: h.curr, } } // SetAnyInRange atomically sets the first unset bit in the specified range in the sequence and returns the corresponding ordinal -func (h *Handle) SetAnyInRange(start, end uint64) (uint64, error) { +func (h *Handle) SetAnyInRange(start, end uint64, serial bool) (uint64, error) { if end < start || end >= h.bits { return invalidPos, fmt.Errorf("invalid bit range [%d, %d]", start, end) } if h.Unselected() == 0 { return invalidPos, ErrNoBitAvailable } - return h.set(0, start, end, true, false) + return h.set(0, start, end, true, false, serial) } // SetAny atomically sets the first unset bit in the sequence and returns the corresponding ordinal -func (h *Handle) SetAny() (uint64, error) { +func (h *Handle) SetAny(serial bool) (uint64, error) { if h.Unselected() == 0 { return invalidPos, ErrNoBitAvailable } - return h.set(0, 0, h.bits-1, true, false) + return h.set(0, 0, h.bits-1, true, false, serial) } // Set atomically sets the corresponding bit in the sequence @@ -220,7 +222,7 @@ func (h *Handle) Set(ordinal uint64) error { if err := h.validateOrdinal(ordinal); err != nil { return err } - _, err := h.set(ordinal, 0, 0, false, false) + _, err := h.set(ordinal, 0, 0, false, false, false) return err } @@ -229,7 +231,7 @@ func (h *Handle) Unset(ordinal uint64) error { if err := h.validateOrdinal(ordinal); err != nil { return err } - _, err := h.set(ordinal, 0, 0, false, true) + _, err := h.set(ordinal, 0, 0, false, true, false) return err } @@ -298,7 +300,7 @@ func (h *Handle) CheckConsistency() error { } // set/reset the bit -func (h *Handle) set(ordinal, start, end uint64, any bool, release bool) (uint64, error) { +func (h *Handle) set(ordinal, start, end uint64, any bool, release bool, serial bool) (uint64, error) { var ( bitPos uint64 bytePos uint64 @@ -308,6 +310,7 @@ func (h *Handle) set(ordinal, start, end uint64, any bool, release bool) (uint64 for { var store datastore.DataStore + curr := uint64(0) h.Lock() store = h.store h.Unlock() @@ -318,15 +321,18 @@ func (h *Handle) set(ordinal, start, end uint64, any bool, release bool) (uint64 } h.Lock() + if serial { + curr = h.curr + } // Get position if available if release { bytePos, bitPos = ordinalToPos(ordinal) } else { if any { - bytePos, bitPos, err = getFirstAvailable(h.head, start) + bytePos, bitPos, err = getAvailableFromCurrent(h.head, start, curr, end) ret = posToOrdinal(bytePos, bitPos) - if end < ret { - err = ErrNoBitAvailable + if err == nil { + h.curr = ret + 1 } } else { bytePos, bitPos, err = checkIfAvailable(h.head, ordinal) @@ -515,6 +521,29 @@ func getFirstAvailable(head *sequence, start uint64) (uint64, uint64, error) { return invalidPos, invalidPos, ErrNoBitAvailable } +// getAvailableFromCurrent will look for available ordinal from the current ordinal. +// If none found then it will loop back to the start to check of the available bit. +// This can be further optimized to check from start till curr in case of a rollover +func getAvailableFromCurrent(head *sequence, start, curr, end uint64) (uint64, uint64, error) { + var bytePos, bitPos uint64 + if curr != 0 && curr > start { + bytePos, bitPos, _ = getFirstAvailable(head, curr) + ret := posToOrdinal(bytePos, bitPos) + if end < ret { + goto begin + } + return bytePos, bitPos, nil + } + +begin: + bytePos, bitPos, _ = getFirstAvailable(head, start) + ret := posToOrdinal(bytePos, bitPos) + if end < ret { + return invalidPos, invalidPos, ErrNoBitAvailable + } + return bytePos, bitPos, nil +} + // checkIfAvailable checks if the bit correspondent to the specified ordinal is unset // If the ordinal is beyond the sequence limits, a negative response is returned func checkIfAvailable(head *sequence, ordinal uint64) (uint64, uint64, error) { diff --git a/vendor/github.com/docker/libnetwork/bitseq/store.go b/vendor/github.com/docker/libnetwork/bitseq/store.go index 5448927eb1..cdb7f04264 100644 --- a/vendor/github.com/docker/libnetwork/bitseq/store.go +++ b/vendor/github.com/docker/libnetwork/bitseq/store.go @@ -87,6 +87,7 @@ func (h *Handle) CopyTo(o datastore.KVObject) error { dstH.dbIndex = h.dbIndex dstH.dbExists = h.dbExists dstH.store = h.store + dstH.curr = h.curr dstH.Unlock() return nil diff --git a/vendor/github.com/docker/libnetwork/drivers/overlay/ovmanager/ovmanager.go b/vendor/github.com/docker/libnetwork/drivers/overlay/ovmanager/ovmanager.go index a80f335892..58cc687d4f 100644 --- a/vendor/github.com/docker/libnetwork/drivers/overlay/ovmanager/ovmanager.go +++ b/vendor/github.com/docker/libnetwork/drivers/overlay/ovmanager/ovmanager.go @@ -165,7 +165,7 @@ func (n *network) obtainVxlanID(s *subnet) error { n.Unlock() if vni == 0 { - vni, err = n.driver.vxlanIdm.GetIDInRange(vxlanIDStart, vxlanIDEnd) + vni, err = n.driver.vxlanIdm.GetIDInRange(vxlanIDStart, vxlanIDEnd, true) if err != nil { return err } diff --git a/vendor/github.com/docker/libnetwork/idm/idm.go b/vendor/github.com/docker/libnetwork/idm/idm.go index 7e449a0dc8..d5843d4a58 100644 --- a/vendor/github.com/docker/libnetwork/idm/idm.go +++ b/vendor/github.com/docker/libnetwork/idm/idm.go @@ -34,11 +34,11 @@ func New(ds datastore.DataStore, id string, start, end uint64) (*Idm, error) { } // GetID returns the first available id in the set -func (i *Idm) GetID() (uint64, error) { +func (i *Idm) GetID(serial bool) (uint64, error) { if i.handle == nil { return 0, errors.New("ID set is not initialized") } - ordinal, err := i.handle.SetAny() + ordinal, err := i.handle.SetAny(serial) return i.start + ordinal, err } @@ -56,7 +56,7 @@ func (i *Idm) GetSpecificID(id uint64) error { } // GetIDInRange returns the first available id in the set within a [start,end] range -func (i *Idm) GetIDInRange(start, end uint64) (uint64, error) { +func (i *Idm) GetIDInRange(start, end uint64, serial bool) (uint64, error) { if i.handle == nil { return 0, errors.New("ID set is not initialized") } @@ -65,7 +65,7 @@ func (i *Idm) GetIDInRange(start, end uint64) (uint64, error) { return 0, errors.New("Requested range does not belong to the set") } - ordinal, err := i.handle.SetAnyInRange(start-i.start, end-i.start) + ordinal, err := i.handle.SetAnyInRange(start-i.start, end-i.start, serial) return i.start + ordinal, err } diff --git a/vendor/github.com/docker/libnetwork/ipam/allocator.go b/vendor/github.com/docker/libnetwork/ipam/allocator.go index 71c9f39531..5beb429dfc 100644 --- a/vendor/github.com/docker/libnetwork/ipam/allocator.go +++ b/vendor/github.com/docker/libnetwork/ipam/allocator.go @@ -457,7 +457,15 @@ func (a *Allocator) RequestAddress(poolID string, prefAddress net.IP, opts map[s return nil, nil, types.InternalErrorf("could not find bitmask in datastore for %s on address %v request from pool %s: %v", k.String(), prefAddress, poolID, err) } - ip, err := a.getAddress(p.Pool, bm, prefAddress, p.Range) + // In order to request for a serial ip address allocation, callers can pass in the option to request + // IP allocation serially or first available IP in the subnet + var serial bool + if opts != nil { + if val, ok := opts[ipamapi.AllocSerialPrefix]; ok { + serial = (val == "true") + } + } + ip, err := a.getAddress(p.Pool, bm, prefAddress, p.Range, serial) if err != nil { return nil, nil, err } @@ -522,7 +530,7 @@ func (a *Allocator) ReleaseAddress(poolID string, address net.IP) error { return bm.Unset(ipToUint64(h)) } -func (a *Allocator) getAddress(nw *net.IPNet, bitmask *bitseq.Handle, prefAddress net.IP, ipr *AddressRange) (net.IP, error) { +func (a *Allocator) getAddress(nw *net.IPNet, bitmask *bitseq.Handle, prefAddress net.IP, ipr *AddressRange, serial bool) (net.IP, error) { var ( ordinal uint64 err error @@ -535,7 +543,7 @@ func (a *Allocator) getAddress(nw *net.IPNet, bitmask *bitseq.Handle, prefAddres return nil, ipamapi.ErrNoAvailableIPs } if ipr == nil && prefAddress == nil { - ordinal, err = bitmask.SetAny() + ordinal, err = bitmask.SetAny(serial) } else if prefAddress != nil { hostPart, e := types.GetHostPartIP(prefAddress, base.Mask) if e != nil { @@ -544,7 +552,7 @@ func (a *Allocator) getAddress(nw *net.IPNet, bitmask *bitseq.Handle, prefAddres ordinal = ipToUint64(types.GetMinimalIP(hostPart)) err = bitmask.Set(ordinal) } else { - ordinal, err = bitmask.SetAnyInRange(ipr.Start, ipr.End) + ordinal, err = bitmask.SetAnyInRange(ipr.Start, ipr.End, serial) } switch err { @@ -579,7 +587,7 @@ func (a *Allocator) DumpDatabase() string { s = fmt.Sprintf("\n\n%s Config", as) aSpace.Lock() for k, config := range aSpace.subnets { - s = fmt.Sprintf("%s%s", s, fmt.Sprintf("\n%v: %v", k, config)) + s += fmt.Sprintf("\n%v: %v", k, config) if config.Range == nil { a.retrieveBitmask(k, config.Pool) } @@ -589,7 +597,7 @@ func (a *Allocator) DumpDatabase() string { s = fmt.Sprintf("%s\n\nBitmasks", s) for k, bm := range a.addresses { - s = fmt.Sprintf("%s%s", s, fmt.Sprintf("\n%s: %s", k, bm)) + s += fmt.Sprintf("\n%s: %s", k, bm) } return s diff --git a/vendor/github.com/docker/libnetwork/ipamapi/labels.go b/vendor/github.com/docker/libnetwork/ipamapi/labels.go new file mode 100644 index 0000000000..e5c7d1cc7e --- /dev/null +++ b/vendor/github.com/docker/libnetwork/ipamapi/labels.go @@ -0,0 +1,10 @@ +package ipamapi + +const ( + // Prefix constant marks the reserved label space for libnetwork + Prefix = "com.docker.network" + + // AllocSerialPrefix constant marks the reserved label space for libnetwork ipam + // allocation ordering.(serial/first available) + AllocSerialPrefix = Prefix + ".ipam.serial" +) diff --git a/vendor/github.com/docker/libnetwork/types/types.go b/vendor/github.com/docker/libnetwork/types/types.go index da113e24cd..164b18096c 100644 --- a/vendor/github.com/docker/libnetwork/types/types.go +++ b/vendor/github.com/docker/libnetwork/types/types.go @@ -129,11 +129,11 @@ func (p *PortBinding) GetCopy() PortBinding { func (p *PortBinding) String() string { ret := fmt.Sprintf("%s/", p.Proto) if p.IP != nil { - ret = fmt.Sprintf("%s%s", ret, p.IP.String()) + ret += p.IP.String() } ret = fmt.Sprintf("%s:%d/", ret, p.Port) if p.HostIP != nil { - ret = fmt.Sprintf("%s%s", ret, p.HostIP.String()) + ret += p.HostIP.String() } ret = fmt.Sprintf("%s:%d", ret, p.HostPort) return ret diff --git a/vendor/github.com/docker/libnetwork/vendor.conf b/vendor/github.com/docker/libnetwork/vendor.conf index 6751cba47e..c97f5517df 100644 --- a/vendor/github.com/docker/libnetwork/vendor.conf +++ b/vendor/github.com/docker/libnetwork/vendor.conf @@ -1,7 +1,7 @@ github.com/Azure/go-ansiterm 19f72df4d05d31cbe1c56bfc8045c96babff6c7e github.com/BurntSushi/toml f706d00e3de6abe700c994cdd545a1a4915af060 github.com/Microsoft/go-winio ce2922f643c8fd76b46cadc7f404a06282678b34 -github.com/Microsoft/hcsshim v0.6.1 +github.com/Microsoft/hcsshim v0.6.3 github.com/armon/go-metrics eb0af217e5e9747e41dd5303755356b62d28e3ec github.com/armon/go-radix e39d623f12e8e41c7b5529e9a9dd67a1e2261f80 github.com/boltdb/bolt c6ba97b89e0454fec9aa92e1d33a4e2c5fc1f631