Merge pull request #221 from eyakubovich/config-changed

Assert lease is compat with config before reuse

subnet/etcd.go

@@ -131,14 +131,23 @@ func (m *EtcdManager) tryAcquireLease(ctx context.Context, network string, confi
 
 	// try to reuse a subnet if there's one that matches our IP
 	if l := findLeaseByIP(leases, extIP); l != nil {
-		resp, err := m.registry.updateSubnet(ctx, network, l.Key(), string(attrBytes), subnetTTL)
-		if err != nil {
-			return nil, err
-		}
+		// make sure the existing subnet is still within the configured network
+		if isSubnetConfigCompat(config, l.Subnet) {
+			log.Infof("Found lease (%v) for current IP (%v), reusing", l.Subnet, extIP)
+			resp, err := m.registry.updateSubnet(ctx, network, l.Key(), string(attrBytes), subnetTTL)
+			if err != nil {
+				return nil, err
+			}
 
-		l.Attrs = attrs
-		l.Expiration = *resp.Node.Expiration
-		return l, nil
+			l.Attrs = attrs
+			l.Expiration = *resp.Node.Expiration
+			return l, nil
+		} else {
+			log.Infof("Found lease (%v) for current IP (%v) but not compatible with current config, deleting", l.Subnet, extIP)
+			if _, err := m.registry.deleteSubnet(ctx, network, l.Key()); err != nil {
+				return nil, err
+			}
+		}
 	}
 
 	// no existing match, grab a new one
@@ -381,11 +390,10 @@ func (m *EtcdManager) watchReset(ctx context.Context, network string) (WatchResu
 	return wr, nil
 }
 
-func interrupted(cancel chan bool) bool {
-	select {
-	case <-cancel:
-		return true
-	default:
+func isSubnetConfigCompat(config *Config, sn ip.IP4Net) bool {
+	if sn.IP < config.SubnetMin || sn.IP > config.SubnetMax {
 		return false
 	}
+
+	return sn.PrefixLen == config.SubnetLen
 }

subnet/mock_registry.go

@@ -58,6 +58,13 @@ func (msr *mockSubnetRegistry) getConfig(ctx context.Context, network string) (*
 	}, nil
 }
 
+func (msr *mockSubnetRegistry) setConfig(config string) {
+	msr.config = &etcd.Node{
+		Key:   "config",
+		Value: config,
+	}
+}
+
 func (msr *mockSubnetRegistry) getSubnets(ctx context.Context, network string) (*etcd.Response, error) {
 	return &etcd.Response{
 		Node:      msr.subnets,
@@ -120,6 +127,29 @@ func (msr *mockSubnetRegistry) updateSubnet(ctx context.Context, network, sn, da
 	return nil, fmt.Errorf("Subnet not found")
 }
 
+func (msr *mockSubnetRegistry) deleteSubnet(ctx context.Context, network, sn string) (*etcd.Response, error) {
+	msr.index += 1
+
+	for i, n := range msr.subnets.Nodes {
+		if n.Key == sn {
+			msr.subnets.Nodes[i] = msr.subnets.Nodes[len(msr.subnets.Nodes)-1]
+			msr.subnets.Nodes = msr.subnets.Nodes[:len(msr.subnets.Nodes)-1]
+			msr.events <- &etcd.Response{
+				Action: "delete",
+				Node:   n,
+			}
+
+			return &etcd.Response{
+				Node:      n,
+				EtcdIndex: msr.index,
+			}, nil
+		}
+	}
+
+	return nil, fmt.Errorf("Subnet not found")
+
+}
+
 func (msr *mockSubnetRegistry) watchSubnets(ctx context.Context, network string, since uint64) (*etcd.Response, error) {
 	for {
 		select {

subnet/registry.go

@@ -32,6 +32,7 @@ type Registry interface {
 	getSubnets(ctx context.Context, network string) (*etcd.Response, error)
 	createSubnet(ctx context.Context, network, sn, data string, ttl uint64) (*etcd.Response, error)
 	updateSubnet(ctx context.Context, network, sn, data string, ttl uint64) (*etcd.Response, error)
+	deleteSubnet(ctx context.Context, network, sn string) (*etcd.Response, error)
 	watchSubnets(ctx context.Context, network string, since uint64) (*etcd.Response, error)
 }
 
@@ -111,6 +112,11 @@ func (esr *etcdSubnetRegistry) updateSubnet(ctx context.Context, network, sn, da
 	return resp, nil
 }
 
+func (esr *etcdSubnetRegistry) deleteSubnet(ctx context.Context, network, sn string) (*etcd.Response, error) {
+	key := path.Join(esr.etcdCfg.Prefix, network, "subnets", sn)
+	return esr.client().Delete(key, false)
+}
+
 type watchResp struct {
 	resp *etcd.Response
 	err  error

subnet/subnet_test.go

@@ -40,7 +40,7 @@ func newDummyRegistry(ttlOverride uint64) *mockSubnetRegistry {
 }
 
 func TestAcquireLease(t *testing.T) {
-	msr := newDummyRegistry(0)
+	msr := newDummyRegistry(1000)
 	sm := newEtcdManager(msr)
 
 	extIP, _ := ip.ParseIP4("1.2.3.4")
@@ -67,6 +67,50 @@ func TestAcquireLease(t *testing.T) {
 	}
 }
 
+func TestConfigChanged(t *testing.T) {
+	msr := newDummyRegistry(1000)
+	sm := newEtcdManager(msr)
+
+	extIP, _ := ip.ParseIP4("1.2.3.4")
+	attrs := LeaseAttrs{
+		PublicIP: extIP,
+	}
+
+	l, err := sm.AcquireLease(context.Background(), "", &attrs)
+	if err != nil {
+		t.Fatal("AcquireLease failed: ", err)
+	}
+
+	if l.Subnet.String() != "10.3.3.0/24" {
+		t.Fatal("Subnet mismatch: expected 10.3.3.0/24, got: ", l.Subnet)
+	}
+
+	// Change config
+	config := `{ "Network": "10.4.0.0/16" }`
+	msr.setConfig(config)
+
+	// Acquire again, should not reuse
+	if l, err = sm.AcquireLease(context.Background(), "", &attrs); err != nil {
+		t.Fatal("AcquireLease failed: ", err)
+	}
+
+	newNet := newIP4Net("10.4.0.0", 16)
+	if !newNet.Contains(l.Subnet.IP) {
+		t.Fatalf("Subnet mismatch: expected within %v, got: %v", newNet, l.Subnet)
+	}
+}
+
+func newIP4Net(ipaddr string, prefix uint) ip.IP4Net {
+	a, err := ip.ParseIP4(ipaddr)
+	if err != nil {
+		panic("failed to parse ipaddr")
+	}
+	return ip.IP4Net{
+		IP:        a,
+		PrefixLen: prefix,
+	}
+}
+
 func TestWatchLeaseAdded(t *testing.T) {
 	msr := newDummyRegistry(0)
 	sm := newEtcdManager(msr)