Browse Source

Assert lease is compat with config before reuse

When starting, only reuse the lease if it is still compatible
with the current network configuration. If values such as
Network, SubnetMin, SubnetMax, or SubnetLen have changed, the
lease may not be usuable in this case and should be deleted and
re-aquired.

Fixes #196
Eugene Yakubovich 9 years ago
parent
commit
bd9bfbafc8
4 changed files with 101 additions and 13 deletions
  1. 20 12
      subnet/etcd.go
  2. 30 0
      subnet/mock_registry.go
  3. 6 0
      subnet/registry.go
  4. 45 1
      subnet/subnet_test.go

+ 20 - 12
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
 }

+ 30 - 0
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 {

+ 6 - 0
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

+ 45 - 1
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)