Fix error message for long-waiting operations

Get HTTP responses together with error, so that we could get full error
messages. It also fixes some edge cases for success responses.

Author: feiskyer

cluster-autoscaler/cloudprovider/azure/azure_agent_pool.go

@@ -17,6 +17,7 @@ limitations under the License.
 package azure
 
 import (
+	"errors"
 	"fmt"
 	"math/rand"
 	"sort"
@@ -209,15 +210,21 @@ func (as *AgentPool) IncreaseSize(delta int) error {
 	}
 	ctx, cancel := getContextWithCancel()
 	defer cancel()
-	_, err = as.manager.azClient.deploymentsClient.CreateOrUpdate(ctx, as.manager.config.ResourceGroup, newDeploymentName, newDeployment)
 	klog.V(3).Infof("Waiting for deploymentsClient.CreateOrUpdate(%s, %s, %v)", as.manager.config.ResourceGroup, newDeploymentName, newDeployment)
-	if err != nil {
-		return err
+	resp, err := as.manager.azClient.deploymentsClient.CreateOrUpdate(ctx, as.manager.config.ResourceGroup, newDeploymentName, newDeployment)
+	if isSuccessHTTPResponse(resp) {
+		klog.V(3).Infof("deploymentsClient.CreateOrUpdate(%s, %s, %v) success", as.manager.config.ResourceGroup, newDeploymentName, newDeployment)
+		as.curSize = int64(expectedSize)
+		as.lastRefresh = time.Now()
+		return nil
 	}
 
-	as.curSize = int64(expectedSize)
-	as.lastRefresh = time.Now()
-	return err
+	errMessage := err.Error()
+	if resp != nil {
+		errMessage = fmt.Sprintf("%s with HTTP status %s", err.Error(), resp.Status)
+	}
+	klog.Errorf("deploymentsClient.CreateOrUpdate for deployment %q failed: %s", newDeploymentName, errMessage)
+	return errors.New(errMessage)
 }
 
 // GetVirtualMachines returns list of nodes for the given agent pool.

cluster-autoscaler/cloudprovider/azure/azure_container_service_pool.go

@@ -17,6 +17,7 @@ limitations under the License.
 package azure
 
 import (
+	"errors"
 	"fmt"
 	"strings"
 
@@ -193,7 +194,20 @@ func (agentPool *ContainerServiceAgentPool) setAKSNodeCount(count int) error {
 		klog.Errorf("Failed to update AKS cluster (%q): %v", agentPool.clusterName, err)
 		return err
 	}
-	return future.WaitForCompletionRef(updateCtx, aksClient.Client)
+
+	err = future.WaitForCompletionRef(updateCtx, aksClient.Client)
+	resp := future.Response()
+	if isSuccessHTTPResponse(resp) {
+		klog.V(3).Infof("aksClient.CreateOrUpdate for aks cluster %q success", agentPool.clusterName)
+		return nil
+	}
+
+	errMessage := err.Error()
+	if resp != nil {
+		errMessage = fmt.Sprintf("%s with HTTP status %s", err.Error(), resp.Status)
+	}
+	klog.Errorf("aksClient.CreateOrUpdate for aks cluster %q failed: %s", agentPool.clusterName, errMessage)
+	return errors.New(errMessage)
 }
 
 // setACSNodeCount sets node count for ACS agent pool.
@@ -226,7 +240,20 @@ func (agentPool *ContainerServiceAgentPool) setACSNodeCount(count int) error {
 		klog.Errorf("Failed to update ACS cluster (%q): %v", agentPool.clusterName, err)
 		return err
 	}
-	return future.WaitForCompletionRef(updateCtx, acsClient.Client)
+
+	err = future.WaitForCompletionRef(updateCtx, acsClient.Client)
+	resp := future.Response()
+	if isSuccessHTTPResponse(resp) {
+		klog.V(3).Infof("acsClient.CreateOrUpdate for acs cluster %q success", agentPool.clusterName)
+		return nil
+	}
+
+	errMessage := err.Error()
+	if resp != nil {
+		errMessage = fmt.Sprintf("%s with HTTP status %s", err.Error(), resp.Status)
+	}
+	klog.Errorf("acsClient.CreateOrUpdate for acs cluster %q failed: %s", agentPool.clusterName, errMessage)
+	return errors.New(errMessage)
 }
 
 //GetNodeCount returns the count of nodes from the managed agent pool profile

cluster-autoscaler/cloudprovider/azure/azure_scale_set.go

@@ -17,6 +17,7 @@ limitations under the License.
 package azure
 
 import (
+	"errors"
 	"fmt"
 	"math/rand"
 	"strings"
@@ -153,15 +154,21 @@ func (scaleSet *ScaleSet) SetScaleSetSize(size int64) error {
 	op.VirtualMachineScaleSetProperties.ProvisioningState = nil
 	updateCtx, updateCancel := getContextWithCancel()
 	defer updateCancel()
-	_, err = scaleSet.manager.azClient.virtualMachineScaleSetsClient.CreateOrUpdate(updateCtx, resourceGroup, scaleSet.Name, op)
-	if err != nil {
-		klog.Errorf("virtualMachineScaleSetsClient.CreateOrUpdate for scale set %q failed: %v", scaleSet.Name, err)
-		return err
+	klog.V(3).Infof("Waiting for virtualMachineScaleSetsClient.CreateOrUpdate(%s)", scaleSet.Name)
+	resp, err := scaleSet.manager.azClient.virtualMachineScaleSetsClient.CreateOrUpdate(updateCtx, resourceGroup, scaleSet.Name, op)
+	if isSuccessHTTPResponse(resp) {
+		klog.V(3).Infof("virtualMachineScaleSetsClient.CreateOrUpdate(%s) success", scaleSet.Name)
+		scaleSet.curSize = size
+		scaleSet.lastRefresh = time.Now()
+		return nil
 	}
 
-	scaleSet.curSize = size
-	scaleSet.lastRefresh = time.Now()
-	return nil
+	errMessage := err.Error()
+	if resp != nil {
+		errMessage = fmt.Sprintf("%s with HTTP status %s", err.Error(), resp.Status)
+	}
+	klog.Errorf("virtualMachineScaleSetsClient.CreateOrUpdate for scale set %q failed: %s", scaleSet.Name, errMessage)
+	return errors.New(errMessage)
 }
 
 // TargetSize returns the current TARGET size of the node group. It is possible that the

cluster-autoscaler/cloudprovider/azure/azure_util.go

@@ -609,3 +609,17 @@ func checkResourceExistsFromError(err error) (bool, error) {
 	}
 	return false, v
 }
+
+// isSuccessHTTPResponse determines if the response from an HTTP request suggests success
+func isSuccessHTTPResponse(resp *http.Response) bool {
+	if resp == nil {
+		return false
+	}
+
+	// HTTP 2xx suggests a successful response
+	if 199 < resp.StatusCode && resp.StatusCode < 300 {
+		return true
+	}
+
+	return false
+}

cluster-autoscaler/cloudprovider/azure/azure_util_test.go

@@ -18,6 +18,7 @@ package azure
 
 import (
 	"fmt"
+	"net/http"
 	"testing"
 
 	"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2018-10-01/compute"
@@ -124,3 +125,33 @@ func TestGetVMNameIndexWindows(t *testing.T) {
 		t.Fatalf("unexpected error: %s", err)
 	}
 }
+
+func TestIsSuccessResponse(t *testing.T) {
+	tests := []struct {
+		code     int
+		expected bool
+	}{
+		{
+			code:     http.StatusNotFound,
+			expected: false,
+		},
+		{
+			code:     http.StatusInternalServerError,
+			expected: false,
+		},
+		{
+			code:     http.StatusOK,
+			expected: true,
+		},
+	}
+
+	for _, test := range tests {
+		resp := http.Response{
+			StatusCode: test.code,
+		}
+		res := isSuccessHTTPResponse(&resp)
+		if res != test.expected {
+			t.Errorf("expected: %v, saw: %v", test.expected, res)
+		}
+	}
+}