Skip to content

Commit

Permalink
Change Xl4 healthcheck timeout
Browse files Browse the repository at this point in the history 
According to l4 healthcheck constants
the default set up has timeout = 8 seconds
and threshold = 3 times. As result it can
lead to 24 (8*3) seconds of lost requests.
This change is supposed to decrease this
time to only 6 seconds for external LB.

Signed-off-by: Elina Akhmanova <elinka@google.com>
  • Loading branch information
@code-elinka
code-elinka committed Oct 3, 2022
1 parent 0d703c9 commit 235301b
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 32 deletions.
31 changes: 24 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)
gceLBHcCheckIntervalSeconds = int64(3) // Internal LB Health check Interval
gceNetLBHcCheckIntervalSeconds = int64(8) // External LB 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)
gceLBHcUnhealthyThreshold = int64(3) // 3 * 8 = 24 seconds before the Internal LB will steer traffic away
gceNetLBHcUnhealthyThreshold = int64(2) // 2 * 3 = 6 seconds before the External LB will steer traffic away
)

var (
Expand Down Expand Up @@ -302,12 +303,28 @@ 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)
}

// Default time for LB to move the traffic from an unhealthy instance differs for internal/external LBs
var (
hcCheckIntervalSeconds int64
hcUnhealthyThreshold int64
)
if l4Type == utils.ILB {
hcCheckIntervalSeconds = gceLBHcCheckIntervalSeconds
hcUnhealthyThreshold = gceLBHcUnhealthyThreshold
} else if l4Type == utils.XLB {
hcCheckIntervalSeconds = gceNetLBHcCheckIntervalSeconds
hcUnhealthyThreshold = gceNetLBHcUnhealthyThreshold
} else {
klog.Errorf("Wrong L4 Load Balancer type. Expected: ILB, XLB. Got: %s", l4Type.ToString())
}

return &composite.HealthCheck{
Name: name,
CheckIntervalSec: gceHcCheckIntervalSeconds,
CheckIntervalSec: hcCheckIntervalSeconds,
TimeoutSec: gceHcTimeoutSeconds,
HealthyThreshold: gceHcHealthyThreshold,
UnhealthyThreshold: gceHcUnhealthyThreshold,
UnhealthyThreshold: hcUnhealthyThreshold,
HttpHealthCheck: &httpSettings,
Type: "HTTP",
Description: desc,
Expand Down
88 changes: 63 additions & 25 deletions pkg/healthchecksl4/healthchecksl4_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ func TestMergeHealthChecks(t *testing.T) {
t.Parallel()
for _, tc := range []struct {
desc string
l4Type utils.L4LBType
checkIntervalSec int64
timeoutSec int64
healthyThreshold int64
Expand All @@ -38,19 +39,24 @@ 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},
{"unchanged", utils.ILB, gceLBHcCheckIntervalSeconds, gceHcTimeoutSeconds, gceHcHealthyThreshold, gceLBHcUnhealthyThreshold, gceLBHcCheckIntervalSeconds, gceHcTimeoutSeconds, gceHcHealthyThreshold, gceLBHcUnhealthyThreshold},
{"unchanged external LB", utils.XLB, gceNetLBHcCheckIntervalSeconds, gceHcTimeoutSeconds, gceHcHealthyThreshold, gceNetLBHcUnhealthyThreshold, gceNetLBHcCheckIntervalSeconds, gceHcTimeoutSeconds, gceHcHealthyThreshold, gceNetLBHcUnhealthyThreshold},
{"interval - too small - should reconcile", utils.ILB, gceLBHcCheckIntervalSeconds - 1, gceHcTimeoutSeconds, gceHcHealthyThreshold, gceLBHcUnhealthyThreshold, gceLBHcCheckIntervalSeconds, gceHcTimeoutSeconds, gceHcHealthyThreshold, gceLBHcUnhealthyThreshold},
{"external LB interval - too small - should reconcile", utils.XLB, gceNetLBHcCheckIntervalSeconds - 1, gceHcTimeoutSeconds, gceHcHealthyThreshold, gceNetLBHcUnhealthyThreshold, gceNetLBHcCheckIntervalSeconds, gceHcTimeoutSeconds, gceHcHealthyThreshold, gceNetLBHcUnhealthyThreshold},
{"timeout - too small - should reconcile", utils.ILB, gceLBHcCheckIntervalSeconds, gceHcTimeoutSeconds - 1, gceHcHealthyThreshold, gceLBHcUnhealthyThreshold, gceLBHcCheckIntervalSeconds, gceHcTimeoutSeconds, gceHcHealthyThreshold, gceLBHcUnhealthyThreshold},
{"healthy threshold - too small - should reconcile", utils.ILB, gceLBHcCheckIntervalSeconds, gceHcTimeoutSeconds, gceHcHealthyThreshold - 1, gceLBHcUnhealthyThreshold, gceLBHcCheckIntervalSeconds, gceHcTimeoutSeconds, gceHcHealthyThreshold, gceLBHcUnhealthyThreshold},
{"unhealthy threshold - too small - should reconcile", utils.ILB, gceLBHcCheckIntervalSeconds, gceHcTimeoutSeconds, gceHcHealthyThreshold, gceLBHcUnhealthyThreshold - 1, gceLBHcCheckIntervalSeconds, gceHcTimeoutSeconds, gceHcHealthyThreshold, gceLBHcUnhealthyThreshold},
{"external LB unhealthy threshold - too small - should reconcile", utils.XLB, gceNetLBHcCheckIntervalSeconds, gceHcTimeoutSeconds, gceHcHealthyThreshold, gceNetLBHcUnhealthyThreshold - 1, gceNetLBHcCheckIntervalSeconds, gceHcTimeoutSeconds, gceHcHealthyThreshold, gceNetLBHcUnhealthyThreshold},
{"interval - user configured - should keep", utils.ILB, gceLBHcCheckIntervalSeconds + 1, gceHcTimeoutSeconds, gceHcHealthyThreshold, gceLBHcUnhealthyThreshold, gceLBHcCheckIntervalSeconds + 1, gceHcTimeoutSeconds, gceHcHealthyThreshold, gceLBHcUnhealthyThreshold},
{"external LB interval - user configured - should keep", utils.XLB, gceNetLBHcCheckIntervalSeconds + 1, gceHcTimeoutSeconds, gceHcHealthyThreshold, gceNetLBHcUnhealthyThreshold, gceNetLBHcCheckIntervalSeconds + 1, gceHcTimeoutSeconds, gceHcHealthyThreshold, gceNetLBHcUnhealthyThreshold},
{"timeout - user configured - should keep", utils.ILB, gceLBHcCheckIntervalSeconds, gceHcTimeoutSeconds + 1, gceHcHealthyThreshold, gceLBHcUnhealthyThreshold, gceLBHcCheckIntervalSeconds, gceHcTimeoutSeconds + 1, gceHcHealthyThreshold, gceLBHcUnhealthyThreshold},
{"healthy threshold - user configured - should keep", utils.ILB, gceLBHcCheckIntervalSeconds, gceHcTimeoutSeconds, gceHcHealthyThreshold + 1, gceLBHcUnhealthyThreshold, gceLBHcCheckIntervalSeconds, gceHcTimeoutSeconds, gceHcHealthyThreshold + 1, gceLBHcUnhealthyThreshold},
{"unhealthy threshold - user configured - should keep", utils.ILB, gceLBHcCheckIntervalSeconds, gceHcTimeoutSeconds, gceHcHealthyThreshold, gceLBHcUnhealthyThreshold + 1, gceLBHcCheckIntervalSeconds, gceHcTimeoutSeconds, gceHcHealthyThreshold, gceLBHcUnhealthyThreshold + 1},
{"external LB unhealthy threshold - user configured - should keep", utils.XLB, gceNetLBHcCheckIntervalSeconds, gceHcTimeoutSeconds, gceHcHealthyThreshold, gceNetLBHcUnhealthyThreshold + 1, gceNetLBHcCheckIntervalSeconds, gceHcTimeoutSeconds, gceHcHealthyThreshold, gceNetLBHcUnhealthyThreshold + 1},
} {
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, "")
wantHC := newL4HealthCheck("hc", types.NamespacedName{Name: "svc", Namespace: "default"}, false, "/", 12345, tc.l4Type, meta.Global, "")
hc := &composite.HealthCheck{
CheckIntervalSec: tc.checkIntervalSec,
TimeoutSec: tc.timeoutSec,
Expand Down Expand Up @@ -78,27 +84,33 @@ func TestCompareHealthChecks(t *testing.T) {
t.Parallel()
for _, tc := range []struct {
desc string
l4Type utils.L4LBType
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},
{"unchanged", utils.ILB, nil, false},
{"unchanged external LB", utils.XLB, nil, false},
{"nil HttpHealthCheck", utils.ILB, func(hc *composite.HealthCheck) { hc.HttpHealthCheck = nil }, true},
{"desc does not match", utils.ILB, func(hc *composite.HealthCheck) { hc.Description = "bad-desc" }, true},
{"port does not match", utils.ILB, func(hc *composite.HealthCheck) { hc.HttpHealthCheck.Port = 54321 }, true},
{"requestPath does not match", utils.ILB, func(hc *composite.HealthCheck) { hc.HttpHealthCheck.RequestPath = "/anotherone" }, true},
{"interval needs update", utils.ILB, func(hc *composite.HealthCheck) { hc.CheckIntervalSec = gceLBHcCheckIntervalSeconds - 1 }, true},
{"external LB interval needs update", utils.XLB, func(hc *composite.HealthCheck) { hc.CheckIntervalSec = gceNetLBHcCheckIntervalSeconds - 1 }, true},
{"timeout needs update", utils.ILB, func(hc *composite.HealthCheck) { hc.TimeoutSec = gceHcTimeoutSeconds - 1 }, true},
{"healthy threshold needs update", utils.ILB, func(hc *composite.HealthCheck) { hc.HealthyThreshold = gceHcHealthyThreshold - 1 }, true},
{"unhealthy threshold needs update", utils.ILB, func(hc *composite.HealthCheck) { hc.UnhealthyThreshold = gceLBHcUnhealthyThreshold - 1 }, true},
{"external LB unhealthy threshold needs update", utils.XLB, func(hc *composite.HealthCheck) { hc.UnhealthyThreshold = gceNetLBHcUnhealthyThreshold - 1 }, true},
{"interval does not need update", utils.ILB, func(hc *composite.HealthCheck) { hc.CheckIntervalSec = gceLBHcCheckIntervalSeconds + 1 }, false},
{"external LB interval does not need update", utils.XLB, func(hc *composite.HealthCheck) { hc.CheckIntervalSec = gceNetLBHcCheckIntervalSeconds + 1 }, false},
{"timeout does not need update", utils.ILB, func(hc *composite.HealthCheck) { hc.TimeoutSec = gceHcTimeoutSeconds + 1 }, false},
{"healthy threshold does not need update", utils.ILB, func(hc *composite.HealthCheck) { hc.HealthyThreshold = gceHcHealthyThreshold + 1 }, false},
{"unhealthy threshold does not need update", utils.ILB, func(hc *composite.HealthCheck) { hc.UnhealthyThreshold = gceLBHcUnhealthyThreshold + 1 }, false},
{"external LB unhealthy threshold does not need update", utils.XLB, func(hc *composite.HealthCheck) { hc.UnhealthyThreshold = gceNetLBHcUnhealthyThreshold + 1 }, false},
} {
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, "")
wantHC := newL4HealthCheck("hc", types.NamespacedName{Name: "svc", Namespace: "default"}, false, "/", 12345, utils.ILB, meta.Global, "")
hc := newL4HealthCheck("hc", types.NamespacedName{Name: "svc", Namespace: "default"}, false, "/", 12345, tc.l4Type, meta.Global, "")
wantHC := newL4HealthCheck("hc", types.NamespacedName{Name: "svc", Namespace: "default"}, false, "/", 12345, tc.l4Type, meta.Global, "")
if tc.modifier != nil {
tc.modifier(hc)
}
Expand All @@ -109,6 +121,32 @@ func TestCompareHealthChecks(t *testing.T) {
}
}

// Checks that correct health check constants used for different LB types (ILB, XLB)
func TestNetLBHealthChecks(t *testing.T) {
t.Parallel()
for _, tc := range []struct {
desc string
l4Type utils.L4LBType
wantCheckIntervalSec int64
wantUnhealthyThreshold int64
}{
{"internal load balancer constants", utils.ILB, gceLBHcCheckIntervalSeconds, gceLBHcUnhealthyThreshold},
{"external load balancer constants", utils.XLB, gceNetLBHcCheckIntervalSeconds, gceNetLBHcUnhealthyThreshold},
} {
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"}, false, "/", 12345, tc.l4Type, 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 235301b

Please sign in to comment.