From fb5edf16e18a75a9590f07b4db0f865d709999f9 Mon Sep 17 00:00:00 2001 From: Bowei Du Date: Sun, 16 Feb 2020 20:49:13 -0800 Subject: [PATCH 1/4] Add support for nil to HTTPStatusCode This allows us to check for HTTPStatusCode == 404 directly without guarding it with a nil check. --- pkg/utils/utils.go | 3 +++ pkg/utils/utils_test.go | 23 ++++++++++++++++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index c062678368..8cc65dd3e9 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -111,6 +111,9 @@ func FakeGoogleAPINotFoundErr() *googleapi.Error { // IsHTTPErrorCode checks if the given error matches the given HTTP Error code. // For this to work the error must be a googleapi Error. func IsHTTPErrorCode(err error, code int) bool { + if err == nil { + return false + } apiErr, ok := err.(*googleapi.Error) return ok && apiErr.Code == code } diff --git a/pkg/utils/utils_test.go b/pkg/utils/utils_test.go index 9b29480040..c0249d5932 100644 --- a/pkg/utils/utils_test.go +++ b/pkg/utils/utils_test.go @@ -17,12 +17,13 @@ limitations under the License. package utils import ( + "errors" "fmt" "testing" "time" "github.com/google/go-cmp/cmp" - + "google.golang.org/api/googleapi" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/ingress-gce/pkg/annotations" "k8s.io/ingress-gce/pkg/flags" @@ -808,3 +809,23 @@ func TestGetPortRanges(t *testing.T) { } } } + +func TestIsHTTPErrorCode(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + err error + code int + want bool + }{ + {nil, 400, false}, + {errors.New("xxx"), 400, false}, + {&googleapi.Error{Code: 200}, 400, false}, + {&googleapi.Error{Code: 400}, 400, true}, + } { + got := IsHTTPErrorCode(tc.err, tc.code) + if got != tc.want { + t.Errorf("IsHTTPErrorCode(%v, %d) = %t; want %t", tc.err, tc.code, got, tc.want) + } + } +} From 581313450edce8462a0ff094e27bf6330d15ed3f Mon Sep 17 00:00:00 2001 From: Bowei Du Date: Mon, 17 Feb 2020 13:32:10 -0800 Subject: [PATCH 2/4] Take healthcheck configuration overrides from the backendconfig This enables taking the healthcheck configuration from a backendconfig associated with the given Service. As part of this PR, we also clean up the override handling to be easier to understand where the settings come from. For a given ServicePort `sp` and its associated healthcheck `hc`: 1. `sp` has some default settings depending on its type (e.g. IG, NEG, ILB). 2. SyncServicePort() will override those settings if the Probe exists. Path, Host, TimeoutSec, CheckIntervalSec. 3. We then sync with what exists in GCE: 3.1 If the healthcheck is does not exist, it will be CREATED. 3.1.1 If it has a backendconfig, then the following settings are used: CheckIntervalSec, TimeoutSec, HealthyThreshold, UnhealthyThreshold, Type, RequestPath We have `Port` in the struct, but it does not seem to be a valid field to use so it is currently ignored. 3.2 Otherwise the healthcheck will need to be UPDATED. The criteria for update can be found in `calculateDiff()`: 3.2.1 Services w/out backendconfig will only update when `Protocol`, `PortSpecification` changes. 3.2.2 Services w/ backendconfig will also consider any fields covered by the backendconfig (see above) 3.2.3 We merge in the settings from the existing healthcheck. As users have (in the past) done edits to their HCs directly, we do not want to break any previous behavior. Thus we merge in all of the settings on HTTPHealthCheck as well as any fields on HealthCheck itself, with the exception of: `Port`, `PortSpecification` 3.2.2 We now consider the backendconfig to be a final override. See list above for settings from the backendconfig. Several situations do not seem to be handled currently: * There is no way to change HC based on probe after it is created. This is because there is no way to tell if the difference is due to a user changing the setting directly in GCE or the HC should be updated. * Changing types to/from NEG, ILB will leak the old healthcheck. User healthchecksettings will not propagate to the new healthcheck. * backendconfig seems to be missing `Host` merge --- pkg/healthchecks/healthchecks.go | 318 ++++++++++++++++++++----------- 1 file changed, 207 insertions(+), 111 deletions(-) diff --git a/pkg/healthchecks/healthchecks.go b/pkg/healthchecks/healthchecks.go index f8966dd413..4561e34510 100644 --- a/pkg/healthchecks/healthchecks.go +++ b/pkg/healthchecks/healthchecks.go @@ -18,8 +18,10 @@ package healthchecks import ( "encoding/json" + "errors" "fmt" "net/http" + "strconv" "strings" "time" @@ -30,6 +32,7 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/ingress-gce/pkg/annotations" + backendconfigv1 "k8s.io/ingress-gce/pkg/apis/backendconfig/v1" "k8s.io/ingress-gce/pkg/composite" "k8s.io/ingress-gce/pkg/flags" "k8s.io/ingress-gce/pkg/loadbalancers/features" @@ -116,12 +119,16 @@ func (h *HealthChecks) SyncServicePort(sp *utils.ServicePort, probe *v1.Probe) ( klog.V(4).Infof("Applying httpGet settings of readinessProbe to health check on port %+v", sp) applyProbeSettingsToHC(probe, hc) } - return h.sync(hc) + var bchcc *backendconfigv1.HealthCheckConfig + if sp.BackendConfig != nil { + bchcc = sp.BackendConfig.Spec.HealthCheck + } + return h.sync(hc, bchcc) } // sync retrieves a health check based on port, checks type and settings and updates/creates if necessary. // sync is only called by the backends.Add func - it's not a pool like other resources. -func (h *HealthChecks) sync(hc *HealthCheck) (string, error) { +func (h *HealthChecks) sync(hc *HealthCheck, bchcc *backendconfigv1.HealthCheckConfig) (string, error) { var scope meta.KeyType // TODO(shance): find a way to remove this if hc.forILB { @@ -131,47 +138,57 @@ func (h *HealthChecks) sync(hc *HealthCheck) (string, error) { } existingHC, err := h.Get(hc.Name, hc.Version(), scope) - if err != nil { - if !utils.IsHTTPErrorCode(err, http.StatusNotFound) { - return "", err - } - - if err = h.create(hc); err != nil { + if utils.IsHTTPErrorCode(err, http.StatusNotFound) { + klog.V(2).Infof("Health check %q does not exist, creating (hc=%+v, bchcc=%+v)", hc.Name, hc, bchcc) + if err = h.create(hc, bchcc); err != nil { + klog.Errorf("Health check %q creation error: %v", hc.Name, err) return "", err } - - return h.getHealthCheckLink(hc.Name, hc.Version(), scope) + // TODO(bowei) -- we don't need to fetch the self-link here as it is + // returned as part of the GCE call. + selfLink, err := h.getHealthCheckLink(hc.Name, hc.Version(), scope) + klog.V(2).Infof("Health check %q selflink = %q", hc.Name, selfLink) + return selfLink, err + } + if err != nil { + return "", err } - if needToUpdate(existingHC, hc) { - err = h.update(existingHC, hc) - return existingHC.SelfLink, err + // First, merge in the configuration from the existing healthcheck to cover + // the case where the user has changed healthcheck settings outside of + // GKE. + premergeHC := hc + hc = mergeUserSettings(existingHC, hc) + klog.V(3).Infof("mergeHealthcheck(%+v,%+v) = %+v", existingHC, premergeHC, hc) + // Then, BackendConfig will override any fields that are explicitly set. + if bchcc != nil { + klog.V(2).Infof("Health check %q has backendconfig override (%+v)", hc.Name, bchcc) + // BackendConfig healthcheck settings always take precedence. + hc.updateFromBackendConfig(bchcc) } - if existingHC.RequestPath != hc.RequestPath { - // TODO: reconcile health checks, and compare headers interval etc. - // Currently Ingress doesn't expose all the health check params - // natively, so some users prefer to hand modify the check. - klog.V(2).Infof("Unexpected request path on health check %v, has %v want %v, NOT reconciling", hc.Name, existingHC.RequestPath, hc.RequestPath) - } else { - klog.V(2).Infof("Health check %v already exists and has the expected path %v", hc.Name, hc.RequestPath) + changes := calculateDiff(existingHC, hc, bchcc) + if changes.hasDiff() { + klog.V(2).Infof("Health check %q needs update (%s)", existingHC.Name, changes) + err := h.update(hc) + if err != nil { + klog.Errorf("Health check %q update error: %v", existingHC.Name, err) + } + return existingHC.SelfLink, err } + klog.V(2).Infof("Health check %q already exists and needs no update", hc.Name) return existingHC.SelfLink, nil } // TODO(shance): merge with existing hc code func (h *HealthChecks) createILB(hc *HealthCheck) error { - cloud := h.cloud.(*gce.Cloud) - - if len(hc.PortSpecification) > 0 { - hc.Port = 0 - } - hc.merge() - compositeType, err := composite.AlphaToHealthCheck(&hc.HealthCheck) + compositeType, err := composite.AlphaToHealthCheck(hc.ToAlphaComputeHealthCheck()) if err != nil { return fmt.Errorf("Error converting hc to composite: %v", err) } + + cloud := h.cloud.(*gce.Cloud) key, err := composite.CreateKey(cloud, hc.Name, features.L7ILBScope()) if err != nil { return err @@ -187,7 +204,12 @@ func (h *HealthChecks) createILB(hc *HealthCheck) error { return nil } -func (h *HealthChecks) create(hc *HealthCheck) error { +func (h *HealthChecks) create(hc *HealthCheck, bchcc *backendconfigv1.HealthCheckConfig) error { + if bchcc != nil { + klog.V(2).Infof("Health check %q has backendconfig override (%+v)", hc.Name, bchcc) + // BackendConfig healthcheck settings always take precedence. + hc.updateFromBackendConfig(bchcc) + } // special case ILB to avoid mucking with stable HC code if hc.forILB { return h.createILB(hc) @@ -217,16 +239,14 @@ func (h *HealthChecks) create(hc *HealthCheck) error { } // TODO(shance): merge with existing hc code -func (h *HealthChecks) updateILB(oldHC, newHC *HealthCheck) error { +func (h *HealthChecks) updateILB(hc *HealthCheck) error { // special case ILB to avoid mucking with stable HC code - cloud := h.cloud.(*gce.Cloud) - - mergedHC := mergeHealthcheck(oldHC, newHC).ToAlphaComputeHealthCheck() - compositeType, err := composite.AlphaToHealthCheck(mergedHC) + compositeType, err := composite.AlphaToHealthCheck(hc.ToAlphaComputeHealthCheck()) if err != nil { return fmt.Errorf("Error converting newHC to composite: %v", err) } - key, err := composite.CreateKey(cloud, mergedHC.Name, features.L7ILBScope()) + cloud := h.cloud.(*gce.Cloud) + key, err := composite.CreateKey(cloud, hc.Name, features.L7ILBScope()) // Update fields compositeType.Version = features.L7ILBVersions().HealthCheck @@ -235,53 +255,68 @@ func (h *HealthChecks) updateILB(oldHC, newHC *HealthCheck) error { return composite.UpdateHealthCheck(cloud, key, compositeType) } -func (h *HealthChecks) update(oldHC, newHC *HealthCheck) error { - // special case ILB to avoid mucking with stable HC code - if oldHC.forILB { - return h.updateILB(oldHC, newHC) +func (h *HealthChecks) update(hc *HealthCheck) error { + if hc.forILB { + return h.updateILB(hc) } - - switch newHC.Version() { + switch hc.Version() { case meta.VersionAlpha: - klog.V(2).Infof("Updating alpha health check with protocol %v", newHC.Type) - return h.cloud.UpdateAlphaHealthCheck(mergeHealthcheck(oldHC, newHC).ToAlphaComputeHealthCheck()) + klog.V(2).Infof("Updating alpha health check with protocol %v", hc.Type) + return h.cloud.UpdateAlphaHealthCheck(hc.ToAlphaComputeHealthCheck()) case meta.VersionBeta: - klog.V(2).Infof("Updating beta health check with protocol %v", newHC.Type) - betaHC, err := mergeHealthcheck(oldHC, newHC).ToBetaComputeHealthCheck() + klog.V(2).Infof("Updating beta health check with protocol %v", hc.Type) + beta, err := hc.ToBetaComputeHealthCheck() if err != nil { return err } - return h.cloud.UpdateBetaHealthCheck(betaHC) + return h.cloud.UpdateBetaHealthCheck(beta) case meta.VersionGA: - klog.V(2).Infof("Updating health check for port %v with protocol %v", newHC.Port, newHC.Type) - v1hc, err := newHC.ToComputeHealthCheck() + klog.V(2).Infof("Updating health check %q for port %v with protocol %v", hc.Name, hc.Port, hc.Type) + ga, err := hc.ToComputeHealthCheck() if err != nil { return err } - return h.cloud.UpdateHealthCheck(v1hc) + return h.cloud.UpdateHealthCheck(ga) default: - return fmt.Errorf("unknown Version: %q", newHC.Version()) + return fmt.Errorf("unknown Version: %q", hc.Version()) } } -// mergeHealthcheck merges old health check configuration (potentially for IG) with the new one. -// This is to preserve the existing health check setting as much as possible. +// mergeUserSettings merges old health check configuration that the user may +// have customized. This is to preserve the existing health check setting as +// much as possible. +// +// TODO(bowei) -- fix below. // WARNING: if a service backend is converted from IG mode to NEG mode, -// the existing health check setting will be preserve, although it may not suit the customer needs. -func mergeHealthcheck(oldHC, newHC *HealthCheck) *HealthCheck { - portSpec := newHC.PortSpecification - port := newHC.Port - newHC.HTTPHealthCheck = oldHC.HTTPHealthCheck +// the existing health check setting will be preserved, although it may not +// suit the customer needs. +// +// TODO(bowei): this is very unstable in combination with Probe, we do not +// have a clear signal as to where the settings are coming from. Once a +// healthcheck is created, it will basically not change. +func mergeUserSettings(existing, newHC *HealthCheck) *HealthCheck { + hc := *newHC // return a copy + + hc.HTTPHealthCheck = existing.HTTPHealthCheck + hc.HealthCheck.CheckIntervalSec = existing.HealthCheck.CheckIntervalSec + hc.HealthCheck.HealthyThreshold = existing.HealthCheck.HealthyThreshold + hc.HealthCheck.TimeoutSec = existing.HealthCheck.TimeoutSec + hc.HealthCheck.UnhealthyThreshold = existing.HealthCheck.UnhealthyThreshold + + if existing.HealthCheck.LogConfig != nil { + l := *existing.HealthCheck.LogConfig + hc.HealthCheck.LogConfig = &l + } // Cannot specify both portSpecification and port field. - if newHC.ForNEG { - newHC.HTTPHealthCheck.Port = 0 - newHC.PortSpecification = portSpec + if hc.forNEG { + hc.HTTPHealthCheck.Port = 0 + hc.PortSpecification = newHC.PortSpecification } else { - newHC.PortSpecification = "" - newHC.Port = port + hc.PortSpecification = "" + hc.Port = newHC.Port } - return newHC + return &hc } func (h *HealthChecks) getHealthCheckLink(name string, version meta.Version, scope meta.KeyType) (string, error) { @@ -348,7 +383,7 @@ func (h *HealthChecks) getILB(name string) (*HealthCheck, error) { // Update fields for future update() calls newHC.forILB = true - newHC.ForNEG = true + newHC.forNEG = true return newHC, nil } @@ -412,7 +447,7 @@ func DefaultHealthCheck(port int64, protocol annotations.AppProtocol) *HealthChe return &HealthCheck{ HTTPHealthCheck: httpSettings, HealthCheck: hcSettings, - ForNEG: false, + forNEG: false, } } @@ -437,7 +472,7 @@ func DefaultNEGHealthCheck(protocol annotations.AppProtocol) *HealthCheck { return &HealthCheck{ HTTPHealthCheck: httpSettings, HealthCheck: hcSettings, - ForNEG: true, + forNEG: true, } } @@ -462,50 +497,46 @@ func defaultILBHealthCheck(protocol annotations.AppProtocol) *HealthCheck { HTTPHealthCheck: httpSettings, HealthCheck: hcSettings, forILB: true, - ForNEG: true, + forNEG: true, } } -// HealthCheck embeds two types - the generic healthcheck compute.HealthCheck -// and the HTTP settings compute.HTTPHealthCheck. By embedding both, consumers can modify -// all relevant settings (HTTP specific and HealthCheck generic) regardless of Type -// Consumers should call .Out() func to generate a compute.HealthCheck -// with the proper child struct (.HttpHealthCheck, .HttpshealthCheck, etc). +// HealthCheck is a wrapper for different versions of the compute struct. +// TODO(bowei): replace inner workings with composite. type HealthCheck struct { + // As the {HTTP, HTTPS, HTTP2} settings are identical, we mantain the + // settings at the outer-level and copy into the appropriate struct + // in the HealthCheck embedded struct (see `merge()`) when getting the + // compute struct back. computealpha.HTTPHealthCheck computealpha.HealthCheck - //compositeHealthCheck composite.HealthCheck - ForNEG bool - // forILB designates whether the health check is for an ILB - // This means that the HC should be alpha + regional + + forNEG bool forILB bool } // NewHealthCheck creates a HealthCheck which abstracts nested structs away func NewHealthCheck(hc *computealpha.HealthCheck) (*HealthCheck, error) { + // TODO(bowei): should never handle nil like this. if hc == nil { - return nil, nil + return nil, errors.New("nil hc to NewHealthCheck") } v := &HealthCheck{HealthCheck: *hc} - var err error switch annotations.AppProtocol(hc.Type) { case annotations.ProtocolHTTP: if hc.HttpHealthCheck == nil { - err = fmt.Errorf(newHealthCheckErrorMessageTemplate, annotations.ProtocolHTTP, hc.Name) - return nil, err + return nil, fmt.Errorf(newHealthCheckErrorMessageTemplate, annotations.ProtocolHTTP, hc.Name) } v.HTTPHealthCheck = *hc.HttpHealthCheck case annotations.ProtocolHTTPS: if hc.HttpsHealthCheck == nil { - err = fmt.Errorf(newHealthCheckErrorMessageTemplate, annotations.ProtocolHTTPS, hc.Name) - return nil, err + return nil, fmt.Errorf(newHealthCheckErrorMessageTemplate, annotations.ProtocolHTTPS, hc.Name) } v.HTTPHealthCheck = computealpha.HTTPHealthCheck(*hc.HttpsHealthCheck) case annotations.ProtocolHTTP2: if hc.Http2HealthCheck == nil { - err = fmt.Errorf(newHealthCheckErrorMessageTemplate, annotations.ProtocolHTTP2, hc.Name) - return nil, err + return nil, fmt.Errorf(newHealthCheckErrorMessageTemplate, annotations.ProtocolHTTP2, hc.Name) } v.HTTPHealthCheck = computealpha.HTTPHealthCheck(*hc.Http2HealthCheck) } @@ -516,7 +547,7 @@ func NewHealthCheck(hc *computealpha.HealthCheck) (*HealthCheck, error) { v.HealthCheck.HttpsHealthCheck = nil v.HealthCheck.Http2HealthCheck = nil - return v, err + return v, nil } // Protocol returns the type cased to AppProtocol @@ -532,25 +563,23 @@ func (hc *HealthCheck) ToComputeHealthCheck() (*compute.HealthCheck, error) { // ToBetaComputeHealthCheck returns a valid computebeta.HealthCheck object func (hc *HealthCheck) ToBetaComputeHealthCheck() (*computebeta.HealthCheck, error) { - // Cannot specify both portSpecification and port field. - if len(hc.PortSpecification) > 0 { - hc.Port = 0 - } hc.merge() return toBetaHealthCheck(&hc.HealthCheck) } // ToAlphaComputeHealthCheck returns a valid computealpha.HealthCheck object func (hc *HealthCheck) ToAlphaComputeHealthCheck() *computealpha.HealthCheck { - // Cannot specify both portSpecification and port field. - if len(hc.PortSpecification) > 0 { - hc.Port = 0 - } hc.merge() - return &hc.HealthCheck + x := hc.HealthCheck // Make a copy to ensure no aliasing. + return &x } func (hc *HealthCheck) merge() { + // Cannot specify both portSpecification and port field. + if hc.PortSpecification != "" { + hc.Port = 0 + } + // Zeroing out child settings as a precaution. GoogleAPI throws an error // if the wrong child struct is set. hc.HealthCheck.Http2HealthCheck = nil @@ -559,7 +588,8 @@ func (hc *HealthCheck) merge() { switch hc.Protocol() { case annotations.ProtocolHTTP: - hc.HealthCheck.HttpHealthCheck = &hc.HTTPHealthCheck + x := hc.HTTPHealthCheck // Make a copy to ensure no aliasing. + hc.HealthCheck.HttpHealthCheck = &x case annotations.ProtocolHTTPS: https := computealpha.HTTPSHealthCheck(hc.HTTPHealthCheck) hc.HealthCheck.HttpsHealthCheck = &https @@ -569,33 +599,89 @@ func (hc *HealthCheck) merge() { } } -func (hc *HealthCheck) isHttp2() bool { - return hc.Protocol() == annotations.ProtocolHTTP2 -} - // Version returns the appropriate API version to handle the health check // Use Beta API for NEG as PORT_SPECIFICATION is required, and HTTP2 func (hc *HealthCheck) Version() meta.Version { if hc.forILB { return features.L7ILBVersions().HealthCheck } - if hc.isHttp2() || hc.ForNEG { + if hc.Protocol() == annotations.ProtocolHTTP2 || hc.forNEG { return meta.VersionBeta } return meta.VersionGA } -func needToUpdate(old, new *HealthCheck) bool { - if old.Protocol() != new.Protocol() { - klog.V(2).Infof("Updating health check %v because it has protocol %v but need %v", old.Name, old.Type, new.Type) - return true +func (hc *HealthCheck) updateFromBackendConfig(c *backendconfigv1.HealthCheckConfig) { + if c.CheckIntervalSec != nil { + hc.CheckIntervalSec = *c.CheckIntervalSec + } + if c.TimeoutSec != nil { + hc.TimeoutSec = *c.TimeoutSec + } + if c.HealthyThreshold != nil { + hc.HealthyThreshold = *c.HealthyThreshold + } + if c.UnhealthyThreshold != nil { + hc.UnhealthyThreshold = *c.UnhealthyThreshold } + if c.Type != nil { + hc.Type = *c.Type + } + if c.RequestPath != nil { + hc.RequestPath = *c.RequestPath + } + if c.Port != nil { + klog.Warningf("Setting Port is not supported (healthcheck %q, backendconfig = %+v)", hc.Name, c) + } +} + +// fieldDiffs encapsulate which fields are different between health checks. +type fieldDiffs struct { + f []string +} + +func (c *fieldDiffs) add(field, oldv, newv string) { + c.f = append(c.f, fmt.Sprintf("%s:%s -> %s", field, oldv, newv)) +} +func (c *fieldDiffs) String() string { return strings.Join(c.f, ", ") } +func (c *fieldDiffs) hasDiff() bool { return len(c.f) > 0 } +func calculateDiff(old, new *HealthCheck, c *backendconfigv1.HealthCheckConfig) *fieldDiffs { + var changes fieldDiffs + + if old.Protocol() != new.Protocol() { + changes.add("Protocol", string(old.Protocol()), string(new.Protocol())) + } if old.PortSpecification != new.PortSpecification { - klog.V(2).Infof("Updating health check %v because it has port specification %q but need %q", old.Name, old.PortSpecification, new.PortSpecification) - return true + changes.add("PortSpecification", old.PortSpecification, new.PortSpecification) + } + + // TODO(bowei): why don't we check Port, timeout etc. + + if c == nil { + return &changes + } + + // This code assumes that the changes wrt to `c` has been applied to `new`. + if c.CheckIntervalSec != nil && old.CheckIntervalSec != new.CheckIntervalSec { + changes.add("CheckIntervalSec", strconv.FormatInt(old.CheckIntervalSec, 10), strconv.FormatInt(new.CheckIntervalSec, 10)) } - return false + if c.TimeoutSec != nil && old.TimeoutSec != new.TimeoutSec { + changes.add("TimeoutSec", strconv.FormatInt(old.TimeoutSec, 10), strconv.FormatInt(new.TimeoutSec, 10)) + } + if c.HealthyThreshold != nil && old.HealthyThreshold != new.HealthyThreshold { + changes.add("HeathyThreshold", strconv.FormatInt(old.HealthyThreshold, 10), strconv.FormatInt(new.HealthyThreshold, 10)) + } + if c.UnhealthyThreshold != nil && old.UnhealthyThreshold != new.UnhealthyThreshold { + changes.add("UnhealthyThreshold", strconv.FormatInt(old.UnhealthyThreshold, 10), strconv.FormatInt(new.UnhealthyThreshold, 10)) + } + // c.Type is handled by Protocol above. + if c.RequestPath != nil && old.RequestPath != new.RequestPath { + changes.add("RequestPath", old.RequestPath, new.RequestPath) + } + // TODO(bowei): Host seems to be missing. + + return &changes } // toV1HealthCheck converts alpha health check to v1 health check. @@ -652,13 +738,23 @@ func copyViaJSON(dest interface{}, src jsonConvertable) error { return json.Unmarshal(bytes, dest) } -// applyProbeSettingsToHC TODO +// applyProbeSettingsToHC takes the Pod healthcheck settings and applies it +// to the healthcheck. +// +// TODO: what if the port changes? +// TODO: does not handle protocol? func applyProbeSettingsToHC(p *v1.Probe, hc *HealthCheck) { + if p.Handler.HTTPGet == nil { + return + } + healthPath := p.Handler.HTTPGet.Path // GCE requires a leading "/" for health check urls. if !strings.HasPrefix(healthPath, "/") { healthPath = "/" + healthPath } + hc.RequestPath = healthPath + // Extract host from HTTP headers host := p.Handler.HTTPGet.Host for _, header := range p.Handler.HTTPGet.HTTPHeaders { @@ -667,16 +763,16 @@ func applyProbeSettingsToHC(p *v1.Probe, hc *HealthCheck) { break } } - - hc.RequestPath = healthPath hc.Host = host - hc.Description = "Kubernetes L7 health check generated with readiness probe settings." + hc.TimeoutSec = int64(p.TimeoutSeconds) - if hc.ForNEG { + if hc.forNEG { // For NEG mode, we can support more aggressive healthcheck interval. hc.CheckIntervalSec = int64(p.PeriodSeconds) } else { // For IG mode, short healthcheck interval may health check flooding problem. hc.CheckIntervalSec = int64(p.PeriodSeconds) + int64(DefaultHealthCheckInterval.Seconds()) } + + hc.Description = "Kubernetes L7 health check generated with readiness probe settings." } From 13844f711916ab948605c33d0133fc68cf63d199 Mon Sep 17 00:00:00 2001 From: Bowei Du Date: Mon, 17 Feb 2020 13:33:25 -0800 Subject: [PATCH 3/4] Add testing for `SyncServicePort` TestSyncServicePort does the following: 1. Setup any preexisting objects (tc.setup) that should exist in the mocked GCE. 2. Run one sync given a ServicePort. 3. Attempt to re-run the sync with the same ServicePort, checking that no update will be needed. --- pkg/healthchecks/healthchecks_test.go | 897 ++++++++++++++++++++++++-- 1 file changed, 832 insertions(+), 65 deletions(-) diff --git a/pkg/healthchecks/healthchecks_test.go b/pkg/healthchecks/healthchecks_test.go index ab449351f1..87c8230db3 100644 --- a/pkg/healthchecks/healthchecks_test.go +++ b/pkg/healthchecks/healthchecks_test.go @@ -17,61 +17,126 @@ limitations under the License. package healthchecks import ( + "context" + "fmt" "net/http" + "reflect" "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" + "github.com/kr/pretty" computealpha "google.golang.org/api/compute/v0.alpha" + computebeta "google.golang.org/api/compute/v0.beta" + "google.golang.org/api/compute/v1" api_v1 "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/ingress-gce/pkg/annotations" + backendconfigv1 "k8s.io/ingress-gce/pkg/apis/backendconfig/v1" "k8s.io/ingress-gce/pkg/utils" namer_util "k8s.io/ingress-gce/pkg/utils/namer" + "k8s.io/klog" "k8s.io/legacy-cloud-providers/gce" ) var ( - namer = namer_util.NewNamer("uid1", "fw1") - + testNamer = namer_util.NewNamer("uid1", "fw1") + testSPs = map[string]*utils.ServicePort{} defaultBackendSvc = types.NamespacedName{Namespace: "system", Name: "default"} ) +func init() { + // Init klog flags so we can see the V logs. + klog.InitFlags(nil) + // var logLevel string + // flag.StringVar(&logLevel, "logLevel", "4", "test") + // flag.Lookup("v").Value.Set(logLevel) + + // Generate many types of ServicePorts. + // Example: sps["HTTP-8000-neg-nil"] is a ServicePort for HTTP with NEG-enabled. + testSPs = map[string]*utils.ServicePort{} + for _, p := range []annotations.AppProtocol{ + annotations.ProtocolHTTP, + annotations.ProtocolHTTPS, + annotations.ProtocolHTTP2, + } { + path := "/foo" + num := int64(1234) + + for npk, np := range map[string]int64{"80": 80, "8000": 8000} { + for _, mode := range []string{"reg", "neg", "ilb"} { + for bck, bc := range map[string]*backendconfigv1.HealthCheckConfig{ + "nil": nil, + "bc": &backendconfigv1.HealthCheckConfig{RequestPath: &path}, + "bcall": &backendconfigv1.HealthCheckConfig{ + RequestPath: &path, + CheckIntervalSec: &num, + TimeoutSec: &num, + HealthyThreshold: &num, + UnhealthyThreshold: &num, + }, + } { + sp := &utils.ServicePort{ + NodePort: np, + Protocol: p, + BackendNamer: testNamer, + } + switch mode { + case "reg": + case "neg": + sp.NEGEnabled = true + case "ilb": + sp.NEGEnabled = true + sp.L7ILBEnabled = true + } + if bc != nil { + sp.BackendConfig = &backendconfigv1.BackendConfig{ + Spec: backendconfigv1.BackendConfigSpec{HealthCheck: bc}, + } + } + testSPs[fmt.Sprintf("%s-%s-%s-%s", p, npk, mode, bck)] = sp + } + } + } + } +} + func TestHealthCheckAdd(t *testing.T) { fakeGCE := gce.NewFakeGCECloud(gce.DefaultTestClusterValues()) healthChecks := NewHealthChecker(fakeGCE, "/", defaultBackendSvc) - sp := &utils.ServicePort{NodePort: 80, Protocol: annotations.ProtocolHTTP, NEGEnabled: false, BackendNamer: namer} + sp := &utils.ServicePort{NodePort: 80, Protocol: annotations.ProtocolHTTP, NEGEnabled: false, BackendNamer: testNamer} _, err := healthChecks.SyncServicePort(sp, nil) if err != nil { t.Fatalf("unexpected error: %v", err) } // Verify the health check exists - _, err = fakeGCE.GetHealthCheck(namer.IGBackend(80)) + _, err = fakeGCE.GetHealthCheck(testNamer.IGBackend(80)) if err != nil { t.Fatalf("expected the health check to exist, err: %v", err) } - sp = &utils.ServicePort{NodePort: 443, Protocol: annotations.ProtocolHTTPS, NEGEnabled: false, BackendNamer: namer} + sp = &utils.ServicePort{NodePort: 443, Protocol: annotations.ProtocolHTTPS, NEGEnabled: false, BackendNamer: testNamer} _, err = healthChecks.SyncServicePort(sp, nil) if err != nil { t.Fatalf("unexpected error: %v", err) } // Verify the health check exists - _, err = fakeGCE.GetHealthCheck(namer.IGBackend(443)) + _, err = fakeGCE.GetHealthCheck(testNamer.IGBackend(443)) if err != nil { t.Fatalf("expected the health check to exist, err: %v", err) } - sp = &utils.ServicePort{NodePort: 3000, Protocol: annotations.ProtocolHTTP2, NEGEnabled: false, BackendNamer: namer} + sp = &utils.ServicePort{NodePort: 3000, Protocol: annotations.ProtocolHTTP2, NEGEnabled: false, BackendNamer: testNamer} _, err = healthChecks.SyncServicePort(sp, nil) if err != nil { t.Fatalf("unexpected error: %v", err) } // Verify the health check exists - _, err = fakeGCE.GetHealthCheck(namer.IGBackend(3000)) + _, err = fakeGCE.GetHealthCheck(testNamer.IGBackend(3000)) if err != nil { t.Fatalf("expected the health check to exist, err: %v", err) } @@ -84,7 +149,7 @@ func TestHealthCheckAddExisting(t *testing.T) { // HTTP // Manually insert a health check httpHC := DefaultHealthCheck(3000, annotations.ProtocolHTTP) - httpHC.Name = namer.IGBackend(3000) + httpHC.Name = testNamer.IGBackend(3000) httpHC.RequestPath = "/my-probes-health" v1hc, err := httpHC.ToComputeHealthCheck() if err != nil { @@ -92,7 +157,7 @@ func TestHealthCheckAddExisting(t *testing.T) { } fakeGCE.CreateHealthCheck(v1hc) - sp := &utils.ServicePort{NodePort: 3000, Protocol: annotations.ProtocolHTTP, NEGEnabled: false, BackendNamer: namer} + sp := &utils.ServicePort{NodePort: 3000, Protocol: annotations.ProtocolHTTP, NEGEnabled: false, BackendNamer: testNamer} // Should not fail adding the same type of health check _, err = healthChecks.SyncServicePort(sp, nil) if err != nil { @@ -107,7 +172,7 @@ func TestHealthCheckAddExisting(t *testing.T) { // HTTPS // Manually insert a health check httpsHC := DefaultHealthCheck(4000, annotations.ProtocolHTTPS) - httpsHC.Name = namer.IGBackend(4000) + httpsHC.Name = testNamer.IGBackend(4000) httpsHC.RequestPath = "/my-probes-health" v1hc, err = httpHC.ToComputeHealthCheck() if err != nil { @@ -115,7 +180,7 @@ func TestHealthCheckAddExisting(t *testing.T) { } fakeGCE.CreateHealthCheck(v1hc) - sp = &utils.ServicePort{NodePort: 4000, Protocol: annotations.ProtocolHTTPS, NEGEnabled: false, BackendNamer: namer} + sp = &utils.ServicePort{NodePort: 4000, Protocol: annotations.ProtocolHTTPS, NEGEnabled: false, BackendNamer: testNamer} _, err = healthChecks.SyncServicePort(sp, nil) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -129,7 +194,7 @@ func TestHealthCheckAddExisting(t *testing.T) { // HTTP2 // Manually insert a health check http2 := DefaultHealthCheck(5000, annotations.ProtocolHTTP2) - http2.Name = namer.IGBackend(5000) + http2.Name = testNamer.IGBackend(5000) http2.RequestPath = "/my-probes-health" v1hc, err = httpHC.ToComputeHealthCheck() if err != nil { @@ -137,7 +202,7 @@ func TestHealthCheckAddExisting(t *testing.T) { } fakeGCE.CreateHealthCheck(v1hc) - sp = &utils.ServicePort{NodePort: 5000, Protocol: annotations.ProtocolHTTPS, NEGEnabled: false, BackendNamer: namer} + sp = &utils.ServicePort{NodePort: 5000, Protocol: annotations.ProtocolHTTPS, NEGEnabled: false, BackendNamer: testNamer} _, err = healthChecks.SyncServicePort(sp, nil) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -155,7 +220,7 @@ func TestHealthCheckDelete(t *testing.T) { // Create HTTP HC for 1234 hc := DefaultHealthCheck(1234, annotations.ProtocolHTTP) - hc.Name = namer.IGBackend(1234) + hc.Name = testNamer.IGBackend(1234) v1hc, err := hc.ToComputeHealthCheck() if err != nil { t.Fatalf("unexpected error: %v", err) @@ -167,7 +232,7 @@ func TestHealthCheckDelete(t *testing.T) { fakeGCE.CreateHealthCheck(v1hc) // Delete only HTTP 1234 - err = healthChecks.Delete(namer.IGBackend(1234), meta.Global) + err = healthChecks.Delete(testNamer.IGBackend(1234), meta.Global) if err != nil { t.Errorf("unexpected error when deleting health check, err: %v", err) } @@ -179,7 +244,7 @@ func TestHealthCheckDelete(t *testing.T) { } // Delete only HTTP 1234 - err = healthChecks.Delete(namer.IGBackend(1234), meta.Global) + err = healthChecks.Delete(testNamer.IGBackend(1234), meta.Global) if err == nil { t.Errorf("expected not-found error when deleting health check, err: %v", err) } @@ -191,13 +256,13 @@ func TestHTTP2HealthCheckDelete(t *testing.T) { // Create HTTP2 HC for 1234 hc := DefaultHealthCheck(1234, annotations.ProtocolHTTP2) - hc.Name = namer.IGBackend(1234) + hc.Name = testNamer.IGBackend(1234) alphahc := hc.ToAlphaComputeHealthCheck() fakeGCE.CreateAlphaHealthCheck(alphahc) // Delete only HTTP2 1234 - err := healthChecks.Delete(namer.IGBackend(1234), meta.Global) + err := healthChecks.Delete(testNamer.IGBackend(1234), meta.Global) if err != nil { t.Errorf("unexpected error when deleting health check, err: %v", err) } @@ -222,7 +287,7 @@ func TestHealthCheckUpdate(t *testing.T) { // HTTP // Manually insert a health check hc := DefaultHealthCheck(3000, annotations.ProtocolHTTP) - hc.Name = namer.IGBackend(3000) + hc.Name = testNamer.IGBackend(3000) hc.RequestPath = "/my-probes-health" v1hc, err := hc.ToComputeHealthCheck() if err != nil { @@ -238,7 +303,7 @@ func TestHealthCheckUpdate(t *testing.T) { // Change to HTTPS hc.Type = string(annotations.ProtocolHTTPS) - _, err = healthChecks.sync(hc) + _, err = healthChecks.sync(hc, nil) if err != nil { t.Fatalf("unexpected err while syncing healthcheck, err %v", err) } @@ -256,7 +321,7 @@ func TestHealthCheckUpdate(t *testing.T) { // Change to HTTP2 hc.Type = string(annotations.ProtocolHTTP2) - _, err = healthChecks.sync(hc) + _, err = healthChecks.sync(hc, nil) if err != nil { t.Fatalf("unexpected err while syncing healthcheck, err %v", err) } @@ -273,9 +338,9 @@ func TestHealthCheckUpdate(t *testing.T) { } // Change to NEG Health Check - hc.ForNEG = true + hc.forNEG = true hc.PortSpecification = UseServingPortSpecification - _, err = healthChecks.sync(hc) + _, err = healthChecks.sync(hc, nil) if err != nil { t.Fatalf("unexpected err while syncing healthcheck, err %v", err) @@ -292,11 +357,11 @@ func TestHealthCheckUpdate(t *testing.T) { } // Change back to regular Health Check - hc.ForNEG = false + hc.forNEG = false hc.Port = 3000 hc.PortSpecification = "" - _, err = healthChecks.sync(hc) + _, err = healthChecks.sync(hc, nil) if err != nil { t.Fatalf("unexpected err while syncing healthcheck, err %v", err) } @@ -316,28 +381,6 @@ func TestHealthCheckUpdate(t *testing.T) { } } -func TestAlphaHealthCheck(t *testing.T) { - fakeGCE := gce.NewFakeGCECloud(gce.DefaultTestClusterValues()) - healthChecks := NewHealthChecker(fakeGCE, "/", defaultBackendSvc) - sp := utils.ServicePort{NodePort: 8000, Protocol: annotations.ProtocolHTTPS, NEGEnabled: true, BackendNamer: namer} - hc := healthChecks.new(sp) - _, err := healthChecks.sync(hc) - if err != nil { - t.Fatalf("got %v, want nil", err) - } - - ret, err := healthChecks.Get(hc.Name, meta.VersionAlpha, meta.Global) - if err != nil { - t.Fatalf("got %v, want nil", err) - } - if ret.Port != 0 { - t.Errorf("got ret.Port == %d, want 0", ret.Port) - } - if ret.PortSpecification != UseServingPortSpecification { - t.Errorf("got ret.PortSpecification = %q, want %q", UseServingPortSpecification, ret.PortSpecification) - } -} - func TestVersion(t *testing.T) { t.Parallel() testCases := []struct { @@ -369,7 +412,7 @@ func TestVersion(t *testing.T) { HealthCheck: computealpha.HealthCheck{ Type: string(annotations.ProtocolHTTP), }, - ForNEG: true, + forNEG: true, }, version: meta.VersionBeta, }, @@ -380,7 +423,7 @@ func TestVersion(t *testing.T) { Type: string(annotations.ProtocolHTTP), }, forILB: true, - ForNEG: true, + forNEG: true, }, version: meta.VersionBeta, }, @@ -397,27 +440,751 @@ func TestVersion(t *testing.T) { } func TestApplyProbeSettingsToHC(t *testing.T) { - p := "healthz" - hc := DefaultHealthCheck(8080, annotations.ProtocolHTTPS) - probe := &api_v1.Probe{ - Handler: api_v1.Handler{ - HTTPGet: &api_v1.HTTPGetAction{ - Scheme: api_v1.URISchemeHTTP, - Path: p, - Port: intstr.IntOrString{ - Type: intstr.Int, - IntVal: 80, + type ws struct { + host string + path string + timeoutSec int64 + checkIntervalSec int64 + port int64 + } + for _, tc := range []struct { + desc string + neg bool + probe *api_v1.Probe + want ws + }{ + { + desc: "no override", + probe: &api_v1.Probe{}, + want: ws{timeoutSec: 60, checkIntervalSec: 60, port: 8080}, + }, + { + desc: "override path", + probe: &api_v1.Probe{ + TimeoutSeconds: 1234, + Handler: api_v1.Handler{ + HTTPGet: &api_v1.HTTPGetAction{Path: "/override"}, + }, + }, + want: ws{path: "/override", timeoutSec: 1234, checkIntervalSec: 60, port: 8080}, + }, + { + desc: "override host", + probe: &api_v1.Probe{ + TimeoutSeconds: 1234, + Handler: api_v1.Handler{ + HTTPGet: &api_v1.HTTPGetAction{Host: "foo.com"}, }, }, + want: ws{host: "foo.com", path: "/", timeoutSec: 1234, checkIntervalSec: 60, port: 8080}, + }, + { + desc: "override host (via header)", + probe: &api_v1.Probe{ + TimeoutSeconds: 1234, + Handler: api_v1.Handler{ + HTTPGet: &api_v1.HTTPGetAction{ + HTTPHeaders: []api_v1.HTTPHeader{{Name: "Host", Value: "foo.com"}}, + }, + }, + }, + want: ws{host: "foo.com", path: "/", timeoutSec: 1234, checkIntervalSec: 60, port: 8080}, + }, + { + // TODO(bowei): this is somewhat subtle behavior. + desc: "ignore port", + probe: &api_v1.Probe{ + TimeoutSeconds: 1234, + Handler: api_v1.Handler{ + HTTPGet: &api_v1.HTTPGetAction{Port: intstr.FromInt(3000)}, + }, + }, + want: ws{path: "/", timeoutSec: 1234, checkIntervalSec: 60, port: 8080}, + }, + { + desc: "override timeouts", + probe: &api_v1.Probe{ + TimeoutSeconds: 50, + PeriodSeconds: 100, + Handler: api_v1.Handler{ + HTTPGet: &api_v1.HTTPGetAction{Port: intstr.FromInt(3000)}, + }, + }, + want: ws{path: "/", timeoutSec: 50, checkIntervalSec: 160, port: 8080}, + }, + { + desc: "override timeouts (neg)", + neg: true, + probe: &api_v1.Probe{ + TimeoutSeconds: 50, + PeriodSeconds: 100, + Handler: api_v1.Handler{ + HTTPGet: &api_v1.HTTPGetAction{Port: intstr.FromInt(3000)}, + }, + }, + want: ws{path: "/", timeoutSec: 50, checkIntervalSec: 100, port: 0}, + }, + } { + t.Run(tc.desc, func(t *testing.T) { + var got *HealthCheck + if tc.neg { + got = DefaultNEGHealthCheck(annotations.ProtocolHTTP) + } else { + got = DefaultHealthCheck(8080, annotations.ProtocolHTTP) + } + applyProbeSettingsToHC(tc.probe, got) + + if got.Host != tc.want.host { + t.Errorf("got.Host = %q, want %q", got.Host, tc.want.host) + } + if got.RequestPath != tc.want.path { + t.Errorf("got.RequestPath = %q, want %q", got.RequestPath, tc.want.path) + } + if got.TimeoutSec != tc.want.timeoutSec { + t.Errorf("got.TimeoutSec = %d, want %d", got.TimeoutSec, tc.want.timeoutSec) + } + if got.CheckIntervalSec != tc.want.checkIntervalSec { + t.Errorf("got.CheckIntervalSec = %d, want %d", got.CheckIntervalSec, tc.want.checkIntervalSec) + } + if got.Port != tc.want.port { + t.Errorf("got.Port = %d, want %d", got.Port, tc.want.port) + } + }) + } +} + +func TestCalculateDiff(t *testing.T) { + t.Parallel() + + type tc struct { + desc string + old, new *HealthCheck + c *backendconfigv1.HealthCheckConfig + hasDiff bool + } + cases := []tc{ + { + desc: "HTTP, no change", + old: DefaultHealthCheck(8080, annotations.ProtocolHTTP), + new: DefaultHealthCheck(8080, annotations.ProtocolHTTP), + }, + { + desc: "HTTP => HTTPS", + old: DefaultHealthCheck(8080, annotations.ProtocolHTTP), + new: DefaultHealthCheck(8080, annotations.ProtocolHTTPS), + hasDiff: true, + }, + { + // TODO(bowei) -- this seems wrong. + desc: "Port change", + old: DefaultHealthCheck(8080, annotations.ProtocolHTTP), + new: DefaultHealthCheck(443, annotations.ProtocolHTTP), + // hasDiff: true + }, + { + desc: "PortSpecification", + old: DefaultHealthCheck(8080, annotations.ProtocolHTTP), + new: DefaultNEGHealthCheck(annotations.ProtocolHTTP), + hasDiff: true, + }, + } + + // TODO(bowei) -- this seems wrong, we should be checking for Port changes. + newHC := DefaultHealthCheck(8080, annotations.ProtocolHTTP) + newHC.Port = 80 + cases = append(cases, tc{ + desc: "Port", + old: DefaultHealthCheck(8080, annotations.ProtocolHTTP), + new: newHC, + // hasDiff: true + }) + + newHC = DefaultHealthCheck(8080, annotations.ProtocolHTTP) + newHC.CheckIntervalSec = 1000 + cases = append(cases, tc{ + desc: "ignore changes without backend config", + old: DefaultHealthCheck(8080, annotations.ProtocolHTTP), + new: newHC, + }) + + i64 := func(i int64) *int64 { return &i } + s := func(s string) *string { return &s } + + newHC = DefaultHealthCheck(8080, annotations.ProtocolHTTP) + newHC.CheckIntervalSec = 1000 + cases = append(cases, tc{ + desc: "CheckIntervalSec", + old: DefaultHealthCheck(8080, annotations.ProtocolHTTP), + new: newHC, + c: &backendconfigv1.HealthCheckConfig{CheckIntervalSec: i64(1000)}, + hasDiff: true, + }) + + newHC = DefaultHealthCheck(8080, annotations.ProtocolHTTP) + newHC.TimeoutSec = 1000 + cases = append(cases, tc{ + desc: "TimeoutSec", + old: DefaultHealthCheck(8080, annotations.ProtocolHTTP), + new: newHC, + c: &backendconfigv1.HealthCheckConfig{TimeoutSec: i64(1000)}, + hasDiff: true, + }) + + newHC = DefaultHealthCheck(8080, annotations.ProtocolHTTP) + newHC.HealthyThreshold = 1000 + cases = append(cases, tc{ + desc: "HealthyThreshold", + old: DefaultHealthCheck(8080, annotations.ProtocolHTTP), + new: newHC, + c: &backendconfigv1.HealthCheckConfig{HealthyThreshold: i64(1000)}, + hasDiff: true, + }) + + newHC = DefaultHealthCheck(8080, annotations.ProtocolHTTP) + newHC.UnhealthyThreshold = 1000 + cases = append(cases, tc{ + desc: "UnhealthyThreshold", + old: DefaultHealthCheck(8080, annotations.ProtocolHTTP), + new: newHC, + c: &backendconfigv1.HealthCheckConfig{UnhealthyThreshold: i64(1000)}, + hasDiff: true, + }) + + newHC = DefaultHealthCheck(8080, annotations.ProtocolHTTP) + newHC.RequestPath = "/foo" + cases = append(cases, tc{ + desc: "RequestPath", + old: DefaultHealthCheck(8080, annotations.ProtocolHTTP), + new: newHC, + c: &backendconfigv1.HealthCheckConfig{RequestPath: s("/foo")}, + hasDiff: true, + }) + + newHC = DefaultHealthCheck(8080, annotations.ProtocolHTTP) + newHC.RequestPath = "/foo" + cases = append(cases, tc{ + desc: "non-backendconfig field is not a diff", + old: DefaultHealthCheck(8080, annotations.ProtocolHTTP), + new: newHC, + c: &backendconfigv1.HealthCheckConfig{}, + }) + + newHC = DefaultHealthCheck(8080, annotations.ProtocolHTTP) + newHC.TimeoutSec = 1000 + newHC.RequestPath = "/foo" + cases = append(cases, tc{ + desc: "multiple changes", + old: DefaultHealthCheck(8080, annotations.ProtocolHTTP), + new: newHC, + c: &backendconfigv1.HealthCheckConfig{TimeoutSec: i64(1000), RequestPath: s("/foo")}, + hasDiff: true, + }) + + for _, tc := range cases { + t.Run(tc.desc, func(t *testing.T) { + diffs := calculateDiff(tc.old, tc.new, tc.c) + t.Logf("\nold=%+v\nnew=%+v\ndiffs = %s", tc.old, tc.new, diffs) + if diffs.hasDiff() != tc.hasDiff { + t.Errorf("calculateDiff(%+v, %+v, %+v) = %s; hasDiff = %t, want %t", tc.old, tc.new, tc.c, diffs, diffs.hasDiff(), tc.hasDiff) + } + }) + } +} + +const ( + regSelfLink = "https://www.googleapis.com/compute/v1/projects/test-project/global/healthChecks/k8s-be-80--uid1" + negSelfLink = "https://www.googleapis.com/compute/beta/projects/test-project/global/healthChecks/k8s1-uid1---0-56ff9a48" + ilbSelfLink = "https://www.googleapis.com/compute/beta/projects/test-project/regions/us-central1/healthChecks/k8s1-uid1---0-56ff9a48" +) + +type syncSPFixture struct{} + +func (*syncSPFixture) hc() *compute.HealthCheck { + return &compute.HealthCheck{ + Name: "k8s-be-80--uid1", + CheckIntervalSec: 60, + HealthyThreshold: 1, + TimeoutSec: 60, + Type: "HTTP", + UnhealthyThreshold: 10, + SelfLink: regSelfLink, + HttpHealthCheck: &compute.HTTPHealthCheck{ + Port: 80, + RequestPath: "/", }, } +} + +func (f *syncSPFixture) hcs() *compute.HealthCheck { return f.toS(f.hc()) } +func (f *syncSPFixture) hc2() *compute.HealthCheck { return f.to2(f.hc()) } +func (f *syncSPFixture) negs() *compute.HealthCheck { return f.toS(f.neg()) } +func (f *syncSPFixture) neg2() *compute.HealthCheck { return f.to2(f.neg()) } +func (f *syncSPFixture) ilbs() *compute.HealthCheck { return f.toS(f.ilb()) } +func (f *syncSPFixture) ilb2() *compute.HealthCheck { return f.to2(f.ilb()) } + +func (f *syncSPFixture) toS(h *compute.HealthCheck) *compute.HealthCheck { + h.Type = "HTTPS" + c := (compute.HTTPSHealthCheck)(*h.HttpHealthCheck) + h.HttpsHealthCheck = &c + h.HttpHealthCheck = nil + return h +} + +func (f *syncSPFixture) to2(h *compute.HealthCheck) *compute.HealthCheck { + h.Type = "HTTP2" + c := (compute.HTTP2HealthCheck)(*h.HttpHealthCheck) + h.Http2HealthCheck = &c + h.HttpHealthCheck = nil + return h +} - applyProbeSettingsToHC(probe, hc) +func (*syncSPFixture) neg() *compute.HealthCheck { + return &compute.HealthCheck{ + Name: "k8s1-uid1---0-56ff9a48", + CheckIntervalSec: 15, + HealthyThreshold: 1, + TimeoutSec: 15, + Type: "HTTP", + UnhealthyThreshold: 2, + SelfLink: negSelfLink, + HttpHealthCheck: &compute.HTTPHealthCheck{ + PortSpecification: UseServingPortSpecification, + RequestPath: "/", + }, + } +} + +func (f *syncSPFixture) ilb() *compute.HealthCheck { + h := f.neg() + h.Region = "us-central1" + h.SelfLink = ilbSelfLink + return h +} + +func (f *syncSPFixture) setupExistingHCFunc(hc *compute.HealthCheck) func(m *cloud.MockGCE) { + return func(m *cloud.MockGCE) { + key := meta.GlobalKey(hc.Name) + m.HealthChecks().Insert(context.Background(), key, hc) + } +} + +func (f *syncSPFixture) setupExistingRegionalHCFunc(region string, hc *compute.HealthCheck) func(m *cloud.MockGCE) { + return func(m *cloud.MockGCE) { + key := meta.RegionalKey(hc.Name, region) + m.RegionHealthChecks().Insert(context.Background(), key, hc) + } +} - if hc.Protocol() != annotations.ProtocolHTTPS || hc.Port != 8080 { - t.Errorf("Basic HC settings changed") +func setupMockUpdate(mock *cloud.MockGCE) { + // Mock does not know how to autogenerate update() methods. + mock.MockHealthChecks.UpdateHook = func(ctx context.Context, k *meta.Key, hc *compute.HealthCheck, m *cloud.MockHealthChecks) error { + m.Objects[*k].Obj = hc + return nil + } + mock.MockAlphaHealthChecks.UpdateHook = func(ctx context.Context, k *meta.Key, hc *computealpha.HealthCheck, m *cloud.MockAlphaHealthChecks) error { + m.Objects[*k].Obj = hc + return nil + } + mock.MockBetaHealthChecks.UpdateHook = func(ctx context.Context, k *meta.Key, hc *computebeta.HealthCheck, m *cloud.MockBetaHealthChecks) error { + m.Objects[*k].Obj = hc + return nil } - if hc.RequestPath != "/"+p { - t.Errorf("Failed to apply probe's requestpath") + mock.MockRegionHealthChecks.UpdateHook = func(ctx context.Context, k *meta.Key, hc *compute.HealthCheck, m *cloud.MockRegionHealthChecks) error { + m.Objects[*k].Obj = hc + return nil + } + mock.MockAlphaRegionHealthChecks.UpdateHook = func(ctx context.Context, k *meta.Key, hc *computealpha.HealthCheck, m *cloud.MockAlphaRegionHealthChecks) error { + m.Objects[*k].Obj = hc + return nil + } + mock.MockBetaRegionHealthChecks.UpdateHook = func(ctx context.Context, k *meta.Key, hc *computebeta.HealthCheck, m *cloud.MockBetaRegionHealthChecks) error { + m.Objects[*k].Obj = hc + return nil } } + +func TestSyncServicePort(t *testing.T) { + type tc struct { + desc string + setup func(*cloud.MockGCE) + sp *utils.ServicePort + probe *v1.Probe + regional bool + + wantSelfLink string + wantErr bool + wantComputeHC *compute.HealthCheck + } + fixture := syncSPFixture{} + + var cases []*tc + + cases = append(cases, &tc{desc: "create http", sp: testSPs["HTTP-80-reg-nil"], wantComputeHC: fixture.hc()}) + cases = append(cases, &tc{desc: "create https", sp: testSPs["HTTPS-80-reg-nil"], wantComputeHC: fixture.hcs()}) + cases = append(cases, &tc{desc: "create http2", sp: testSPs["HTTP2-80-reg-nil"], wantComputeHC: fixture.hc2()}) + cases = append(cases, &tc{desc: "create neg", sp: testSPs["HTTP-80-neg-nil"], wantComputeHC: fixture.neg()}) + cases = append(cases, &tc{desc: "create ilb", sp: testSPs["HTTP-80-ilb-nil"], regional: true, wantComputeHC: fixture.ilb()}) + + // Probe override + chc := fixture.hc() + chc.HttpHealthCheck.RequestPath = "/foo" + chc.HttpHealthCheck.Host = "foo.com" + chc.CheckIntervalSec = 61 + chc.TimeoutSec = 1234 + cases = append(cases, &tc{ + desc: "create probe", + sp: testSPs["HTTP-80-reg-nil"], + probe: &v1.Probe{ + Handler: v1.Handler{ + HTTPGet: &v1.HTTPGetAction{Path: "/foo", Host: "foo.com"}, + }, + PeriodSeconds: 1, + TimeoutSeconds: 1234, + }, + wantComputeHC: chc, + }) + + // Probe override (NEG) + chc = fixture.neg() + chc.HttpHealthCheck.RequestPath = "/foo" + chc.HttpHealthCheck.Host = "foo.com" + chc.CheckIntervalSec = 1234 + chc.TimeoutSec = 5678 + cases = append(cases, &tc{ + desc: "create probe neg", + sp: testSPs["HTTP-80-neg-nil"], + probe: &v1.Probe{ + Handler: v1.Handler{ + HTTPGet: &v1.HTTPGetAction{Path: "/foo", Host: "foo.com"}, + }, + PeriodSeconds: 1234, + TimeoutSeconds: 5678, + }, + wantComputeHC: chc, + }) + + // Probe override (ILB) + chc = fixture.ilb() + chc.HttpHealthCheck.RequestPath = "/foo" + chc.HttpHealthCheck.Host = "foo.com" + chc.CheckIntervalSec = 1234 + chc.TimeoutSec = 5678 + cases = append(cases, &tc{ + desc: "create probe ilb", + sp: testSPs["HTTP-80-ilb-nil"], + regional: true, + probe: &v1.Probe{ + Handler: v1.Handler{ + HTTPGet: &v1.HTTPGetAction{Path: "/foo", Host: "foo.com"}, + }, + PeriodSeconds: 1234, + TimeoutSeconds: 5678, + }, + wantComputeHC: chc, + }) + + // BackendConfig + chc = fixture.hc() + chc.HttpHealthCheck.RequestPath = "/foo" + cases = append(cases, &tc{desc: "create backendconfig", sp: testSPs["HTTP-80-reg-bc"], wantComputeHC: chc}) + + // BackendConfig all + chc = fixture.hc() + chc.HttpHealthCheck.RequestPath = "/foo" + chc.CheckIntervalSec = 1234 + chc.HealthyThreshold = 1234 + chc.UnhealthyThreshold = 1234 + chc.TimeoutSec = 1234 + cases = append(cases, &tc{desc: "create backendconfig all", sp: testSPs["HTTP-80-reg-bcall"], wantComputeHC: chc}) + + // BackendConfig neg + chc = fixture.neg() + chc.HttpHealthCheck.RequestPath = "/foo" + cases = append(cases, &tc{ + desc: "create backendconfig neg", + sp: testSPs["HTTP-80-neg-bc"], + wantComputeHC: chc, + }) + + // BackendConfig ilb + chc = fixture.ilb() + chc.HttpHealthCheck.RequestPath = "/foo" + cases = append(cases, &tc{ + desc: "create backendconfig ilb", + sp: testSPs["HTTP-80-ilb-bc"], + regional: true, + wantComputeHC: chc, + }) + + // Probe and BackendConfig override + chc = fixture.hc() + chc.HttpHealthCheck.RequestPath = "/foo" + chc.HttpHealthCheck.Host = "foo.com" + chc.CheckIntervalSec = 61 + chc.TimeoutSec = 1234 + cases = append(cases, &tc{ + desc: "create probe and backendconfig", + sp: testSPs["HTTP-80-reg-bc"], + probe: &v1.Probe{ + Handler: v1.Handler{ + HTTPGet: &v1.HTTPGetAction{Path: "/bar", Host: "foo.com"}, + }, + PeriodSeconds: 1, + TimeoutSeconds: 1234, + }, + wantComputeHC: chc, + }) + + // Update tests + cases = append(cases, &tc{ + desc: "update http no change", + setup: fixture.setupExistingHCFunc(fixture.hc()), + sp: testSPs["HTTP-80-reg-nil"], + wantComputeHC: fixture.hc(), + }) + cases = append(cases, &tc{ + desc: "update https no change", + setup: fixture.setupExistingHCFunc(fixture.hcs()), + sp: testSPs["HTTPS-80-reg-nil"], + wantComputeHC: fixture.hcs(), + }) + cases = append(cases, &tc{ + desc: "update http2 no change", + setup: fixture.setupExistingHCFunc(fixture.hc2()), + sp: testSPs["HTTP2-80-reg-nil"], + wantComputeHC: fixture.hc2(), + }) + cases = append(cases, &tc{ + desc: "update neg no change", + setup: fixture.setupExistingHCFunc(fixture.neg()), + sp: testSPs["HTTP-80-neg-nil"], + wantComputeHC: fixture.neg(), + }) + cases = append(cases, &tc{ + desc: "update ilb no change", + setup: fixture.setupExistingHCFunc(fixture.ilb()), + sp: testSPs["HTTP-80-ilb-nil"], + regional: true, + wantComputeHC: fixture.ilb(), + }) + + // Update protocol + cases = append(cases, &tc{ + desc: "update http to https", + setup: fixture.setupExistingHCFunc(fixture.hc()), + sp: testSPs["HTTPS-80-reg-nil"], + wantComputeHC: fixture.hcs(), + }) + cases = append(cases, &tc{ + desc: "update http to http2", + setup: fixture.setupExistingHCFunc(fixture.hc()), + sp: testSPs["HTTP2-80-reg-nil"], + wantComputeHC: fixture.hc2(), + }) + cases = append(cases, &tc{ + desc: "update https to http", + setup: fixture.setupExistingHCFunc(fixture.hcs()), + sp: testSPs["HTTP-80-reg-nil"], + wantComputeHC: fixture.hc(), + }) + cases = append(cases, &tc{ + desc: "update https to http2", + setup: fixture.setupExistingHCFunc(fixture.hcs()), + sp: testSPs["HTTP2-80-reg-nil"], + wantComputeHC: fixture.hc2(), + }) + cases = append(cases, &tc{ + desc: "update neg protocol", + setup: fixture.setupExistingHCFunc(fixture.neg()), + sp: testSPs["HTTPS-80-neg-nil"], + wantComputeHC: fixture.negs(), + }) + cases = append(cases, &tc{ + desc: "update ilb protocol", + setup: fixture.setupExistingRegionalHCFunc("us-central1", fixture.ilb()), + sp: testSPs["HTTPS-80-ilb-nil"], + regional: true, + wantComputeHC: fixture.ilbs(), + }) + + // Preserve user settings. + chc = fixture.hc() + chc.HttpHealthCheck.RequestPath = "/user-path" + chc.CheckIntervalSec = 1234 + cases = append(cases, &tc{ + desc: "update preserve", + setup: fixture.setupExistingHCFunc(chc), + sp: testSPs["HTTP-80-reg-nil"], + wantComputeHC: chc, + }) + + // Preserve user settings while changing the protocol + chc = fixture.hc() + chc.HttpHealthCheck.RequestPath = "/user-path" + chc.CheckIntervalSec = 1234 + wantCHC := fixture.hcs() + wantCHC.HttpsHealthCheck.RequestPath = "/user-path" + wantCHC.CheckIntervalSec = 1234 + cases = append(cases, &tc{ + desc: "update preserve across protocol change", + setup: fixture.setupExistingHCFunc(chc), + sp: testSPs["HTTPS-80-reg-nil"], + wantComputeHC: wantCHC, + }) + + // Preserve user settings while changing the protocol (neg) + chc = fixture.neg() + chc.HttpHealthCheck.RequestPath = "/user-path" + chc.CheckIntervalSec = 1234 + wantCHC = fixture.negs() + wantCHC.HttpsHealthCheck.RequestPath = "/user-path" + wantCHC.CheckIntervalSec = 1234 + cases = append(cases, &tc{ + desc: "update preserve across protocol change neg", + setup: fixture.setupExistingHCFunc(chc), + sp: testSPs["HTTPS-80-neg-nil"], + wantComputeHC: wantCHC, + }) + + // Preserve user settings while changing the protocol (ilb) + chc = fixture.ilb() + chc.HttpHealthCheck.RequestPath = "/user-path" + chc.CheckIntervalSec = 1234 + wantCHC = fixture.ilbs() + wantCHC.HttpsHealthCheck.RequestPath = "/user-path" + wantCHC.CheckIntervalSec = 1234 + cases = append(cases, &tc{ + desc: "update preserve across protocol change ilb", + setup: fixture.setupExistingRegionalHCFunc("us-central1", chc), + sp: testSPs["HTTPS-80-ilb-nil"], + regional: true, + wantComputeHC: wantCHC, + }) + + // Preserve some settings, but override some in the backend config + chc = fixture.hc() + chc.HttpHealthCheck.RequestPath = "/user-path" + chc.CheckIntervalSec = 1234 + wantCHC = fixture.hc() + wantCHC.HttpHealthCheck.RequestPath = "/foo" // from bc + wantCHC.CheckIntervalSec = 1234 // same + cases = append(cases, &tc{ + desc: "update preserve and backendconfig (path)", + sp: testSPs["HTTP-80-reg-bc"], + setup: fixture.setupExistingHCFunc(chc), + wantComputeHC: wantCHC, + }) + + // Override all settings from backendconfig. + chc = fixture.hc() + chc.HttpHealthCheck.RequestPath = "/user-path" + chc.CheckIntervalSec = 1234 + wantCHC = fixture.hc() + wantCHC.HttpHealthCheck.RequestPath = "/foo" // from bc + wantCHC.CheckIntervalSec = 1234 + wantCHC.HealthyThreshold = 1234 + wantCHC.UnhealthyThreshold = 1234 + wantCHC.TimeoutSec = 1234 + cases = append(cases, &tc{ + desc: "update preserve backendconfig all", + setup: fixture.setupExistingHCFunc(chc), + sp: testSPs["HTTP-80-reg-bcall"], + wantComputeHC: wantCHC, + }) + + // BUG: changing probe settings has not effect on the healthcheck + // update. + // TODO(bowei): document this. + chc = fixture.hc() + chc.HttpHealthCheck.RequestPath = "/user-path" + cases = append(cases, &tc{ + desc: "update probe has no effect (bug)", + setup: fixture.setupExistingHCFunc(chc), + sp: testSPs["HTTP-80-reg-nil"], + probe: &v1.Probe{ + Handler: v1.Handler{ + HTTPGet: &v1.HTTPGetAction{Path: "/foo", Host: "foo.com"}, + }, + PeriodSeconds: 1, + TimeoutSeconds: 1234, + }, + wantComputeHC: chc, + }) + + // BUG: Enable NEG, leaks old healthcheck, does not preserve old + // settings. + + // BUG: Switch to/from ILB, leaks old healthcheck, does not preserve old + // settings. + + for _, tc := range cases { + t.Run(tc.desc, func(t *testing.T) { + fakeGCE := gce.NewFakeGCECloud(gce.DefaultTestClusterValues()) + + mock := fakeGCE.Compute().(*cloud.MockGCE) + setupMockUpdate(mock) + + if tc.setup != nil { + tc.setup(mock) + } + + hcs := NewHealthChecker(fakeGCE, "/", defaultBackendSvc) + + gotSelfLink, err := hcs.SyncServicePort(tc.sp, tc.probe) + if gotErr := err != nil; gotErr != tc.wantErr { + t.Errorf("hcs.SyncServicePort(tc.sp, tc.probe) = _, %v; gotErr = %t, want %t\nsp = %s\nprobe = %s", err, gotErr, tc.wantErr, pretty.Sprint(tc.sp), pretty.Sprint(tc.probe)) + } + if tc.wantSelfLink != "" && gotSelfLink != tc.wantSelfLink { + t.Errorf("hcs.SyncServicePort(tc.sp, tc.probe) = %q, _; want = %q", gotSelfLink, tc.wantSelfLink) + } + + verify := func() { + t.Helper() + var computeHCs []*compute.HealthCheck + if tc.regional { + computeHCs, _ = fakeGCE.Compute().RegionHealthChecks().List(context.Background(), fakeGCE.Region(), nil) + } else { + computeHCs, _ = fakeGCE.Compute().HealthChecks().List(context.Background(), nil) + } + if len(computeHCs) != 1 { + t.Fatalf("Got %d healthchecks, want 1\n%s", len(computeHCs), pretty.Sprint(computeHCs)) + } + + gotHC := computeHCs[0] + // Filter out fields that are hard to deal with in the mock and + // test cases. + filter := func(hc *compute.HealthCheck) { + hc.Description = "" + hc.SelfLink = "" + } + filter(gotHC) + filter(tc.wantComputeHC) + + if !reflect.DeepEqual(gotHC, tc.wantComputeHC) { + t.Fatalf("Compute healthcheck is:\n%s, want:\n%s", pretty.Sprint(gotHC), pretty.Sprint(tc.wantComputeHC)) + } + } + + verify() + + // Check that resync should not have an effect and does not issue + // an update to GCE. Hook Update() to fail. + mock.MockHealthChecks.UpdateHook = func(context.Context, *meta.Key, *compute.HealthCheck, *cloud.MockHealthChecks) error { + t.Fatalf("resync should not result in an update") + return nil + } + + gotSelfLink, err = hcs.SyncServicePort(tc.sp, tc.probe) + if gotErr := err != nil; gotErr != tc.wantErr { + t.Errorf("hcs.SyncServicePort(tc.sp, tc.probe) = %v; gotErr = %t, want %t\nsp = %s\nprobe = %s", err, gotErr, tc.wantErr, pretty.Sprint(tc.sp), pretty.Sprint(tc.probe)) + } + if tc.wantSelfLink != "" && gotSelfLink != tc.wantSelfLink { + t.Errorf("hcs.SyncServicePort(tc.sp, tc.probe) = %q, _; want = %q", gotSelfLink, tc.wantSelfLink) + } + verify() + }) + } +} + +// TODO(bowei): test regional delete +// TODO(bowei): test errors from GCE From 168a7bc683782018c4b8d98f3a7ba39f2f8f3d6a Mon Sep 17 00:00:00 2001 From: Bowei Du Date: Sat, 22 Feb 2020 14:55:26 -0800 Subject: [PATCH 4/4] Update code generation (was missing healthcheck fields) --- go.sum | 1 + pkg/apis/backendconfig/v1/types.go | 26 +++++-- .../backendconfig/v1/zz_generated.openapi.go | 69 ++++++++++++++++++- pkg/apis/backendconfig/v1beta1/types.go | 26 +++++-- .../v1beta1/zz_generated.openapi.go | 69 ++++++++++++++++++- 5 files changed, 175 insertions(+), 16 deletions(-) diff --git a/go.sum b/go.sum index 38b22fbbd7..e6f9da1136 100644 --- a/go.sum +++ b/go.sum @@ -514,6 +514,7 @@ k8s.io/component-base v0.17.0 h1:BnDFcmBDq+RPpxXjmuYnZXb59XNN9CaFrX8ba9+3xrA= k8s.io/component-base v0.17.0/go.mod h1:rKuRAokNMY2nn2A6LP/MiwpoaMRHpfRnrPaUJJj1Yoc= k8s.io/cri-api v0.0.0-20190531030430-6117653b35f1/go.mod h1:K6Ux7uDbzKhacgqW0OJg3rjXk/SR9kprCPfSUDXGB5A= k8s.io/csi-translation-lib v0.0.0-20190620090114-816aa063c73d/go.mod h1:q5k0Vv3qsOTu2PUrTh+4mRAj+Pt+1BRyWiiwr5LBFOM= +k8s.io/gengo v0.0.0-20190116091435-f8a0810f38af h1:SwjZbO0u5ZuaV6TRMWOGB40iaycX8sbdMQHtjNZ19dk= k8s.io/gengo v0.0.0-20190116091435-f8a0810f38af/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0= k8s.io/gengo v0.0.0-20190128074634-0689ccc1d7d6/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0= k8s.io/heapster v1.2.0-beta.1/go.mod h1:h1uhptVXMwC8xtZBYsPXKVi8fpdlYkTs6k949KozGrM= diff --git a/pkg/apis/backendconfig/v1/types.go b/pkg/apis/backendconfig/v1/types.go index abd1b5cca2..d61c33f6d1 100644 --- a/pkg/apis/backendconfig/v1/types.go +++ b/pkg/apis/backendconfig/v1/types.go @@ -137,11 +137,23 @@ type CustomRequestHeadersConfig struct { // HealthCheckConfig contains configuration for the health check. // +k8s:openapi-gen=true type HealthCheckConfig struct { - CheckIntervalSec *int64 `json:"checkIntervalSec,omitempty"` - TimeoutSec *int64 `json:"timeoutSec,omitempty"` - HealthyThreshold *int64 `json:"healthyThreshold,omitempty"` - UnhealthyThreshold *int64 `json:"unhealthyThreshold,omitempty"` - Type *string `json:"type,omitempty"` - Port *int64 `json:"port,omitempty"` - RequestPath *string `json:"requestPath,omitempty"` + // CheckIntervalSec is a health check parameter. See + // https://cloud.google.com/compute/docs/reference/rest/v1/healthChecks. + CheckIntervalSec *int64 `json:"checkIntervalSec,omitempty"` + // TimeoutSec is a health check parameter. See + // https://cloud.google.com/compute/docs/reference/rest/v1/healthChecks. + TimeoutSec *int64 `json:"timeoutSec,omitempty"` + // HealthyThreshold is a health check parameter. See + // https://cloud.google.com/compute/docs/reference/rest/v1/healthChecks. + HealthyThreshold *int64 `json:"healthyThreshold,omitempty"` + // UnhealthyThreshold is a health check parameter. See + // https://cloud.google.com/compute/docs/reference/rest/v1/healthChecks. + UnhealthyThreshold *int64 `json:"unhealthyThreshold,omitempty"` + // Type is a health check parameter. See + // https://cloud.google.com/compute/docs/reference/rest/v1/healthChecks. + Type *string `json:"type,omitempty"` + Port *int64 `json:"port,omitempty"` + // RequestPath is a health check parameter. See + // https://cloud.google.com/compute/docs/reference/rest/v1/healthChecks. + RequestPath *string `json:"requestPath,omitempty"` } diff --git a/pkg/apis/backendconfig/v1/zz_generated.openapi.go b/pkg/apis/backendconfig/v1/zz_generated.openapi.go index 40d53df8dd..f1c97e7d60 100644 --- a/pkg/apis/backendconfig/v1/zz_generated.openapi.go +++ b/pkg/apis/backendconfig/v1/zz_generated.openapi.go @@ -35,6 +35,7 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA "k8s.io/ingress-gce/pkg/apis/backendconfig/v1.CacheKeyPolicy": schema_pkg_apis_backendconfig_v1_CacheKeyPolicy(ref), "k8s.io/ingress-gce/pkg/apis/backendconfig/v1.ConnectionDrainingConfig": schema_pkg_apis_backendconfig_v1_ConnectionDrainingConfig(ref), "k8s.io/ingress-gce/pkg/apis/backendconfig/v1.CustomRequestHeadersConfig": schema_pkg_apis_backendconfig_v1_CustomRequestHeadersConfig(ref), + "k8s.io/ingress-gce/pkg/apis/backendconfig/v1.HealthCheckConfig": schema_pkg_apis_backendconfig_v1_HealthCheckConfig(ref), "k8s.io/ingress-gce/pkg/apis/backendconfig/v1.IAPConfig": schema_pkg_apis_backendconfig_v1_IAPConfig(ref), "k8s.io/ingress-gce/pkg/apis/backendconfig/v1.OAuthClientCredentials": schema_pkg_apis_backendconfig_v1_OAuthClientCredentials(ref), "k8s.io/ingress-gce/pkg/apis/backendconfig/v1.SessionAffinityConfig": schema_pkg_apis_backendconfig_v1_SessionAffinityConfig(ref), @@ -127,11 +128,16 @@ func schema_pkg_apis_backendconfig_v1_BackendConfigSpec(ref common.ReferenceCall Ref: ref("k8s.io/ingress-gce/pkg/apis/backendconfig/v1.CustomRequestHeadersConfig"), }, }, + "healthCheck": { + SchemaProps: spec.SchemaProps{ + Ref: ref("k8s.io/ingress-gce/pkg/apis/backendconfig/v1.HealthCheckConfig"), + }, + }, }, }, }, Dependencies: []string{ - "k8s.io/ingress-gce/pkg/apis/backendconfig/v1.CDNConfig", "k8s.io/ingress-gce/pkg/apis/backendconfig/v1.ConnectionDrainingConfig", "k8s.io/ingress-gce/pkg/apis/backendconfig/v1.CustomRequestHeadersConfig", "k8s.io/ingress-gce/pkg/apis/backendconfig/v1.IAPConfig", "k8s.io/ingress-gce/pkg/apis/backendconfig/v1.SecurityPolicyConfig", "k8s.io/ingress-gce/pkg/apis/backendconfig/v1.SessionAffinityConfig"}, + "k8s.io/ingress-gce/pkg/apis/backendconfig/v1.CDNConfig", "k8s.io/ingress-gce/pkg/apis/backendconfig/v1.ConnectionDrainingConfig", "k8s.io/ingress-gce/pkg/apis/backendconfig/v1.CustomRequestHeadersConfig", "k8s.io/ingress-gce/pkg/apis/backendconfig/v1.HealthCheckConfig", "k8s.io/ingress-gce/pkg/apis/backendconfig/v1.IAPConfig", "k8s.io/ingress-gce/pkg/apis/backendconfig/v1.SecurityPolicyConfig", "k8s.io/ingress-gce/pkg/apis/backendconfig/v1.SessionAffinityConfig"}, } } @@ -270,6 +276,67 @@ func schema_pkg_apis_backendconfig_v1_CustomRequestHeadersConfig(ref common.Refe } } +func schema_pkg_apis_backendconfig_v1_HealthCheckConfig(ref common.ReferenceCallback) common.OpenAPIDefinition { + return common.OpenAPIDefinition{ + Schema: spec.Schema{ + SchemaProps: spec.SchemaProps{ + Description: "HealthCheckConfig contains configuration for the health check.", + Type: []string{"object"}, + Properties: map[string]spec.Schema{ + "checkIntervalSec": { + SchemaProps: spec.SchemaProps{ + Description: "CheckIntervalSec is a health check parameter. See https://cloud.google.com/compute/docs/reference/rest/v1/healthChecks.", + Type: []string{"integer"}, + Format: "int64", + }, + }, + "timeoutSec": { + SchemaProps: spec.SchemaProps{ + Description: "TimeoutSec is a health check parameter. See https://cloud.google.com/compute/docs/reference/rest/v1/healthChecks.", + Type: []string{"integer"}, + Format: "int64", + }, + }, + "healthyThreshold": { + SchemaProps: spec.SchemaProps{ + Description: "HealthyThreshold is a health check parameter. See https://cloud.google.com/compute/docs/reference/rest/v1/healthChecks.", + Type: []string{"integer"}, + Format: "int64", + }, + }, + "unhealthyThreshold": { + SchemaProps: spec.SchemaProps{ + Description: "UnhealthyThreshold is a health check parameter. See https://cloud.google.com/compute/docs/reference/rest/v1/healthChecks.", + Type: []string{"integer"}, + Format: "int64", + }, + }, + "type": { + SchemaProps: spec.SchemaProps{ + Description: "Type is a health check parameter. See https://cloud.google.com/compute/docs/reference/rest/v1/healthChecks.", + Type: []string{"string"}, + Format: "", + }, + }, + "port": { + SchemaProps: spec.SchemaProps{ + Type: []string{"integer"}, + Format: "int64", + }, + }, + "requestPath": { + SchemaProps: spec.SchemaProps{ + Description: "RequestPath is a health check parameter. See https://cloud.google.com/compute/docs/reference/rest/v1/healthChecks.", + Type: []string{"string"}, + Format: "", + }, + }, + }, + }, + }, + } +} + func schema_pkg_apis_backendconfig_v1_IAPConfig(ref common.ReferenceCallback) common.OpenAPIDefinition { return common.OpenAPIDefinition{ Schema: spec.Schema{ diff --git a/pkg/apis/backendconfig/v1beta1/types.go b/pkg/apis/backendconfig/v1beta1/types.go index 78eed087e1..750dff6f40 100644 --- a/pkg/apis/backendconfig/v1beta1/types.go +++ b/pkg/apis/backendconfig/v1beta1/types.go @@ -137,11 +137,23 @@ type CustomRequestHeadersConfig struct { // HealthCheckConfig contains configuration for the health check. // +k8s:openapi-gen=true type HealthCheckConfig struct { - CheckIntervalSec *int64 `json:"checkIntervalSec,omitempty"` - TimeoutSec *int64 `json:"timeoutSec,omitempty"` - HealthyThreshold *int64 `json:"healthyThreshold,omitempty"` - UnhealthyThreshold *int64 `json:"unhealthyThreshold,omitempty"` - Type *string `json:"type,omitempty"` - Port *int64 `json:"port,omitempty"` - RequestPath *string `json:"requestPath,omitempty"` + // CheckIntervalSec is a health check parameter. See + // https://cloud.google.com/compute/docs/reference/rest/v1/healthChecks. + CheckIntervalSec *int64 `json:"checkIntervalSec,omitempty"` + // TimeoutSec is a health check parameter. See + // https://cloud.google.com/compute/docs/reference/rest/v1/healthChecks. + TimeoutSec *int64 `json:"timeoutSec,omitempty"` + // HealthyThreshold is a health check parameter. See + // https://cloud.google.com/compute/docs/reference/rest/v1/healthChecks. + HealthyThreshold *int64 `json:"healthyThreshold,omitempty"` + // UnhealthyThreshold is a health check parameter. See + // https://cloud.google.com/compute/docs/reference/rest/v1/healthChecks. + UnhealthyThreshold *int64 `json:"unhealthyThreshold,omitempty"` + // Type is a health check parameter. See + // https://cloud.google.com/compute/docs/reference/rest/v1/healthChecks. + Type *string `json:"type,omitempty"` + Port *int64 `json:"port,omitempty"` + // RequestPath is a health check parameter. See + // https://cloud.google.com/compute/docs/reference/rest/v1/healthChecks. + RequestPath *string `json:"requestPath,omitempty"` } diff --git a/pkg/apis/backendconfig/v1beta1/zz_generated.openapi.go b/pkg/apis/backendconfig/v1beta1/zz_generated.openapi.go index 60ddfca94f..e7b4f4200e 100644 --- a/pkg/apis/backendconfig/v1beta1/zz_generated.openapi.go +++ b/pkg/apis/backendconfig/v1beta1/zz_generated.openapi.go @@ -35,6 +35,7 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA "k8s.io/ingress-gce/pkg/apis/backendconfig/v1beta1.CacheKeyPolicy": schema_pkg_apis_backendconfig_v1beta1_CacheKeyPolicy(ref), "k8s.io/ingress-gce/pkg/apis/backendconfig/v1beta1.ConnectionDrainingConfig": schema_pkg_apis_backendconfig_v1beta1_ConnectionDrainingConfig(ref), "k8s.io/ingress-gce/pkg/apis/backendconfig/v1beta1.CustomRequestHeadersConfig": schema_pkg_apis_backendconfig_v1beta1_CustomRequestHeadersConfig(ref), + "k8s.io/ingress-gce/pkg/apis/backendconfig/v1beta1.HealthCheckConfig": schema_pkg_apis_backendconfig_v1beta1_HealthCheckConfig(ref), "k8s.io/ingress-gce/pkg/apis/backendconfig/v1beta1.IAPConfig": schema_pkg_apis_backendconfig_v1beta1_IAPConfig(ref), "k8s.io/ingress-gce/pkg/apis/backendconfig/v1beta1.OAuthClientCredentials": schema_pkg_apis_backendconfig_v1beta1_OAuthClientCredentials(ref), "k8s.io/ingress-gce/pkg/apis/backendconfig/v1beta1.SessionAffinityConfig": schema_pkg_apis_backendconfig_v1beta1_SessionAffinityConfig(ref), @@ -127,11 +128,16 @@ func schema_pkg_apis_backendconfig_v1beta1_BackendConfigSpec(ref common.Referenc Ref: ref("k8s.io/ingress-gce/pkg/apis/backendconfig/v1beta1.CustomRequestHeadersConfig"), }, }, + "healthCheck": { + SchemaProps: spec.SchemaProps{ + Ref: ref("k8s.io/ingress-gce/pkg/apis/backendconfig/v1beta1.HealthCheckConfig"), + }, + }, }, }, }, Dependencies: []string{ - "k8s.io/ingress-gce/pkg/apis/backendconfig/v1beta1.CDNConfig", "k8s.io/ingress-gce/pkg/apis/backendconfig/v1beta1.ConnectionDrainingConfig", "k8s.io/ingress-gce/pkg/apis/backendconfig/v1beta1.CustomRequestHeadersConfig", "k8s.io/ingress-gce/pkg/apis/backendconfig/v1beta1.IAPConfig", "k8s.io/ingress-gce/pkg/apis/backendconfig/v1beta1.SecurityPolicyConfig", "k8s.io/ingress-gce/pkg/apis/backendconfig/v1beta1.SessionAffinityConfig"}, + "k8s.io/ingress-gce/pkg/apis/backendconfig/v1beta1.CDNConfig", "k8s.io/ingress-gce/pkg/apis/backendconfig/v1beta1.ConnectionDrainingConfig", "k8s.io/ingress-gce/pkg/apis/backendconfig/v1beta1.CustomRequestHeadersConfig", "k8s.io/ingress-gce/pkg/apis/backendconfig/v1beta1.HealthCheckConfig", "k8s.io/ingress-gce/pkg/apis/backendconfig/v1beta1.IAPConfig", "k8s.io/ingress-gce/pkg/apis/backendconfig/v1beta1.SecurityPolicyConfig", "k8s.io/ingress-gce/pkg/apis/backendconfig/v1beta1.SessionAffinityConfig"}, } } @@ -270,6 +276,67 @@ func schema_pkg_apis_backendconfig_v1beta1_CustomRequestHeadersConfig(ref common } } +func schema_pkg_apis_backendconfig_v1beta1_HealthCheckConfig(ref common.ReferenceCallback) common.OpenAPIDefinition { + return common.OpenAPIDefinition{ + Schema: spec.Schema{ + SchemaProps: spec.SchemaProps{ + Description: "HealthCheckConfig contains configuration for the health check.", + Type: []string{"object"}, + Properties: map[string]spec.Schema{ + "checkIntervalSec": { + SchemaProps: spec.SchemaProps{ + Description: "CheckIntervalSec is a health check parameter. See https://cloud.google.com/compute/docs/reference/rest/v1/healthChecks.", + Type: []string{"integer"}, + Format: "int64", + }, + }, + "timeoutSec": { + SchemaProps: spec.SchemaProps{ + Description: "TimeoutSec is a health check parameter. See https://cloud.google.com/compute/docs/reference/rest/v1/healthChecks.", + Type: []string{"integer"}, + Format: "int64", + }, + }, + "healthyThreshold": { + SchemaProps: spec.SchemaProps{ + Description: "HealthyThreshold is a health check parameter. See https://cloud.google.com/compute/docs/reference/rest/v1/healthChecks.", + Type: []string{"integer"}, + Format: "int64", + }, + }, + "unhealthyThreshold": { + SchemaProps: spec.SchemaProps{ + Description: "UnhealthyThreshold is a health check parameter. See https://cloud.google.com/compute/docs/reference/rest/v1/healthChecks.", + Type: []string{"integer"}, + Format: "int64", + }, + }, + "type": { + SchemaProps: spec.SchemaProps{ + Description: "Type is a health check parameter. See https://cloud.google.com/compute/docs/reference/rest/v1/healthChecks.", + Type: []string{"string"}, + Format: "", + }, + }, + "port": { + SchemaProps: spec.SchemaProps{ + Type: []string{"integer"}, + Format: "int64", + }, + }, + "requestPath": { + SchemaProps: spec.SchemaProps{ + Description: "RequestPath is a health check parameter. See https://cloud.google.com/compute/docs/reference/rest/v1/healthChecks.", + Type: []string{"string"}, + Format: "", + }, + }, + }, + }, + }, + } +} + func schema_pkg_apis_backendconfig_v1beta1_IAPConfig(ref common.ReferenceCallback) common.OpenAPIDefinition { return common.OpenAPIDefinition{ Schema: spec.Schema{