Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup backend namer workflow #861

Merged
merged 1 commit into from
Oct 9, 2019
Merged

Cleanup backend namer workflow #861

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions pkg/backends/backends.go
View file
Open in desktop
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
View file
Open in desktop
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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about dropping the IGName function and just use BackendName?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry. please skip it.

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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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