Rationalize ipam arguments

- rename -defaultsubnet to -ipsubnet
- default subnet defaults to whole -iprange
- parse and check inside ipam package
- removes some error cases

ipam/address/address.go

@@ -40,15 +40,13 @@ func ParseCIDR(s string) (Address, CIDR, error) {
 
 func (cidr CIDR) Size() Offset { return 1 << uint(32-cidr.PrefixLen) }
 func (cidr CIDR) End() Address { return Add(cidr.Start, cidr.Size()) }
-func (cidr CIDR) Blank() bool  { return cidr.Start == 0 && cidr.PrefixLen == 0 }
 
 func (cidr CIDR) String() string {
 	return fmt.Sprintf("%s/%d", cidr.Start.String(), cidr.PrefixLen)
 }
 
 func (cidr1 CIDR) Overlaps(cidr2 CIDR) bool {
-	return cidr1.Start >= cidr2.Start && cidr1.Start < cidr2.End() ||
-		cidr2.Start >= cidr1.Start && cidr2.Start < cidr1.End()
+	return !(cidr1.Start >= cidr2.End() || cidr1.End() <= cidr2.Start)
 }
 
 func (cidr CIDR) Contains(addr Address) bool {

ipam/allocator.go

@@ -21,6 +21,7 @@ const (
 	msgRingUpdate
 
 	paxosInterval = time.Second * 5
+	minSubnetSize = 4 // first and last addresses are excluded, so 2 would be too small
 )
 
 // operation represents something which Allocator wants to do, but
@@ -60,16 +61,21 @@ type Allocator struct {
 }
 
 // NewAllocator creates and initialises a new Allocator
-func NewAllocator(ourName router.PeerName, ourUID router.PeerUID, ourNickname string, universe address.CIDR, quorum uint) *Allocator {
-	return &Allocator{
-		ourName:   ourName,
-		universe:  universe,
-		ring:      ring.New(universe.Start, address.Add(universe.Start, universe.Size()), ourName),
-		owned:     make(map[string][]address.Address),
-		paxos:     paxos.NewNode(ourName, ourUID, quorum),
-		nicknames: map[router.PeerName]string{ourName: ourNickname},
-		now:       time.Now,
+func NewAllocator(ourName router.PeerName, ourUID router.PeerUID, ourNickname string, universeStr string, quorum uint) (*Allocator, error) {
+	universe, err := parseAndCheckCIDR(universeStr)
+	if err != nil {
+		return nil, err
 	}
+	return &Allocator{
+		ourName:       ourName,
+		universe:      universe,
+		defaultSubnet: universe,
+		ring:          ring.New(universe.Start, address.Add(universe.Start, universe.Size()), ourName),
+		owned:         make(map[string][]address.Address),
+		paxos:         paxos.NewNode(ourName, ourUID, quorum),
+		nicknames:     map[router.PeerName]string{ourName: ourNickname},
+		now:           time.Now,
+	}, nil
 }
 
 // Start runs the allocator goroutine
@@ -176,11 +182,12 @@ func hasBeenCancelled(cancelChan <-chan bool) func() bool {
 
 // Actor client API
 
-func (alloc *Allocator) SetDefaultSubnet(subnet address.CIDR) error {
-	resultChan := make(chan error)
-	if subnet.Size() < 4 {
-		return fmt.Errorf("Allocation range smaller than minimum size 4: %s", subnet)
+func (alloc *Allocator) SetDefaultSubnet(subnetStr string) error {
+	subnet, err := parseAndCheckCIDR(subnetStr)
+	if err != nil {
+		return err
 	}
+	resultChan := make(chan error)
 	alloc.actionChan <- func() {
 		if !alloc.universe.Overlaps(subnet) {
 			resultChan <- fmt.Errorf("Default subnet %s out of bounds: %s", subnet, alloc.universe)
@@ -227,6 +234,12 @@ func (alloc *Allocator) Claim(ident string, addr address.Address, cancelChan <-c
 	return <-resultChan
 }
 
+// ContainerDied is provided to satisfy the updater interface; does a 'Delete' underneath.  Sync.
+func (alloc *Allocator) ContainerDied(ident string) error {
+	alloc.debugln("Container", ident, "died; releasing addresses")
+	return alloc.Delete(ident)
+}
+
 // Delete (Sync) - release all IP addresses for container with given name
 func (alloc *Allocator) Delete(ident string) error {
 	errChan := make(chan error)
@@ -271,12 +284,6 @@ func (alloc *Allocator) Free(ident string, addrToFree address.Address) error {
 	return <-errChan
 }
 
-// ContainerDied is provided to satisfy the updater interface; does a 'Delete' underneath.  Sync.
-func (alloc *Allocator) ContainerDied(ident string) error {
-	alloc.debugln("Container", ident, "died; releasing addresses")
-	return alloc.Delete(ident)
-}
-
 // Sync.
 func (alloc *Allocator) String() string {
 	resultChan := make(chan string)
@@ -487,12 +494,20 @@ func (alloc *Allocator) actorLoop(actionChan <-chan func()) {
 
 // Helper functions
 
+func parseAndCheckCIDR(cidrStr string) (address.CIDR, error) {
+	_, cidr, err := address.ParseCIDR(cidrStr)
+	if err != nil {
+		return address.CIDR{}, err
+	}
+	if cidr.Size() < minSubnetSize {
+		return address.CIDR{}, fmt.Errorf("Allocation range smaller than minimum size %d: %s", minSubnetSize, cidrStr)
+	}
+	return cidr, nil
+}
+
 func (alloc *Allocator) string() string {
 	var buf bytes.Buffer
-	fmt.Fprintf(&buf, "Allocator universe %s", alloc.universe)
-	if !alloc.defaultSubnet.Blank() {
-		fmt.Fprintf(&buf, " (default subnet %s)", alloc.defaultSubnet)
-	}
+	fmt.Fprintf(&buf, "Allocator range %s (default subnet %s)", alloc.universe, alloc.defaultSubnet)
 
 	if alloc.ring.Empty() {
 		if alloc.paxosTicker != nil {

ipam/http.go

@@ -50,10 +50,6 @@ func (alloc *Allocator) HandleHTTP(router *mux.Router) {
 	})
 
 	router.Methods("GET").Path("/ip/{id}").HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		if alloc.defaultSubnet.Blank() {
-			badRequest(w, fmt.Errorf("No subnet specified"))
-			return
-		}
 		addr, err := alloc.Lookup(mux.Vars(r)["id"], alloc.defaultSubnet)
 		if err != nil {
 			http.NotFound(w, r)
@@ -86,10 +82,6 @@ func (alloc *Allocator) HandleHTTP(router *mux.Router) {
 	})
 
 	router.Methods("POST").Path("/ip/{id}").HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		if alloc.defaultSubnet.Blank() {
-			badRequest(w, fmt.Errorf("No subnet specified"))
-			return
-		}
 		closedChan := w.(http.CloseNotifier).CloseNotify()
 		ident := mux.Vars(r)["id"]
 		newAddr, err := alloc.Allocate(ident, alloc.defaultSubnet, closedChan)

ipam/testutils_test.go

@@ -134,13 +134,11 @@ func makeAllocator(name string, cidrStr string, quorum uint) *Allocator {
 		panic(err)
 	}
 
-	_, cidr, err := address.ParseCIDR(cidrStr)
+	alloc, err := NewAllocator(peername, router.PeerUID(rand.Int63()),
+		"nick-"+name, cidrStr, quorum)
 	if err != nil {
 		panic(err)
 	}
-	alloc := NewAllocator(peername, router.PeerUID(rand.Int63()),
-		"nick-"+name, cidr, quorum)
-	alloc.defaultSubnet = cidr
 
 	return alloc
 }

prog/weaver/main.go

@@ -17,7 +17,6 @@ import (
 	. "github.com/weaveworks/weave/common"
 	"github.com/weaveworks/weave/common/updater"
 	"github.com/weaveworks/weave/ipam"
-	"github.com/weaveworks/weave/ipam/address"
 	weavenet "github.com/weaveworks/weave/net"
 	weave "github.com/weaveworks/weave/router"
 )
@@ -38,24 +37,24 @@ func main() {
 	runtime.GOMAXPROCS(procs)
 
 	var (
-		config      weave.Config
-		justVersion bool
-		ifaceName   string
-		routerName  string
-		nickName    string
-		password    string
-		wait        int
-		debug       bool
-		pktdebug    bool
-		prof        string
-		bufSzMB     int
-		noDiscovery bool
-		httpAddr    string
-		iprangeCIDR string
-		defltSubnet string
-		peerCount   int
-		apiPath     string
-		peers       []string
+		config       weave.Config
+		justVersion  bool
+		ifaceName    string
+		routerName   string
+		nickName     string
+		password     string
+		wait         int
+		debug        bool
+		pktdebug     bool
+		prof         string
+		bufSzMB      int
+		noDiscovery  bool
+		httpAddr     string
+		iprangeCIDR  string
+		ipsubnetCIDR string
+		peerCount    int
+		apiPath      string
+		peers        []string
 	)
 
 	flag.BoolVar(&justVersion, "version", false, "print version and exit")
@@ -72,8 +71,8 @@ func main() {
 	flag.BoolVar(&noDiscovery, "nodiscovery", false, "disable peer discovery")
 	flag.IntVar(&bufSzMB, "bufsz", 8, "capture buffer size in MB")
 	flag.StringVar(&httpAddr, "httpaddr", fmt.Sprintf(":%d", weave.HTTPPort), "address to bind HTTP interface to (disabled if blank, absolute path indicates unix domain socket)")
-	flag.StringVar(&iprangeCIDR, "iprange", "", "IP address range to allocate within, in CIDR notation")
-	flag.StringVar(&defltSubnet, "defaultsubnet", "", "default subnet for IP address allocation, in CIDR notation")
+	flag.StringVar(&iprangeCIDR, "iprange", "", "IP address range reserved for automatic allocation, in CIDR notation")
+	flag.StringVar(&ipsubnetCIDR, "ipsubnet", "", "subnet to allocate within by default, in CIDR notation")
 	flag.IntVar(&peerCount, "initpeercount", 0, "number of peers in network (for IP address allocation)")
 	flag.StringVar(&apiPath, "api", "unix:///var/run/docker.sock", "Path to Docker API socket")
 	flag.Parse()
@@ -141,9 +140,9 @@ func main() {
 
 	var allocator *ipam.Allocator
 	if iprangeCIDR != "" {
-		allocator = createAllocator(router, apiPath, iprangeCIDR, defltSubnet, determineQuorum(peerCount, peers))
+		allocator = createAllocator(router, apiPath, iprangeCIDR, ipsubnetCIDR, determineQuorum(peerCount, peers))
 	} else if peerCount > 0 {
-		log.Fatal("-initpeercount flag specified without -iprange or -ipuniverse")
+		log.Fatal("-initpeercount flag specified without -iprange")
 	} else {
 		router.NewGossip("IPallocation", &ipam.DummyAllocator{})
 	}
@@ -197,23 +196,15 @@ func logFrameFunc(debug bool) weave.LogFrameFunc {
 	}
 }
 
-func createAllocator(router *weave.Router, apiPath string, ipUniverse string, defaultSubnet string, quorum uint) *ipam.Allocator {
-	_, universe, err := address.ParseCIDR(ipUniverse)
+func createAllocator(router *weave.Router, apiPath string, ipRangeStr string, defaultSubnetStr string, quorum uint) *ipam.Allocator {
+	allocator, err := ipam.NewAllocator(router.Ourself.Peer.Name, router.Ourself.Peer.UID, router.Ourself.Peer.NickName, ipRangeStr, quorum)
 	if err != nil {
 		log.Fatal(err)
 	}
-	if universe.Size() < 4 {
-		log.Fatal("IP allocation range too small")
-	}
-	allocator := ipam.NewAllocator(router.Ourself.Peer.Name, router.Ourself.Peer.UID, router.Ourself.Peer.NickName, universe, quorum)
 	allocator.SetInterfaces(router.NewGossip("IPallocation", allocator))
 	allocator.Start()
-	if defaultSubnet != "" {
-		_, subnet, err := address.ParseCIDR(defaultSubnet)
-		if err != nil {
-			log.Fatal(err)
-		}
-		if err = allocator.SetDefaultSubnet(subnet); err != nil {
+	if defaultSubnetStr != "" {
+		if err = allocator.SetDefaultSubnet(defaultSubnetStr); err != nil {
 			log.Fatal(err)
 		}
 	}

weave

@@ -782,10 +782,6 @@ ipam_cidrs() {
     fi
     # If no addresses passed in, get one in the default subnet
     CIDR_ARGS="$(http_call $CONTAINER_NAME $HTTP_PORT POST /ip/$CONTAINER_ID)"
-    if [ "$CIDR_ARGS" = "No subnet specified" ] ; then
-        echo "No IP address supplied (use the -iprange and -defaultsubnet options on 'weave launch' to enable IP address allocation)" >&2
-        return 1
-    fi
     if [ "$CIDR_ARGS" = "404 page not found" ] ; then
         echo "No IP address supplied (use the -iprange option on 'weave launch' to enable IP address allocation)" >&2
         return 1