[release-1.23] fix: Do not retry regional when request is not GET

pkg/azureclients/armclient/azure_armclient.go

@@ -98,7 +98,6 @@ func New(authorizer autorest.Authorizer, clientConfig azureclients.ClientConfig,
 	client.client.Sender = autorest.DecorateSender(client.client,
 		autorest.DoCloseIfError(),
 		retry.DoExponentialBackoffRetry(backoff),
-		DoHackRegionalRetryDecorator(client),
 	)
 
 	client.client.Sender = autorest.DecorateSender(client.client.Sender, sendDecoraters...)
@@ -126,8 +125,8 @@ func NormalizeAzureRegion(name string) string {
 	return strings.ToLower(region)
 }
 
-// DoHackRegionalRetryDecorator returns an autorest.SendDecorator which performs retry with customizable backoff policy.
-func DoHackRegionalRetryDecorator(c *Client) autorest.SendDecorator {
+// DoHackRegionalRetryForGET checks if GET request returns empty response and retries regional server or returns error.
+func DoHackRegionalRetryForGET(c *Client) autorest.SendDecorator {
 	return func(s autorest.Sender) autorest.Sender {
 		return autorest.SenderFunc(func(request *http.Request) (*http.Response, error) {
 			response, rerr := s.Do(request)
@@ -180,9 +179,10 @@ func DoHackRegionalRetryDecorator(c *Client) autorest.SendDecorator {
 
 			request.Host = c.regionalEndpoint
 			request.URL.Host = c.regionalEndpoint
-			klog.V(5).Infof("Send.sendRegionalRequest on ResourceGroupNotFound error. Retrying regional host: %s", html.EscapeString(request.Host))
+			klog.V(6).Infof("Send.sendRegionalRequest on ResourceGroupNotFound error. Retrying regional host: %s", html.EscapeString(request.Host))
 
 			regionalResponse, regionalError := s.Do(request)
+
 			// only use the result if the regional request actually goes through and returns 2xx status code, for two reasons:
 			// 1. the retry on regional ARM host approach is a hack.
 			// 2. the concatenated regional uri could be wrong as the rule is not officially declared by ARM.
@@ -192,7 +192,7 @@ func DoHackRegionalRetryDecorator(c *Client) autorest.SendDecorator {
 					regionalErrStr = regionalError.Error()
 				}
 
-				klog.V(5).Infof("Send.sendRegionalRequest failed to get response from regional host, error: %q. Ignoring the result.", regionalErrStr)
+				klog.V(6).Infof("Send.sendRegionalRequest failed to get response from regional host, error: %q. Ignoring the result.", regionalErrStr)
 				return response, rerr
 			}
 
@@ -369,9 +369,6 @@ func (c *Client) GetResourceWithExpandQuery(ctx context.Context, resourceID, exp
 // GetResourceWithExpandAPIVersionQuery get a resource by resource ID with expand and API version.
 func (c *Client) GetResourceWithExpandAPIVersionQuery(ctx context.Context, resourceID, expand, apiVersion string) (*http.Response, *retry.Error) {
 	decorators := []autorest.PrepareDecorator{
-		autorest.AsGet(),
-		autorest.WithBaseURL(c.baseURI),
-		autorest.WithPathParameters("{resourceID}", map[string]interface{}{"resourceID": resourceID}),
 		withAPIVersion(apiVersion),
 	}
 	if expand != "" {
@@ -380,15 +377,7 @@ func (c *Client) GetResourceWithExpandAPIVersionQuery(ctx context.Context, resou
 		}))
 	}
 
-	preparer := autorest.CreatePreparer(decorators...)
-	request, err := preparer.Prepare((&http.Request{}).WithContext(ctx))
-
-	if err != nil {
-		klog.V(5).Infof("Received error in %s: resourceID: %s, error: %s", "get.prepare", resourceID, err)
-		return nil, retry.NewError(false, err)
-	}
-
-	return c.Send(ctx, request)
+	return c.GetResource(ctx, resourceID, decorators...)
 }
 
 // GetResourceWithDecorators get a resource with decorators by resource ID
@@ -402,7 +391,7 @@ func (c *Client) GetResource(ctx context.Context, resourceID string, decorators
 		return nil, retry.NewError(false, err)
 	}
 
-	return c.Send(ctx, request)
+	return c.Send(ctx, request, DoHackRegionalRetryForGET(c))
 }
 
 // PutResource puts a resource by resource ID

pkg/azureclients/armclient/azure_armclient_test.go

@@ -75,7 +75,7 @@ func TestSend(t *testing.T) {
 	assert.Equal(t, 2, count)
 	assert.Equal(t, http.StatusOK, response.StatusCode)
 }
-func TestSendFailureRegionalRetry(t *testing.T) {
+func TestDoHackRegionalRetryForGET(t *testing.T) {
 	testcases := []struct {
 		description               string
 		globalServerErrMsg        string
@@ -121,24 +121,13 @@ func TestSendFailureRegionalRetry(t *testing.T) {
 			armClient := New(nil, azConfig, server.URL, "2019-01-01")
 			targetURL, _ := url.Parse(server.URL)
 			armClient.regionalEndpoint = targetURL.Host
-			pathParameters := map[string]interface{}{
-				"resourceGroupName": autorest.Encode("path", "testgroup"),
-				"subscriptionId":    autorest.Encode("path", "testid"),
-				"resourceName":      autorest.Encode("path", "testname"),
-			}
-
-			decorators := []autorest.PrepareDecorator{
-				autorest.WithPathParameters(
-					"/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Network/vNets/{resourceName}", pathParameters),
-				autorest.WithBaseURL(globalServer.URL),
-			}
+			armClient.baseURI = globalServer.URL
 
+			resourceID := "/subscriptions/testid/resourceGroups/restgroup/providers/Microsoft.Network/vNets/testname"
 			ctx := context.Background()
-			request, err := armClient.PrepareGetRequest(ctx, decorators...)
-			assert.NoError(t, err)
-
-			response, rerr := armClient.Send(ctx, request)
-			assert.Nil(t, rerr, rerr.Error())
+			response, rerr := armClient.GetResource(ctx, resourceID)
+			assert.Nil(t, rerr)
+			assert.NotNil(t, response)
 			assert.Equal(t, http.StatusOK, response.StatusCode)
 			assert.Equal(t, targetURL.Host, response.Request.URL.Host)
 		})