Merge pull request kubernetes#66045 from cpuguy83/az_lb_timeout

Automatic merge from submit-queue (batch tested with PRs 66121, 66140, 66045). 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>.

Support setting azure LB idle timeout

**What this PR does / why we need it**:

Adds a new annotation to allow users to configure the idle timeout of
the Azure LB.

**Release note**:

```release-note
Support configuring the Azure load balancer idle connection timeout for services
```

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.

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)
+			}
+		})
+	}
+}