Merge pull request #56024 from dimpavloff/aws-elb-set-hc-params

Automatic merge from submit-queue (batch tested with PRs 56211, 56024). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

allow ELB Healthcheck configuration via Service annotations

**What this PR does / why we need it**:
The default settings which are set on the ELB HC work well but there are cases when it would be better to tweak its parameters -- for example, faster detection of unhealthy backends. This PR makes it possible to override any of the healthcheck's parameters via annotations on the Service, with the exception of the Target setting which continues to be inferred from the Service's spec.

**Release note**:
```release-note
It is now possible to override the healthcheck parameters for AWS ELBs via annotations on the corresponding service. The new annotations are `healthy-threshold`, `unhealthy-threshold`, `timeout`, `interval` (all prefixed with `service.beta.kubernetes.io/aws-load-balancer-healthcheck-`)
```
This commit is contained in:
Kubernetes Submit Queue 2017-11-22 08:48:43 -08:00 committed by GitHub
commit 6b1b6d504a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 221 additions and 29 deletions

View File

@ -81,6 +81,7 @@ go_test(
"//vendor/github.com/aws/aws-sdk-go/service/elb:go_default_library",
"//vendor/github.com/stretchr/testify/assert:go_default_library",
"//vendor/github.com/stretchr/testify/mock:go_default_library",
"//vendor/github.com/stretchr/testify/require:go_default_library",
"//vendor/k8s.io/api/core/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/types:go_default_library",

View File

@ -172,6 +172,25 @@ const ServiceAnnotationLoadBalancerBEProtocol = "service.beta.kubernetes.io/aws-
// For example: "Key1=Val1,Key2=Val2,KeyNoVal1=,KeyNoVal2"
const ServiceAnnotationLoadBalancerAdditionalTags = "service.beta.kubernetes.io/aws-load-balancer-additional-resource-tags"
// ServiceAnnotationLoadBalancerHCHealthyThreshold is the annotation used on
// the service to specify the number of successive successful health checks
// required for a backend to be considered healthy for traffic.
const ServiceAnnotationLoadBalancerHCHealthyThreshold = "service.beta.kubernetes.io/aws-load-balancer-healthcheck-healthy-threshold"
// ServiceAnnotationLoadBalancerHCUnhealthyThreshold is the annotation used
// on the service to specify the number of unsuccessful health checks
// required for a backend to be considered unhealthy for traffic
const ServiceAnnotationLoadBalancerHCUnhealthyThreshold = "service.beta.kubernetes.io/aws-load-balancer-healthcheck-unhealthy-threshold"
// ServiceAnnotationLoadBalancerHCTimeout is the annotation used on the
// service to specify, in seconds, how long to wait before marking a health
// check as failed.
const ServiceAnnotationLoadBalancerHCTimeout = "service.beta.kubernetes.io/aws-load-balancer-healthcheck-timeout"
// ServiceAnnotationLoadBalancerHCInterval is the annotation used on the
// service to specify, in seconds, the interval between health checks.
const ServiceAnnotationLoadBalancerHCInterval = "service.beta.kubernetes.io/aws-load-balancer-healthcheck-interval"
// Event key when a volume is stuck on attaching state when being attached to a volume
const volumeAttachmentStuck = "VolumeAttachmentStuck"
@ -3375,7 +3394,7 @@ func (c *Cloud) EnsureLoadBalancer(clusterName string, apiService *v1.Service, n
if path, healthCheckNodePort := service.GetServiceHealthCheckPathPort(apiService); path != "" {
glog.V(4).Infof("service %v (%v) needs health checks on :%d%s)", apiService.Name, loadBalancerName, healthCheckNodePort, path)
err = c.ensureLoadBalancerHealthCheck(loadBalancer, "HTTP", healthCheckNodePort, path)
err = c.ensureLoadBalancerHealthCheck(loadBalancer, "HTTP", healthCheckNodePort, path, annotations)
if err != nil {
return nil, fmt.Errorf("Failed to ensure health check for localized service %v on node port %v: %q", loadBalancerName, healthCheckNodePort, err)
}
@ -3391,7 +3410,7 @@ func (c *Cloud) EnsureLoadBalancer(clusterName string, apiService *v1.Service, n
break
}
// there must be no path on TCP health check
err = c.ensureLoadBalancerHealthCheck(loadBalancer, "TCP", tcpHealthCheckPort, "")
err = c.ensureLoadBalancerHealthCheck(loadBalancer, "TCP", tcpHealthCheckPort, "", annotations)
if err != nil {
return nil, err
}

View File

@ -34,9 +34,19 @@ import (
"k8s.io/apimachinery/pkg/util/sets"
)
const ProxyProtocolPolicyName = "k8s-proxyprotocol-enabled"
const (
ProxyProtocolPolicyName = "k8s-proxyprotocol-enabled"
const SSLNegotiationPolicyNameFormat = "k8s-SSLNegotiationPolicy-%s"
SSLNegotiationPolicyNameFormat = "k8s-SSLNegotiationPolicy-%s"
)
var (
// Defaults for ELB Healthcheck
defaultHCHealthyThreshold = int64(2)
defaultHCUnhealthyThreshold = int64(6)
defaultHCTimeout = int64(5)
defaultHCInterval = int64(10)
)
func isNLB(annotations map[string]string) bool {
if annotations[ServiceAnnotationLoadBalancerType] == "nlb" {
@ -1173,44 +1183,72 @@ func awsArnEquals(l, r *string) bool {
return strings.EqualFold(aws.StringValue(l), aws.StringValue(r))
}
// getExpectedHealthCheck returns an elb.Healthcheck for the provided target
// and using either sensible defaults or overrides via Service annotations
func (c *Cloud) getExpectedHealthCheck(target string, annotations map[string]string) (*elb.HealthCheck, error) {
healthcheck := &elb.HealthCheck{Target: &target}
getOrDefault := func(annotation string, defaultValue int64) (*int64, error) {
i64 := defaultValue
var err error
if s, ok := annotations[annotation]; ok {
i64, err = strconv.ParseInt(s, 10, 0)
if err != nil {
return nil, fmt.Errorf("failed parsing health check annotation value: %v", err)
}
}
return &i64, nil
}
var err error
healthcheck.HealthyThreshold, err = getOrDefault(ServiceAnnotationLoadBalancerHCHealthyThreshold, defaultHCHealthyThreshold)
if err != nil {
return nil, err
}
healthcheck.UnhealthyThreshold, err = getOrDefault(ServiceAnnotationLoadBalancerHCUnhealthyThreshold, defaultHCUnhealthyThreshold)
if err != nil {
return nil, err
}
healthcheck.Timeout, err = getOrDefault(ServiceAnnotationLoadBalancerHCTimeout, defaultHCTimeout)
if err != nil {
return nil, err
}
healthcheck.Interval, err = getOrDefault(ServiceAnnotationLoadBalancerHCInterval, defaultHCInterval)
if err != nil {
return nil, err
}
if err = healthcheck.Validate(); err != nil {
return nil, fmt.Errorf("some of the load balancer health check parameters are invalid: %v", err)
}
return healthcheck, nil
}
// Makes sure that the health check for an ELB matches the configured health check node port
func (c *Cloud) ensureLoadBalancerHealthCheck(loadBalancer *elb.LoadBalancerDescription, protocol string, port int32, path string) error {
func (c *Cloud) ensureLoadBalancerHealthCheck(loadBalancer *elb.LoadBalancerDescription, protocol string, port int32, path string, annotations map[string]string) error {
name := aws.StringValue(loadBalancer.LoadBalancerName)
actual := loadBalancer.HealthCheck
// Default AWS settings
expectedHealthyThreshold := int64(2)
expectedUnhealthyThreshold := int64(6)
expectedTimeout := int64(5)
expectedInterval := int64(10)
expectedTarget := protocol + ":" + strconv.FormatInt(int64(port), 10) + path
expected, err := c.getExpectedHealthCheck(expectedTarget, annotations)
if err != nil {
return fmt.Errorf("cannot update health check for load balancer %q: %q", name, err)
}
if expectedTarget == aws.StringValue(actual.Target) &&
expectedHealthyThreshold == orZero(actual.HealthyThreshold) &&
expectedUnhealthyThreshold == orZero(actual.UnhealthyThreshold) &&
expectedTimeout == orZero(actual.Timeout) &&
expectedInterval == orZero(actual.Interval) {
// comparing attributes 1 by 1 to avoid breakage in case a new field is
// added to the HC which breaks the equality
if aws.StringValue(expected.Target) == aws.StringValue(actual.Target) &&
aws.Int64Value(expected.HealthyThreshold) == aws.Int64Value(actual.HealthyThreshold) &&
aws.Int64Value(expected.UnhealthyThreshold) == aws.Int64Value(actual.UnhealthyThreshold) &&
aws.Int64Value(expected.Interval) == aws.Int64Value(actual.Interval) &&
aws.Int64Value(expected.Timeout) == aws.Int64Value(actual.Timeout) {
return nil
}
glog.V(2).Infof("Updating load-balancer health-check for %q", name)
healthCheck := &elb.HealthCheck{}
healthCheck.HealthyThreshold = &expectedHealthyThreshold
healthCheck.UnhealthyThreshold = &expectedUnhealthyThreshold
healthCheck.Timeout = &expectedTimeout
healthCheck.Interval = &expectedInterval
healthCheck.Target = &expectedTarget
request := &elb.ConfigureHealthCheckInput{}
request.HealthCheck = healthCheck
request.HealthCheck = expected
request.LoadBalancerName = loadBalancer.LoadBalancerName
_, err := c.elb.ConfigureHealthCheck(request)
_, err = c.elb.ConfigureHealthCheck(request)
if err != nil {
return fmt.Errorf("error configuring load-balancer health-check for %q: %q", name, err)
return fmt.Errorf("error configuring load balancer health check for %q: %q", name, err)
}
return nil

View File

@ -32,6 +32,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
"k8s.io/apimachinery/pkg/types"
kubeletapis "k8s.io/kubernetes/pkg/kubelet/apis"
)
@ -87,6 +88,24 @@ func (m *MockedFakeELB) AddTags(input *elb.AddTagsInput) (*elb.AddTagsOutput, er
return args.Get(0).(*elb.AddTagsOutput), nil
}
func (m *MockedFakeELB) ConfigureHealthCheck(input *elb.ConfigureHealthCheckInput) (*elb.ConfigureHealthCheckOutput, error) {
args := m.Called(input)
if args.Get(0) == nil {
return nil, args.Error(1)
}
return args.Get(0).(*elb.ConfigureHealthCheckOutput), args.Error(1)
}
func (m *MockedFakeELB) expectConfigureHealthCheck(loadBalancerName *string, expectedHC *elb.HealthCheck, returnErr error) {
expected := &elb.ConfigureHealthCheckInput{HealthCheck: expectedHC, LoadBalancerName: loadBalancerName}
call := m.On("ConfigureHealthCheck", expected)
if returnErr != nil {
call.Return(nil, returnErr)
} else {
call.Return(&elb.ConfigureHealthCheckOutput{}, nil)
}
}
func TestReadAWSCloudConfig(t *testing.T) {
tests := []struct {
name string
@ -1160,6 +1179,121 @@ func TestAddLoadBalancerTags(t *testing.T) {
awsServices.elb.(*MockedFakeELB).AssertExpectations(t)
}
func TestEnsureLoadBalancerHealthCheck(t *testing.T) {
tests := []struct {
name string
annotations map[string]string
overriddenFieldName string
overriddenValue int64
}{
{"falls back to HC defaults", map[string]string{}, "", int64(0)},
{"healthy threshold override", map[string]string{ServiceAnnotationLoadBalancerHCHealthyThreshold: "7"}, "HealthyThreshold", int64(7)},
{"unhealthy threshold override", map[string]string{ServiceAnnotationLoadBalancerHCUnhealthyThreshold: "7"}, "UnhealthyThreshold", int64(7)},
{"timeout override", map[string]string{ServiceAnnotationLoadBalancerHCTimeout: "7"}, "Timeout", int64(7)},
{"interval override", map[string]string{ServiceAnnotationLoadBalancerHCInterval: "7"}, "Interval", int64(7)},
}
lbName := "myLB"
// this HC will always differ from the expected HC and thus it is expected an
// API call will be made to update it
currentHC := &elb.HealthCheck{}
elbDesc := &elb.LoadBalancerDescription{LoadBalancerName: &lbName, HealthCheck: currentHC}
defaultHealthyThreshold := int64(2)
defaultUnhealthyThreshold := int64(6)
defaultTimeout := int64(5)
defaultInterval := int64(10)
protocol, path, port := "tcp", "", int32(8080)
target := "tcp:8080"
defaultHC := &elb.HealthCheck{
HealthyThreshold: &defaultHealthyThreshold,
UnhealthyThreshold: &defaultUnhealthyThreshold,
Timeout: &defaultTimeout,
Interval: &defaultInterval,
Target: &target,
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
awsServices := newMockedFakeAWSServices(TestClusterId)
c, err := newAWSCloud(strings.NewReader("[global]"), awsServices)
assert.Nil(t, err, "Error building aws cloud: %v", err)
expectedHC := *defaultHC
if test.overriddenFieldName != "" { // cater for test case with no overrides
value := reflect.ValueOf(&test.overriddenValue)
reflect.ValueOf(&expectedHC).Elem().FieldByName(test.overriddenFieldName).Set(value)
}
awsServices.elb.(*MockedFakeELB).expectConfigureHealthCheck(&lbName, &expectedHC, nil)
err = c.ensureLoadBalancerHealthCheck(elbDesc, protocol, port, path, test.annotations)
require.Nil(t, err)
awsServices.elb.(*MockedFakeELB).AssertExpectations(t)
})
}
t.Run("does not make an API call if the current health check is the same", func(t *testing.T) {
awsServices := newMockedFakeAWSServices(TestClusterId)
c, err := newAWSCloud(strings.NewReader("[global]"), awsServices)
assert.Nil(t, err, "Error building aws cloud: %v", err)
expectedHC := *defaultHC
timeout := int64(3)
expectedHC.Timeout = &timeout
annotations := map[string]string{ServiceAnnotationLoadBalancerHCTimeout: "3"}
var currentHC elb.HealthCheck
currentHC = expectedHC
// NOTE no call expectations are set on the ELB mock
// test default HC
elbDesc := &elb.LoadBalancerDescription{LoadBalancerName: &lbName, HealthCheck: defaultHC}
err = c.ensureLoadBalancerHealthCheck(elbDesc, protocol, port, path, map[string]string{})
assert.Nil(t, err)
// test HC with override
elbDesc = &elb.LoadBalancerDescription{LoadBalancerName: &lbName, HealthCheck: &currentHC}
err = c.ensureLoadBalancerHealthCheck(elbDesc, protocol, port, path, annotations)
assert.Nil(t, err)
})
t.Run("validates resulting expected health check before making an API call", func(t *testing.T) {
awsServices := newMockedFakeAWSServices(TestClusterId)
c, err := newAWSCloud(strings.NewReader("[global]"), awsServices)
assert.Nil(t, err, "Error building aws cloud: %v", err)
expectedHC := *defaultHC
invalidThreshold := int64(1)
expectedHC.HealthyThreshold = &invalidThreshold
require.Error(t, expectedHC.Validate()) // confirm test precondition
annotations := map[string]string{ServiceAnnotationLoadBalancerHCTimeout: "1"}
// NOTE no call expectations are set on the ELB mock
err = c.ensureLoadBalancerHealthCheck(elbDesc, protocol, port, path, annotations)
require.Error(t, err)
})
t.Run("handles invalid override values", func(t *testing.T) {
awsServices := newMockedFakeAWSServices(TestClusterId)
c, err := newAWSCloud(strings.NewReader("[global]"), awsServices)
assert.Nil(t, err, "Error building aws cloud: %v", err)
annotations := map[string]string{ServiceAnnotationLoadBalancerHCTimeout: "3.3"}
// NOTE no call expectations are set on the ELB mock
err = c.ensureLoadBalancerHealthCheck(elbDesc, protocol, port, path, annotations)
require.Error(t, err)
})
t.Run("returns error when updating the health check fails", func(t *testing.T) {
awsServices := newMockedFakeAWSServices(TestClusterId)
c, err := newAWSCloud(strings.NewReader("[global]"), awsServices)
assert.Nil(t, err, "Error building aws cloud: %v", err)
returnErr := fmt.Errorf("throttling error")
awsServices.elb.(*MockedFakeELB).expectConfigureHealthCheck(&lbName, defaultHC, returnErr)
err = c.ensureLoadBalancerHealthCheck(elbDesc, protocol, port, path, map[string]string{})
require.Error(t, err)
awsServices.elb.(*MockedFakeELB).AssertExpectations(t)
})
}
func newMockedFakeAWSServices(id string) *FakeAWSServices {
s := NewFakeAWSServices(id)
s.ec2 = &MockedFakeEC2{FakeEC2Impl: s.ec2.(*FakeEC2Impl)}