Skip to content

Commit

Permalink
Merge pull request #1828 from code-elinka/change-l4-healthcheck
Browse files Browse the repository at this point in the history 
Change l4 healthcheck timeout
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Oct 6, 2022
2 parents 43e9cf6 + 22247d2 commit 6b01d42
Show file tree
Hide file tree
Showing 2 changed files with 294 additions and 33 deletions.
37 changes: 30 additions & 7 deletions pkg/healthchecksl4/healthchecksl4.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,13 @@ 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
)

var (
Expand Down Expand Up @@ -79,6 +80,24 @@ func Fake(cloud *gce.Cloud, recorder record.EventRecorder) *l4HealthChecks {
}
}

// 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
}
}

// EnsureHealthCheckWithFirewall exist for the L4
// LoadBalancer Service.
//
Expand Down Expand Up @@ -285,12 +304,16 @@ 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,
Expand Down
290 changes: 264 additions & 26 deletions pkg/healthchecksl4/healthchecksl4_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (

func TestMergeHealthChecks(t *testing.T) {
t.Parallel()
for _, tc := range []struct {
testCases := []struct {
desc string
checkIntervalSec int64
timeoutSec int64
Expand All @@ -38,16 +38,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) {
// healthcheck intervals and thresholds are common for Global and Regional healthchecks. Hence testing only Global case.
wantHC := newL4HealthCheck("hc", types.NamespacedName{Name: "svc", Namespace: "default"}, false, "/", 12345, utils.ILB, meta.Global, "")
Expand Down Expand Up @@ -76,25 +167,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) {
// healthcheck intervals and thresholds are common for Global and Regional healthchecks. Hence testing only Global case.
hc := newL4HealthCheck("hc", types.NamespacedName{Name: "svc", Namespace: "default"}, false, "/", 12345, utils.ILB, meta.Global, "")
Expand All @@ -109,6 +253,100 @@ 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)
}
})
}
}

// Checks that newL4HealthCheck() returns correct CheckInterval and UnhealthyThreshold
func TestSharedHealthChecks(t *testing.T) {

t.Parallel()
testCases := []struct {
desc string
shared bool
wantCheckIntervalSec int64
wantUnhealthyThreshold int64
}{
{
desc: "shared - check interval and unhealthy threshold",
shared: true,
wantCheckIntervalSec: gceSharedHcCheckIntervalSeconds,
wantUnhealthyThreshold: gceSharedHcUnhealthyThreshold,
},
{
desc: "local - check interval and unhealthy threshold",
shared: false,
wantCheckIntervalSec: gceLocalHcCheckIntervalSeconds,
wantUnhealthyThreshold: gceLocalHcUnhealthyThreshold,
},
}

for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
// healthcheck intervals and thresholds are common for Global and Regional healthchecks. Hence testing only Global case.
gotHC := newL4HealthCheck("hc", types.NamespacedName{Name: "svc", Namespace: "default"}, tc.shared, "/", 12345, utils.ILB, meta.Global, "")
if gotHC.CheckIntervalSec != tc.wantCheckIntervalSec {
t.Errorf("gotHC.CheckIntervalSec = %d; want %d", gotHC.CheckIntervalSec, tc.wantCheckIntervalSec)
}
if gotHC.UnhealthyThreshold != tc.wantUnhealthyThreshold {
t.Errorf("gotHC.UnhealthyThreshold = %d; want %d", gotHC.UnhealthyThreshold, tc.wantUnhealthyThreshold)
}
})
}
}

func TestNewHealthCheck(t *testing.T) {
t.Parallel()
namespaceName := types.NamespacedName{Name: "svc", Namespace: "default"}
Expand Down

0 comments on commit 6b01d42

Please sign in to comment.