From 55569494d64dd93777ca42437055884d6a99c46d Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Tue, 10 Jul 2018 10:30:01 -0700 Subject: [PATCH] Support setting azure LB idle timeout Adds a new annotation to allow users to configure the idle timeout of the Azure LB. --- .../providers/azure/azure_loadbalancer.go | 40 ++++++++++++++++++- .../azure/azure_loadbalancer_test.go | 33 +++++++++++++++ 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/pkg/cloudprovider/providers/azure/azure_loadbalancer.go b/pkg/cloudprovider/providers/azure/azure_loadbalancer.go index fca68fbb404..8c8e1ec493b 100644 --- a/pkg/cloudprovider/providers/azure/azure_loadbalancer.go +++ b/pkg/cloudprovider/providers/azure/azure_loadbalancer.go @@ -74,6 +74,10 @@ const ( // ServiceAnnotationAllowedServiceTag is the annotation used on the service // to specify a list of allowed service tags separated by comma ServiceAnnotationAllowedServiceTag = "service.beta.kubernetes.io/azure-allowed-service-tags" + + // ServiceAnnotationLoadBalancerIdleTimeout is the annotation used on the service + // to specify the idle timeout for connections on the load balancer in minutes. + ServiceAnnotationLoadBalancerIdleTimeout = "service.beta.kubernetes.io/azure-load-balancer-tcp-idle-timeout" ) var ( @@ -467,6 +471,31 @@ func (az *Cloud) ensurePublicIPExists(service *v1.Service, pipName string, domai return &pip, nil } +func getIdleTimeout(s *v1.Service) (*int32, error) { + const ( + min = 4 + max = 30 + ) + + val, ok := s.Annotations[ServiceAnnotationLoadBalancerIdleTimeout] + if !ok { + // Return a nil here as this will set the value to the azure default + return nil, nil + } + + errInvalidTimeout := fmt.Errorf("idle timeout value must be a whole number representing minutes between %d and %d", min, max) + to, err := strconv.Atoi(val) + if err != nil { + return nil, fmt.Errorf("error parsing idle timeout value: %v: %v", err, errInvalidTimeout) + } + to32 := int32(to) + + if to32 < min || to32 > max { + return nil, errInvalidTimeout + } + return &to32, nil +} + // This ensures load balancer exists and the frontend ip config is setup. // This also reconciles the Service's Ports with the LoadBalancer config. // This entails adding rules/probes for expected Ports and removing stale rules/ports. @@ -487,6 +516,11 @@ func (az *Cloud) reconcileLoadBalancer(clusterName string, service *v1.Service, lbBackendPoolName := getBackendPoolName(clusterName) lbBackendPoolID := az.getBackendPoolID(lbName, lbBackendPoolName) + lbIdleTimeout, err := getIdleTimeout(service) + if err != nil { + return nil, err + } + dirtyLb := false // Ensure LoadBalancer's Backend Pool Configuration @@ -683,6 +717,9 @@ func (az *Cloud) reconcileLoadBalancer(clusterName string, service *v1.Service, EnableFloatingIP: to.BoolPtr(true), }, } + if port.Protocol == v1.ProtocolTCP { + expectedRule.LoadBalancingRulePropertiesFormat.IdleTimeoutInMinutes = lbIdleTimeout + } // we didn't construct the probe objects for UDP because they're not used/needed/allowed if port.Protocol != v1.ProtocolUDP { @@ -1280,7 +1317,8 @@ func equalLoadBalancingRulePropertiesFormat(s, t *network.LoadBalancingRulePrope reflect.DeepEqual(s.LoadDistribution, t.LoadDistribution) && reflect.DeepEqual(s.FrontendPort, t.FrontendPort) && reflect.DeepEqual(s.BackendPort, t.BackendPort) && - reflect.DeepEqual(s.EnableFloatingIP, t.EnableFloatingIP) + reflect.DeepEqual(s.EnableFloatingIP, t.EnableFloatingIP) && + reflect.DeepEqual(s.IdleTimeoutInMinutes, t.IdleTimeoutInMinutes) } // This compares rule's Name, Protocol, SourcePortRange, DestinationPortRange, SourceAddressPrefix, Access, and Direction. diff --git a/pkg/cloudprovider/providers/azure/azure_loadbalancer_test.go b/pkg/cloudprovider/providers/azure/azure_loadbalancer_test.go index ee0ed967de2..f11742e5f39 100644 --- a/pkg/cloudprovider/providers/azure/azure_loadbalancer_test.go +++ b/pkg/cloudprovider/providers/azure/azure_loadbalancer_test.go @@ -18,11 +18,13 @@ package azure import ( "fmt" + "reflect" "testing" "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2017-09-01/network" "github.com/Azure/go-autorest/autorest/to" "github.com/stretchr/testify/assert" + "k8s.io/api/core/v1" ) func TestFindProbe(t *testing.T) { @@ -210,3 +212,34 @@ func TestFindRule(t *testing.T) { assert.Equal(t, test.expected, findResult, fmt.Sprintf("TestCase[%d]: %s", i, test.msg)) } } + +func TestGetIdleTimeout(t *testing.T) { + for _, c := range []struct { + desc string + annotations map[string]string + i *int32 + err bool + }{ + {desc: "no annotation"}, + {desc: "annotation empty value", annotations: map[string]string{ServiceAnnotationLoadBalancerIdleTimeout: ""}, err: true}, + {desc: "annotation not a number", annotations: map[string]string{ServiceAnnotationLoadBalancerIdleTimeout: "cookies"}, err: true}, + {desc: "annotation negative value", annotations: map[string]string{ServiceAnnotationLoadBalancerIdleTimeout: "-6"}, err: true}, + {desc: "annotation zero value", annotations: map[string]string{ServiceAnnotationLoadBalancerIdleTimeout: "0"}, err: true}, + {desc: "annotation too low value", annotations: map[string]string{ServiceAnnotationLoadBalancerIdleTimeout: "3"}, err: true}, + {desc: "annotation too high value", annotations: map[string]string{ServiceAnnotationLoadBalancerIdleTimeout: "31"}, err: true}, + {desc: "annotation good value", annotations: map[string]string{ServiceAnnotationLoadBalancerIdleTimeout: "24"}, i: to.Int32Ptr(24)}, + } { + t.Run(c.desc, func(t *testing.T) { + s := &v1.Service{} + s.Annotations = c.annotations + i, err := getIdleTimeout(s) + + if !reflect.DeepEqual(c.i, i) { + t.Fatalf("got unexpected value: %d", to.Int32(i)) + } + if (err != nil) != c.err { + t.Fatalf("expected error=%v, got %v", c.err, err) + } + }) + } +}