diff --git a/controllers/gce/backends/backends.go b/controllers/gce/backends/backends.go index 5c66b3dfa..a14749749 100644 --- a/controllers/gce/backends/backends.go +++ b/controllers/gce/backends/backends.go @@ -171,13 +171,22 @@ func (b *Backends) Get(port int64) (*compute.BackendService, error) { func (b *Backends) ensureHealthCheck(sp ServicePort) (string, error) { hc := b.healthChecker.New(sp.Port, sp.Protocol) - if b.prober != nil { + + existingLegacyHC, err := b.healthChecker.GetLegacy(sp.Port) + if err != nil && !utils.IsNotFoundError(err) { + return "", err + } + + if existingLegacyHC != nil { + glog.V(4).Infof("Applying settings of existing health check to newer health check on port %+v", sp) + applyLegacyHCToHC(existingLegacyHC, hc) + } else if b.prober != nil { probe, err := b.prober.GetProbe(sp) if err != nil { return "", err } if probe != nil { - glog.V(2).Infof("Applying httpGet settings of readinessProbe to health check on port %+v", sp) + glog.V(4).Infof("Applying httpGet settings of readinessProbe to health check on port %+v", sp) applyProbeSettingsToHC(probe, hc) } } @@ -247,7 +256,7 @@ func (b *Backends) Add(p ServicePort) error { // If previous health check was legacy type, we need to delete it. if existingHCLink != hcLink && strings.Contains(existingHCLink, "/httpHealthChecks/") { if err = b.healthChecker.DeleteLegacy(p.Port); err != nil { - return err + glog.Warning("Failed to delete legacy HttpHealthCheck %v; Will not try again, err: %v", pName, err) } } @@ -433,6 +442,16 @@ func (b *Backends) Status(name string) string { return hs.HealthStatus[0].HealthState } +func applyLegacyHCToHC(existing *compute.HttpHealthCheck, hc *healthchecks.HealthCheck) { + hc.CheckIntervalSec = existing.CheckIntervalSec + hc.HealthyThreshold = existing.HealthyThreshold + hc.Host = existing.Host + hc.Port = existing.Port + hc.RequestPath = existing.RequestPath + hc.TimeoutSec = existing.TimeoutSec + hc.UnhealthyThreshold = existing.UnhealthyThreshold +} + func applyProbeSettingsToHC(p *api_v1.Probe, hc *healthchecks.HealthCheck) { healthPath := p.Handler.HTTPGet.Path // GCE requires a leading "/" for health check urls. diff --git a/controllers/gce/backends/backends_test.go b/controllers/gce/backends/backends_test.go index 99cfd45ba..bfe215c65 100644 --- a/controllers/gce/backends/backends_test.go +++ b/controllers/gce/backends/backends_test.go @@ -50,21 +50,23 @@ var existingProbe = &api_v1.Probe{ }, } -func newBackendPool(f BackendServices, fakeIGs instances.InstanceGroups, syncWithCloud bool) *Backends { +func newTestJig(f BackendServices, fakeIGs instances.InstanceGroups, syncWithCloud bool) (*Backends, healthchecks.HealthCheckProvider) { namer := &utils.Namer{} nodePool := instances.NewNodePool(fakeIGs) nodePool.Init(&instances.FakeZoneLister{Zones: []string{defaultZone}}) - healthChecks := healthchecks.NewHealthChecker(healthchecks.NewFakeHealthCheckProvider(), "/", namer) + healthCheckProvider := healthchecks.NewFakeHealthCheckProvider() + healthChecks := healthchecks.NewHealthChecker(healthCheckProvider, "/", namer) bp := NewBackendPool(f, healthChecks, nodePool, namer, []int64{}, syncWithCloud) probes := map[ServicePort]*api_v1.Probe{{Port: 443, Protocol: utils.ProtocolHTTPS}: existingProbe} bp.Init(NewFakeProbeProvider(probes)) - return bp + + return bp, healthCheckProvider } func TestBackendPoolAdd(t *testing.T) { f := NewFakeBackendServices(noOpErrFunc) fakeIGs := instances.NewFakeInstanceGroups(sets.NewString()) - pool := newBackendPool(f, fakeIGs, false) + pool, _ := newTestJig(f, fakeIGs, false) namer := utils.Namer{} testCases := []ServicePort{ @@ -105,7 +107,6 @@ func TestBackendPoolAdd(t *testing.T) { } // Check the created healthcheck is the correct protocol - // pool.healthChecker. hc, err := pool.healthChecker.Get(nodePort.Port) if err != nil { t.Fatalf("Unexpected err when querying fake healthchecker: %v", err) @@ -122,10 +123,46 @@ func TestBackendPoolAdd(t *testing.T) { } } +func TestHealthCheckMigration(t *testing.T) { + f := NewFakeBackendServices(noOpErrFunc) + fakeIGs := instances.NewFakeInstanceGroups(sets.NewString()) + pool, hcp := newTestJig(f, fakeIGs, false) + namer := utils.Namer{} + + p := ServicePort{Port: 7000, Protocol: utils.ProtocolHTTP} + + // Create a legacy health check and insert it into the HC provider. + legacyHC := &compute.HttpHealthCheck{ + Name: namer.BeName(p.Port), + RequestPath: "/my-healthz-path", + Host: "k8s.io", + UnhealthyThreshold: 30, + CheckIntervalSec: 40, + } + hcp.CreateHttpHealthCheck(legacyHC) + + // Add the service port to the backend pool + pool.Add(p) + + // Assert the proper health check was created + hc, _ := pool.healthChecker.Get(p.Port) + if hc == nil || hc.Protocol() != p.Protocol { + t.Fatalf("Expected %s health check, received %v: ", p.Protocol, hc) + } + + // Assert the newer health check has the legacy health check settings + if hc.RequestPath != legacyHC.RequestPath || + hc.Host != legacyHC.Host || + hc.UnhealthyThreshold != legacyHC.UnhealthyThreshold || + hc.CheckIntervalSec != legacyHC.CheckIntervalSec { + t.Fatalf("Expected newer health check to have identical settings to legacy health check. Legacy: %+v, New: %+v", legacyHC, hc) + } +} + func TestBackendPoolUpdate(t *testing.T) { f := NewFakeBackendServices(noOpErrFunc) fakeIGs := instances.NewFakeInstanceGroups(sets.NewString()) - pool := newBackendPool(f, fakeIGs, false) + pool, _ := newTestJig(f, fakeIGs, false) namer := utils.Namer{} p := ServicePort{Port: 3000, Protocol: utils.ProtocolHTTP} @@ -171,7 +208,7 @@ func TestBackendPoolUpdate(t *testing.T) { func TestBackendPoolChaosMonkey(t *testing.T) { f := NewFakeBackendServices(noOpErrFunc) fakeIGs := instances.NewFakeInstanceGroups(sets.NewString()) - pool := newBackendPool(f, fakeIGs, false) + pool, _ := newTestJig(f, fakeIGs, false) namer := utils.Namer{} nodePort := ServicePort{Port: 8080, Protocol: utils.ProtocolHTTP} @@ -220,7 +257,7 @@ func TestBackendPoolSync(t *testing.T) { svcNodePorts := []ServicePort{{Port: 81, Protocol: utils.ProtocolHTTP}, {Port: 82, Protocol: utils.ProtocolHTTPS}, {Port: 83, Protocol: utils.ProtocolHTTP}} f := NewFakeBackendServices(noOpErrFunc) fakeIGs := instances.NewFakeInstanceGroups(sets.NewString()) - pool := newBackendPool(f, fakeIGs, true) + pool, _ := newTestJig(f, fakeIGs, true) pool.Add(ServicePort{Port: 81}) pool.Add(ServicePort{Port: 90}) if err := pool.Sync(svcNodePorts); err != nil { @@ -345,7 +382,7 @@ func TestBackendPoolDeleteLegacyHealthChecks(t *testing.T) { func TestBackendPoolShutdown(t *testing.T) { f := NewFakeBackendServices(noOpErrFunc) fakeIGs := instances.NewFakeInstanceGroups(sets.NewString()) - pool := newBackendPool(f, fakeIGs, false) + pool, _ := newTestJig(f, fakeIGs, false) namer := utils.Namer{} // Add a backend-service and verify that it doesn't exist after Shutdown() @@ -359,7 +396,7 @@ func TestBackendPoolShutdown(t *testing.T) { func TestBackendInstanceGroupClobbering(t *testing.T) { f := NewFakeBackendServices(noOpErrFunc) fakeIGs := instances.NewFakeInstanceGroups(sets.NewString()) - pool := newBackendPool(f, fakeIGs, false) + pool, _ := newTestJig(f, fakeIGs, false) namer := utils.Namer{} // This will add the instance group k8s-ig to the instance pool @@ -405,7 +442,7 @@ func TestBackendCreateBalancingMode(t *testing.T) { f := NewFakeBackendServices(noOpErrFunc) fakeIGs := instances.NewFakeInstanceGroups(sets.NewString()) - pool := newBackendPool(f, fakeIGs, false) + pool, _ := newTestJig(f, fakeIGs, false) namer := utils.Namer{} nodePort := ServicePort{Port: 8080} modes := []BalancingMode{Rate, Utilization} diff --git a/controllers/gce/healthchecks/healthchecks.go b/controllers/gce/healthchecks/healthchecks.go index 98f0a5f43..b150f2f2a 100644 --- a/controllers/gce/healthchecks/healthchecks.go +++ b/controllers/gce/healthchecks/healthchecks.go @@ -125,6 +125,12 @@ func (h *HealthChecks) Get(port int64) (*HealthCheck, error) { return NewHealthCheck(hc), err } +// GetLegacy deletes legacy HTTP health checks +func (h *HealthChecks) GetLegacy(port int64) (*compute.HttpHealthCheck, error) { + name := h.namer.BeName(port) + return h.cloud.GetHttpHealthCheck(name) +} + // DeleteLegacy deletes legacy HTTP health checks func (h *HealthChecks) DeleteLegacy(port int64) error { name := h.namer.BeName(port) diff --git a/controllers/gce/healthchecks/interfaces.go b/controllers/gce/healthchecks/interfaces.go index cdf8c635d..e63f1f491 100644 --- a/controllers/gce/healthchecks/interfaces.go +++ b/controllers/gce/healthchecks/interfaces.go @@ -41,5 +41,6 @@ type HealthChecker interface { Sync(hc *HealthCheck) (string, error) Delete(port int64) error Get(port int64) (*HealthCheck, error) + GetLegacy(port int64) (*compute.HttpHealthCheck, error) DeleteLegacy(port int64) error } diff --git a/controllers/gce/utils/utils.go b/controllers/gce/utils/utils.go index 3b628ef6c..ec35f7412 100644 --- a/controllers/gce/utils/utils.go +++ b/controllers/gce/utils/utils.go @@ -332,6 +332,20 @@ func IgnoreHTTPNotFound(err error) error { return err } +// IsInUsedByError returns true if the resource is being used by another GCP resource +func IsInUsedByError(err error) bool { + apiErr, ok := err.(*googleapi.Error) + if !ok || apiErr.Code != http.StatusBadRequest { + return false + } + return strings.Contains(apiErr.Message, "being used by") +} + +// IsNotFoundError returns true if the resource does not exist +func IsNotFoundError(err error) bool { + return IsHTTPErrorCode(err, http.StatusNotFound) +} + // CompareLinks returns true if the 2 self links are equal. func CompareLinks(l1, l2 string) bool { // TODO: These can be partial links