Skip to content

Commit

Permalink
Cleanup backend namer workflow
Browse files Browse the repository at this point in the history
  • Loading branch information
skmatti committed Oct 9, 2019
1 parent a52c0ed commit f0fc341
Show file tree
Hide file tree
Showing 21 changed files with 241 additions and 217 deletions.
6 changes: 3 additions & 3 deletions pkg/backends/backends.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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,
Expand All @@ -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,
Expand Down
10 changes: 3 additions & 7 deletions pkg/backends/ig_linker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)
}
Expand All @@ -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
}
Expand Down
16 changes: 8 additions & 8 deletions pkg/backends/ig_linker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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) {
Expand All @@ -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 {
Expand All @@ -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)
}
Expand All @@ -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
Expand Down Expand Up @@ -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)
}
Expand All @@ -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))
}
}
12 changes: 6 additions & 6 deletions pkg/backends/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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,
}
}
Expand All @@ -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)
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
Expand Down
10 changes: 3 additions & 7 deletions pkg/backends/neg_linker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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,
}
}
Expand All @@ -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 {
Expand All @@ -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)
Expand Down
15 changes: 8 additions & 7 deletions pkg/backends/neg_linker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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.
Expand All @@ -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)
Expand Down
12 changes: 4 additions & 8 deletions pkg/backends/syncer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -37,7 +36,6 @@ type backendSyncer struct {
backendPool Pool
healthChecker healthchecks.HealthChecker
prober ProbeProvider
namer *namer.Namer
cloud *gce.Cloud
}

Expand All @@ -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,
}
}
Expand All @@ -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)

Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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 {
Expand Down
Loading

0 comments on commit f0fc341

Please sign in to comment.