Merge pull request #861 from skmatti/benamer-cleanup

Cleanup backend namer workflow

pkg/backends/backends.go

@@ -30,7 +30,7 @@ import (
 // Backends handles CRUD operations for backends.
 type Backends struct {
 	cloud *gce.Cloud
-	namer *namer.Namer
+	namer namer.BackendNamer
 }
 
 // Backends is a Pool.
@@ -39,7 +39,7 @@ var _ Pool = (*Backends)(nil)
 // NewPool returns a new backend pool.
 // - cloud: implements BackendServices
 // - namer: produces names for backends.
-func NewPool(cloud *gce.Cloud, namer *namer.Namer) *Backends {
+func NewPool(cloud *gce.Cloud, namer namer.BackendNamer) *Backends {
 	return &Backends{
 		cloud: cloud,
 		namer: namer,
@@ -60,7 +60,7 @@ func ensureDescription(be *composite.BackendService, sp *utils.ServicePort) (nee
 
 // Create implements Pool.
 func (b *Backends) Create(sp utils.ServicePort, hcLink string) (*composite.BackendService, error) {
-	name := sp.BackendName(b.namer)
+	name := sp.BackendName()
 	namedPort := &compute.NamedPort{
 		Name: b.namer.NamedPort(sp.NodePort),
 		Port: sp.NodePort,

pkg/backends/ig_linker.go

@@ -23,7 +23,6 @@ import (
 	"k8s.io/ingress-gce/pkg/composite"
 	"k8s.io/ingress-gce/pkg/instances"
 	"k8s.io/ingress-gce/pkg/utils"
-	"k8s.io/ingress-gce/pkg/utils/namer"
 	"k8s.io/klog"
 )
 
@@ -66,28 +65,25 @@ const maxRPS = 1
 type instanceGroupLinker struct {
 	instancePool instances.NodePool
 	backendPool  Pool
-	namer        *namer.Namer
 }
 
 // instanceGroupLinker is a Linker
 var _ Linker = (*instanceGroupLinker)(nil)
 
 func NewInstanceGroupLinker(
 	instancePool instances.NodePool,
-	backendPool Pool,
-	namer *namer.Namer) Linker {
+	backendPool Pool) Linker {
 	return &instanceGroupLinker{
 		instancePool: instancePool,
 		backendPool:  backendPool,
-		namer:        namer,
 	}
 }
 
 // Link implements Link.
 func (l *instanceGroupLinker) Link(sp utils.ServicePort, groups []GroupKey) error {
 	var igLinks []string
 	for _, group := range groups {
-		ig, err := l.instancePool.Get(l.namer.InstanceGroup(), group.Zone)
+		ig, err := l.instancePool.Get(sp.IGName(), group.Zone)
 		if err != nil {
 			return fmt.Errorf("error retrieving IG for linking with backend %+v: %v", sp, err)
 		}
@@ -97,7 +93,7 @@ func (l *instanceGroupLinker) Link(sp utils.ServicePort, groups []GroupKey) erro
 	// ig_linker only supports L7 HTTP(s) External Load Balancer
 	// Hardcoded here since IGs are not supported for non GA-Global right now
 	// TODO(shance): find a way to remove hardcoded values
-	be, err := l.backendPool.Get(sp.BackendName(l.namer), meta.VersionGA, meta.Global)
+	be, err := l.backendPool.Get(sp.BackendName(), meta.VersionGA, meta.Global)
 	if err != nil {
 		return err
 	}

pkg/backends/ig_linker_test.go

@@ -15,17 +15,17 @@ package backends
 
 import (
 	"context"
-	"k8s.io/ingress-gce/pkg/backends/features"
 	"net/http"
 	"testing"
 
 	"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud"
 	"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
 	"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/mock"
-	compute "google.golang.org/api/compute/v1"
+	"google.golang.org/api/compute/v1"
 	"google.golang.org/api/googleapi"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/ingress-gce/pkg/annotations"
+	"k8s.io/ingress-gce/pkg/backends/features"
 	"k8s.io/ingress-gce/pkg/instances"
 	"k8s.io/ingress-gce/pkg/utils"
 	"k8s.io/legacy-cloud-providers/gce"
@@ -42,7 +42,7 @@ func newTestIGLinker(fakeGCE *gce.Cloud, fakeInstancePool instances.NodePool) *i
 	(fakeGCE.Compute().(*cloud.MockGCE)).MockBetaBackendServices.UpdateHook = mock.UpdateBetaBackendServiceHook
 	(fakeGCE.Compute().(*cloud.MockGCE)).MockBackendServices.UpdateHook = mock.UpdateBackendServiceHook
 
-	return &instanceGroupLinker{fakeInstancePool, fakeBackendPool, defaultNamer}
+	return &instanceGroupLinker{fakeInstancePool, fakeBackendPool}
 }
 
 func TestLink(t *testing.T) {
@@ -51,7 +51,7 @@ func TestLink(t *testing.T) {
 	fakeGCE := gce.NewFakeGCECloud(gce.DefaultTestClusterValues())
 	linker := newTestIGLinker(fakeGCE, fakeNodePool)
 
-	sp := utils.ServicePort{NodePort: 8080, Protocol: annotations.ProtocolHTTP}
+	sp := utils.ServicePort{NodePort: 8080, Protocol: annotations.ProtocolHTTP, BackendNamer: defaultNamer}
 
 	// Mimic the instance group being created
 	if _, err := linker.instancePool.EnsureInstanceGroupsAndPorts(defaultNamer.InstanceGroup(), []int64{sp.NodePort}); err != nil {
@@ -65,7 +65,7 @@ func TestLink(t *testing.T) {
 		t.Fatalf("%v", err)
 	}
 
-	be, err := fakeGCE.GetGlobalBackendService(sp.BackendName(defaultNamer))
+	be, err := fakeGCE.GetGlobalBackendService(sp.BackendName())
 	if err != nil {
 		t.Fatalf("%v", err)
 	}
@@ -81,7 +81,7 @@ func TestLinkWithCreationModeError(t *testing.T) {
 	fakeGCE := gce.NewFakeGCECloud(gce.DefaultTestClusterValues())
 	linker := newTestIGLinker(fakeGCE, fakeNodePool)
 
-	sp := utils.ServicePort{NodePort: 8080, Protocol: annotations.ProtocolHTTP}
+	sp := utils.ServicePort{NodePort: 8080, Protocol: annotations.ProtocolHTTP, BackendNamer: defaultNamer}
 	modes := []BalancingMode{Rate, Utilization}
 
 	// block the update of Backends with the given balancingMode
@@ -109,7 +109,7 @@ func TestLinkWithCreationModeError(t *testing.T) {
 			t.Fatalf("%v", err)
 		}
 
-		be, err := fakeGCE.GetGlobalBackendService(sp.BackendName(defaultNamer))
+		be, err := fakeGCE.GetGlobalBackendService(sp.BackendName())
 		if err != nil {
 			t.Fatalf("%v", err)
 		}
@@ -123,6 +123,6 @@ func TestLinkWithCreationModeError(t *testing.T) {
 				t.Fatalf("Wrong balancing mode, expected %v got %v", modes[(i+1)%len(modes)], b.BalancingMode)
 			}
 		}
-		linker.backendPool.Delete(sp.BackendName(defaultNamer), features.VersionFromServicePort(&sp), features.ScopeFromServicePort(&sp))
+		linker.backendPool.Delete(sp.BackendName(), features.VersionFromServicePort(&sp), features.ScopeFromServicePort(&sp))
 	}
 }

pkg/backends/integration_test.go

@@ -41,7 +41,7 @@ type Jig struct {
 }
 
 func newTestJig(fakeGCE *gce.Cloud) *Jig {
-	fakeHealthChecks := healthchecks.NewHealthChecker(fakeGCE, "/", "/healthz", defaultNamer, defaultBackendSvc)
+	fakeHealthChecks := healthchecks.NewHealthChecker(fakeGCE, "/", "/healthz", defaultBackendSvc)
 	fakeBackendPool := NewPool(fakeGCE, defaultNamer)
 
 	fakeIGs := instances.NewFakeInstanceGroups(sets.NewString(), defaultNamer)
@@ -58,8 +58,8 @@ func newTestJig(fakeGCE *gce.Cloud) *Jig {
 
 	return &Jig{
 		fakeInstancePool: fakeInstancePool,
-		linker:           NewInstanceGroupLinker(fakeInstancePool, fakeBackendPool, defaultNamer),
-		syncer:           NewBackendSyncer(fakeBackendPool, fakeHealthChecks, defaultNamer, fakeGCE),
+		linker:           NewInstanceGroupLinker(fakeInstancePool, fakeBackendPool),
+		syncer:           NewBackendSyncer(fakeBackendPool, fakeHealthChecks, fakeGCE),
 		pool:             fakeBackendPool,
 	}
 }
@@ -68,7 +68,7 @@ func TestBackendInstanceGroupClobbering(t *testing.T) {
 	fakeGCE := gce.NewFakeGCECloud(gce.DefaultTestClusterValues())
 	jig := newTestJig(fakeGCE)
 
-	sp := utils.ServicePort{NodePort: 80}
+	sp := utils.ServicePort{NodePort: 80, BackendNamer: defaultNamer}
 	_, err := jig.fakeInstancePool.EnsureInstanceGroupsAndPorts(defaultNamer.InstanceGroup(), []int64{sp.NodePort})
 	if err != nil {
 		t.Fatalf("Did not expect error when ensuring IG for ServicePort %+v: %v", sp, err)
@@ -140,7 +140,7 @@ func TestSyncChaosMonkey(t *testing.T) {
 	fakeGCE := gce.NewFakeGCECloud(gce.DefaultTestClusterValues())
 	jig := newTestJig(fakeGCE)
 
-	sp := utils.ServicePort{NodePort: 8080, Protocol: annotations.ProtocolHTTP}
+	sp := utils.ServicePort{NodePort: 8080, Protocol: annotations.ProtocolHTTP, BackendNamer: defaultNamer}
 
 	_, err := jig.fakeInstancePool.EnsureInstanceGroupsAndPorts(defaultNamer.InstanceGroup(), []int64{sp.NodePort})
 	if err != nil {
@@ -154,7 +154,7 @@ func TestSyncChaosMonkey(t *testing.T) {
 		t.Fatalf("Did not expect error when linking backend with port %v to groups, err: %v", sp.NodePort, err)
 	}
 
-	beName := sp.BackendName(defaultNamer)
+	beName := sp.BackendName()
 	be, err := fakeGCE.GetGlobalBackendService(beName)
 	if err != nil {
 		t.Fatalf("%v", err)

pkg/backends/neg_linker.go

@@ -19,15 +19,13 @@ import (
 	befeatures "k8s.io/ingress-gce/pkg/backends/features"
 	"k8s.io/ingress-gce/pkg/composite"
 	"k8s.io/ingress-gce/pkg/utils"
-	"k8s.io/ingress-gce/pkg/utils/namer"
 	"k8s.io/legacy-cloud-providers/gce"
 )
 
 // negLinker handles linking backends to NEG's.
 type negLinker struct {
 	backendPool Pool
 	negGetter   NEGGetter
-	namer       *namer.Namer
 	cloud       *gce.Cloud
 }
 
@@ -37,12 +35,10 @@ var _ Linker = (*negLinker)(nil)
 func NewNEGLinker(
 	backendPool Pool,
 	negGetter NEGGetter,
-	namer *namer.Namer,
 	cloud *gce.Cloud) Linker {
 	return &negLinker{
 		backendPool: backendPool,
 		negGetter:   negGetter,
-		namer:       namer,
 		cloud:       cloud,
 	}
 }
@@ -53,10 +49,10 @@ func (l *negLinker) Link(sp utils.ServicePort, groups []GroupKey) error {
 	var err error
 	for _, group := range groups {
 		// If the group key contains a name, then use that.
-		// Otherwise, generate the name using the namer.
+		// Otherwise, get the name from svc port.
 		negName := group.Name
 		if negName == "" {
-			negName = sp.BackendName(l.namer)
+			negName = sp.BackendName()
 		}
 		neg, err := l.negGetter.GetNetworkEndpointGroup(negName, group.Zone)
 		if err != nil {
@@ -65,7 +61,7 @@ func (l *negLinker) Link(sp utils.ServicePort, groups []GroupKey) error {
 		negs = append(negs, neg)
 	}
 
-	beName := sp.BackendName(l.namer)
+	beName := sp.BackendName()
 
 	version := befeatures.VersionFromServicePort(&sp)
 	scope := befeatures.ScopeFromServicePort(&sp)

pkg/backends/neg_linker_test.go

@@ -35,7 +35,7 @@ func newTestNEGLinker(fakeNEG negtypes.NetworkEndpointGroupCloud, fakeGCE *gce.C
 	(fakeGCE.Compute().(*cloud.MockGCE)).MockBetaBackendServices.UpdateHook = mock.UpdateBetaBackendServiceHook
 	(fakeGCE.Compute().(*cloud.MockGCE)).MockBackendServices.UpdateHook = mock.UpdateBackendServiceHook
 
-	return &negLinker{fakeBackendPool, fakeNEG, defaultNamer, fakeGCE}
+	return &negLinker{fakeBackendPool, fakeNEG, fakeGCE}
 }
 
 func TestLinkBackendServiceToNEG(t *testing.T) {
@@ -53,11 +53,12 @@ func TestLinkBackendServiceToNEG(t *testing.T) {
 				Name:      name,
 			},
 		},
-		Port:       80,
-		NodePort:   30001,
-		Protocol:   annotations.ProtocolHTTP,
-		TargetPort: port,
-		NEGEnabled: true,
+		Port:         80,
+		NodePort:     30001,
+		Protocol:     annotations.ProtocolHTTP,
+		TargetPort:   port,
+		NEGEnabled:   true,
+		BackendNamer: defaultNamer,
 	}
 
 	// Mimic how the syncer would create the backend.
@@ -76,7 +77,7 @@ func TestLinkBackendServiceToNEG(t *testing.T) {
 		t.Fatalf("Failed to link backend service to NEG: %v", err)
 	}
 
-	beName := svcPort.BackendName(defaultNamer)
+	beName := svcPort.BackendName()
 	bs, err := fakeGCE.GetGlobalBackendService(beName)
 	if err != nil {
 		t.Fatalf("Failed to retrieve backend service: %v", err)

pkg/backends/syncer.go

@@ -27,7 +27,6 @@ import (
 	"k8s.io/ingress-gce/pkg/healthchecks"
 	lbfeatures "k8s.io/ingress-gce/pkg/loadbalancers/features"
 	"k8s.io/ingress-gce/pkg/utils"
-	"k8s.io/ingress-gce/pkg/utils/namer"
 	"k8s.io/klog"
 	"k8s.io/legacy-cloud-providers/gce"
 )
@@ -37,7 +36,6 @@ type backendSyncer struct {
 	backendPool   Pool
 	healthChecker healthchecks.HealthChecker
 	prober        ProbeProvider
-	namer         *namer.Namer
 	cloud         *gce.Cloud
 }
 
@@ -47,12 +45,10 @@ var _ Syncer = (*backendSyncer)(nil)
 func NewBackendSyncer(
 	backendPool Pool,
 	healthChecker healthchecks.HealthChecker,
-	namer *namer.Namer,
 	cloud *gce.Cloud) Syncer {
 	return &backendSyncer{
 		backendPool:   backendPool,
 		healthChecker: healthChecker,
-		namer:         namer,
 		cloud:         cloud,
 	}
 }
@@ -78,7 +74,7 @@ func (s *backendSyncer) ensureBackendService(sp utils.ServicePort) error {
 	// We must track the ports even if creating the backends failed, because
 	// we might've created health-check for them.
 	be := &composite.BackendService{}
-	beName := sp.BackendName(s.namer)
+	beName := sp.BackendName()
 	version := features.VersionFromServicePort(&sp)
 	scope := features.ScopeFromServicePort(&sp)
 
@@ -141,7 +137,7 @@ func (s *backendSyncer) ensureBackendService(sp utils.ServicePort) error {
 
 // GC implements Syncer.
 func (s *backendSyncer) GC(svcPorts []utils.ServicePort) error {
-	knownPorts, err := knownPortsFromServicePorts(s.cloud, s.namer, svcPorts)
+	knownPorts, err := knownPortsFromServicePorts(s.cloud, svcPorts)
 	if err != nil {
 		return err
 	}
@@ -216,11 +212,11 @@ func (s *backendSyncer) gc(backends []*composite.BackendService, knownPorts sets
 }
 
 // TODO: (shance) add unit tests
-func knownPortsFromServicePorts(cloud *gce.Cloud, namer *namer.Namer, svcPorts []utils.ServicePort) (sets.String, error) {
+func knownPortsFromServicePorts(cloud *gce.Cloud, svcPorts []utils.ServicePort) (sets.String, error) {
 	knownPorts := sets.NewString()
 
 	for _, sp := range svcPorts {
-		name := sp.BackendName(namer)
+		name := sp.BackendName()
 		if key, err := composite.CreateKey(cloud, name, features.ScopeFromServicePort(&sp)); err != nil {
 			return nil, err
 		} else {