Merge pull request #431 from rramkumar1/master

Fix bug in backend syncer where backend service was being created without health check
kubernetes · Aug 14, 2018 · ec7aa4d · ec7aa4d
2 parents 7079695 + 8e9dc82
commit ec7aa4d
Show file tree

Hide file tree

Showing 6 changed files with 19 additions and 14 deletions.
diff --git a/pkg/backends/backends.go b/pkg/backends/backends.go
@@ -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),
@@ -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 {

diff --git a/pkg/backends/ig_linker_test.go b/pkg/backends/ig_linker_test.go
@@ -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)
@@ -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)

diff --git a/pkg/backends/interfaces.go b/pkg/backends/interfaces.go
@@ -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.

diff --git a/pkg/backends/neg_linker_test.go b/pkg/backends/neg_linker_test.go
@@ -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{

diff --git a/pkg/backends/syncer.go b/pkg/backends/syncer.go
@@ -100,7 +100,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
 		}

diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go
@@ -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
 	}
 
@@ -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
 		}
 	}