Merge pull request openshift#116 from JoelSpeed/client-timeouts

OCPBUGS-29012: Improvements to client timeouts to prevent hangs
openshift-cloud-team · Apr 30, 2024 · 0e95532 · 0e95532
2 parents 8c9dff6 + 77d95e6
commit 0e95532
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 22 deletions.
diff --git a/pkg/azclient/utils/transport.go b/pkg/azclient/utils/transport.go
@@ -35,13 +35,16 @@ func init() {
 				Timeout:   30 * time.Second,
 				KeepAlive: 30 * time.Second,
 			}).DialContext,
-			ForceAttemptHTTP2:   true,
-			MaxIdleConns:        100,
-			MaxConnsPerHost:     100,
-			IdleConnTimeout:     90 * time.Second,
-			TLSHandshakeTimeout: 10 * time.Second,
+			ForceAttemptHTTP2:     true,
+			MaxIdleConns:          100,
+			MaxConnsPerHost:       100,
+			IdleConnTimeout:       90 * time.Second,
+			TLSHandshakeTimeout:   10 * time.Second,
+			ExpectContinueTimeout: 1 * time.Second, // the same as default transport
+			ResponseHeaderTimeout: 60 * time.Second,
 			TLSClientConfig: &tls.Config{
-				MinVersion: tls.VersionTLS12,
+				MinVersion:    tls.VersionTLS12,
+				Renegotiation: tls.RenegotiateNever, // the same as default transport https://pkg.go.dev/crypto/tls#RenegotiationSupport
 			},
 		}
 	})

diff --git a/pkg/azureclients/armclient/azure_armclient.go b/pkg/azureclients/armclient/azure_armclient.go
@@ -81,6 +81,7 @@ func sender() autorest.Sender {
 			IdleConnTimeout:       90 * time.Second, // the same as default transport
 			TLSHandshakeTimeout:   10 * time.Second, // the same as default transport
 			ExpectContinueTimeout: 1 * time.Second,  // the same as default transport
+			ResponseHeaderTimeout: 60 * time.Second,
 			TLSClientConfig: &tls.Config{
 				MinVersion:    tls.VersionTLS12,     //force to use TLS 1.2
 				Renegotiation: tls.RenegotiateNever, // the same as default transport https://pkg.go.dev/crypto/tls#RenegotiationSupport

diff --git a/pkg/provider/azure_loadbalancer_repo.go b/pkg/provider/azure_loadbalancer_repo.go
@@ -40,9 +40,8 @@ import (
 
 // DeleteLB invokes az.LoadBalancerClient.Delete with exponential backoff retry
 func (az *Cloud) DeleteLB(service *v1.Service, lbName string) *retry.Error {
-	ctx, cancel := getContextWithCancel()
-	defer cancel()
-
+	ctx, cancelFunc := context.WithTimeout(context.Background(), 60*time.Second)
+	defer cancelFunc()
 	rgName := az.getLoadBalancerResourceGroup()
 	rerr := az.LoadBalancerClient.Delete(ctx, rgName, lbName)
 	if rerr == nil {
@@ -58,9 +57,8 @@ func (az *Cloud) DeleteLB(service *v1.Service, lbName string) *retry.Error {
 
 // ListLB invokes az.LoadBalancerClient.List with exponential backoff retry
 func (az *Cloud) ListLB(service *v1.Service) ([]network.LoadBalancer, error) {
-	ctx, cancel := getContextWithCancel()
-	defer cancel()
-
+	ctx, cancelFunc := context.WithTimeout(context.Background(), 60*time.Second)
+	defer cancelFunc()
 	rgName := az.getLoadBalancerResourceGroup()
 	allLBs, rerr := az.LoadBalancerClient.List(ctx, rgName)
 	if rerr != nil {
@@ -133,9 +131,8 @@ func (az *Cloud) ListManagedLBs(service *v1.Service, nodes []*v1.Node, clusterNa
 
 // CreateOrUpdateLB invokes az.LoadBalancerClient.CreateOrUpdate with exponential backoff retry
 func (az *Cloud) CreateOrUpdateLB(service *v1.Service, lb network.LoadBalancer) error {
-	ctx, cancel := getContextWithCancel()
-	defer cancel()
-
+	ctx, cancelFunc := context.WithTimeout(context.Background(), 60*time.Second)
+	defer cancelFunc()
 	lb = cleanupSubnetInFrontendIPConfigurations(&lb)
 
 	rgName := az.getLoadBalancerResourceGroup()
@@ -192,9 +189,8 @@ func (az *Cloud) CreateOrUpdateLB(service *v1.Service, lb network.LoadBalancer)
 }
 
 func (az *Cloud) CreateOrUpdateLBBackendPool(lbName string, backendPool network.BackendAddressPool) error {
-	ctx, cancel := getContextWithCancel()
-	defer cancel()
-
+	ctx, cancelFunc := context.WithTimeout(context.Background(), 60*time.Second)
+	defer cancelFunc()
 	klog.V(4).Infof("CreateOrUpdateLBBackendPool: updating backend pool %s in LB %s", pointer.StringDeref(backendPool.Name, ""), lbName)
 	rerr := az.LoadBalancerClient.CreateOrUpdateBackendPools(ctx, az.getLoadBalancerResourceGroup(), lbName, pointer.StringDeref(backendPool.Name, ""), backendPool, pointer.StringDeref(backendPool.Etag, ""))
 	if rerr == nil {
@@ -220,9 +216,8 @@ func (az *Cloud) CreateOrUpdateLBBackendPool(lbName string, backendPool network.
 }
 
 func (az *Cloud) DeleteLBBackendPool(lbName, backendPoolName string) error {
-	ctx, cancel := getContextWithCancel()
-	defer cancel()
-
+	ctx, cancelFunc := context.WithTimeout(context.Background(), 60*time.Second)
+	defer cancelFunc()
 	klog.V(4).Infof("DeleteLBBackendPool: deleting backend pool %s in LB %s", backendPoolName, lbName)
 	rerr := az.LoadBalancerClient.DeleteLBBackendPool(ctx, az.getLoadBalancerResourceGroup(), lbName, backendPoolName)
 	if rerr == nil {
@@ -280,7 +275,9 @@ func cleanupSubnetInFrontendIPConfigurations(lb *network.LoadBalancer) network.L
 func (az *Cloud) MigrateToIPBasedBackendPoolAndWaitForCompletion(
 	lbName string, backendPoolNames []string, nicsCountMap map[string]int,
 ) error {
-	if rerr := az.LoadBalancerClient.MigrateToIPBasedBackendPool(context.Background(), az.ResourceGroup, lbName, backendPoolNames); rerr != nil {
+	ctx, cancelFunc := context.WithTimeout(context.Background(), 60*time.Second)
+	defer cancelFunc()
+	if rerr := az.LoadBalancerClient.MigrateToIPBasedBackendPool(ctx, az.ResourceGroup, lbName, backendPoolNames); rerr != nil {
 		backendPoolNamesStr := strings.Join(backendPoolNames, ",")
 		klog.Errorf("MigrateToIPBasedBackendPoolAndWaitForCompletion: Failed to migrate to IP based backend pool for lb %s, backend pool %s: %s", lbName, backendPoolNamesStr, rerr.Error().Error())
 		return rerr.Error()