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.

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

Fix bug in backend syncer where backend service was being created without health check #431

Merged
merged 1 commit into from
Aug 14, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 7 additions & 6 deletions pkg/backends/backends.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func ensureDescription(be *composite.BackendService, sp *utils.ServicePort) (nee
}

// Create implements Pool.
func (b *Backends) Create(sp utils.ServicePort) (*composite.BackendService, error) {
func (b *Backends) Create(sp utils.ServicePort, hcLink string) (*composite.BackendService, error) {
name := sp.BackendName(b.namer)
namedPort := &compute.NamedPort{
Name: b.namer.NamedPort(sp.NodePort),
Expand All @@ -88,11 +88,12 @@ func (b *Backends) Create(sp utils.ServicePort) (*composite.BackendService, erro

version := features.VersionFromServicePort(&sp)
be := &composite.BackendService{
Version: version,
Name: name,
Protocol: string(sp.Protocol),
Port: namedPort.Port,
PortName: namedPort.Name,
Version: version,
Name: name,
Protocol: string(sp.Protocol),
Port: namedPort.Port,
PortName: namedPort.Name,
HealthChecks: []string{hcLink},
}
ensureDescription(be, &sp)
if err := composite.CreateBackendService(be, b.cloud); err != nil {
Expand Down
4 changes: 2 additions & 2 deletions pkg/backends/ig_linker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func TestLink(t *testing.T) {
}

// Mimic the syncer creating the backend.
linker.backendPool.Create(sp)
linker.backendPool.Create(sp, "fake-health-check-link")

if err := linker.Link(sp, []GroupKey{{defaultZone}}); err != nil {
t.Fatalf("%v", err)
Expand Down Expand Up @@ -102,7 +102,7 @@ func TestLinkWithCreationModeError(t *testing.T) {
}

// Mimic the syncer creating the backend.
linker.backendPool.Create(sp)
linker.backendPool.Create(sp, "fake-health-check-link")

if err := linker.Link(sp, []GroupKey{{defaultZone}}); err != nil {
t.Fatalf("%v", err)
Expand Down
2 changes: 1 addition & 1 deletion pkg/backends/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ type Pool interface {
// Get a composite BackendService given a required version.
Get(name string, version meta.Version) (*composite.BackendService, error)
// Create a composite BackendService and returns it.
Create(sp utils.ServicePort) (*composite.BackendService, error)
Create(sp utils.ServicePort, hcLink string) (*composite.BackendService, error)
// Update a BackendService given the composite type.
Update(be *composite.BackendService) error
// Delete a BackendService given its name.
Expand Down
2 changes: 1 addition & 1 deletion pkg/backends/neg_linker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func TestLinkBackendServiceToNEG(t *testing.T) {
}

// Mimic how the syncer would create the backend.
linker.backendPool.Create(svcPort)
linker.backendPool.Create(svcPort, "fake-healthcheck-link")

for _, key := range zones {
err := fakeNEG.CreateNetworkEndpointGroup(&computebeta.NetworkEndpointGroup{
Expand Down
2 changes: 1 addition & 1 deletion pkg/backends/syncer.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ func (s *backendSyncer) ensureBackendService(sp utils.ServicePort) error {
}
// Only create the backend service if the error was 404.
glog.V(2).Infof("Creating backend service for port %v named %v", sp.NodePort, beName)
be, err = s.backendPool.Create(sp)
be, err = s.backendPool.Create(sp, hcLink)
if err != nil {
return err
}
Expand Down
10 changes: 7 additions & 3 deletions pkg/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ func (lbc *LoadBalancerController) ensureIngress(ing *extensions.Ingress, nodeNa
}

// Sync the backends
if lbc.backendSyncer.Sync(ingSvcPorts); err != nil {
if err := lbc.backendSyncer.Sync(ingSvcPorts); err != nil {
return err
}

Expand All @@ -349,12 +349,16 @@ func (lbc *LoadBalancerController) ensureIngress(ing *extensions.Ingress, nodeNa

// Link backends to groups.
for _, sp := range ingSvcPorts {
var linkErr error
if sp.NEGEnabled {
// Link backend to NEG's if the backend has NEG enabled.
lbc.negLinker.Link(sp, groupKeys)
linkErr = lbc.negLinker.Link(sp, groupKeys)
} else {
// Otherwise, link backend to IG's.
lbc.igLinker.Link(sp, groupKeys)
linkErr = lbc.igLinker.Link(sp, groupKeys)
}
if linkErr != nil {
return linkErr
}
}

Expand Down