diff --git a/pkg/healthchecks/healthchecks_l4.go b/pkg/healthchecks/healthchecks_l4.go index 933d0ebda3..ac443b0724 100644 --- a/pkg/healthchecks/healthchecks_l4.go +++ b/pkg/healthchecks/healthchecks_l4.go @@ -31,14 +31,33 @@ import ( const ( // L4 Load Balancer parameters - gceHcCheckIntervalSeconds = int64(8) - gceHcTimeoutSeconds = int64(1) + gceSharedHcCheckIntervalSeconds = int64(8) // Shared Health check Interval + gceLocalHcCheckIntervalSeconds = int64(3) // Local Health check Interval + gceHcTimeoutSeconds = int64(1) // Start sending requests as soon as one healthcheck succeeds. - gceHcHealthyThreshold = int64(1) - // Defaults to 3 * 8 = 24 seconds before the LB will steer traffic away. - gceHcUnhealthyThreshold = int64(3) + gceHcHealthyThreshold = int64(1) + gceSharedHcUnhealthyThreshold = int64(3) // 3 * 8 = 24 seconds before the LB will steer traffic away + gceLocalHcUnhealthyThreshold = int64(2) // 2 * 3 = 6 seconds before the LB will steer traffic away ) +// Returns an interval constant based of shared/local status +func healthcheckInterval(isShared bool) int64 { + if isShared { + return gceSharedHcCheckIntervalSeconds + } else { + return gceLocalHcCheckIntervalSeconds + } +} + +// Returns a threshold for unhealthy instance based of shared/local status +func healthcheckUnhealthyThreshold(isShared bool) int64 { + if isShared { + return gceSharedHcUnhealthyThreshold + } else { + return gceLocalHcUnhealthyThreshold + } +} + // EnsureL4HealthCheck creates a new HTTP health check for an L4 LoadBalancer service, based on the parameters provided. // If the healthcheck already exists, it is updated as needed. func EnsureL4HealthCheck(cloud *gce.Cloud, name string, svcName types.NamespacedName, shared bool, path string, port int32) (*composite.HealthCheck, string, error) { @@ -96,12 +115,17 @@ func NewL4HealthCheck(name string, svcName types.NamespacedName, shared bool, pa if err != nil { klog.Warningf("Failed to generate description for L4HealthCheck %s, err %v", name, err) } + + // Get constant values based on shared/local status + interval := healthcheckInterval(shared) + unhealthyThreshold := healthcheckUnhealthyThreshold(shared) + return &composite.HealthCheck{ Name: name, - CheckIntervalSec: gceHcCheckIntervalSeconds, + CheckIntervalSec: interval, TimeoutSec: gceHcTimeoutSeconds, HealthyThreshold: gceHcHealthyThreshold, - UnhealthyThreshold: gceHcUnhealthyThreshold, + UnhealthyThreshold: unhealthyThreshold, HttpHealthCheck: &httpSettings, Type: "HTTP", Description: desc, diff --git a/pkg/healthchecks/healthchecks_l4_test.go b/pkg/healthchecks/healthchecks_l4_test.go index 1c863cbb8c..a1a5bf9144 100644 --- a/pkg/healthchecks/healthchecks_l4_test.go +++ b/pkg/healthchecks/healthchecks_l4_test.go @@ -17,14 +17,15 @@ limitations under the License. package healthchecks import ( + "testing" + "k8s.io/apimachinery/pkg/types" "k8s.io/ingress-gce/pkg/composite" - "testing" ) func TestMergeHealthChecks(t *testing.T) { t.Parallel() - for _, tc := range []struct { + testCases := []struct { desc string checkIntervalSec int64 timeoutSec int64 @@ -35,16 +36,107 @@ func TestMergeHealthChecks(t *testing.T) { wantHealthyThreshold int64 wantUnhealthyThreshold int64 }{ - {"unchanged", gceHcCheckIntervalSeconds, gceHcTimeoutSeconds, gceHcHealthyThreshold, gceHcUnhealthyThreshold, gceHcCheckIntervalSeconds, gceHcTimeoutSeconds, gceHcHealthyThreshold, gceHcUnhealthyThreshold}, - {"interval - too small - should reconcile", gceHcCheckIntervalSeconds - 1, gceHcTimeoutSeconds, gceHcHealthyThreshold, gceHcUnhealthyThreshold, gceHcCheckIntervalSeconds, gceHcTimeoutSeconds, gceHcHealthyThreshold, gceHcUnhealthyThreshold}, - {"timeout - too small - should reconcile", gceHcCheckIntervalSeconds, gceHcTimeoutSeconds - 1, gceHcHealthyThreshold, gceHcUnhealthyThreshold, gceHcCheckIntervalSeconds, gceHcTimeoutSeconds, gceHcHealthyThreshold, gceHcUnhealthyThreshold}, - {"healthy threshold - too small - should reconcile", gceHcCheckIntervalSeconds, gceHcTimeoutSeconds, gceHcHealthyThreshold - 1, gceHcUnhealthyThreshold, gceHcCheckIntervalSeconds, gceHcTimeoutSeconds, gceHcHealthyThreshold, gceHcUnhealthyThreshold}, - {"unhealthy threshold - too small - should reconcile", gceHcCheckIntervalSeconds, gceHcTimeoutSeconds, gceHcHealthyThreshold, gceHcUnhealthyThreshold - 1, gceHcCheckIntervalSeconds, gceHcTimeoutSeconds, gceHcHealthyThreshold, gceHcUnhealthyThreshold}, - {"interval - user configured - should keep", gceHcCheckIntervalSeconds + 1, gceHcTimeoutSeconds, gceHcHealthyThreshold, gceHcUnhealthyThreshold, gceHcCheckIntervalSeconds + 1, gceHcTimeoutSeconds, gceHcHealthyThreshold, gceHcUnhealthyThreshold}, - {"timeout - user configured - should keep", gceHcCheckIntervalSeconds, gceHcTimeoutSeconds + 1, gceHcHealthyThreshold, gceHcUnhealthyThreshold, gceHcCheckIntervalSeconds, gceHcTimeoutSeconds + 1, gceHcHealthyThreshold, gceHcUnhealthyThreshold}, - {"healthy threshold - user configured - should keep", gceHcCheckIntervalSeconds, gceHcTimeoutSeconds, gceHcHealthyThreshold + 1, gceHcUnhealthyThreshold, gceHcCheckIntervalSeconds, gceHcTimeoutSeconds, gceHcHealthyThreshold + 1, gceHcUnhealthyThreshold}, - {"unhealthy threshold - user configured - should keep", gceHcCheckIntervalSeconds, gceHcTimeoutSeconds, gceHcHealthyThreshold, gceHcUnhealthyThreshold + 1, gceHcCheckIntervalSeconds, gceHcTimeoutSeconds, gceHcHealthyThreshold, gceHcUnhealthyThreshold + 1}, - } { + { + desc: "unchanged", + checkIntervalSec: gceLocalHcCheckIntervalSeconds, + timeoutSec: gceHcTimeoutSeconds, + healthyThreshold: gceHcHealthyThreshold, + unhealthyThreshold: gceLocalHcUnhealthyThreshold, + wantCheckIntervalSec: gceLocalHcCheckIntervalSeconds, + wantTimeoutSec: gceHcTimeoutSeconds, + wantHealthyThreshold: gceHcHealthyThreshold, + wantUnhealthyThreshold: gceLocalHcUnhealthyThreshold, + }, + { + desc: "interval - too small - should reconcile", + checkIntervalSec: gceLocalHcCheckIntervalSeconds - 1, + timeoutSec: gceHcTimeoutSeconds, + healthyThreshold: gceHcHealthyThreshold, + unhealthyThreshold: gceLocalHcUnhealthyThreshold, + wantCheckIntervalSec: gceLocalHcCheckIntervalSeconds, + wantTimeoutSec: gceHcTimeoutSeconds, + wantHealthyThreshold: gceHcHealthyThreshold, + wantUnhealthyThreshold: gceLocalHcUnhealthyThreshold, + }, + { + desc: "timeout - too small - should reconcile", + checkIntervalSec: gceLocalHcCheckIntervalSeconds, + timeoutSec: gceHcTimeoutSeconds - 1, + healthyThreshold: gceHcHealthyThreshold, + unhealthyThreshold: gceLocalHcUnhealthyThreshold, + wantCheckIntervalSec: gceLocalHcCheckIntervalSeconds, + wantTimeoutSec: gceHcTimeoutSeconds, + wantHealthyThreshold: gceHcHealthyThreshold, + wantUnhealthyThreshold: gceLocalHcUnhealthyThreshold, + }, + { + desc: "healthy threshold - too small - should reconcil", + checkIntervalSec: gceLocalHcCheckIntervalSeconds, + timeoutSec: gceHcTimeoutSeconds, + healthyThreshold: gceHcHealthyThreshold - 1, + unhealthyThreshold: gceLocalHcUnhealthyThreshold, + wantCheckIntervalSec: gceLocalHcCheckIntervalSeconds, + wantTimeoutSec: gceHcTimeoutSeconds, + wantHealthyThreshold: gceHcHealthyThreshold, + wantUnhealthyThreshold: gceLocalHcUnhealthyThreshold, + }, + { + desc: "unhealthy threshold - too small - should reconcile", + checkIntervalSec: gceLocalHcCheckIntervalSeconds, + timeoutSec: gceHcTimeoutSeconds, + healthyThreshold: gceHcHealthyThreshold, + unhealthyThreshold: gceLocalHcUnhealthyThreshold - 1, + wantCheckIntervalSec: gceLocalHcCheckIntervalSeconds, + wantTimeoutSec: gceHcTimeoutSeconds, + wantHealthyThreshold: gceHcHealthyThreshold, + wantUnhealthyThreshold: gceLocalHcUnhealthyThreshold, + }, + { + desc: "interval - user configured - should keep", + checkIntervalSec: gceLocalHcCheckIntervalSeconds + 1, + timeoutSec: gceHcTimeoutSeconds, + healthyThreshold: gceHcHealthyThreshold, + unhealthyThreshold: gceLocalHcUnhealthyThreshold, + wantCheckIntervalSec: gceLocalHcCheckIntervalSeconds + 1, + wantTimeoutSec: gceHcTimeoutSeconds, + wantHealthyThreshold: gceHcHealthyThreshold, + wantUnhealthyThreshold: gceLocalHcUnhealthyThreshold, + }, + { + desc: "timeout - user configured - should keep", + checkIntervalSec: gceLocalHcCheckIntervalSeconds, + timeoutSec: gceHcTimeoutSeconds + 1, + healthyThreshold: gceHcHealthyThreshold, + unhealthyThreshold: gceLocalHcUnhealthyThreshold, + wantCheckIntervalSec: gceLocalHcCheckIntervalSeconds, + wantTimeoutSec: gceHcTimeoutSeconds + 1, + wantHealthyThreshold: gceHcHealthyThreshold, + wantUnhealthyThreshold: gceLocalHcUnhealthyThreshold, + }, + { + desc: "healthy threshold - user configured - should keep", + checkIntervalSec: gceLocalHcCheckIntervalSeconds, + timeoutSec: gceHcTimeoutSeconds, + healthyThreshold: gceHcHealthyThreshold + 1, + unhealthyThreshold: gceLocalHcUnhealthyThreshold, + wantCheckIntervalSec: gceLocalHcCheckIntervalSeconds, + wantTimeoutSec: gceHcTimeoutSeconds, + wantHealthyThreshold: gceHcHealthyThreshold + 1, + wantUnhealthyThreshold: gceLocalHcUnhealthyThreshold, + }, + { + desc: "unhealthy threshold - user configured - should keep", + checkIntervalSec: gceLocalHcCheckIntervalSeconds, + timeoutSec: gceHcTimeoutSeconds, + healthyThreshold: gceHcHealthyThreshold, + unhealthyThreshold: gceLocalHcUnhealthyThreshold + 1, + wantCheckIntervalSec: gceLocalHcCheckIntervalSeconds, + wantTimeoutSec: gceHcTimeoutSeconds, + wantHealthyThreshold: gceHcHealthyThreshold, + wantUnhealthyThreshold: gceLocalHcUnhealthyThreshold + 1, + }, + } + for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { wantHC := NewL4HealthCheck("hc", types.NamespacedName{Name: "svc", Namespace: "default"}, false, "/", 12345) hc := &composite.HealthCheck{ @@ -72,25 +164,78 @@ func TestMergeHealthChecks(t *testing.T) { func TestCompareHealthChecks(t *testing.T) { t.Parallel() - for _, tc := range []struct { + testCases := []struct { desc string modifier func(*composite.HealthCheck) wantChanged bool }{ - {"unchanged", nil, false}, - {"nil HttpHealthCheck", func(hc *composite.HealthCheck) { hc.HttpHealthCheck = nil }, true}, - {"desc does not match", func(hc *composite.HealthCheck) { hc.Description = "bad-desc" }, true}, - {"port does not match", func(hc *composite.HealthCheck) { hc.HttpHealthCheck.Port = 54321 }, true}, - {"requestPath does not match", func(hc *composite.HealthCheck) { hc.HttpHealthCheck.RequestPath = "/anotherone" }, true}, - {"interval needs update", func(hc *composite.HealthCheck) { hc.CheckIntervalSec = gceHcCheckIntervalSeconds - 1 }, true}, - {"timeout needs update", func(hc *composite.HealthCheck) { hc.TimeoutSec = gceHcTimeoutSeconds - 1 }, true}, - {"healthy threshold needs update", func(hc *composite.HealthCheck) { hc.HealthyThreshold = gceHcHealthyThreshold - 1 }, true}, - {"unhealthy threshold needs update", func(hc *composite.HealthCheck) { hc.UnhealthyThreshold = gceHcUnhealthyThreshold - 1 }, true}, - {"interval does not need update", func(hc *composite.HealthCheck) { hc.CheckIntervalSec = gceHcCheckIntervalSeconds + 1 }, false}, - {"timeout does not need update", func(hc *composite.HealthCheck) { hc.TimeoutSec = gceHcTimeoutSeconds + 1 }, false}, - {"healthy threshold does not need update", func(hc *composite.HealthCheck) { hc.HealthyThreshold = gceHcHealthyThreshold + 1 }, false}, - {"unhealthy threshold does not need update", func(hc *composite.HealthCheck) { hc.UnhealthyThreshold = gceHcUnhealthyThreshold + 1 }, false}, - } { + { + desc: "unchanged", + modifier: nil, + wantChanged: false, + }, + { + desc: "nil HttpHealthCheck", + modifier: func(hc *composite.HealthCheck) { hc.HttpHealthCheck = nil }, + wantChanged: true, + }, + { + desc: "desc does not match", + modifier: func(hc *composite.HealthCheck) { hc.Description = "bad-desc" }, + wantChanged: true, + }, + { + desc: "port does not match", + modifier: func(hc *composite.HealthCheck) { hc.HttpHealthCheck.Port = 54321 }, + wantChanged: true, + }, + { + desc: "requestPath does not match", + modifier: func(hc *composite.HealthCheck) { hc.HttpHealthCheck.RequestPath = "/anotherone" }, + wantChanged: true, + }, + { + desc: "interval needs update", + modifier: func(hc *composite.HealthCheck) { hc.CheckIntervalSec = gceLocalHcCheckIntervalSeconds - 1 }, + wantChanged: true, + }, + { + desc: "timeout needs update", + modifier: func(hc *composite.HealthCheck) { hc.TimeoutSec = gceHcTimeoutSeconds - 1 }, + wantChanged: true, + }, + { + desc: "healthy threshold needs update", + modifier: func(hc *composite.HealthCheck) { hc.HealthyThreshold = gceHcHealthyThreshold - 1 }, + wantChanged: true, + }, + { + desc: "unhealthy threshold needs update", + modifier: func(hc *composite.HealthCheck) { hc.UnhealthyThreshold = gceLocalHcUnhealthyThreshold - 1 }, + wantChanged: true, + }, + { + desc: "interval does not need update", + modifier: func(hc *composite.HealthCheck) { hc.CheckIntervalSec = gceLocalHcCheckIntervalSeconds + 1 }, + wantChanged: false, + }, + { + desc: "timeout does not need update", + modifier: func(hc *composite.HealthCheck) { hc.TimeoutSec = gceHcTimeoutSeconds + 1 }, + wantChanged: false, + }, + { + desc: "healthy threshold does not need update", + modifier: func(hc *composite.HealthCheck) { hc.HealthyThreshold = gceHcHealthyThreshold + 1 }, + wantChanged: false, + }, + { + desc: "unhealthy threshold does not need update", + modifier: func(hc *composite.HealthCheck) { hc.UnhealthyThreshold = gceLocalHcUnhealthyThreshold + 1 }, + wantChanged: false, + }, + } + for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { hc := NewL4HealthCheck("hc", types.NamespacedName{Name: "svc", Namespace: "default"}, false, "/", 12345) wantHC := NewL4HealthCheck("hc", types.NamespacedName{Name: "svc", Namespace: "default"}, false, "/", 12345) @@ -103,3 +248,59 @@ func TestCompareHealthChecks(t *testing.T) { }) } } + +func TestHealthcheckInterval(t *testing.T) { + t.Parallel() + testCases := []struct { + desc string + shared bool + wantCheckIntervalSec int64 + }{ + { + desc: "shared - check interval function", + shared: true, + wantCheckIntervalSec: gceSharedHcCheckIntervalSeconds, + }, + { + desc: "local - check interval function", + shared: false, + wantCheckIntervalSec: gceLocalHcCheckIntervalSeconds, + }, + } + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + gotInterval := healthcheckInterval(tc.shared) + if gotInterval != tc.wantCheckIntervalSec { + t.Errorf("got Health Check Interval: %d, want: %d", gotInterval, tc.wantCheckIntervalSec) + } + }) + } +} + +func TestHealthcheckUnhealthyThreshold(t *testing.T) { + t.Parallel() + testCases := []struct { + desc string + shared bool + wantUnhealthyThreshold int64 + }{ + { + desc: "shared - check unhealthy threshold function", + shared: true, + wantUnhealthyThreshold: gceSharedHcUnhealthyThreshold, + }, + { + desc: "local - check unhealthy threshold function", + shared: false, + wantUnhealthyThreshold: gceLocalHcUnhealthyThreshold, + }, + } + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + gotUnhealthyThreshold := healthcheckUnhealthyThreshold(tc.shared) + if gotUnhealthyThreshold != tc.wantUnhealthyThreshold { + t.Errorf("got Unhealthy Threshold: %d, want: %d", gotUnhealthyThreshold, tc.wantUnhealthyThreshold) + } + }) + } +}