diff --git a/pkg/api/types.go b/pkg/api/types.go index 2d680472e4..eb056ea74b 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -149,8 +149,6 @@ const ( Migrating ProvisioningState = "Migrating" // Upgrading means an existing ContainerService resource is being upgraded Upgrading ProvisioningState = "Upgrading" - // Canceled means a deployment has been canceled - Canceled ProvisioningState = "Canceled" ) // OrchestratorProfile contains Orchestrator properties diff --git a/pkg/apierror/apierror.go b/pkg/apierror/apierror.go deleted file mode 100644 index d32b751180..0000000000 --- a/pkg/apierror/apierror.go +++ /dev/null @@ -1,16 +0,0 @@ -//------------------------------------------------------------ -// Copyright (c) Microsoft Corporation. All rights reserved. -//------------------------------------------------------------ - -package apierror - -// New creates an ErrorResponse -func New(errorCategory ErrorCategory, errorCode ErrorCode, message string) *ErrorResponse { - return &ErrorResponse{ - Body: Error{ - Code: errorCode, - Message: message, - Category: errorCategory, - }, - } -} diff --git a/pkg/apierror/apierror_test.go b/pkg/apierror/apierror_test.go deleted file mode 100644 index 563911de2f..0000000000 --- a/pkg/apierror/apierror_test.go +++ /dev/null @@ -1,33 +0,0 @@ -//------------------------------------------------------------ -// Copyright (c) Microsoft Corporation. All rights reserved. -//------------------------------------------------------------ - -package apierror - -import ( - "testing" - - . "github.com/onsi/gomega" -) - -func TestNewAPIError(t *testing.T) { - RegisterTestingT(t) - - apiError := New( - ClientError, - InvalidParameter, - "error test") - - Expect(apiError.Body.Code).Should(Equal(ErrorCode("InvalidParameter"))) -} - -func TestAcsNewAPIError(t *testing.T) { - RegisterTestingT(t) - - apiError := New( - ClientError, - InvalidSubscriptionStateTransition, - "error test") - - Expect(apiError.Body.Code).Should(Equal(ErrorCode("InvalidSubscriptionStateTransition"))) -} diff --git a/pkg/apierror/const.go b/pkg/apierror/const.go deleted file mode 100644 index b5fcb7eade..0000000000 --- a/pkg/apierror/const.go +++ /dev/null @@ -1,71 +0,0 @@ -//------------------------------------------------------------ -// Copyright (c) Microsoft Corporation. All rights reserved. -//------------------------------------------------------------ - -package apierror - -// ErrorCategory indicates the kind of error -type ErrorCategory string - -const ( - // ClientError is expected error - ClientError ErrorCategory = "ClientError" - - // InternalError is system or internal error - InternalError ErrorCategory = "InternalError" -) - -// ErrorCode is Common Azure Resource Provider API error code -type ErrorCode string - -const ( - // From Microsoft.Azure.ResourceProvider.API.ErrorCode - - // InvalidParameter error - InvalidParameter ErrorCode = "InvalidParameter" - // BadRequest error - BadRequest ErrorCode = "BadRequest" - // NotFound error - NotFound ErrorCode = "NotFound" - // Conflict error - Conflict ErrorCode = "Conflict" - // PreconditionFailed error - PreconditionFailed ErrorCode = "PreconditionFailed" - // OperationNotAllowed error - OperationNotAllowed ErrorCode = "OperationNotAllowed" - // OperationPreempted error - OperationPreempted ErrorCode = "OperationPreempted" - // PropertyChangeNotAllowed error - PropertyChangeNotAllowed ErrorCode = "PropertyChangeNotAllowed" - // InternalOperationError error - InternalOperationError ErrorCode = "InternalOperationError" - // InvalidSubscriptionStateTransition error - InvalidSubscriptionStateTransition ErrorCode = "InvalidSubscriptionStateTransition" - // UnregisterWithResourcesNotAllowed error - UnregisterWithResourcesNotAllowed ErrorCode = "UnregisterWithResourcesNotAllowed" - // InvalidParameterConflictingProperties error - InvalidParameterConflictingProperties ErrorCode = "InvalidParameterConflictingProperties" - // SubscriptionNotRegistered error - SubscriptionNotRegistered ErrorCode = "SubscriptionNotRegistered" - // ConflictingUserInput error - ConflictingUserInput ErrorCode = "ConflictingUserInput" - // ProvisioningInternalError error - ProvisioningInternalError ErrorCode = "ProvisioningInternalError" - // ProvisioningFailed error - ProvisioningFailed ErrorCode = "ProvisioningFailed" - // NetworkingInternalOperationError error - NetworkingInternalOperationError ErrorCode = "NetworkingInternalOperationError" - // QuotaExceeded error - QuotaExceeded ErrorCode = "QuotaExceeded" - // Unauthorized error - Unauthorized ErrorCode = "Unauthorized" - // ResourcesOverConstrained error - ResourcesOverConstrained ErrorCode = "ResourcesOverConstrained" - - // ResourceDeploymentFailure error - ResourceDeploymentFailure ErrorCode = "ResourceDeploymentFailure" - // InvalidTemplateDeployment error - InvalidTemplateDeployment ErrorCode = "InvalidTemplateDeployment" - // DeploymentFailed error - DeploymentFailed ErrorCode = "DeploymentFailed" -) diff --git a/pkg/apierror/helper.go b/pkg/apierror/helper.go deleted file mode 100644 index 4b96a479de..0000000000 --- a/pkg/apierror/helper.go +++ /dev/null @@ -1,47 +0,0 @@ -package apierror - -import ( - "encoding/json" - "net/http" - - "github.com/Azure/azure-sdk-for-go/arm/resources/resources" -) - -// ExtractCodeFromARMHttpResponse returns the ARM error's Code field -// If not found return defaultCode -func ExtractCodeFromARMHttpResponse(resp *http.Response, defaultCode ErrorCode) ErrorCode { - if resp == nil { - return defaultCode - } - decoder := json.NewDecoder(resp.Body) - errorJSON := ErrorResponse{} - if err := decoder.Decode(&errorJSON); err != nil { - return defaultCode - } - - if errorJSON.Body.Code == "" { - return defaultCode - } - return ErrorCode(errorJSON.Body.Code) -} - -//ConvertToAPIError turns a ManagementErrorWithDetails into a apierror.Error -func ConvertToAPIError(mError *resources.ManagementErrorWithDetails) *Error { - retVal := &Error{} - if mError.Code != nil { - retVal.Code = ErrorCode(*mError.Code) - } - if mError.Message != nil { - retVal.Message = *mError.Message - } - if mError.Target != nil { - retVal.Target = *mError.Target - } - if mError.Details != nil { - retVal.Details = []Error{} - for _, me := range *mError.Details { - retVal.Details = append(retVal.Details, *ConvertToAPIError(&me)) - } - } - return retVal -} diff --git a/pkg/apierror/helper_test.go b/pkg/apierror/helper_test.go deleted file mode 100644 index 0c45711974..0000000000 --- a/pkg/apierror/helper_test.go +++ /dev/null @@ -1,21 +0,0 @@ -package apierror - -import ( - "bytes" - "io/ioutil" - "net/http" - "testing" - - . "github.com/onsi/gomega" -) - -func TestExtractCodeFromARMHttpResponse(t *testing.T) { - RegisterTestingT(t) - - resp := &http.Response{ - Body: ioutil.NopCloser(bytes.NewBufferString(`{"error":{"code":"ResourceGroupNotFound","message":"Resource group 'jiren-fakegroup' could not be found."}}`)), - } - - code := ExtractCodeFromARMHttpResponse(resp, "") - Expect(code).To(Equal(ErrorCode("ResourceGroupNotFound"))) -} diff --git a/pkg/apierror/types.go b/pkg/apierror/types.go deleted file mode 100644 index 307044a1a3..0000000000 --- a/pkg/apierror/types.go +++ /dev/null @@ -1,39 +0,0 @@ -//------------------------------------------------------------ -// Copyright (c) Microsoft Corporation. All rights reserved. -//------------------------------------------------------------ - -package apierror - -import "encoding/json" - -// Error is the OData v4 format, used by the RPC and -// will go into the v2.2 Azure REST API guidelines -type Error struct { - Code ErrorCode `json:"code"` - Message string `json:"message"` - Target string `json:"target,omitempty"` - Details []Error `json:"details,omitempty"` - - Category ErrorCategory `json:"-"` - ExceptionType string `json:"-"` - InternalDetail string `json:"-"` -} - -// ErrorResponse defines Resource Provider API 2.0 Error Response Content structure -type ErrorResponse struct { - Body Error `json:"error"` -} - -// Error implements error interface to return error in json -func (e *ErrorResponse) Error() string { - return e.Body.Error() -} - -// Error implements error interface to return error in json -func (e *Error) Error() string { - output, err := json.MarshalIndent(e, " ", " ") - if err != nil { - return err.Error() - } - return string(output) -} diff --git a/pkg/armhelpers/deploymentError.go b/pkg/armhelpers/deploymentError.go index 3e63710c32..9a11f123c9 100644 --- a/pkg/armhelpers/deploymentError.go +++ b/pkg/armhelpers/deploymentError.go @@ -7,220 +7,115 @@ import ( "strings" "github.com/Azure/acs-engine/pkg/api" - "github.com/Azure/acs-engine/pkg/apierror" "github.com/Azure/azure-sdk-for-go/arm/resources/resources" "github.com/sirupsen/logrus" ) -func parseDeploymentOperation(logger *logrus.Entry, operation resources.DeploymentOperation) (*apierror.Error, error) { - if operation.Properties == nil || operation.Properties.StatusMessage == nil { - return nil, fmt.Errorf("DeploymentOperation.Properties is not set") - } - b, err := json.MarshalIndent(operation.Properties.StatusMessage, "", " ") - if err != nil { - logger.Errorf("Error occurred marshalling JSON: '%v'", err) - return nil, err - } - return toError(logger, b) +// DeploymentError contains the root deployment error along with deployment operation errors +type DeploymentError struct { + TopError error + StatusCode int + Response []byte + ProvisioningState string + Operations [][]byte } -func toError(logger *logrus.Entry, b []byte) (*apierror.Error, error) { - errresp := &apierror.ErrorResponse{} - - if err := json.Unmarshal(b, errresp); err != nil { - logger.Errorf("Error occurred unmarshalling JSON: '%v' JSON: '%s'", err, string(b)) - return nil, err +// Error implements error interface +func (e *DeploymentError) Error() string { + var str string + if e.TopError != nil { + str = e.TopError.Error() } - - armError := &errresp.Body - // If error code is ResourceDeploymentFailure then RP error is defined in the child object field: "details - switch armError.Code { - case apierror.ResourceDeploymentFailure, - apierror.InvalidTemplateDeployment, - apierror.DeploymentFailed: - // StatusMessage.error.details array supports multiple errors but in this particular case - // DeploymentOperationProperties contains error from one specific resource type so the - // chances of multiple deployment errors being returned for a single resource type is slim - // (but possible) based on current error/QoS analysis. In those cases where multiple errors - // are returned ACS will pick the first error code for determining whether this is an internal - // or a client error. This can be reevaluated later based on practical experience. - // However, note that customer will be returned the entire contents of "StatusMessage" object - // (like before) so they have access to all the errors returned by ARM. - logger.Infof("Found %s error code - error response = '%v'", armError.Code, armError) - if len(armError.Details) > 0 { - armError = &armError.Details[0] - } - } - armError.Category = getErrorCategory(armError.Code) - return armError, nil -} - -func getErrorCategory(code apierror.ErrorCode) apierror.ErrorCategory { - switch code { - case apierror.InvalidParameter, - apierror.BadRequest, - apierror.OperationNotAllowed, - apierror.PropertyChangeNotAllowed, - apierror.UnregisterWithResourcesNotAllowed, - apierror.InvalidParameterConflictingProperties, - apierror.SubscriptionNotRegistered, - apierror.ConflictingUserInput, - apierror.QuotaExceeded, - apierror.Unauthorized, - apierror.ResourcesOverConstrained: - return apierror.ClientError - default: - return apierror.InternalError + var ops []string + for _, b := range e.Operations { + ops = append(ops, string(b)) } + return fmt.Sprintf("TopError[%s] StatusCode[%d] Response[%s] ProvisioningState[%s] Operations[%s]", + str, e.StatusCode, e.Response, e.ProvisioningState, strings.Join(ops, " | ")) } -// DeployTemplateSync deploys the template and returns apierror +// DeployTemplateSync deploys the template and returns ArmError func DeployTemplateSync(az ACSEngineClient, logger *logrus.Entry, resourceGroupName, deploymentName string, template map[string]interface{}, parameters map[string]interface{}) error { - depExt, depErr := az.DeployTemplate(resourceGroupName, deploymentName, template, parameters, nil) - if depErr == nil { + deploymentExtended, err := az.DeployTemplate(resourceGroupName, deploymentName, template, parameters, nil) + if err == nil && deploymentExtended == nil { return nil } logger.Infof("Getting detailed deployment errors for %s", deploymentName) + deploymentErr := &DeploymentError{} + deploymentErr.TopError = err - if depExt == nil { + if deploymentExtended == nil { logger.Warn("DeploymentExtended is nil") - return &apierror.Error{ - Code: apierror.InternalOperationError, - Message: depErr.Error(), - Category: apierror.InternalError} + return deploymentErr } // try to extract error from ARM Response - var armErr *apierror.Error - if depExt.Response.Response != nil && depExt.Body != nil { + if deploymentExtended.Response.Response != nil && deploymentExtended.Body != nil { buf := new(bytes.Buffer) - buf.ReadFrom(depExt.Body) - logger.Infof("StatusCode: %d, Error: %s", depExt.Response.StatusCode, buf.String()) - if resp, err := toError(logger, buf.Bytes()); err == nil { - switch { - case depExt.Response.StatusCode < 500 && depExt.Response.StatusCode >= 400: - resp.Category = apierror.ClientError - case depExt.Response.StatusCode >= 500: - resp.Category = apierror.InternalError - } - armErr = resp - } else { - logger.Errorf("unable to unmarshal response into apierror: %v", err) - } + buf.ReadFrom(deploymentExtended.Body) + logger.Infof("StatusCode: %d, Error: %s", deploymentExtended.Response.StatusCode, buf.String()) + deploymentErr.Response = buf.Bytes() + deploymentErr.StatusCode = deploymentExtended.Response.StatusCode } else { logger.Errorf("Got error from Azure SDK without response from ARM") // This is the failed sdk validation before calling ARM path - return &apierror.Error{ - Code: apierror.InternalOperationError, - Message: depErr.Error(), - Category: apierror.InternalError} - } - - // Check that ARM returned ErrorResponse - if armErr == nil || len(armErr.Message) == 0 || len(armErr.Code) == 0 { - logger.Warn("Not an ARM Response") - return &apierror.Error{ - Code: apierror.InternalOperationError, - Message: depErr.Error(), - Category: apierror.InternalError} + return deploymentErr } - if depExt.Properties == nil || depExt.Properties.ProvisioningState == nil { + if deploymentExtended.Properties == nil || deploymentExtended.Properties.ProvisioningState == nil { logger.Warn("No resources.DeploymentExtended.Properties") - return armErr + return deploymentErr } - properties := depExt.Properties + properties := deploymentExtended.Properties + deploymentErr.ProvisioningState = *properties.ProvisioningState - switch *properties.ProvisioningState { - case string(api.Canceled): - logger.Warning("template deployment has been canceled") - return &apierror.Error{ - Code: apierror.ProvisioningFailed, - Message: "template deployment has been canceled", - Category: apierror.ClientError} + var top int32 = 1 + res, err := az.ListDeploymentOperations(resourceGroupName, deploymentName, &top) + if err != nil { + logger.Errorf("unable to list deployment operations %s. error: %v", deploymentName, err) + return deploymentErr + } + getOperationError(res, deploymentErr, logger) - case string(api.Failed): - var top int32 = 1 - results := make([]resources.DeploymentOperationsListResult, top) - res, err := az.ListDeploymentOperations(resourceGroupName, deploymentName, &top) + for res.NextLink != nil { + res, err = az.ListDeploymentOperationsNextResults(res) if err != nil { - logger.Errorf("unable to list deployment operations %s. error: %v", deploymentName, err) - return armErr + logger.Warningf("unable to list next deployment operations %s. error: %v", deploymentName, err) + break } - results[0] = res - - for res.NextLink != nil { - res, err = az.ListDeploymentOperationsNextResults(res) - if err != nil { - logger.Warningf("unable to list next deployment operations %s. error: %v", deploymentName, err) - break - } - - results = append(results, res) - } - apierr, err := analyzeDeploymentResultAndSaveError(resourceGroupName, deploymentName, results, logger) - if err != nil || apierr == nil { - return armErr - } - return apierr - - default: - logger.Warningf("Unexpected ProvisioningState %s", *properties.ProvisioningState) - return armErr + getOperationError(res, deploymentErr, logger) } + return deploymentErr } -func analyzeDeploymentResultAndSaveError(resourceGroupName, deploymentName string, - operationLists []resources.DeploymentOperationsListResult, logger *logrus.Entry) (*apierror.Error, error) { - var apierr *apierror.Error - var err error - errs := []string{} - isInternalErr := false +func getOperationError(operationsList resources.DeploymentOperationsListResult, deploymentErr *DeploymentError, logger *logrus.Entry) { + if operationsList.Value == nil { + return + } - for _, operationsList := range operationLists { - if operationsList.Value == nil { + for _, operation := range *operationsList.Value { + if operation.Properties == nil || *operation.Properties.ProvisioningState != string(api.Failed) { continue } - for _, operation := range *operationsList.Value { - if operation.Properties == nil || *operation.Properties.ProvisioningState != string(api.Failed) { - continue - } - - // log the full deployment operation error response - if operation.ID != nil && operation.OperationID != nil { - b, _ := json.Marshal(operation.Properties) - logger.Infof("deployment operation ID %s, operationID %s, prooperties: %s", *operation.ID, *operation.OperationID, b) - } else { - logger.Error("either deployment ID or operationID is nil") - } - - apierr, err = parseDeploymentOperation(logger, operation) - if err != nil { - logger.Errorf("unable to convert deployment operation to error response in deployment %s from ARM. error: %v", deploymentName, err) - return nil, err - } - if apierr.Category == apierror.InternalError { - isInternalErr = true - } - errs = append(errs, apierr.Error()) - } - } - provisionErr := &apierror.Error{} - if len(errs) > 0 { - if isInternalErr { - provisionErr.Category = apierror.InternalError + // log the full deployment operation error response + if operation.ID != nil && operation.OperationID != nil { + b, _ := json.Marshal(operation.Properties) + logger.Infof("deployment operation ID %s, operationID %s, prooperties: %s", *operation.ID, *operation.OperationID, b) } else { - provisionErr.Category = apierror.ClientError + logger.Error("either deployment ID or operationID is nil") } - if len(errs) == 1 { - provisionErr = apierr - } else { - provisionErr.Code = apierror.ProvisioningFailed - provisionErr.Message = strings.Join(errs, "\n") + + if operation.Properties == nil || operation.Properties.StatusMessage == nil { + logger.Error("DeploymentOperation.Properties is not set") + continue + } + b, err := json.MarshalIndent(operation.Properties.StatusMessage, "", " ") + if err != nil { + logger.Errorf("Error occurred marshalling JSON: '%v'", err) + continue } - return provisionErr, nil + deploymentErr.Operations = append(deploymentErr.Operations, b) } - return nil, nil } diff --git a/pkg/armhelpers/deploymentError_test.go b/pkg/armhelpers/deploymentError_test.go index 0872561d6a..f932baf4cc 100644 --- a/pkg/armhelpers/deploymentError_test.go +++ b/pkg/armhelpers/deploymentError_test.go @@ -3,7 +3,6 @@ package armhelpers import ( "testing" - "github.com/Azure/acs-engine/pkg/apierror" . "github.com/Azure/acs-engine/pkg/test" . "github.com/onsi/gomega" @@ -24,9 +23,13 @@ var _ = Describe("Template deployment tests", func() { err := DeployTemplateSync(mockClient, logger, "rg1", "agentvm", map[string]interface{}{}, map[string]interface{}{}) Expect(err).NotTo(BeNil()) - apierr, ok := err.(*apierror.Error) + deplErr, ok := err.(*DeploymentError) Expect(ok).To(BeTrue()) - Expect(apierr.Code).To(Equal(apierror.InternalOperationError)) + Expect(deplErr.TopError).NotTo(BeNil()) + Expect(deplErr.TopError.Error()).To(Equal("DeployTemplate failed")) + Expect(deplErr.ProvisioningState).To(Equal("")) + Expect(deplErr.StatusCode).To(Equal(0)) + Expect(len(deplErr.Operations)).To(Equal(0)) }) It("Should return QuotaExceeded error code, specified in details", func() { @@ -36,9 +39,13 @@ var _ = Describe("Template deployment tests", func() { err := DeployTemplateSync(mockClient, logger, "rg1", "agentvm", map[string]interface{}{}, map[string]interface{}{}) Expect(err).NotTo(BeNil()) - apierr, ok := err.(*apierror.Error) + deplErr, ok := err.(*DeploymentError) Expect(ok).To(BeTrue()) - Expect(apierr.Code).To(Equal(apierror.QuotaExceeded)) + Expect(deplErr.TopError).NotTo(BeNil()) + Expect(deplErr.ProvisioningState).To(Equal("")) + Expect(deplErr.StatusCode).To(Equal(400)) + Expect(string(deplErr.Response)).To(ContainSubstring("\"code\":\"QuotaExceeded\"")) + Expect(len(deplErr.Operations)).To(Equal(0)) }) It("Should return Conflict error code, specified in details", func() { @@ -48,8 +55,12 @@ var _ = Describe("Template deployment tests", func() { err := DeployTemplateSync(mockClient, logger, "rg1", "agentvm", map[string]interface{}{}, map[string]interface{}{}) Expect(err).NotTo(BeNil()) - apierr, ok := err.(*apierror.Error) + deplErr, ok := err.(*DeploymentError) Expect(ok).To(BeTrue()) - Expect(apierr.Code).To(Equal(apierror.Conflict)) + Expect(deplErr.TopError).NotTo(BeNil()) + Expect(deplErr.ProvisioningState).To(Equal("")) + Expect(deplErr.StatusCode).To(Equal(200)) + Expect(string(deplErr.Response)).To(ContainSubstring("\"code\":\"Conflict\"")) + Expect(len(deplErr.Operations)).To(Equal(0)) }) })