From 0fe9cdca05fbc33bfb334dbfc118e8b1bdb52d9b Mon Sep 17 00:00:00 2001 From: Nik Shampur Date: Fri, 11 Jan 2019 10:08:49 -0800 Subject: [PATCH 1/4] Reuse the http client for ASM calls. Throttle retries when ServiceUnavailable responses are received --- services/classic/management/client.go | 12 +++++++-- services/classic/management/http.go | 35 +++++++++++++++++---------- 2 files changed, 32 insertions(+), 15 deletions(-) diff --git a/services/classic/management/client.go b/services/classic/management/client.go index 5d75b73a754f..6d3d350b6e47 100644 --- a/services/classic/management/client.go +++ b/services/classic/management/client.go @@ -21,6 +21,7 @@ package management import ( "errors" "fmt" + "net/http" "runtime" "time" @@ -44,6 +45,7 @@ const ( type client struct { publishSettings publishSettings config ClientConfig + httpClient *http.Client } // Client is the base Azure Service Management API client instance that @@ -154,10 +156,16 @@ func makeClient(subscriptionID string, managementCert []byte, config ClientConfi config.UserAgent = DefaultUserAgent } - return client{ + clientObj := client{ publishSettings: publishSettings, config: config, - }, nil + } + var err error + clientObj.httpClient, err = clientObj.createHTTPClient() + if err != nil { + return nil, err + } + return clientObj, nil } func userAgent() string { diff --git a/services/classic/management/http.go b/services/classic/management/http.go index 3573010017af..83bdd5f9c25b 100644 --- a/services/classic/management/http.go +++ b/services/classic/management/http.go @@ -21,6 +21,7 @@ import ( "crypto/tls" "fmt" "net/http" + "time" ) const ( @@ -100,22 +101,26 @@ func (client client) sendAzureRequest(method, url, contentType string, data []by } // createHTTPClient creates an HTTP Client configured with the key pair for -// the subscription for this client. +// the subscription for this client. If a client already exists, then it returns +// the instance func (client client) createHTTPClient() (*http.Client, error) { - cert, err := tls.X509KeyPair(client.publishSettings.SubscriptionCert, client.publishSettings.SubscriptionKey) - if err != nil { - return nil, err - } + if client.httpClient == nil { + cert, err := tls.X509KeyPair(client.publishSettings.SubscriptionCert, client.publishSettings.SubscriptionKey) + if err != nil { + return nil, err + } - return &http.Client{ - Transport: &http.Transport{ - Proxy: http.ProxyFromEnvironment, - TLSClientConfig: &tls.Config{ - Renegotiation: tls.RenegotiateOnceAsClient, - Certificates: []tls.Certificate{cert}, + client.httpClient = &http.Client{ + Transport: &http.Transport{ + Proxy: http.ProxyFromEnvironment, + TLSClientConfig: &tls.Config{ + Renegotiation: tls.RenegotiateFreelyAsClient, + Certificates: []tls.Certificate{cert}, + }, }, - }, - }, nil + } + } + return client.httpClient, nil } // sendRequest sends a request to the Azure management API using the given @@ -161,6 +166,10 @@ func (client client) sendRequest(httpClient *http.Client, url, requestType, cont if numberOfRetries == 0 { return nil, azureErr } + if response.StatusCode == http.StatusServiceUnavailable { + // Wait before retrying the operation + time.Sleep(30 * time.Second) + } return client.sendRequest(httpClient, url, requestType, contentType, data, numberOfRetries-1) } From c572a78b897d10e71e6c70d18b1de07590a4e252 Mon Sep 17 00:00:00 2001 From: Nik Shampur Date: Fri, 11 Jan 2019 13:20:40 -0800 Subject: [PATCH 2/4] code cleanup:- Removed the only other occurence in sendAzureRequest() since the httpClient was already setup in makeClient(). Removed the nil check inside the function as the parameter is passed in as a value type. --- services/classic/management/http.go | 37 +++++++++++------------------ 1 file changed, 14 insertions(+), 23 deletions(-) diff --git a/services/classic/management/http.go b/services/classic/management/http.go index 83bdd5f9c25b..ccb886755482 100644 --- a/services/classic/management/http.go +++ b/services/classic/management/http.go @@ -87,12 +87,7 @@ func (client client) sendAzureRequest(method, url, contentType string, data []by return nil, fmt.Errorf(errParamNotSpecified, "url") } - httpClient, err := client.createHTTPClient() - if err != nil { - return nil, err - } - - response, err := client.sendRequest(httpClient, url, method, contentType, data, 5) + response, err := client.sendRequest(client.httpClient, url, method, contentType, data, 5) if err != nil { return nil, err } @@ -101,26 +96,22 @@ func (client client) sendAzureRequest(method, url, contentType string, data []by } // createHTTPClient creates an HTTP Client configured with the key pair for -// the subscription for this client. If a client already exists, then it returns -// the instance +// the subscription for this client. func (client client) createHTTPClient() (*http.Client, error) { - if client.httpClient == nil { - cert, err := tls.X509KeyPair(client.publishSettings.SubscriptionCert, client.publishSettings.SubscriptionKey) - if err != nil { - return nil, err - } + cert, err := tls.X509KeyPair(client.publishSettings.SubscriptionCert, client.publishSettings.SubscriptionKey) + if err != nil { + return nil, err + } - client.httpClient = &http.Client{ - Transport: &http.Transport{ - Proxy: http.ProxyFromEnvironment, - TLSClientConfig: &tls.Config{ - Renegotiation: tls.RenegotiateFreelyAsClient, - Certificates: []tls.Certificate{cert}, - }, + return &http.Client{ + Transport: &http.Transport{ + Proxy: http.ProxyFromEnvironment, + TLSClientConfig: &tls.Config{ + Renegotiation: tls.RenegotiateFreelyAsClient, + Certificates: []tls.Certificate{cert}, }, - } - } - return client.httpClient, nil + }, + }, nil } // sendRequest sends a request to the Azure management API using the given From 092dc5bb27fb407926937162c43e5252f78f39b8 Mon Sep 17 00:00:00 2001 From: Nik Shampur Date: Fri, 11 Jan 2019 13:44:26 -0800 Subject: [PATCH 3/4] Reverted the change as it was inadvertent during testing --- services/classic/management/http.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/classic/management/http.go b/services/classic/management/http.go index ccb886755482..acf5a586620d 100644 --- a/services/classic/management/http.go +++ b/services/classic/management/http.go @@ -107,7 +107,7 @@ func (client client) createHTTPClient() (*http.Client, error) { Transport: &http.Transport{ Proxy: http.ProxyFromEnvironment, TLSClientConfig: &tls.Config{ - Renegotiation: tls.RenegotiateFreelyAsClient, + Renegotiation: tls.RenegotiateOnceAsClient, Certificates: []tls.Certificate{cert}, }, }, From 9a1e03d4582f804d6869cd2a0788be443678da82 Mon Sep 17 00:00:00 2001 From: Nik Shampur Date: Fri, 11 Jan 2019 14:15:09 -0800 Subject: [PATCH 4/4] Code improvements. Use the configurable Poll interval and check for both service unavaialbe and throttling messages --- services/classic/management/http.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/classic/management/http.go b/services/classic/management/http.go index acf5a586620d..a12f8a156ded 100644 --- a/services/classic/management/http.go +++ b/services/classic/management/http.go @@ -157,9 +157,9 @@ func (client client) sendRequest(httpClient *http.Client, url, requestType, cont if numberOfRetries == 0 { return nil, azureErr } - if response.StatusCode == http.StatusServiceUnavailable { + if response.StatusCode == http.StatusServiceUnavailable || response.StatusCode == http.StatusTooManyRequests { // Wait before retrying the operation - time.Sleep(30 * time.Second) + time.Sleep(client.config.OperationPollInterval) } return client.sendRequest(httpClient, url, requestType, contentType, data, numberOfRetries-1)