{"action":"created","comment":{"url":"https://api.github.com/repos/docker/swarmkit/pulls/comments/183912520","pull_request_review_id":115003383,"id":183912520,"diff_hunk":"@@ -0,0 +1,729 @@\n+package ipam\n+\n+import (\n+\t\"fmt\"\n+\t\"net\"\n+\n+\t\"github.com/docker/libnetwork/ipamapi\"\n+\t\"github.com/docker/libnetwork/netlabel\"\n+\n+\t\"github.com/docker/swarmkit/api\"\n+)\n+\n+// DrvRegistry is an interface defining\n+type DrvRegistry interface {\n+\tIPAM(name string) (ipamapi.Ipam, *ipamapi.Capability)\n+}\n+\n+// network is a private type used to hold the internal state of a network in\n+// the IP Allocator\n+type network struct {\n+\t// nw is a local cache of the network object found in the store\n+\tnw *api.Network\n+\n+\t// pools is used to save the internal poolIDs needed when releasing a pool.\n+\t// It maps an ip address to a pool ID.\n+\tpools map[string]string\n+\n+\t// endpoints is a map of the endpoint IP to the poolID from which it was\n+\t// allocated.\n+\tendpoints map[string]string\n+}\n+\n+// Allocator is an interface that represents the IP address allocator. It\n+// exists mainly for testing purposes, so that the allocator can be more easily\n+// mocked out.\n+type Allocator interface {\n+\tRestore([]*api.Network, []*api.Endpoint, []*api.NetworkAttachment) error\n+\tAllocateNetwork(*api.Network) error\n+\tDeallocateNetwork(*api.Network)\n+\tAllocateVIPs(*api.Endpoint, []string) error\n+\tDeallocateVIPs(*api.Endpoint)\n+\tAllocateAttachments([]*api.NetworkAttachmentConfig) ([]*api.NetworkAttachment, error)\n+\tDeallocateAttachments([]*api.NetworkAttachment)\n+}\n+\n+// allocator is an allocator for IP addresses and IPAM pools. It handles all\n+// allocation and deallocation of IP addresses, both for VIPs and endpoints, in\n+// addition to handling IPAM pools for networks.\n+type allocator struct {\n+\t// networks maps network IDs to the locally stored network object\n+\tnetworks map[string]*network\n+\n+\t// drvRegistry is the driver registry, from which we can get active network\n+\t// drivers\n+\tdrvRegistry DrvRegistry\n+}\n+\n+// NewAllocator takes a drvRegistry and creates a new IP allocator\n+func NewAllocator(reg DrvRegistry) Allocator {\n+\treturn &allocator{\n+\t\tnetworks:    make(map[string]*network),\n+\t\tdrvRegistry: reg,\n+\t}\n+}\n+\n+// Restore restores the state of the provided networks to the Allocator. It can\n+// return errors if the initialization of its dependencies fails.\n+func (a *allocator) Restore(networks []*api.Network, endpoints []*api.Endpoint, attachments []*api.NetworkAttachment) error {\n+\t// Initialize the state of the networks. this gets the initial IPAM pools.\n+\tfor _, nw := range networks {\n+\t\t// if the network has no IPAM field, it has no state, and there is\n+\t\t// nothing to do\n+\t\tif nw.IPAM == nil ||\n+\t\t\t// if the network has no IPAM driver, it has no IPAM state, and there\n+\t\t\t// is nothing to do.\n+\t\t\tnw.IPAM.Driver == nil ||\n+\t\t\t// if the network has no ipam configs, then it has no state, and there\n+\t\t\t// is nothing to do\n+\t\t\tlen(nw.IPAM.Configs) == 0 {\n+\t\t\tcontinue\n+\t\t}\n+\t\tlocal := &network{\n+\t\t\tnw:        nw,\n+\t\t\tpools:     make(map[string]string),\n+\t\t\tendpoints: make(map[string]string),\n+\t\t}\n+\t\tipamName := nw.IPAM.Driver.Name\n+\t\tipamOpts := nw.IPAM.Driver.Options\n+\t\tif ipamOpts == nil {\n+\t\t\tipamOpts = map[string]string{}\n+\t\t}\n+\t\t// if we have an ipam driver name and we have IPAM configs, but for\n+\t\t// some reason we don't have IPAM options, just fill that field in.\n+\t\t// shouldn't do any harm. i doubt we'll ever hit this in prod\n+\t\tlocal.nw.IPAM.Driver.Options = ipamOpts\n+\t\t// IPAM returns the Ipam object and also its capabilities. We don't\n+\t\t// use the capabilities or care about them right now, so just ignore\n+\t\t// that part of the return value\n+\t\tipam, _ := a.drvRegistry.IPAM(ipamName)\n+\t\tif ipam == nil {\n+\t\t\treturn ErrInvalidIPAM{ipamName, nw.ID}\n+\t\t}\n+\t\t_, addressSpace, err := ipam.GetDefaultAddressSpaces()\n+\t\t// the only errors here result from having an invalid ipam driver\n+\t\tif err != nil {\n+\t\t\treturn ErrBustedIPAM{ipamName, nw.ID, err}\n+\t\t}\n+\n+\t\t// now initialize the IPAM pools. IPAM pools are the set of addresses\n+\t\t// available for a specific network. There is one IPAM config for every\n+\t\t// pool. In order for this restore operation to be consistent, a\n+\t\t// network must have no ipam configs if it hasn't been allocated\n+\t\tfor _, config := range local.nw.IPAM.Configs {\n+\t\t\t// the last param of RequestPool is \"v6\", meaning IPv6, which we\n+\t\t\t// don't support, hence passing \"false\"\n+\t\t\tpoolID, poolIP, _, err := ipam.RequestPool(addressSpace, config.Subnet, config.Range, ipamOpts, false)\n+\t\t\tif err != nil {\n+\t\t\t\t// typically, if there was an error in requesting a pool, we\n+\t\t\t\t// would release the pools we've already allocated. However, if\n+\t\t\t\t// there is an error at this stage, it means the whole object\n+\t\t\t\t// store is in a bad state, so we don't do that, we just\n+\t\t\t\t// abandon everything.\n+\t\t\t\treturn ErrBadState{\n+\t\t\t\t\tlocal.nw.ID,\n+\t\t\t\t\tfmt.Sprintf(\"error reserving ipam pool: %v\", err),\n+\t\t\t\t}\n+\t\t\t}\n+\t\t\t// each IPAM pool we use has a separate ID referring to it. We also\n+\t\t\t// keep a map of each IP address we have allocated for this network\n+\t\t\t// and what pool it belongs to, so we can deallocate an address\n+\t\t\t// from the correct pool\n+\t\t\tlocal.pools[poolIP.String()] = poolID\n+\t\t\t// now we just need to reserve the gateway address for this pool.\n+\t\t\t// set the IPAM request address type to \"Gateway\" to tell the IPAM\n+\t\t\t// driver that's the kind of address we're requesting\n+\t\t\tipamOpts[ipamapi.RequestAddressType] = netlabel.Gateway\n+\t\t\t// and now, if we have a Gateway address for this network, we need\n+\t\t\t// to allocate it. This condition should never happen; a network\n+\t\t\t// with a valid IPAM config but an empty gateway is a recipe for\n+\t\t\t// disaster.  This is just included for completeness if an older\n+\t\t\t// version has incorrect state\n+\t\t\tif config.Gateway != \"\" {\n+\t\t\t\t_, _, err := ipam.RequestAddress(poolID, net.ParseIP(config.Gateway), ipamOpts)\n+\t\t\t\tif err != nil {\n+\t\t\t\t\treturn ErrBadState{\n+\t\t\t\t\t\tlocal.nw.ID,\n+\t\t\t\t\t\tfmt.Sprintf(\"error requesting already assigned gateway address %v\", err),\n+\t\t\t\t\t}\n+\t\t\t\t}\n+\t\t\t\t// NOTE(dperny): this check was originally here:\n+\t\t\t\t// if gwIP.IP.String() != config.Gateway {\n+\t\t\t\t//   // if we get an IP address from this that isn't the one we\n+\t\t\t\t//   // requested, that's Very Bad.\n+\t\t\t\t//   return ErrBadState{\n+\t\t\t\t//     local.nw.ID,\n+\t\t\t\t//     fmt.Sprintf(\"got back gateway ip %v, but requested ip %v\", gwIP, config.Gateway),\n+\t\t\t\t//   }\n+\t\t\t\t// }\n+\t\t\t\t// it has been removed because we don't need it. this is a\n+\t\t\t\t// check that IPAM is behaving correctly. We don't need to\n+\t\t\t\t// check that IPAM is behaving correctly. Doing so makes this\n+\t\t\t\t// code less clear and harder to test. This has been left in\n+\t\t\t\t// so that this bolt of wisdom is shared with future\n+\t\t\t\t// contributors. A similar check was found in restoreAddress\n+\t\t\t\t// in the analogous location.\n+\n+\t\t\t\t// most addresses need to be added to the map of\n+\t\t\t\t// address -> poolid, but we don't need to do this with the\n+\t\t\t\t// gateway address because it's store in the ipam config with\n+\t\t\t\t// the subnet, which is the key for the pools map.\n+\t\t\t}\n+\t\t\t// delete the Gateway option from the IPAM options when we're done\n+\t\t\tdelete(ipamOpts, ipamapi.RequestAddressType)\n+\t\t\t// finally, add the network to the list of networks we're keeping\n+\t\t\t// track of.\n+\t\t\ta.networks[local.nw.ID] = local\n+\t\t}\n+\t}\n+\n+\t// now restore the VIPs and attachment addresses\n+\n+\t// first the VIPs\n+\tfor _, endpoint := range endpoints {\n+\t\tfor _, vip := range endpoint.VirtualIPs {\n+\t\t\t// there shouldn't be nil vips but lord knows what kind of black\n+\t\t\t// magic goes on in other parts of the code or in the old\n+\t\t\t// allocator, and it doesn't really cost anything\n+\t\t\t// also, check that the VIP address is allocated, so we don't try\n+\t\t\t// to allocate a new VIP. that's the most common class of error in\n+\t\t\t// the old allocator\n+\t\t\tif vip != nil {\n+\t\t\t\tif err := a.restoreAddress(vip.NetworkID, vip.Addr); err != nil {\n+\t\t\t\t\treturn err\n+\t\t\t\t}\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\t// now the attachments\n+\tfor _, attachment := range attachments {\n+\t\t// nil checking for the same reaason as VIPs. why is everything a\n+\t\t// pointer\n+\t\tif attachment != nil && attachment.Network != nil {\n+\t\t\tnwid := attachment.Network.ID\n+\t\t\tfor _, addr := range attachment.Addresses {\n+\t\t\t\tif err := a.restoreAddress(nwid, addr); err != nil {\n+\t\t\t\t\treturn err\n+\t\t\t\t}\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\t// finally, everything is reallocated and we're ready to go and allocate\n+\t// new things.\n+\treturn nil\n+}\n+\n+// restoreAddress is the common functionality needed to mark a given address in\n+// use for the given network ID. if the address given is accidentally empty,\n+// we'll return nil as there is nothing to restore but no error has occurred\n+func (a *allocator) restoreAddress(nwid string, address string) error {\n+\t// this check on address is shared by restoring VIPs and attachments.\n+\tif address == \"\" {\n+\t\treturn nil\n+\t}\n+\t// first, get the local network state and IPAM driver\n+\tlocal, ok := a.networks[nwid]\n+\tif !ok {\n+\t\treturn ErrNetworkNotAllocated{nwid}\n+\t}\n+\tipam, _ := a.drvRegistry.IPAM(local.nw.IPAM.Driver.Name)\n+\tif ipam == nil {\n+\t\treturn ErrInvalidIPAM{local.nw.IPAM.Driver.Name, local.nw.ID}\n+\t}\n+\tipamOpts := local.nw.IPAM.Driver.Options\n+\t// NOTE(dperny): this code, where we try parsing as CIDR and\n+\t// then as a regular IP, is from the old allocator. I do not\n+\t// know why this is done this way\n+\taddr, _, err := net.ParseCIDR(address)\n+\tif err != nil {\n+\t\taddr = net.ParseIP(address)\n+\t\tif addr == nil {\n+\t\t\treturn ErrInvalidAddress{address}\n+\t\t}\n+\t}\n+\t// we don't know which pool this address belongs to, so go through each\n+\t// pool in this network and try to request this address.\n+\t//\n+\t// NOTE(dperny): this code is couched in 2 assumptions:\n+\t//   - IPAM pools can't overlap\n+\t//   - IPAM address requests will prefer \"out of range\" to \"no available\n+\t//\t   IPs\"\n+\t// The first one I'm rather sure of, but the second i'm not... I don't know\n+\t// what the response would be if the pool had no remaining addresses but\n+\t// the requested address was out of range anyway\n+\tfor _, poolID := range local.pools {\n+\t\tip, _, err := ipam.RequestAddress(poolID, addr, ipamOpts)\n+\t\tif err == ipamapi.ErrIPOutOfRange {\n+\t\t\tcontinue\n+\t\t}\n+\t\tif err != nil {\n+\t\t\treturn ErrBadState{\n+\t\t\t\tlocal.nw.ID,\n+\t\t\t\tfmt.Sprintf(\n+\t\t\t\t\t\"error with driver %v requesting address %v: %v\",\n+\t\t\t\t\tlocal.nw.IPAM.Driver.Name,\n+\t\t\t\t\taddr.String(),\n+\t\t\t\t\terr,\n+\t\t\t\t),\n+\t\t\t}\n+\t\t}\n+\t\t// if we get this far, the address belongs to this pool. add to the\n+\t\t// endpoints map for deallocation later and return nil, for no error\n+\t\tlocal.endpoints[ip.String()] = poolID\n+\t\treturn nil\n+\t}\n+\n+\t// if we get all the way through this loop, without jumping to\n+\t// the next iteration of the addresses loop, then we're in a\n+\t// weird situation where the address is out of range for\n+\t// _every_ pool on the network.\n+\treturn ErrBadState{\n+\t\tlocal.nw.ID,\n+\t\tfmt.Sprintf(\"requested address %v is out of range\", addr),\n+\t}\n+}\n+\n+// AllocateNetwork allocates the IPAM pools for the given network. The network\n+// must not be nil, and must not use a node-local driver.\n+//\n+// We can't use a node-lcal driver, because we don't have access to the\n+// network's driver information. We only have acccess to the IPAM driver.\n+func (a *allocator) AllocateNetwork(n *api.Network) (rerr error) {\n+\t// check if this network is already being managed and return an error if it\n+\t// is. networks are immutable and cannot be updated.\n+\tif _, ok := a.networks[n.ID]; ok {\n+\t\treturn ErrNetworkAllocated{n.ID}\n+\t}\n+\n+\t// Now get the IPAM driver and options, either the defaults or the user's\n+\t// specified options.\n+\tipamName := ipamapi.DefaultIPAM\n+\tif n.Spec.IPAM != nil && n.Spec.IPAM.Driver != nil && n.Spec.IPAM.Driver.Name != \"\" {\n+\t\tipamName = n.Spec.IPAM.Driver.Name\n+\t}\n+\tipam, _ := a.drvRegistry.IPAM(ipamName)\n+\tif ipam == nil {\n+\t\treturn ErrInvalidIPAM{ipamName, n.ID}\n+\t}\n+\n+\t// make sure here that ipamOpts is not nil so we don't need to nil check it\n+\t// everywhere later\n+\tipamOpts := map[string]string{}\n+\tif n.Spec.IPAM != nil && n.Spec.IPAM.Driver != nil && n.Spec.IPAM.Driver.Options != nil {\n+\t\tipamOpts = n.Spec.IPAM.Driver.Options\n+\t}\n+\t// if the IPAM serial allocation option has not been explicitly set one\n+\t// way or another, set it here. this (roughly speaking) encourages IPAM to\n+\t// prefer allocating addresses that haven't been recently freed, mitigating\n+\t// (but not eliminating) some issues with objects being deleted before\n+\t// they've fully finished terminating.\n+\tif _, ok := ipamOpts[ipamapi.AllocSerialPrefix]; !ok {\n+\t\tipamOpts[ipamapi.AllocSerialPrefix] = \"true\"\n+\t}\n+\n+\t_, addressSpace, err := ipam.GetDefaultAddressSpaces()\n+\tif err != nil {\n+\t\treturn ErrBustedIPAM{ipamName, n.ID, err}\n+\t}\n+\n+\t// now create the local network state object\n+\tlocal := &network{\n+\t\tnw:        n,\n+\t\tpools:     map[string]string{},\n+\t\tendpoints: map[string]string{},\n+\t}\n+\tvar ipamConfigs []*api.IPAMConfig\n+\tif n.Spec.IPAM != nil && len(n.Spec.IPAM.Configs) != 0 {\n+\t\t// if the user has specified any IPAM configs, we'll use those\n+\t\tipamConfigs = n.Spec.IPAM.Configs\n+\t} else {\n+\t\t// otherwise, we'll create a single default IPAM config.\n+\t\tipamConfigs = []*api.IPAMConfig{\n+\t\t\t{\n+\t\t\t\tFamily: api.IPAMConfig_IPV4,\n+\t\t\t},\n+\t\t}\n+\t}\n+\t// make the slice to hold final IPAM configs\n+\tfinalConfigs := make([]*api.IPAMConfig, 0, len(ipamConfigs))\n+\t// before we start allocating from ipam, set up this defer to roll back\n+\t// allocation in the case that allocation of any particular pool fails\n+\tdefer func() {\n+\t\tif rerr != nil {\n+\t\t\tfor _, config := range finalConfigs {\n+\t\t\t\t// only free addresses that were actually allocated\n+\t\t\t\tif ip := net.ParseIP(config.Gateway); ip != nil {\n+\t\t\t\t\tif err := ipam.ReleaseAddress(local.pools[config.Subnet], ip); err != nil {\n+\t\t\t\t\t\trerr = ErrDoubleFault{rerr, err}\n+\t\t\t\t\t\treturn\n+\t\t\t\t\t}\n+\t\t\t\t}\n+\t\t\t}\n+\t\t\tfor _, pool := range local.pools {\n+\t\t\t\tif err := ipam.ReleasePool(pool); err != nil {\n+\t\t\t\t\trerr = ErrDoubleFault{rerr, err}\n+\t\t\t\t\treturn\n+\t\t\t\t}\n+\t\t\t}\n+\t\t}\n+\t}()\n+\n+\t// now go through all of the IPAM configs and allocate them. in the\n+\t// process, copy those configs to the object's configs. we do copies in\n+\t// order to avoid inadvertently modifying the spec.\n+\t//\n+\t// NOTE(dperny): be careful with this! if this loop doesn't run (because\n+\t// there were no items in ipamConfigs) then this will fail silently!\n+\tfor _, specConfig := range ipamConfigs {\n+\t\tconfig := specConfig.Copy()\n+\t\t// the last parameter of this is \"v6 bool\", but we don't support ipv6\n+\t\t// so we just pass \"false\"\n+\t\tpoolID, poolIP, meta, err := ipam.RequestPool(addressSpace, config.Subnet, config.Range, ipamOpts, false)\n+\t\tif err != nil {\n+\t\t\treturn ErrFailedPoolRequest{config.Subnet, config.Range, err}\n+\t\t}\n+\t\tlocal.pools[poolIP.String()] = poolID\n+\t\t// The IPAM contract allows the IPAM driver to autonomously provide a\n+\t\t// network gateway in response to the pool request.  But if the network\n+\t\t// spec contains a gateway, we will allocate it irrespective of whether\n+\t\t// the ipam driver returned one already.  If none of the above is true,\n+\t\t// we need to allocate one now, and let the driver know this request is\n+\t\t// for the network gateway.\n+\t\tvar (\n+\t\t\tgwIP *net.IPNet\n+\t\t\tip   net.IP\n+\t\t)\n+\n+\t\tif gws, ok := meta[netlabel.Gateway]; ok {\n+\t\t\tif ip, gwIP, err = net.ParseCIDR(gws); err != nil {\n+\t\t\t\treturn ErrBustedIPAM{\n+\t\t\t\t\tipamName,\n+\t\t\t\t\tn.ID,\n+\t\t\t\t\tfmt.Errorf(\n+\t\t\t\t\t\t\"can't parse gateway address (%v) returned by the ipam driver: %v\",\n+\t\t\t\t\t\tgws, err,\n+\t\t\t\t\t),\n+\t\t\t\t}\n+\t\t\t}\n+\t\t\tgwIP.IP = ip\n+\t\t}\n+\t\t// add the option indicating that we're gonna request a gateway, and\n+\t\t// remove it before we exit this function\n+\t\tipamOpts[ipamapi.RequestAddressType] = netlabel.Gateway\n+\t\tdefer delete(ipamOpts, ipamapi.RequestAddressType)\n+\t\tif config.Gateway != \"\" || gwIP == nil {\n+\t\t\tgwIP, _, err = ipam.RequestAddress(poolID, net.ParseIP(config.Gateway), ipamOpts)\n+\t\t\tif err != nil {\n+\t\t\t\treturn ErrFailedAddressRequest{config.Gateway, err}\n+\t\t\t}\n+\t\t}\n+\t\tif config.Subnet == \"\" {\n+\t\t\tconfig.Subnet = poolIP.String()\n+\t\t}\n+\t\tif config.Gateway == \"\" {\n+\t\t\tconfig.Gateway = gwIP.IP.String()\n+\t\t}\n+\t\tfinalConfigs = append(finalConfigs, config)\n+\t}\n+\n+\t// now that everythign has succeeded, add the fields to the network object\n+\tn.IPAM = &api.IPAMOptions{\n+\t\tDriver: &api.Driver{\n+\t\t\tName:    ipamName,\n+\t\t\tOptions: ipamOpts,\n+\t\t},\n+\t}\n+\tn.IPAM.Configs = finalConfigs\n+\n+\t// finally, add this network to the set of allocated networks.\n+\ta.networks[n.ID] = local\n+\treturn nil\n+}\n+\n+// DeallocateNetwork takes a network that has been allocated, and releases all\n+// of the IPAM resource associated with it.\n+func (a *allocator) DeallocateNetwork(network *api.Network) {\n+\tlocal, ok := a.networks[network.ID]\n+\tif !ok {\n+\t\t// if the network was never allocated, nothing to do\n+\t\treturn\n+\t}\n+\n+\t// we know, because we allocated this network, that its IPAM field is fully\n+\t// allocated, and we can use it without nil checking\n+\tipam, _ := a.drvRegistry.IPAM(local.nw.IPAM.Driver.Name)\n+\n+\tfor _, config := range local.nw.IPAM.Configs {\n+\t\t// these things can return errors, but we literally can't do anything\n+\t\t// about it if they do, so just ignore it.\n+\t\tipam.ReleaseAddress(local.pools[config.Subnet], net.ParseIP(config.Gateway))\n+\t\tipam.ReleasePool(local.pools[config.Subnet])\n+\t}\n+\tdelete(a.networks, network.ID)\n+}\n+\n+// AllocateVIPs allocates the VIPs for the provided endpoint and network ids.\n+func (a *allocator) AllocateVIPs(endpoint *api.Endpoint, networkIDs []string) (rerr error) {\n+\t// if the endpoint spec mode has changed to DNSRR, then what we're actually\n+\t// doing is freeing all of the endpoint specs.\n+\tif endpoint.Spec != nil {\n+\t\tif endpoint.Spec.Mode == api.ResolutionModeDNSRoundRobin {\n+\t\t\ta.deallocateVIPs(endpoint.VirtualIPs)\n+\t\t\treturn nil\n+\t\t}\n+\t}\n+\t// first, go through and check that every network we want a VIP for is\n+\t// allocated. if not, return an error. We can't allocate VIPs until the\n+\t// network is allocated\n+\tfor _, nwid := range networkIDs {\n+\t\tif _, ok := a.networks[nwid]; !ok {\n+\t\t\treturn ErrNetworkNotAllocated{nwid}\n+\t\t}\n+\t}\n+\n+\t// now compute the changes\n+\tallocate := []string{}\n+\t// keep is the set of all virtual IPs we'll retain between the previous and\n+\t// current spec. we make it with a capacity the same as the current vips\n+\t// because typically, the vips in use won't change much, and we can avoid\n+\t// allocation by using this as a guess\n+\tkeep := make([]*api.Endpoint_VirtualIP, 0, len(endpoint.VirtualIPs))\n+\tdeallocate := []*api.Endpoint_VirtualIP{}\n+\t// first, we need to figure out if any virtual IPs are being removed\n+\t// continues are bad and hard to follow, so here's the plain english\n+\t// explanation:\n+\t// for every VIP currently allocated\n+\t//     go through the list of desired network IDs\n+\t//         if a network ID matches the ID on the VIP\n+\t//             then we're keeping this vip, so add it to the keep list and\n+\t//             go to the next VIPs, skipping the bottom of this loop\n+\t//     if we get to this point, then we have been through every desired\n+\t//     network ID and not found one matching the one on this VIP, so we\n+\t//     can add it to the list of VIPs to deallocate\n+vips:\n+\tfor _, vip := range endpoint.VirtualIPs {\n+\t\tfor _, nwid := range networkIDs {\n+\t\t\tif vip.NetworkID == nwid {\n+\t\t\t\tkeep = append(keep, vip)\n+\t\t\t\tcontinue vips\n+\t\t\t}\n+\t\t}\n+\t\tdeallocate = append(deallocate, vip)\n+\t}\n+\n+\t// now figure out which new network IDs we've added, which is the same loop\n+\t// above but swapped around to check network IDs against VIPs\n+newvips:\n+\tfor _, nwid := range networkIDs {\n+\t\tfor _, vip := range endpoint.VirtualIPs {\n+\t\t\tif vip.NetworkID == nwid {\n+\t\t\t\tcontinue newvips\n+\t\t\t}\n+\t\t}\n+\t\tallocate = append(allocate, nwid)\n+\t}","path":"manager/allocator/network/ipam/ipam.go","position":526,"original_position":526,"commit_id":"4582f8c189257d3f10d692409be8b61ab91cc4f0","original_commit_id":"4582f8c189257d3f10d692409be8b61ab91cc4f0","user":{"login":"jlhawn","id":1441929,"avatar_url":"https://avatars2.githubusercontent.com/u/1441929?v=4","gravatar_id":"","url":"https://api.github.com/users/jlhawn","html_url":"https://github.com/jlhawn","followers_url":"https://api.github.com/users/jlhawn/followers","following_url":"https://api.github.com/users/jlhawn/following{/other_user}","gists_url":"https://api.github.com/users/jlhawn/gists{/gist_id}","starred_url":"https://api.github.com/users/jlhawn/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jlhawn/subscriptions","organizations_url":"https://api.github.com/users/jlhawn/orgs","repos_url":"https://api.github.com/users/jlhawn/repos","events_url":"https://api.github.com/users/jlhawn/events{/privacy}","received_events_url":"https://api.github.com/users/jlhawn/received_events","type":"User","site_admin":false},"body":"I'd prefer to avoid code with `continue LABEL` if possible. These two loops could written to loop over a `map[string]struct{}` which acts as a set of the requested network IDs. Likewise, we could have a mapping of existing vip network IDs to the VIP. This would allow us to replace these 2 doubly-nested loops with a single loop:\r\n\r\n```go\r\nfor netID, currentVIP := range currentVIPsByNetID {\r\n\tif _, shouldKeep := requestedNetIDs[netID]; shouldKeep {\r\n\t\tkeep = append(keep, currentVIP)\r\n\t} else {\r\n\t\tdeallocate = append(deallocate, currentVIP)\r\n\t}\r\n\tdelete(requestedNetIDs, netID)\r\n}\r\n\r\n// Now `keep` contains all of the VIPs we should keep, `deallocate` contains\r\n// all of the VIPs we should deallocate, and any network IDs which remain in\r\n// the `requestedNetIDs` set are IDs for networks from which we need to\r\n// allocate new VIPs.\r\n```","created_at":"2018-04-24T23:52:43Z","updated_at":"2018-04-24T23:52:43Z","html_url":"https://github.com/docker/swarmkit/pull/2605#discussion_r183912520","pull_request_url":"https://api.github.com/repos/docker/swarmkit/pulls/2605","author_association":"CONTRIBUTOR","_links":{"self":{"href":"https://api.github.com/repos/docker/swarmkit/pulls/comments/183912520"},"html":{"href":"https://github.com/docker/swarmkit/pull/2605#discussion_r183912520"},"pull_request":{"href":"https://api.github.com/repos/docker/swarmkit/pulls/2605"}}},"pull_request":{"url":"https://api.github.com/repos/docker/swarmkit/pulls/2605","id":181617436,"html_url":"https://github.com/docker/swarmkit/pull/2605","diff_url":"https://github.com/docker/swarmkit/pull/2605.diff","patch_url":"https://github.com/docker/swarmkit/pull/2605.patch","issue_url":"https://api.github.com/repos/docker/swarmkit/issues/2605","number":2605,"state":"open","locked":false,"title":"[WIP] Add new IPAM subcomponent","user":{"login":"dperny","id":2367858,"avatar_url":"https://avatars3.githubusercontent.com/u/2367858?v=4","gravatar_id":"","url":"https://api.github.com/users/dperny","html_url":"https://github.com/dperny","followers_url":"https://api.github.com/users/dperny/followers","following_url":"https://api.github.com/users/dperny/following{/other_user}","gists_url":"https://api.github.com/users/dperny/gists{/gist_id}","starred_url":"https://api.github.com/users/dperny/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dperny/subscriptions","organizations_url":"https://api.github.com/users/dperny/orgs","repos_url":"https://api.github.com/users/dperny/repos","events_url":"https://api.github.com/users/dperny/events{/privacy}","received_events_url":"https://api.github.com/users/dperny/received_events","type":"User","site_admin":false},"body":"This PR is marked WIP because it still needs tests written, but the file itself is mostly finished and ready for review.\r\n\r\nPart of a series of commits rewriting the Allocator. Adds another subcomponent, the purpose of which is to translate between the libnetwork ipam library and the swarmkit allocator. It is responsible  for filling out IP addresses.\r\n\r\nNote that this PR includes the commit from #2579, so in review, you should focus only on changes in the `ipam` package. I will fix this Eventually but it's a pain so I've opted not to yet.","created_at":"2018-04-13T23:46:27Z","updated_at":"2018-04-24T23:52:43Z","closed_at":null,"merged_at":null,"merge_commit_sha":"bda8c9e16fc9db10a87a9a3d0b26548a104fcc74","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[],"milestone":null,"commits_url":"https://api.github.com/repos/docker/swarmkit/pulls/2605/commits","review_comments_url":"https://api.github.com/repos/docker/swarmkit/pulls/2605/comments","review_comment_url":"https://api.github.com/repos/docker/swarmkit/pulls/comments{/number}","comments_url":"https://api.github.com/repos/docker/swarmkit/issues/2605/comments","statuses_url":"https://api.github.com/repos/docker/swarmkit/statuses/4582f8c189257d3f10d692409be8b61ab91cc4f0","head":{"label":"dperny:rewrite-allocator-add-ipam","ref":"rewrite-allocator-add-ipam","sha":"4582f8c189257d3f10d692409be8b61ab91cc4f0","user":{"login":"dperny","id":2367858,"avatar_url":"https://avatars3.githubusercontent.com/u/2367858?v=4","gravatar_id":"","url":"https://api.github.com/users/dperny","html_url":"https://github.com/dperny","followers_url":"https://api.github.com/users/dperny/followers","following_url":"https://api.github.com/users/dperny/following{/other_user}","gists_url":"https://api.github.com/users/dperny/gists{/gist_id}","starred_url":"https://api.github.com/users/dperny/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dperny/subscriptions","organizations_url":"https://api.github.com/users/dperny/orgs","repos_url":"https://api.github.com/users/dperny/repos","events_url":"https://api.github.com/users/dperny/events{/privacy}","received_events_url":"https://api.github.com/users/dperny/received_events","type":"User","site_admin":false},"repo":{"id":60813040,"name":"swarmkit-1","full_name":"dperny/swarmkit-1","owner":{"login":"dperny","id":2367858,"avatar_url":"https://avatars3.githubusercontent.com/u/2367858?v=4","gravatar_id":"","url":"https://api.github.com/users/dperny","html_url":"https://github.com/dperny","followers_url":"https://api.github.com/users/dperny/followers","following_url":"https://api.github.com/users/dperny/following{/other_user}","gists_url":"https://api.github.com/users/dperny/gists{/gist_id}","starred_url":"https://api.github.com/users/dperny/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dperny/subscriptions","organizations_url":"https://api.github.com/users/dperny/orgs","repos_url":"https://api.github.com/users/dperny/repos","events_url":"https://api.github.com/users/dperny/events{/privacy}","received_events_url":"https://api.github.com/users/dperny/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/dperny/swarmkit-1","description":"A toolkit for orchestrating distributed systems at any scale. It includes primitives for node discovery, raft-based consensus, task scheduling and more.","fork":true,"url":"https://api.github.com/repos/dperny/swarmkit-1","forks_url":"https://api.github.com/repos/dperny/swarmkit-1/forks","keys_url":"https://api.github.com/repos/dperny/swarmkit-1/keys{/key_id}","collaborators_url":"https://api.github.com/repos/dperny/swarmkit-1/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/dperny/swarmkit-1/teams","hooks_url":"https://api.github.com/repos/dperny/swarmkit-1/hooks","issue_events_url":"https://api.github.com/repos/dperny/swarmkit-1/issues/events{/number}","events_url":"https://api.github.com/repos/dperny/swarmkit-1/events","assignees_url":"https://api.github.com/repos/dperny/swarmkit-1/assignees{/user}","branches_url":"https://api.github.com/repos/dperny/swarmkit-1/branches{/branch}","tags_url":"https://api.github.com/repos/dperny/swarmkit-1/tags","blobs_url":"https://api.github.com/repos/dperny/swarmkit-1/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/dperny/swarmkit-1/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/dperny/swarmkit-1/git/refs{/sha}","trees_url":"https://api.github.com/repos/dperny/swarmkit-1/git/trees{/sha}","statuses_url":"https://api.github.com/repos/dperny/swarmkit-1/statuses/{sha}","languages_url":"https://api.github.com/repos/dperny/swarmkit-1/languages","stargazers_url":"https://api.github.com/repos/dperny/swarmkit-1/stargazers","contributors_url":"https://api.github.com/repos/dperny/swarmkit-1/contributors","subscribers_url":"https://api.github.com/repos/dperny/swarmkit-1/subscribers","subscription_url":"https://api.github.com/repos/dperny/swarmkit-1/subscription","commits_url":"https://api.github.com/repos/dperny/swarmkit-1/commits{/sha}","git_commits_url":"https://api.github.com/repos/dperny/swarmkit-1/git/commits{/sha}","comments_url":"https://api.github.com/repos/dperny/swarmkit-1/comments{/number}","issue_comment_url":"https://api.github.com/repos/dperny/swarmkit-1/issues/comments{/number}","contents_url":"https://api.github.com/repos/dperny/swarmkit-1/contents/{+path}","compare_url":"https://api.github.com/repos/dperny/swarmkit-1/compare/{base}...{head}","merges_url":"https://api.github.com/repos/dperny/swarmkit-1/merges","archive_url":"https://api.github.com/repos/dperny/swarmkit-1/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/dperny/swarmkit-1/downloads","issues_url":"https://api.github.com/repos/dperny/swarmkit-1/issues{/number}","pulls_url":"https://api.github.com/repos/dperny/swarmkit-1/pulls{/number}","milestones_url":"https://api.github.com/repos/dperny/swarmkit-1/milestones{/number}","notifications_url":"https://api.github.com/repos/dperny/swarmkit-1/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/dperny/swarmkit-1/labels{/name}","releases_url":"https://api.github.com/repos/dperny/swarmkit-1/releases{/id}","deployments_url":"https://api.github.com/repos/dperny/swarmkit-1/deployments","created_at":"2016-06-09T23:18:00Z","updated_at":"2016-06-09T23:18:01Z","pushed_at":"2018-04-17T23:23:51Z","git_url":"git://github.com/dperny/swarmkit-1.git","ssh_url":"git@github.com:dperny/swarmkit-1.git","clone_url":"https://github.com/dperny/swarmkit-1.git","svn_url":"https://github.com/dperny/swarmkit-1","homepage":"","size":20161,"stargazers_count":0,"watchers_count":0,"language":"Go","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":0,"mirror_url":null,"archived":false,"open_issues_count":0,"license":{"key":"apache-2.0","name":"Apache License 2.0","spdx_id":"Apache-2.0","url":"https://api.github.com/licenses/apache-2.0"},"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"docker:master","ref":"master","sha":"9c2aa152c3054371b833483a7ddad8d15052ec4f","user":{"login":"docker","id":5429470,"avatar_url":"https://avatars1.githubusercontent.com/u/5429470?v=4","gravatar_id":"","url":"https://api.github.com/users/docker","html_url":"https://github.com/docker","followers_url":"https://api.github.com/users/docker/followers","following_url":"https://api.github.com/users/docker/following{/other_user}","gists_url":"https://api.github.com/users/docker/gists{/gist_id}","starred_url":"https://api.github.com/users/docker/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/docker/subscriptions","organizations_url":"https://api.github.com/users/docker/orgs","repos_url":"https://api.github.com/users/docker/repos","events_url":"https://api.github.com/users/docker/events{/privacy}","received_events_url":"https://api.github.com/users/docker/received_events","type":"Organization","site_admin":false},"repo":{"id":51556828,"name":"swarmkit","full_name":"docker/swarmkit","owner":{"login":"docker","id":5429470,"avatar_url":"https://avatars1.githubusercontent.com/u/5429470?v=4","gravatar_id":"","url":"https://api.github.com/users/docker","html_url":"https://github.com/docker","followers_url":"https://api.github.com/users/docker/followers","following_url":"https://api.github.com/users/docker/following{/other_user}","gists_url":"https://api.github.com/users/docker/gists{/gist_id}","starred_url":"https://api.github.com/users/docker/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/docker/subscriptions","organizations_url":"https://api.github.com/users/docker/orgs","repos_url":"https://api.github.com/users/docker/repos","events_url":"https://api.github.com/users/docker/events{/privacy}","received_events_url":"https://api.github.com/users/docker/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/docker/swarmkit","description":"A toolkit for orchestrating distributed systems at any scale. It includes primitives for node discovery, raft-based consensus, task scheduling and more.","fork":false,"url":"https://api.github.com/repos/docker/swarmkit","forks_url":"https://api.github.com/repos/docker/swarmkit/forks","keys_url":"https://api.github.com/repos/docker/swarmkit/keys{/key_id}","collaborators_url":"https://api.github.com/repos/docker/swarmkit/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/docker/swarmkit/teams","hooks_url":"https://api.github.com/repos/docker/swarmkit/hooks","issue_events_url":"https://api.github.com/repos/docker/swarmkit/issues/events{/number}","events_url":"https://api.github.com/repos/docker/swarmkit/events","assignees_url":"https://api.github.com/repos/docker/swarmkit/assignees{/user}","branches_url":"https://api.github.com/repos/docker/swarmkit/branches{/branch}","tags_url":"https://api.github.com/repos/docker/swarmkit/tags","blobs_url":"https://api.github.com/repos/docker/swarmkit/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/docker/swarmkit/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/docker/swarmkit/git/refs{/sha}","trees_url":"https://api.github.com/repos/docker/swarmkit/git/trees{/sha}","statuses_url":"https://api.github.com/repos/docker/swarmkit/statuses/{sha}","languages_url":"https://api.github.com/repos/docker/swarmkit/languages","stargazers_url":"https://api.github.com/repos/docker/swarmkit/stargazers","contributors_url":"https://api.github.com/repos/docker/swarmkit/contributors","subscribers_url":"https://api.github.com/repos/docker/swarmkit/subscribers","subscription_url":"https://api.github.com/repos/docker/swarmkit/subscription","commits_url":"https://api.github.com/repos/docker/swarmkit/commits{/sha}","git_commits_url":"https://api.github.com/repos/docker/swarmkit/git/commits{/sha}","comments_url":"https://api.github.com/repos/docker/swarmkit/comments{/number}","issue_comment_url":"https://api.github.com/repos/docker/swarmkit/issues/comments{/number}","contents_url":"https://api.github.com/repos/docker/swarmkit/contents/{+path}","compare_url":"https://api.github.com/repos/docker/swarmkit/compare/{base}...{head}","merges_url":"https://api.github.com/repos/docker/swarmkit/merges","archive_url":"https://api.github.com/repos/docker/swarmkit/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/docker/swarmkit/downloads","issues_url":"https://api.github.com/repos/docker/swarmkit/issues{/number}","pulls_url":"https://api.github.com/repos/docker/swarmkit/pulls{/number}","milestones_url":"https://api.github.com/repos/docker/swarmkit/milestones{/number}","notifications_url":"https://api.github.com/repos/docker/swarmkit/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/docker/swarmkit/labels{/name}","releases_url":"https://api.github.com/repos/docker/swarmkit/releases{/id}","deployments_url":"https://api.github.com/repos/docker/swarmkit/deployments","created_at":"2016-02-12T00:02:15Z","updated_at":"2018-04-24T08:15:19Z","pushed_at":"2018-04-24T13:28:14Z","git_url":"git://github.com/docker/swarmkit.git","ssh_url":"git@github.com:docker/swarmkit.git","clone_url":"https://github.com/docker/swarmkit.git","svn_url":"https://github.com/docker/swarmkit","homepage":"","size":19028,"stargazers_count":1655,"watchers_count":1655,"language":"Go","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":382,"mirror_url":null,"archived":false,"open_issues_count":218,"license":{"key":"apache-2.0","name":"Apache License 2.0","spdx_id":"Apache-2.0","url":"https://api.github.com/licenses/apache-2.0"},"forks":382,"open_issues":218,"watchers":1655,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/docker/swarmkit/pulls/2605"},"html":{"href":"https://github.com/docker/swarmkit/pull/2605"},"issue":{"href":"https://api.github.com/repos/docker/swarmkit/issues/2605"},"comments":{"href":"https://api.github.com/repos/docker/swarmkit/issues/2605/comments"},"review_comments":{"href":"https://api.github.com/repos/docker/swarmkit/pulls/2605/comments"},"review_comment":{"href":"https://api.github.com/repos/docker/swarmkit/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/docker/swarmkit/pulls/2605/commits"},"statuses":{"href":"https://api.github.com/repos/docker/swarmkit/statuses/4582f8c189257d3f10d692409be8b61ab91cc4f0"}},"author_association":"MEMBER"}}
{"actor":{"display_login":"jlhawn"}}