Repository: kumahq/kuma
Pull request #1213 (2020-11-25 12:23:24)

fix(kuma-cp) resync IPAM in case of Kuma CP restart
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>

### Summary

IPAM has to be synced between Kuma CP restarts. This PR fixes it



Patch:
diff --git a/pkg/dns/ip.go b/pkg/dns/ip.go
index cd3c8d055..03b1813aa 100644
--- a/pkg/dns/ip.go
+++ b/pkg/dns/ip.go
@@ -2,6 +2,7 @@ package dns
 
 import (
 	"net"
+	"strings"
 
 	"github.com/Nordix/simple-ipam/pkg/ipam"
 )
@@ -9,6 +10,11 @@ import (
 type IPAM interface {
 	AllocateIP() (string, error)
 	FreeIP(ip string) error
+	ReserveIP(ip string) error
+}
+
+func IsAddressAlreadyAllocated(err error) bool {
+	return err != nil && strings.HasPrefix(err.Error(), "Address already allocated")
 }
 
 type SimpleIPAM struct {
@@ -38,7 +44,7 @@ func (i *SimpleIPAM) FreeIP(ip string) error {
 
 	// ensure the IP is reserved before deleting it
 	err := i.Reserve(parsedIP)
-	if err != nil && err.Error() != "Address already allocated" {
+	if err != nil && !IsAddressAlreadyAllocated(err) {
 		return err
 	}
 
@@ -46,3 +52,7 @@ func (i *SimpleIPAM) FreeIP(ip string) error {
 
 	return nil
 }
+
+func (i *SimpleIPAM) ReserveIP(ip string) error {
+	return i.Reserve(net.ParseIP(ip))
+}
diff --git a/pkg/dns/vips/persistence.go b/pkg/dns/vips/persistence.go
index 86d540294..170429881 100644
--- a/pkg/dns/vips/persistence.go
+++ b/pkg/dns/vips/persistence.go
@@ -48,16 +48,17 @@ func NewPersistence(resourceManager manager.ReadOnlyResourceManager, configManag
 	}
 }
 
-func (m *Persistence) Get() (List, error) {
+func (m *Persistence) Get() (global List, meshed map[string]List, errs error) {
 	resourceList := &config_model.ConfigResourceList{}
 	if err := m.configManager.List(context.Background(), resourceList); err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 
-	var errs error
-	vips := List{}
+	global = List{}
+	meshed = map[string]List{}
 	for _, resource := range resourceList.Items {
-		if _, ok := MeshFromConfigKey(resource.Meta.GetName()); !ok {
+		mesh, ok := MeshFromConfigKey(resource.Meta.GetName())
+		if !ok {
 			continue
 		}
 		if resource.Spec.Config == "" {
@@ -68,9 +69,10 @@ func (m *Persistence) Get() (List, error) {
 			errs = multierr.Append(errs, err)
 			continue
 		}
-		vips.Append(v)
+		global.Append(v)
+		meshed[mesh] = v
 	}
-	return vips, nil
+	return
 }
 
 func (m *Persistence) GetByMesh(mesh string) (List, error) {
diff --git a/pkg/dns/vips_allocator.go b/pkg/dns/vips_allocator.go
index 79f311d2a..2870ccf28 100644
--- a/pkg/dns/vips_allocator.go
+++ b/pkg/dns/vips_allocator.go
@@ -23,10 +23,10 @@ var vipsAllocatorLog = core.Log.WithName("dns-vips-allocator")
 
 type VIPsAllocator struct {
 	rm          manager.ReadOnlyResourceManager
-	ipam        IPAM
 	persistence *vips.Persistence
 	resolver    resolver.DNSResolver
 	newTicker   func() *time.Ticker
+	cidr        string
 }
 
 // NewVIPsAllocator creates new object of VIPsAllocator. You can either
@@ -34,14 +34,10 @@ type VIPsAllocator struct {
 // In the latter scenario it will call CreateOrUpdateVIPConfig every 'tickInterval'
 // for all meshes in the store.
 func NewVIPsAllocator(rm manager.ReadOnlyResourceManager, configManager config_manager.ConfigManager, cidr string, resolver resolver.DNSResolver) (*VIPsAllocator, error) {
-	ipam, err := NewSimpleIPAM(cidr)
-	if err != nil {
-		return nil, err
-	}
 	return &VIPsAllocator{
 		rm:          rm,
 		persistence: vips.NewPersistence(rm, configManager),
-		ipam:        ipam,
+		cidr:        cidr,
 		resolver:    resolver,
 		newTicker: func() *time.Ticker {
 			return time.NewTicker(tickInterval)
@@ -61,8 +57,8 @@ func (d *VIPsAllocator) Start(stop <-chan struct{}) error {
 	for {
 		select {
 		case <-ticker.C:
-			if err := d.createOrUpdateVIPConfigs(); err != nil {
-				vipsAllocatorLog.Error(err, "unable to create or update VIP configs")
+			if err := d.CreateOrUpdateVIPConfigs(); err != nil {
+				vipsAllocatorLog.Error(err, "errors during updating VIP configs")
 			}
 		case <-stop:
 			vipsAllocatorLog.Info("stopping")
@@ -71,49 +67,80 @@ func (d *VIPsAllocator) Start(stop <-chan struct{}) error {
 	}
 }
 
-func (d *VIPsAllocator) createOrUpdateVIPConfigs() error {
-	meshes := core_mesh.MeshResourceList{}
-	err := d.rm.List(context.Background(), &meshes)
-	if err != nil {
+func (d *VIPsAllocator) CreateOrUpdateVIPConfigs() error {
+	meshRes := core_mesh.MeshResourceList{}
+	if err := d.rm.List(context.Background(), &meshRes); err != nil {
 		return err
 	}
 
-	for _, mesh := range meshes.Items {
-		if err := d.CreateOrUpdateVIPConfig(mesh.GetMeta().GetName()); err != nil {
-			vipsAllocatorLog.Error(err, "unable to create or update VIP config", "mesh", mesh.GetMeta().GetName())
-		}
+	meshes := []string{}
+	for _, mesh := range meshRes.Items {
+		meshes = append(meshes, mesh.GetMeta().GetName())
 	}
-	return nil
+
+	return d.createOrUpdateVIPConfigs(meshes...)
 }
 
 func (d *VIPsAllocator) CreateOrUpdateVIPConfig(mesh string) error {
-	serviceSet, err := BuildServiceSet(d.rm, mesh)
-	if err != nil {
-		return err
-	}
+	return d.createOrUpdateVIPConfigs(mesh)
+}
 
-	global, err := d.persistence.Get()
+func (d *VIPsAllocator) createOrUpdateVIPConfigs(meshes ...string) (errs error) {
+	global, byMesh, err := d.persistence.Get()
 	if err != nil {
 		return err
 	}
 
-	meshed, err := d.persistence.GetByMesh(mesh)
+	ipam, err := d.newIPAM(global)
 	if err != nil {
 		return err
 	}
 
-	updated, updError := UpdateMeshedVIPs(global, meshed, d.ipam, serviceSet)
-	if !updated {
+	forEachMesh := func(mesh string, meshed vips.List) error {
+		serviceSet, err := BuildServiceSet(d.rm, mesh)
+		if err != nil {
+			return err
+		}
+
+		updated, updError := UpdateMeshedVIPs(global, meshed, ipam, serviceSet)
+		if !updated {
+			return updError
+		}
+
+		if err := d.persistence.Set(mesh, meshed); err != nil {
+			return multierr.Append(updError, err)
+		}
+
+		d.resolver.SetVIPs(meshed)
 		return updError
 	}
 
-	if err := d.persistence.Set(mesh, meshed); err != nil {
-		return multierr.Append(updError, err)
+	for _, mesh := range meshes {
+		meshed, ok := byMesh[mesh]
+		if !ok {
+			meshed = vips.List{}
+		}
+		if err := forEachMesh(mesh, meshed); err != nil {
+			errs = multierr.Append(errs, errors.Wrapf(err, "errors during updating VIP config for mesh %s", mesh))
+		}
 	}
 
-	d.resolver.SetVIPs(meshed)
+	return errs
+}
+
+func (d *VIPsAllocator) newIPAM(initialVIPs vips.List) (IPAM, error) {
+	ipam, err := NewSimpleIPAM(d.cidr)
+	if err != nil {
+		return nil, err
+	}
+
+	for _, vip := range initialVIPs {
+		if err := ipam.ReserveIP(vip); err != nil && !IsAddressAlreadyAllocated(err) {
+			return nil, err
+		}
+	}
 
-	return updError
+	return ipam, nil
 }
 
 type ServiceSet map[string]bool
@@ -178,12 +205,14 @@ func UpdateMeshedVIPs(global, meshed vips.List, ipam IPAM, serviceSet ServiceSet
 		}
 		meshed[service] = ip
 		updated = true
+		vipsAllocatorLog.Info("adding", "service", service, "ip", ip)
 	}
 	for service, ip := range meshed {
 		if _, found := serviceSet[service]; !found {
 			updated = true
 			_ = ipam.FreeIP(ip)
 			delete(meshed, service)
+			vipsAllocatorLog.Info("deleting", "service", service, "ip", ip)
 		}
 	}
 	return
diff --git a/pkg/dns/vips_synchronizer.go b/pkg/dns/vips_synchronizer.go
index 10a30d976..8f9147cab 100644
--- a/pkg/dns/vips_synchronizer.go
+++ b/pkg/dns/vips_synchronizer.go
@@ -63,7 +63,7 @@ func (d *vipsSynchronizer) synchronize() error {
 	if d.leadInfo.IsLeader() {
 		return nil // when CP is leader we skip this because VIP allocator updates DNSResolver
 	}
-	vipList, err := d.persistence.Get()
+	vipList, _, err := d.persistence.Get()
 	if err != nil {
 		return err
 	}
diff --git a/pkg/plugins/runtime/k8s/plugin.go b/pkg/plugins/runtime/k8s/plugin.go
index 9c5f648c6..8c11d7526 100644
--- a/pkg/plugins/runtime/k8s/plugin.go
+++ b/pkg/plugins/runtime/k8s/plugin.go
@@ -153,7 +153,7 @@ func addDNS(mgr kube_ctrl.Manager, rt core_runtime.Runtime) error {
 		return nil
 	}
 	vipsAllocator, err := dns.NewVIPsAllocator(
-		rt.ReadOnlyResourceManager(),
+		rt.ResourceManager(),
 		rt.ConfigManager(),
 		rt.Config().DNSServer.CIDR,
 		rt.DNSResolver(),


Test patch:
diff --git a/pkg/dns/vips/persistence_test.go b/pkg/dns/vips/persistence_test.go
index 40d1413ff..1f7e7fb8f 100644
--- a/pkg/dns/vips/persistence_test.go
+++ b/pkg/dns/vips/persistence_test.go
@@ -109,7 +109,7 @@ var _ = Describe("Meshed Persistence", func() {
 		})
 
 		It("should merge vips from several configs", func() {
-			actual, err := meshedPersistence.Get()
+			actual, _, err := meshedPersistence.Get()
 			Expect(err).ToNot(HaveOccurred())
 
 			expected := vips.List{
diff --git a/pkg/dns/vips_allocator_test.go b/pkg/dns/vips_allocator_test.go
index 55baab9f3..7671b8396 100644
--- a/pkg/dns/vips_allocator_test.go
+++ b/pkg/dns/vips_allocator_test.go
@@ -6,6 +6,9 @@ import (
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 
+	config_manager "github.com/kumahq/kuma/pkg/core/config/manager"
+	"github.com/kumahq/kuma/pkg/dns/resolver"
+
 	"github.com/kumahq/kuma/pkg/dns"
 	"github.com/kumahq/kuma/pkg/dns/vips"
 
@@ -17,214 +20,289 @@ import (
 	"github.com/kumahq/kuma/pkg/plugins/resources/memory"
 )
 
+func dp(services ...string) *mesh_proto.Dataplane {
+	inbound := []*mesh_proto.Dataplane_Networking_Inbound{}
+	for _, service := range services {
+		inbound = append(inbound, &mesh_proto.Dataplane_Networking_Inbound{
+			Port: 8080,
+			Tags: map[string]string{
+				mesh_proto.ServiceTag: service,
+			},
+		})
+	}
+	return &mesh_proto.Dataplane{
+		Networking: &mesh_proto.Dataplane_Networking{
+			Address: "127.0.0.1",
+			Inbound: inbound,
+		},
+	}
+}
+
 var _ = Describe("VIP Allocator", func() {
-	Context("BuildServiceSet", func() {
-		var rm manager.ResourceManager
+	var rm manager.ResourceManager
+	var cm config_manager.ConfigManager
+	var allocator *dns.VIPsAllocator
+
+	BeforeEach(func() {
+		s := memory.NewStore()
+		rm = manager.NewResourceManager(s)
+		cm = config_manager.NewConfigManager(s)
+
+		err := rm.Create(context.Background(), &mesh.MeshResource{}, store.CreateByKey("mesh-1", model.NoMesh))
+		Expect(err).ToNot(HaveOccurred())
+
+		err = rm.Create(context.Background(), &mesh.MeshResource{}, store.CreateByKey("mesh-2", model.NoMesh))
+		Expect(err).ToNot(HaveOccurred())
+
+		err = rm.Create(context.Background(), &mesh.DataplaneResource{Spec: *dp("backend")}, store.CreateByKey("dp-1", "mesh-1"))
+		Expect(err).ToNot(HaveOccurred())
+
+		err = rm.Create(context.Background(), &mesh.DataplaneResource{Spec: *dp("frontend")}, store.CreateByKey("dp-2", "mesh-1"))
+		Expect(err).ToNot(HaveOccurred())
 
-		BeforeEach(func() {
-			rm = manager.NewResourceManager(memory.NewStore())
+		err = rm.Create(context.Background(), &mesh.DataplaneResource{Spec: *dp("web")}, store.CreateByKey("dp-3", "mesh-2"))
+		Expect(err).ToNot(HaveOccurred())
+
+		allocator, err = dns.NewVIPsAllocator(rm, cm, "240.0.0.0/24", resolver.NewDNSResolver("mesh"))
+		Expect(err).ToNot(HaveOccurred())
+	})
+
+	It("should create VIP config for each mesh", func() {
+		// when
+		err := allocator.CreateOrUpdateVIPConfigs()
+		Expect(err).ToNot(HaveOccurred())
+
+		persistence := vips.NewPersistence(rm, cm)
+
+		// then
+		vipList, err := persistence.GetByMesh("mesh-1")
+		Expect(err).ToNot(HaveOccurred())
+		Expect(vipList).To(HaveLen(2))
+
+		vipList, err = persistence.GetByMesh("mesh-2")
+		Expect(err).ToNot(HaveOccurred())
+
+		Expect(vipList).To(HaveLen(1))
+	})
+
+	It("should respect already allocated VIPs in case of IPAM restarts", func() {
+		// setup
+		persistence := vips.NewPersistence(rm, cm)
+		// we add VIPs directly to the 'persistence' object
+		// that emulates situation when IPAM is fresh and doesn't aware of allocated VIPs
+		err := persistence.Set("mesh-1", vips.List{
+			"frontend": "240.0.0.0",
+			"backend":  "240.0.0.1",
 		})
+		Expect(err).ToNot(HaveOccurred())
 
-		It("should build service set for mesh", func() {
-			// setup meshes
-			err := rm.Create(context.Background(), &mesh.MeshResource{}, store.CreateByKey("mesh-1", model.NoMesh))
-			Expect(err).ToNot(HaveOccurred())
-
-			err = rm.Create(context.Background(), &mesh.MeshResource{}, store.CreateByKey("mesh-2", model.NoMesh))
-			Expect(err).ToNot(HaveOccurred())
-
-			dp := func(services ...string) *mesh_proto.Dataplane {
-				inbound := []*mesh_proto.Dataplane_Networking_Inbound{}
-				for _, service := range services {
-					inbound = append(inbound, &mesh_proto.Dataplane_Networking_Inbound{
-						Port: 8080,
-						Tags: map[string]string{
-							mesh_proto.ServiceTag: service,
-						},
-					})
-				}
-				return &mesh_proto.Dataplane{
-					Networking: &mesh_proto.Dataplane_Networking{
-						Address: "127.0.0.1",
-						Inbound: inbound,
-					},
-				}
-			}
+		err = rm.Create(context.Background(), &mesh.DataplaneResource{Spec: *dp("database")}, store.CreateByKey("dp-3", "mesh-1"))
+		Expect(err).ToNot(HaveOccurred())
 
-			// setup dataplanes
-			err = rm.Create(context.Background(), &mesh.DataplaneResource{Spec: *dp("backend")}, store.CreateByKey("backend-1", "mesh-1"))
-			Expect(err).ToNot(HaveOccurred())
+		// when
+		err = allocator.CreateOrUpdateVIPConfig("mesh-1")
+		Expect(err).ToNot(HaveOccurred())
 
-			err = rm.Create(context.Background(), &mesh.DataplaneResource{Spec: *dp("frontend")}, store.CreateByKey("frontend-1", "mesh-1"))
-			Expect(err).ToNot(HaveOccurred())
+		vipList, err := persistence.GetByMesh("mesh-1")
+		Expect(err).ToNot(HaveOccurred())
+		// then
+		Expect(vipList).To(Equal(vips.List{
+			"frontend": "240.0.0.0",
+			"backend":  "240.0.0.1",
+			"database": "240.0.0.2",
+		}))
+	})
 
-			err = rm.Create(context.Background(), &mesh.DataplaneResource{Spec: *dp("frontend")}, store.CreateByKey("frontend-2", "mesh-1"))
-			Expect(err).ToNot(HaveOccurred())
+})
 
-			err = rm.Create(context.Background(), &mesh.DataplaneResource{Spec: *dp("database", "metrics")}, store.CreateByKey("db-m-1", "mesh-1"))
-			Expect(err).ToNot(HaveOccurred())
+var _ = Describe("BuildServiceSet", func() {
+	var rm manager.ResourceManager
 
-			err = rm.Create(context.Background(), &mesh.DataplaneResource{Spec: *dp("another-mesh-svc")}, store.CreateByKey("another-mesh-dp-1", "mesh-2"))
-			Expect(err).ToNot(HaveOccurred())
+	BeforeEach(func() {
+		rm = manager.NewResourceManager(memory.NewStore())
+	})
 
-			// setup ingress
-			err = rm.Create(context.Background(), &mesh.DataplaneResource{Spec: mesh_proto.Dataplane{
-				Networking: &mesh_proto.Dataplane_Networking{
-					Inbound: []*mesh_proto.Dataplane_Networking_Inbound{
-						{
-							Port: 10001,
-						},
+	It("should build service set for mesh", func() {
+		// setup meshes
+		err := rm.Create(context.Background(), &mesh.MeshResource{}, store.CreateByKey("mesh-1", model.NoMesh))
+		Expect(err).ToNot(HaveOccurred())
+
+		err = rm.Create(context.Background(), &mesh.MeshResource{}, store.CreateByKey("mesh-2", model.NoMesh))
+		Expect(err).ToNot(HaveOccurred())
+
+		// setup dataplanes
+		err = rm.Create(context.Background(), &mesh.DataplaneResource{Spec: *dp("backend")}, store.CreateByKey("backend-1", "mesh-1"))
+		Expect(err).ToNot(HaveOccurred())
+
+		err = rm.Create(context.Background(), &mesh.DataplaneResource{Spec: *dp("frontend")}, store.CreateByKey("frontend-1", "mesh-1"))
+		Expect(err).ToNot(HaveOccurred())
+
+		err = rm.Create(context.Background(), &mesh.DataplaneResource{Spec: *dp("frontend")}, store.CreateByKey("frontend-2", "mesh-1"))
+		Expect(err).ToNot(HaveOccurred())
+
+		err = rm.Create(context.Background(), &mesh.DataplaneResource{Spec: *dp("database", "metrics")}, store.CreateByKey("db-m-1", "mesh-1"))
+		Expect(err).ToNot(HaveOccurred())
+
+		err = rm.Create(context.Background(), &mesh.DataplaneResource{Spec: *dp("another-mesh-svc")}, store.CreateByKey("another-mesh-dp-1", "mesh-2"))
+		Expect(err).ToNot(HaveOccurred())
+
+		// setup ingress
+		err = rm.Create(context.Background(), &mesh.DataplaneResource{Spec: mesh_proto.Dataplane{
+			Networking: &mesh_proto.Dataplane_Networking{
+				Inbound: []*mesh_proto.Dataplane_Networking_Inbound{
+					{
+						Port: 10001,
 					},
-					Ingress: &mesh_proto.Dataplane_Networking_Ingress{
-						AvailableServices: []*mesh_proto.Dataplane_Networking_Ingress_AvailableService{
-							{
-								Mesh:      "mesh-1",
-								Instances: 2,
-								Tags: map[string]string{
-									mesh_proto.ServiceTag: "ingress-svc",
-								},
+				},
+				Ingress: &mesh_proto.Dataplane_Networking_Ingress{
+					AvailableServices: []*mesh_proto.Dataplane_Networking_Ingress_AvailableService{
+						{
+							Mesh:      "mesh-1",
+							Instances: 2,
+							Tags: map[string]string{
+								mesh_proto.ServiceTag: "ingress-svc",
 							},
-							{
-								Mesh:      "mesh-2",
-								Instances: 3,
-								Tags: map[string]string{
-									mesh_proto.ServiceTag: "another-mesh-ingress-svc",
-								},
+						},
+						{
+							Mesh:      "mesh-2",
+							Instances: 3,
+							Tags: map[string]string{
+								mesh_proto.ServiceTag: "another-mesh-ingress-svc",
 							},
 						},
 					},
 				},
-			}}, store.CreateByKey("ingress-1", "mesh-1"))
-			Expect(err).ToNot(HaveOccurred())
-
-			// setup external services
-			es := func(service string) *mesh_proto.ExternalService {
-				return &mesh_proto.ExternalService{
-					Networking: &mesh_proto.ExternalService_Networking{
-						Address: "external.service.com:8080",
-					},
-					Tags: map[string]string{
-						mesh_proto.ServiceTag: service,
-					},
-				}
+			},
+		}}, store.CreateByKey("ingress-1", "mesh-1"))
+		Expect(err).ToNot(HaveOccurred())
+
+		// setup external services
+		es := func(service string) *mesh_proto.ExternalService {
+			return &mesh_proto.ExternalService{
+				Networking: &mesh_proto.ExternalService_Networking{
+					Address: "external.service.com:8080",
+				},
+				Tags: map[string]string{
+					mesh_proto.ServiceTag: service,
+				},
 			}
+		}
 
-			err = rm.Create(context.Background(), &mesh.ExternalServiceResource{Spec: *es("es-backend")}, store.CreateByKey("es-backend-1", "mesh-1"))
-			Expect(err).ToNot(HaveOccurred())
-
-			err = rm.Create(context.Background(), &mesh.ExternalServiceResource{Spec: *es("another-mesh-es")}, store.CreateByKey("es-backend-1", "mesh-2"))
-			Expect(err).ToNot(HaveOccurred())
-
-			// when
-			serviceSet, err := dns.BuildServiceSet(rm, "mesh-1")
-			Expect(err).ToNot(HaveOccurred())
-
-			// then
-			Expect(serviceSet).To(Equal(dns.ServiceSet{
-				"backend":     true,
-				"frontend":    true,
-				"database":    true,
-				"metrics":     true,
-				"ingress-svc": true,
-				"es-backend":  true,
-			}))
-		})
+		err = rm.Create(context.Background(), &mesh.ExternalServiceResource{Spec: *es("es-backend")}, store.CreateByKey("es-backend-1", "mesh-1"))
+		Expect(err).ToNot(HaveOccurred())
+
+		err = rm.Create(context.Background(), &mesh.ExternalServiceResource{Spec: *es("another-mesh-es")}, store.CreateByKey("es-backend-1", "mesh-2"))
+		Expect(err).ToNot(HaveOccurred())
+
+		// when
+		serviceSet, err := dns.BuildServiceSet(rm, "mesh-1")
+		Expect(err).ToNot(HaveOccurred())
+
+		// then
+		Expect(serviceSet).To(Equal(dns.ServiceSet{
+			"backend":     true,
+			"frontend":    true,
+			"database":    true,
+			"metrics":     true,
+			"ingress-svc": true,
+			"es-backend":  true,
+		}))
 	})
+})
 
-	Context("UpdateMeshedVIPs", func() {
-		It("should allocate new VIPs", func() {
-			// setup
-			vipsList := vips.List{}
-			ipam, err := dns.NewSimpleIPAM("240.0.0.0/4")
-			Expect(err).ToNot(HaveOccurred())
-			serviceSet := dns.ServiceSet{
-				"backend":  true,
-				"frontend": true,
-			}
-			// when
-			updated, err := dns.UpdateMeshedVIPs(vipsList, vipsList, ipam, serviceSet)
-			Expect(err).ToNot(HaveOccurred())
-			// then
-			Expect(err).ToNot(HaveOccurred())
-			Expect(updated).To(BeTrue())
-			Expect(vipsList).To(Equal(vips.List{
-				"backend":  "240.0.0.0",
-				"frontend": "240.0.0.1",
-			}))
-		})
+var _ = Describe("UpdateMeshedVIPs", func() {
+	It("should allocate new VIPs", func() {
+		// setup
+		vipsList := vips.List{}
+		ipam, err := dns.NewSimpleIPAM("240.0.0.0/4")
+		Expect(err).ToNot(HaveOccurred())
+		serviceSet := dns.ServiceSet{
+			"backend":  true,
+			"frontend": true,
+		}
+		// when
+		updated, err := dns.UpdateMeshedVIPs(vipsList, vipsList, ipam, serviceSet)
+		Expect(err).ToNot(HaveOccurred())
+		// then
+		Expect(err).ToNot(HaveOccurred())
+		Expect(updated).To(BeTrue())
+		Expect(vipsList).To(Equal(vips.List{
+			"backend":  "240.0.0.0",
+			"frontend": "240.0.0.1",
+		}))
+	})
 
-		It("should free IP for deleted service", func() {
-			// setup
-			vipsList := vips.List{
-				"backend":  "240.0.0.0",
-				"frontend": "240.0.0.1",
-			}
-			ipam, err := dns.NewSimpleIPAM("240.0.0.0/4")
-			Expect(err).ToNot(HaveOccurred())
-			serviceSet := dns.ServiceSet{
-				"backend": true,
-			}
-			// when
-			updated, err := dns.UpdateMeshedVIPs(vipsList, vipsList, ipam, serviceSet)
-			Expect(err).ToNot(HaveOccurred())
-			// then
-			Expect(updated).To(BeTrue())
-			Expect(vipsList).To(Equal(vips.List{
-				"backend": "240.0.0.0",
-			}))
-		})
+	It("should free IP for deleted service", func() {
+		// setup
+		vipsList := vips.List{
+			"backend":  "240.0.0.0",
+			"frontend": "240.0.0.1",
+		}
+		ipam, err := dns.NewSimpleIPAM("240.0.0.0/4")
+		Expect(err).ToNot(HaveOccurred())
+		serviceSet := dns.ServiceSet{
+			"backend": true,
+		}
+		// when
+		updated, err := dns.UpdateMeshedVIPs(vipsList, vipsList, ipam, serviceSet)
+		Expect(err).ToNot(HaveOccurred())
+		// then
+		Expect(updated).To(BeTrue())
+		Expect(vipsList).To(Equal(vips.List{
+			"backend": "240.0.0.0",
+		}))
+	})
 
-		It("should return updated=false if nothing changed", func() {
-			// setup
-			vipsList := vips.List{
-				"backend":  "240.0.0.0",
-				"frontend": "240.0.0.1",
-			}
-			ipam, err := dns.NewSimpleIPAM("240.0.0.0/4")
-			Expect(err).ToNot(HaveOccurred())
-			serviceSet := dns.ServiceSet{
-				"backend":  true,
-				"frontend": true,
-			}
-			// when
-			updated, err := dns.UpdateMeshedVIPs(vipsList, vipsList, ipam, serviceSet)
-			Expect(err).ToNot(HaveOccurred())
-			// then
-			Expect(updated).To(BeFalse())
-			Expect(vipsList).To(Equal(vips.List{
-				"backend":  "240.0.0.0",
-				"frontend": "240.0.0.1",
-			}))
-		})
+	It("should return updated=false if nothing changed", func() {
+		// setup
+		vipsList := vips.List{
+			"backend":  "240.0.0.0",
+			"frontend": "240.0.0.1",
+		}
+		ipam, err := dns.NewSimpleIPAM("240.0.0.0/4")
+		Expect(err).ToNot(HaveOccurred())
+		serviceSet := dns.ServiceSet{
+			"backend":  true,
+			"frontend": true,
+		}
+		// when
+		updated, err := dns.UpdateMeshedVIPs(vipsList, vipsList, ipam, serviceSet)
+		Expect(err).ToNot(HaveOccurred())
+		// then
+		Expect(updated).To(BeFalse())
+		Expect(vipsList).To(Equal(vips.List{
+			"backend":  "240.0.0.0",
+			"frontend": "240.0.0.1",
+		}))
+	})
 
-		It("should generate the same VIP for services across meshes", func() {
-			// setup
-			global := vips.List{
-				"backend":  "240.0.0.0",
-				"frontend": "240.0.0.1",
-				"database": "240.0.0.10",
-			}
-			meshed := vips.List{
-				"backend":  "240.0.0.0",
-				"frontend": "240.0.0.1",
-			}
-			ipam, err := dns.NewSimpleIPAM("240.0.0.0/4")
-			Expect(err).ToNot(HaveOccurred())
-			serviceSet := dns.ServiceSet{
-				"backend":  true,
-				"frontend": true,
-				"database": true,
-			}
-			// when
-			updated, err := dns.UpdateMeshedVIPs(global, meshed, ipam, serviceSet)
-			Expect(err).ToNot(HaveOccurred())
-			// then
-			Expect(updated).To(BeTrue())
-			Expect(meshed).To(Equal(vips.List{
-				"backend":  "240.0.0.0",
-				"frontend": "240.0.0.1",
-				"database": "240.0.0.10",
-			}))
-		})
+	It("should generate the same VIP for services across meshes", func() {
+		// setup
+		global := vips.List{
+			"backend":  "240.0.0.0",
+			"frontend": "240.0.0.1",
+			"database": "240.0.0.10",
+		}
+		meshed := vips.List{
+			"backend":  "240.0.0.0",
+			"frontend": "240.0.0.1",
+		}
+		ipam, err := dns.NewSimpleIPAM("240.0.0.0/4")
+		Expect(err).ToNot(HaveOccurred())
+		serviceSet := dns.ServiceSet{
+			"backend":  true,
+			"frontend": true,
+			"database": true,
+		}
+		// when
+		updated, err := dns.UpdateMeshedVIPs(global, meshed, ipam, serviceSet)
+		Expect(err).ToNot(HaveOccurred())
+		// then
+		Expect(updated).To(BeTrue())
+		Expect(meshed).To(Equal(vips.List{
+			"backend":  "240.0.0.0",
+			"frontend": "240.0.0.1",
+			"database": "240.0.0.10",
+		}))
 	})
 })