diff --git a/controllers/gce/backends/backends.go b/controllers/gce/backends/backends.go index e7e1cbbd0..6224758fe 100644 --- a/controllers/gce/backends/backends.go +++ b/controllers/gce/backends/backends.go @@ -173,55 +173,18 @@ func (b *Backends) ensureHealthCheck(sp ServicePort) (string, error) { return b.healthChecker.Sync(hc) } -func (b *Backends) create(igs []*compute.InstanceGroup, namedPort *compute.NamedPort, hcLink string, protocol utils.AppProtocol, name string) (*compute.BackendService, error) { - var errs []string - // We first try to create the backend with balancingMode=RATE. If this - // fails, it's mostly likely because there are existing backends with - // balancingMode=UTILIZATION. This failure mode throws a googleapi error - // which wraps a HTTP 400 status code. We handle it in the loop below - // and come around to retry with the right balancing mode. The goal is to - // switch everyone to using RATE. - for _, bm := range []BalancingMode{Rate, Utilization} { - // Create a new backend - bs := newBackendService(igs, bm, namedPort, []string{hcLink}, protocol, name) - if err := b.cloud.CreateBackendService(bs); err != nil { - // This is probably a failure because we tried to create the backend - // with balancingMode=RATE when there are already backends with - // balancingMode=UTILIZATION. Just ignore it and retry setting - // balancingMode=UTILIZATION (b/35102911). - if utils.IsHTTPErrorCode(err, http.StatusBadRequest) { - glog.Infof("Error creating backend service with balancing mode %v:%v", bm, err) - errs = append(errs, fmt.Sprintf("%v", err)) - continue - } - return nil, err - } - return b.Get(namedPort.Port) - } - return nil, fmt.Errorf("%v", strings.Join(errs, "\n")) -} - -func newBackendService(igs []*compute.InstanceGroup, bm BalancingMode, namedPort *compute.NamedPort, healthCheckLinks []string, protocol utils.AppProtocol, name string) *compute.BackendService { - backends := getBackendsForIGs(igs) - for _, b := range backends { - switch bm { - case Rate: - b.MaxRatePerInstance = maxRPS - default: - // TODO: Set utilization and connection limits when we accept them - // as valid fields. - } - b.BalancingMode = string(bm) - } - - return &compute.BackendService{ +func (b *Backends) create(namedPort *compute.NamedPort, hcLink string, protocol utils.AppProtocol, name string) (*compute.BackendService, error) { + bs := &compute.BackendService{ Name: name, Protocol: string(protocol), - Backends: backends, - HealthChecks: healthCheckLinks, + HealthChecks: []string{hcLink}, Port: namedPort.Port, PortName: namedPort.Name, } + if err := b.cloud.CreateBackendService(bs); err != nil { + return nil, err + } + return b.Get(namedPort.Port) } // Add will get or create a Backend for the given port. @@ -242,21 +205,22 @@ func (b *Backends) Add(p ServicePort) error { return err } + // Verify existance of a backend service for the proper port, but do not specify any backends/igs pName := b.namer.BeName(p.Port) be, _ = b.Get(p.Port) if be == nil { - glog.V(2).Infof("Creating backend for %d instance groups, port %v named port %v", len(igs), p.Port, namedPort) - be, err = b.create(igs, namedPort, hcLink, p.Protocol, pName) + glog.V(2).Infof("Creating backend service for port %v named port %v", p.Port, namedPort) + be, err = b.create(namedPort, hcLink, p.Protocol, pName) if err != nil { return err } } + // Check that the backend service has the correct protocol and health check link existingHCLink := "" if len(be.HealthChecks) == 1 { existingHCLink = be.HealthChecks[0] } - if be.Protocol != string(p.Protocol) || existingHCLink != hcLink { glog.V(2).Infof("Updating backend protocol %v (%v) for change in protocol (%v) or health check", pName, be.Protocol, string(p.Protocol)) be.Protocol = string(p.Protocol) @@ -277,6 +241,8 @@ func (b *Backends) Add(p ServicePort) error { if len(igs) == 0 { return nil } + + // Verify that backend service contains links to all backends/instance-groups return b.edgeHop(be, igs) } @@ -315,10 +281,22 @@ func (b *Backends) List() ([]interface{}, error) { return interList, nil } -func getBackendsForIGs(igs []*compute.InstanceGroup) []*compute.Backend { - backends := []*compute.Backend{} +func getBackendsForIGs(igs []*compute.InstanceGroup, bm BalancingMode) []*compute.Backend { + var backends []*compute.Backend for _, ig := range igs { - backends = append(backends, &compute.Backend{Group: ig.SelfLink}) + b := &compute.Backend{ + Group: ig.SelfLink, + BalancingMode: string(bm), + } + switch bm { + case Rate: + b.MaxRatePerInstance = maxRPS + default: + // TODO: Set utilization and connection limits when we accept them + // as valid fields. + } + + backends = append(backends, b) } return backends } @@ -337,20 +315,45 @@ func (b *Backends) edgeHop(be *compute.BackendService, igs []*compute.InstanceGr if beIGs.IsSuperset(igLinks) { return nil } - glog.Infof("Backend %v has a broken edge, expected igs %+v, current igs %+v", - be.Name, igLinks.List(), beIGs.List()) + glog.V(2).Infof("Updating backend service %v with %d backends: expected igs %+v, current igs %+v", + be.Name, igLinks.Len(), igLinks.List(), beIGs.List()) - newBackends := []*compute.Backend{} - for _, b := range getBackendsForIGs(igs) { - if !beIGs.Has(b.Group) { - newBackends = append(newBackends, b) + originalBackends := be.Backends + var addIGs []*compute.InstanceGroup + for _, ig := range igs { + if !beIGs.Has(ig.SelfLink) { + addIGs = append(addIGs, ig) } } - be.Backends = append(be.Backends, newBackends...) - if err := b.cloud.UpdateBackendService(be); err != nil { - return err + + // We first try to create the backend with balancingMode=RATE. If this + // fails, it's mostly likely because there are existing backends with + // balancingMode=UTILIZATION. This failure mode throws a googleapi error + // which wraps a HTTP 400 status code. We handle it in the loop below + // and come around to retry with the right balancing mode. The goal is to + // switch everyone to using RATE. + var errs []string + for _, bm := range []BalancingMode{Rate, Utilization} { + // Generate backends with given instance groups with a specific mode + newBackends := getBackendsForIGs(addIGs, bm) + be.Backends = append(originalBackends, newBackends...) + + if err := b.cloud.UpdateBackendService(be); err != nil { + if utils.IsHTTPErrorCode(err, http.StatusBadRequest) { + glog.V(2).Infof("Updating backend service backends with balancing mode %v failed, will try another mode. err:%v", bm, err) + errs = append(errs, err.Error()) + // This is probably a failure because we tried to create the backend + // with balancingMode=RATE when there are already backends with + // balancingMode=UTILIZATION. Just ignore it and retry setting + // balancingMode=UTILIZATION (b/35102911). + continue + } + glog.V(2).Infof("Error updating backend service backends with balancing mode %v:%v", bm, err) + return err + } + return nil } - return nil + return fmt.Errorf("received errors when updating backend service: %v", strings.Join(errs, "\n")) } // Sync syncs backend services corresponding to ports in the given list. diff --git a/controllers/gce/backends/fakes.go b/controllers/gce/backends/fakes.go index 6a6c1cbb0..4ad8d0c28 100644 --- a/controllers/gce/backends/fakes.go +++ b/controllers/gce/backends/fakes.go @@ -99,6 +99,11 @@ func (f *FakeBackendServices) ListBackendServices() (*compute.BackendServiceList // UpdateBackendService fakes updating a backend service. func (f *FakeBackendServices) UpdateBackendService(be *compute.BackendService) error { + if f.errFunc != nil { + if err := f.errFunc(utils.Update, be); err != nil { + return err + } + } f.calls = append(f.calls, utils.Update) return f.backendServices.Update(be) }