Skip to content

Commit

Permalink
[Cherrypick kubernetes#1828] l4 healthcheck timeouts change
Browse files Browse the repository at this point in the history 
This change adds changed l4 healthcheck timeouts
to the v1.14 release.
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 sharedHc=false.

Signed-off-by: Elina Akhmanova <elinka@google.com>
  • Loading branch information
@code-elinka
code-elinka committed Oct 13, 2022
1 parent b5153b8 commit 093ba2c
Show file tree
Hide file tree
Showing 2 changed files with 259 additions and 34 deletions.
38 changes: 31 additions & 7 deletions pkg/healthchecks/healthchecks_l4.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
Expand Down
255 changes: 228 additions & 27 deletions pkg/healthchecks/healthchecks_l4_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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{
Expand Down Expand Up @@ -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)
Expand All @@ -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)
}
})
}
}

0 comments on commit 093ba2c

Please sign in to comment.