Merge pull request #279 from rramkumar1/use-ig-links

Ensure Load Balancer using IG links instead of IG compute object

pkg/backends/backends.go

@@ -297,12 +297,11 @@ func (b *Backends) create(namedPort *compute.NamedPort, hcLink string, sp utils.
 }
 
 // Ensure will update or create Backends for the given ports.
-// Uses the given instance groups if non-nil, else creates instance groups.
-func (b *Backends) Ensure(svcPorts []utils.ServicePort, igs []*compute.InstanceGroup) error {
+func (b *Backends) Ensure(svcPorts []utils.ServicePort, igLinks []string) error {
 	glog.V(3).Infof("Sync: backends %v", svcPorts)
 	// create backends for new ports, perform an edge hop for existing ports
 	for _, port := range svcPorts {
-		if err := b.ensureBackendService(port, igs); err != nil {
+		if err := b.ensureBackendService(port, igLinks); err != nil {
 			return err
 		}
 	}
@@ -312,7 +311,7 @@ func (b *Backends) Ensure(svcPorts []utils.ServicePort, igs []*compute.InstanceG
 // ensureBackendService will update or create a Backend for the given port.
 // It assumes that the instance groups have been created and required named port has been added.
 // If not, then Ensure should be called instead.
-func (b *Backends) ensureBackendService(sp utils.ServicePort, igs []*compute.InstanceGroup) error {
+func (b *Backends) ensureBackendService(sp utils.ServicePort, igLinks []string) error {
 	// We must track the ports even if creating the backends failed, because
 	// we might've created health-check for them.
 	be := &BackendService{}
@@ -365,17 +364,17 @@ func (b *Backends) ensureBackendService(sp utils.ServicePort, igs []*compute.Ins
 	// If there are instance pools(node pool is synced) and NEG is not enabled,
 	// perform edgeHop to verify that BackendServices contains links to all
 	// backends/instancegroups
-	if len(igs) > 0 && !sp.NEGEnabled {
-		return b.edgeHop(be, igs)
+	if len(igLinks) > 0 && !sp.NEGEnabled {
+		return b.edgeHop(be, igLinks)
 	}
 
 	return nil
 }
 
 // edgeHop checks the links of the given backend by executing an edge hop.
 // It fixes broken links and updates the Backend accordingly.
-func (b *Backends) edgeHop(be *BackendService, igs []*compute.InstanceGroup) error {
-	addIGs := getInstanceGroupsToAdd(be, igs)
+func (b *Backends) edgeHop(be *BackendService, igLinks []string) error {
+	addIGs := getInstanceGroupsToAdd(be, igLinks)
 	if len(addIGs) == 0 {
 		return nil
 	}
@@ -482,11 +481,11 @@ func (b *Backends) List() ([]interface{}, error) {
 	return ret, nil
 }
 
-func getBackendsForIGs(igs []*compute.InstanceGroup, bm BalancingMode) []*compute.Backend {
+func getBackendsForIGs(igLinks []string, bm BalancingMode) []*compute.Backend {
 	var backends []*compute.Backend
-	for _, ig := range igs {
+	for _, igLink := range igLinks {
 		b := &compute.Backend{
-			Group:         ig.SelfLink,
+			Group:         igLink,
 			BalancingMode: string(bm),
 		}
 		switch bm {
@@ -502,11 +501,11 @@ func getBackendsForIGs(igs []*compute.InstanceGroup, bm BalancingMode) []*comput
 	return backends
 }
 
-func getAlphaBackendsForIGs(igs []*compute.InstanceGroup, bm BalancingMode) []*computealpha.Backend {
+func getAlphaBackendsForIGs(igLinks []string, bm BalancingMode) []*computealpha.Backend {
 	var backends []*computealpha.Backend
-	for _, ig := range igs {
+	for _, igLink := range igLinks {
 		b := &computealpha.Backend{
-			Group:         ig.SelfLink,
+			Group:         igLink,
 			BalancingMode: string(bm),
 		}
 		switch bm {
@@ -535,7 +534,7 @@ func getBackendsForNEGs(negs []*computealpha.NetworkEndpointGroup) []*computealp
 	return backends
 }
 
-func getInstanceGroupsToAdd(be *BackendService, igs []*compute.InstanceGroup) []*compute.InstanceGroup {
+func getInstanceGroupsToAdd(be *BackendService, igLinks []string) []string {
 	// A GA link can be used to reference an alpha object - so we only need to
 	// check the GA InstanceGroups.
 	beName := be.Ga.Name
@@ -545,8 +544,8 @@ func getInstanceGroupsToAdd(be *BackendService, igs []*compute.InstanceGroup) []
 	}
 
 	expectedIGs := sets.String{}
-	for _, ig := range igs {
-		expectedIGs.Insert(comparableGroupPath(ig.SelfLink))
+	for _, igLink := range igLinks {
+		expectedIGs.Insert(comparableGroupPath(igLink))
 	}
 
 	if beIGs.IsSuperset(expectedIGs) {
@@ -555,10 +554,10 @@ func getInstanceGroupsToAdd(be *BackendService, igs []*compute.InstanceGroup) []
 	glog.V(2).Infof("Expected igs for backend service %v: %+v, current igs %+v",
 		beName, expectedIGs.List(), beIGs.List())
 
-	var addIGs []*compute.InstanceGroup
-	for _, ig := range igs {
-		if !beIGs.Has(comparableGroupPath(ig.SelfLink)) {
-			addIGs = append(addIGs, ig)
+	var addIGs []string
+	for _, igLink := range igLinks {
+		if !beIGs.Has(comparableGroupPath(igLink)) {
+			addIGs = append(addIGs, igLink)
 		}
 	}
 

pkg/backends/backends_test.go

@@ -93,7 +93,7 @@ func TestBackendPoolAdd(t *testing.T) {
 			// Add a backend for a port, then re-add the same port and
 			// make sure it corrects a broken link from the backend to
 			// the instance group.
-			err = pool.Ensure([]utils.ServicePort{sp}, igs)
+			err = pool.Ensure([]utils.ServicePort{sp}, utils.IGLinks(igs))
 			if err != nil {
 				t.Fatalf("Did not expect error when ensuring a ServicePort %+v: %v", sp, err)
 			}
@@ -314,8 +314,11 @@ func TestBackendPoolChaosMonkey(t *testing.T) {
 	pool, _ := newTestJig(f, fakeIGs, false)
 
 	sp := utils.ServicePort{NodePort: 8080, Protocol: annotations.ProtocolHTTP}
-	igs, _ := pool.nodePool.EnsureInstanceGroupsAndPorts(defaultNamer.InstanceGroup(), []int64{sp.NodePort})
-	pool.Ensure([]utils.ServicePort{sp}, igs)
+	igs, err := pool.nodePool.EnsureInstanceGroupsAndPorts(defaultNamer.InstanceGroup(), []int64{sp.NodePort})
+	if err != nil {
+		t.Fatalf("Did not expect error when ensuring IG for ServicePort %+v: %v", sp, err)
+	}
+	pool.Ensure([]utils.ServicePort{sp}, utils.IGLinks(igs))
 	beName := sp.BackendName(defaultNamer)
 
 	be, _ := f.GetGlobalBackendService(beName)
@@ -328,8 +331,11 @@ func TestBackendPoolChaosMonkey(t *testing.T) {
 	f.calls = []int{}
 	f.UpdateGlobalBackendService(be)
 
-	igs, _ = pool.nodePool.EnsureInstanceGroupsAndPorts(defaultNamer.InstanceGroup(), []int64{sp.NodePort})
-	pool.Ensure([]utils.ServicePort{sp}, igs)
+	igs, err = pool.nodePool.EnsureInstanceGroupsAndPorts(defaultNamer.InstanceGroup(), []int64{sp.NodePort})
+	if err != nil {
+		t.Fatalf("Did not expect error when ensuring IG for ServicePort %+v: %v", sp, err)
+	}
+	pool.Ensure([]utils.ServicePort{sp}, utils.IGLinks(igs))
 	for _, call := range f.calls {
 		if call == utils.Create {
 			t.Fatalf("Unexpected create for existing backend service")
@@ -669,8 +675,11 @@ func TestBackendInstanceGroupClobbering(t *testing.T) {
 	pool, _ := newTestJig(f, fakeIGs, false)
 
 	sp := utils.ServicePort{NodePort: 80}
-	igs, _ := pool.nodePool.EnsureInstanceGroupsAndPorts(defaultNamer.InstanceGroup(), []int64{sp.NodePort})
-	pool.Ensure([]utils.ServicePort{sp}, igs)
+	igs, err := pool.nodePool.EnsureInstanceGroupsAndPorts(defaultNamer.InstanceGroup(), []int64{sp.NodePort})
+	if err != nil {
+		t.Fatalf("Did not expect error when ensuring IG for ServicePort %+v: %v", sp, err)
+	}
+	pool.Ensure([]utils.ServicePort{sp}, utils.IGLinks(igs))
 
 	be, err := f.GetGlobalBackendService(defaultNamer.IGBackend(80))
 	if err != nil {
@@ -688,8 +697,11 @@ func TestBackendInstanceGroupClobbering(t *testing.T) {
 	}
 
 	// Make sure repeated adds don't clobber the inserted instance group
-	igs, _ = pool.nodePool.EnsureInstanceGroupsAndPorts(defaultNamer.InstanceGroup(), []int64{sp.NodePort})
-	pool.Ensure([]utils.ServicePort{sp}, igs)
+	igs, err = pool.nodePool.EnsureInstanceGroupsAndPorts(defaultNamer.InstanceGroup(), []int64{sp.NodePort})
+	if err != nil {
+		t.Fatalf("Did not expect error when ensuring IG for ServicePort %+v: %v", sp, err)
+	}
+	pool.Ensure([]utils.ServicePort{sp}, utils.IGLinks(igs))
 	be, err = f.GetGlobalBackendService(defaultNamer.IGBackend(80))
 	if err != nil {
 		t.Fatalf("%v", err)
@@ -729,8 +741,11 @@ func TestBackendCreateBalancingMode(t *testing.T) {
 			return nil
 		}
 
-		igs, _ := pool.nodePool.EnsureInstanceGroupsAndPorts(defaultNamer.InstanceGroup(), []int64{sp.NodePort})
-		pool.Ensure([]utils.ServicePort{sp}, igs)
+		igs, err := pool.nodePool.EnsureInstanceGroupsAndPorts(defaultNamer.InstanceGroup(), []int64{sp.NodePort})
+		if err != nil {
+			t.Fatalf("Did not expect error when ensuring IG for ServicePort %+v: %v", sp, err)
+		}
+		pool.Ensure([]utils.ServicePort{sp}, utils.IGLinks(igs))
 		be, err := f.GetGlobalBackendService(sp.BackendName(defaultNamer))
 		if err != nil {
 			t.Fatalf("%v", err)

pkg/backends/interfaces.go

@@ -32,7 +32,7 @@ type ProbeProvider interface {
 // as gce backendServices, and sync them through the BackendServices interface.
 type BackendPool interface {
 	Init(p ProbeProvider)
-	Ensure(ports []utils.ServicePort, igs []*compute.InstanceGroup) error
+	Ensure(ports []utils.ServicePort, igLinks []string) error
 	Get(name string, isAlpha bool) (*BackendService, error)
 	Delete(name string) error
 	GC(ports []utils.ServicePort) error

pkg/controller/cluster_manager.go

@@ -92,9 +92,9 @@ func (c *ClusterManager) shutdown() error {
 // - lbServicePorts are the ports for which we require Backend Services.
 // - instanceGroups are the groups to be referenced by the Backend Services..
 // If GCE runs out of quota, a googleapi 403 is returned.
-func (c *ClusterManager) EnsureLoadBalancer(lb *loadbalancers.L7RuntimeInfo, lbServicePorts []utils.ServicePort, instanceGroups []*compute.InstanceGroup) error {
-	glog.V(4).Infof("EnsureLoadBalancer(%q lb, %v lbServicePorts, %v instanceGroups)", lb.String(), len(lbServicePorts), len(instanceGroups))
-	if err := c.backendPool.Ensure(uniq(lbServicePorts), instanceGroups); err != nil {
+func (c *ClusterManager) EnsureLoadBalancer(lb *loadbalancers.L7RuntimeInfo, lbServicePorts []utils.ServicePort, igLinks []string) error {
+	glog.V(4).Infof("EnsureLoadBalancer(%q lb, %v lbServicePorts, %v instanceGroups)", lb.String(), len(lbServicePorts), len(igLinks))
+	if err := c.backendPool.Ensure(uniq(lbServicePorts), igLinks); err != nil {
 		return err
 	}
 

pkg/controller/controller.go

@@ -364,7 +364,8 @@ func (lbc *LoadBalancerController) ensureIngress(ing *extensions.Ingress, nodeNa
 	lb.UrlMap = urlMap
 
 	// Create the backend services and higher-level LB resources.
-	if err = lbc.CloudClusterManager.EnsureLoadBalancer(lb, ingSvcPorts, igs); err != nil {
+	// Note: To ensure the load balancer, we only need the IG links.
+	if err = lbc.CloudClusterManager.EnsureLoadBalancer(lb, ingSvcPorts, utils.IGLinks(igs)); err != nil {
 		return err
 	}
 

pkg/utils/utils.go

@@ -23,6 +23,7 @@ import (
 	"net/http"
 	"strings"
 
+	compute "google.golang.org/api/compute/v1"
 	"google.golang.org/api/googleapi"
 	"k8s.io/apimachinery/pkg/types"
 )
@@ -181,3 +182,12 @@ func BackendServiceComparablePath(url string) string {
 	}
 	return fmt.Sprintf("global/%s", path_parts[1])
 }
+
+// IGLinks returns a list of links extracted from the passed in list of
+// compute.InstanceGroup's.
+func IGLinks(igs []*compute.InstanceGroup) (igLinks []string) {
+	for _, ig := range igs {
+		igLinks = append(igLinks, ig.SelfLink)
+	}
+	return
+}