diff --git a/internal/actionplan/actionplan.go b/internal/actionplan/actionplan.go index 97bc540..632cd1b 100644 --- a/internal/actionplan/actionplan.go +++ b/internal/actionplan/actionplan.go @@ -16,6 +16,7 @@ import ( "github.com/Azure/azure-extension-platform/pkg/extensionevents" "github.com/Azure/azure-extension-platform/pkg/handlerenv" "github.com/Azure/azure-extension-platform/pkg/logging" + vmextensionhelper "github.com/Azure/azure-extension-platform/vmextension" ) type action struct { @@ -117,14 +118,19 @@ func updateFailDeploymentError(failDeploymentError *failedDeploymentError, act * } type ExecuteError struct { - failedDeploymentErr *failedDeploymentError - combinedExecuteErrors error + failedDeploymentErr *failedDeploymentError + combinedExecuteErrors error + errorWithClarification vmextensionhelper.ErrorWithClarification } func (executeError *ExecuteError) GetCombinedExecuteError() error { return executeError.combinedExecuteErrors } +func (executeError *ExecuteError) GetErrorWithClarification() vmextensionhelper.ErrorWithClarification { + return executeError.errorWithClarification +} + func (executeError *ExecuteError) SetFailedDeploymentErr(err *failedDeploymentError) { executeError.failedDeploymentErr = err } @@ -133,9 +139,12 @@ func (executeError *ExecuteError) SetCombinedExecuteErrors(errs error) { executeError.combinedExecuteErrors = errs } -func (exeucuteError *ExecuteError) update(act *action, singleExecutionError error) { +func (exeucuteError *ExecuteError) update(act *action, singleExecutionError error, code ...vmextensionhelper.ErrorWithClarification) { exeucuteError.failedDeploymentErr = updateFailDeploymentError(exeucuteError.failedDeploymentErr, act, singleExecutionError) exeucuteError.combinedExecuteErrors = extensionerrors.CombineErrors(exeucuteError.combinedExecuteErrors, singleExecutionError) + if len(code) != 0 { + exeucuteError.errorWithClarification = vmextensionhelper.NewErrorWithClarification(code[0].ErrorCode, code[0].Err) + } } func (exeucuteError *ExecuteError) GetErrorIfDeploymentFailed() error { @@ -233,7 +242,7 @@ func (actionPlan *ActionPlan) insertOperation(order *int, dependentActions1 ...* } func (actionPlan *ActionPlan) Execute(registryHandler packageregistry.IPackageRegistry, eem *extensionevents.ExtensionEventManager, commandHandler commandhandler.ICommandHandler) (*ExecuteError, IResult) { - var executeError *ExecuteError = &ExecuteError{failedDeploymentErr: nil, combinedExecuteErrors: nil} + var executeError *ExecuteError = &ExecuteError{failedDeploymentErr: nil, combinedExecuteErrors: nil, errorWithClarification: vmextensionhelper.ErrorWithClarification{}} // registry will be mutated and written to disk so that we can keep track of all the actions that have happened registry, err := registryHandler.GetExistingPackages() if err != nil { @@ -243,9 +252,9 @@ func (actionPlan *ActionPlan) Execute(registryHandler packageregistry.IPackageRe // handle unordered implicit uninstalls for _, act := range actionPlan.unorderedImplicitUninstalls { - newError := actionPlan.executeHelper(registryHandler, commandHandler, registry, act, eem) + ewc, newError := actionPlan.executeHelper(registryHandler, commandHandler, registry, act, eem) appendExecutionResult(&executionResult, act, newError) - executeError.update(act, newError) + executeError.update(act, newError, ewc) } // handle ordered operations @@ -274,8 +283,8 @@ func (actionPlan *ActionPlan) Execute(registryHandler packageregistry.IPackageRe break } - newError := actionPlan.executeHelper(registryHandler, commandHandler, registry, act, eem) - executeError.update(act, newError) + ewc, newError := actionPlan.executeHelper(registryHandler, commandHandler, registry, act, eem) + executeError.update(act, newError, ewc) appendExecutionResult(&executionResult, act, newError) if newError != nil { @@ -292,8 +301,8 @@ func (actionPlan *ActionPlan) Execute(registryHandler packageregistry.IPackageRe // handle remaining unordered operations for _, depActions := range actionPlan.unorderedOperations { for _, act := range depActions { - newError := actionPlan.executeHelper(registryHandler, commandHandler, registry, act, eem) - executeError.update(act, newError) + ewc, newError := actionPlan.executeHelper(registryHandler, commandHandler, registry, act, eem) + executeError.update(act, newError, ewc) appendExecutionResult(&executionResult, act, newError) if newError != nil { diff --git a/internal/actionplan/actionplan_test.go b/internal/actionplan/actionplan_test.go index 103e29a..1d97609 100644 --- a/internal/actionplan/actionplan_test.go +++ b/internal/actionplan/actionplan_test.go @@ -10,11 +10,13 @@ import ( "github.com/Azure/VMApplication-Extension/internal/hostgacommunicator" "github.com/Azure/VMApplication-Extension/internal/packageregistry" + "github.com/Azure/VMApplication-Extension/pkg/utils" "github.com/Azure/azure-extension-platform/pkg/commandhandler" "github.com/Azure/azure-extension-platform/pkg/constants" "github.com/Azure/azure-extension-platform/pkg/extensionevents" "github.com/Azure/azure-extension-platform/pkg/handlerenv" "github.com/Azure/azure-extension-platform/pkg/logging" + vmextensionhelper "github.com/Azure/azure-extension-platform/vmextension" "github.com/pkg/errors" "github.com/stretchr/testify/assert" ) @@ -766,3 +768,336 @@ func validateApplicationAfterReboot(t *testing.T, applicationName string, numReb assert.Contains(t, app.Result, "Reboot detected during 'Install' operation") } } + +func TestExecuteError_Update(t *testing.T) { + tests := []struct { + name string + initialExecuteError *ExecuteError + action *action + singleExecutionError error + errorWithClarification []vmextensionhelper.ErrorWithClarification + expectedFailedDeployment bool + expectedErrorCount int + expectedErrorCode int + }{ + { + name: "Update with no error and no clarification", + initialExecuteError: &ExecuteError{ + failedDeploymentErr: nil, + combinedExecuteErrors: nil, + errorWithClarification: vmextensionhelper.ErrorWithClarification{}, + }, + action: &action{ + vmAppPackage: getVmAppPackageCurrent("testapp", "1.0"), + treatFailureAsDeploymentFailure: false, + actionToPerform: packageregistry.Install, + }, + singleExecutionError: nil, + errorWithClarification: []vmextensionhelper.ErrorWithClarification{}, + expectedFailedDeployment: false, + expectedErrorCount: 0, + expectedErrorCode: 0, + }, + { + name: "Update with install action error and treatFailureAsDeploymentFailure true", + initialExecuteError: &ExecuteError{ + failedDeploymentErr: nil, + combinedExecuteErrors: nil, + errorWithClarification: vmextensionhelper.ErrorWithClarification{}, + }, + action: &action{ + vmAppPackage: getVmAppPackageCurrent("testapp", "1.0"), + treatFailureAsDeploymentFailure: true, + actionToPerform: packageregistry.Install, + }, + singleExecutionError: errors.New("install failed"), + errorWithClarification: []vmextensionhelper.ErrorWithClarification{}, + expectedFailedDeployment: true, + expectedErrorCount: 1, + expectedErrorCode: 0, + }, + { + name: "Update with update action error and treatFailureAsDeploymentFailure true", + initialExecuteError: &ExecuteError{ + failedDeploymentErr: nil, + combinedExecuteErrors: nil, + errorWithClarification: vmextensionhelper.ErrorWithClarification{}, + }, + action: &action{ + vmAppPackage: getVmAppPackageCurrent("testapp", "1.0"), + treatFailureAsDeploymentFailure: true, + actionToPerform: packageregistry.Update, + }, + singleExecutionError: errors.New("update failed"), + errorWithClarification: []vmextensionhelper.ErrorWithClarification{}, + expectedFailedDeployment: true, + expectedErrorCount: 1, + expectedErrorCode: 0, + }, + { + name: "Update with remove action error and treatFailureAsDeploymentFailure true - should not fail deployment", + initialExecuteError: &ExecuteError{ + failedDeploymentErr: nil, + combinedExecuteErrors: nil, + errorWithClarification: vmextensionhelper.ErrorWithClarification{}, + }, + action: &action{ + vmAppPackage: getVmAppPackageCurrent("testapp", "1.0"), + treatFailureAsDeploymentFailure: true, + actionToPerform: packageregistry.Remove, + }, + singleExecutionError: errors.New("remove failed"), + errorWithClarification: []vmextensionhelper.ErrorWithClarification{}, + expectedFailedDeployment: false, + expectedErrorCount: 1, + expectedErrorCode: 0, + }, + { + name: "Update with error clarification", + initialExecuteError: &ExecuteError{ + failedDeploymentErr: nil, + combinedExecuteErrors: nil, + errorWithClarification: vmextensionhelper.ErrorWithClarification{}, + }, + action: &action{ + vmAppPackage: getVmAppPackageCurrent("testapp", "1.0"), + treatFailureAsDeploymentFailure: false, + actionToPerform: packageregistry.Install, + }, + singleExecutionError: errors.New("execution failed"), + errorWithClarification: []vmextensionhelper.ErrorWithClarification{ + vmextensionhelper.NewErrorWithClarification(utils.CommandExecutionError, errors.New("command execution error")), + }, + expectedFailedDeployment: false, + expectedErrorCount: 1, + expectedErrorCode: utils.CommandExecutionError, + }, + { + name: "Update multiple errors with existing failed deployment", + initialExecuteError: &ExecuteError{ + failedDeploymentErr: &failedDeploymentError{ + appsWithTreatFailureAsDeploymentFailure: []string{"existingapp"}, + additionalErrorForContext: nil, + }, + combinedExecuteErrors: errors.New("existing error"), + errorWithClarification: vmextensionhelper.ErrorWithClarification{}, + }, + action: &action{ + vmAppPackage: getVmAppPackageCurrent("newapp", "2.0"), + treatFailureAsDeploymentFailure: true, + actionToPerform: packageregistry.Install, + }, + singleExecutionError: errors.New("new install failed"), + errorWithClarification: []vmextensionhelper.ErrorWithClarification{}, + expectedFailedDeployment: true, + expectedErrorCount: 2, + expectedErrorCode: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Perform the update + tt.initialExecuteError.update(tt.action, tt.singleExecutionError, tt.errorWithClarification...) + + // Check failed deployment error + deploymentError := tt.initialExecuteError.GetErrorIfDeploymentFailed() + if tt.expectedFailedDeployment { + assert.NotNil(t, deploymentError, "Expected deployment failure error") + assert.Contains(t, deploymentError.Error(), tt.action.vmAppPackage.ApplicationName, "Error should contain app name") + } else { + assert.Nil(t, deploymentError, "Expected no deployment failure error") + } + + // Check combined errors + if tt.expectedErrorCount > 0 { + assert.NotNil(t, tt.initialExecuteError.combinedExecuteErrors, "Expected combined errors") + } else { + assert.Nil(t, tt.initialExecuteError.combinedExecuteErrors, "Expected no combined errors") + } + + // Check error clarification + if tt.expectedErrorCode != 0 { + errorWithClarification := tt.initialExecuteError.GetErrorWithClarification() + assert.Equal(t, tt.expectedErrorCode, errorWithClarification.ErrorCode, "Error code should match") + } + }) + } +} + +func TestExecuteError_UpdateFailDeploymentError(t *testing.T) { + tests := []struct { + name string + initialFailedDeploymentErr *failedDeploymentError + action *action + singleExecutionError error + expectedAppCount int + expectedToReturnError bool + }{ + { + name: "No initial error, install with failure, treatFailureAsDeploymentFailure true", + initialFailedDeploymentErr: nil, + action: &action{ + vmAppPackage: getVmAppPackageCurrent("app1", "1.0"), + treatFailureAsDeploymentFailure: true, + actionToPerform: packageregistry.Install, + }, + singleExecutionError: errors.New("install failed"), + expectedAppCount: 1, + expectedToReturnError: true, + }, + { + name: "No initial error, update with failure, treatFailureAsDeploymentFailure true", + initialFailedDeploymentErr: nil, + action: &action{ + vmAppPackage: getVmAppPackageCurrent("app1", "1.0"), + treatFailureAsDeploymentFailure: true, + actionToPerform: packageregistry.Update, + }, + singleExecutionError: errors.New("update failed"), + expectedAppCount: 1, + expectedToReturnError: true, + }, + { + name: "No initial error, remove with failure, treatFailureAsDeploymentFailure true", + initialFailedDeploymentErr: nil, + action: &action{ + vmAppPackage: getVmAppPackageCurrent("app1", "1.0"), + treatFailureAsDeploymentFailure: true, + actionToPerform: packageregistry.Remove, + }, + singleExecutionError: errors.New("remove failed"), + expectedAppCount: 0, + expectedToReturnError: false, + }, + { + name: "No initial error, install with failure, treatFailureAsDeploymentFailure false", + initialFailedDeploymentErr: nil, + action: &action{ + vmAppPackage: getVmAppPackageCurrent("app1", "1.0"), + treatFailureAsDeploymentFailure: false, + actionToPerform: packageregistry.Install, + }, + singleExecutionError: errors.New("install failed"), + expectedAppCount: 0, + expectedToReturnError: false, + }, + { + name: "Existing error, add another app failure", + initialFailedDeploymentErr: &failedDeploymentError{ + appsWithTreatFailureAsDeploymentFailure: []string{"existingapp"}, + additionalErrorForContext: nil, + }, + action: &action{ + vmAppPackage: getVmAppPackageCurrent("app2", "1.0"), + treatFailureAsDeploymentFailure: true, + actionToPerform: packageregistry.Install, + }, + singleExecutionError: errors.New("install failed"), + expectedAppCount: 2, + expectedToReturnError: true, + }, + { + name: "No error occurred", + initialFailedDeploymentErr: nil, + action: &action{ + vmAppPackage: getVmAppPackageCurrent("app1", "1.0"), + treatFailureAsDeploymentFailure: true, + actionToPerform: packageregistry.Install, + }, + singleExecutionError: nil, + expectedAppCount: 0, + expectedToReturnError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := updateFailDeploymentError(tt.initialFailedDeploymentErr, tt.action, tt.singleExecutionError) + + if tt.expectedToReturnError { + assert.NotNil(t, result, "Expected failed deployment error to be returned") + assert.Equal(t, tt.expectedAppCount, len(result.appsWithTreatFailureAsDeploymentFailure), "Expected app count should match") + if tt.expectedAppCount > 0 { + assert.Contains(t, result.appsWithTreatFailureAsDeploymentFailure, tt.action.vmAppPackage.ApplicationName, "Should contain the failed app name") + } + } else { + if tt.expectedAppCount == 0 { + assert.Nil(t, result, "Expected no failed deployment error") + } else { + assert.NotNil(t, result, "Expected existing failed deployment error to be preserved") + assert.Equal(t, tt.expectedAppCount, len(result.appsWithTreatFailureAsDeploymentFailure), "Expected app count should match") + } + } + }) + } +} + +func TestExecuteError_GetErrorIfDeploymentFailed(t *testing.T) { + tests := []struct { + name string + failedDeploymentErr *failedDeploymentError + combinedExecuteErrors error + expectNil bool + expectedToContainAppName string + expectedToContainAdditionalError bool + }{ + { + name: "No failed deployment error", + failedDeploymentErr: nil, + combinedExecuteErrors: errors.New("some error"), + expectNil: true, + }, + { + name: "Failed deployment error without additional context", + failedDeploymentErr: &failedDeploymentError{ + appsWithTreatFailureAsDeploymentFailure: []string{"app1"}, + additionalErrorForContext: nil, + }, + combinedExecuteErrors: nil, + expectNil: false, + expectedToContainAppName: "app1", + expectedToContainAdditionalError: false, + }, + { + name: "Failed deployment error with additional context", + failedDeploymentErr: &failedDeploymentError{ + appsWithTreatFailureAsDeploymentFailure: []string{"app1", "app2"}, + additionalErrorForContext: nil, + }, + combinedExecuteErrors: errors.New("additional error context"), + expectNil: false, + expectedToContainAppName: "app1", + expectedToContainAdditionalError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + executeError := &ExecuteError{ + failedDeploymentErr: tt.failedDeploymentErr, + combinedExecuteErrors: tt.combinedExecuteErrors, + } + + result := executeError.GetErrorIfDeploymentFailed() + + if tt.expectNil { + assert.Nil(t, result, "Expected nil error") + } else { + assert.NotNil(t, result, "Expected non-nil error") + errorMessage := result.Error() + + if tt.expectedToContainAppName != "" { + assert.Contains(t, errorMessage, tt.expectedToContainAppName, "Error message should contain app name") + } + + if tt.expectedToContainAdditionalError { + assert.Contains(t, errorMessage, "Additional errors:", "Error message should contain additional error context") + assert.Contains(t, errorMessage, tt.combinedExecuteErrors.Error(), "Error message should contain the combined error text") + // Verify that the additional error context was set + assert.Equal(t, tt.combinedExecuteErrors, tt.failedDeploymentErr.additionalErrorForContext, "Additional error context should be set") + } + } + }) + } +} diff --git a/internal/actionplan/executehelper.go b/internal/actionplan/executehelper.go index b96d742..a7f71f7 100644 --- a/internal/actionplan/executehelper.go +++ b/internal/actionplan/executehelper.go @@ -12,11 +12,13 @@ import ( "time" "github.com/Azure/VMApplication-Extension/internal/packageregistry" + "github.com/Azure/VMApplication-Extension/pkg/utils" "github.com/Azure/azure-extension-platform/pkg/commandhandler" "github.com/Azure/azure-extension-platform/pkg/constants" "github.com/Azure/azure-extension-platform/pkg/exithelper" "github.com/Azure/azure-extension-platform/pkg/extensionerrors" "github.com/Azure/azure-extension-platform/pkg/extensionevents" + vmextensionhelper "github.com/Azure/azure-extension-platform/vmextension" "github.com/pkg/errors" ) @@ -24,10 +26,11 @@ const MaxReboots = 3 func (actionPlan *ActionPlan) executeHelper(registryHandler packageregistry.IPackageRegistry, commandHandler commandhandler.ICommandHandler, registry packageregistry.CurrentPackageRegistry, - act *action, eem *extensionevents.ExtensionEventManager) (errorMessageToReturn error) { + act *action, eem *extensionevents.ExtensionEventManager) (errorWithClarification vmextensionhelper.ErrorWithClarification, errorMessageToReturn error) { errorMessageToReturn = nil appName := act.vmAppPackage.ApplicationName version := act.vmAppPackage.Version + ewc := vmextensionhelper.ErrorWithClarification{} // record new operation in the packageRegistry vmAppPackageCurrent := act.vmAppPackage registry[appName] = &vmAppPackageCurrent @@ -40,11 +43,14 @@ func (actionPlan *ActionPlan) executeHelper(registryHandler packageregistry.IPac // return early for Cleanup operation if vmAppPackageCurrent.OngoingOperation == packageregistry.Cleanup { delete(registry, appName) - return registryHandler.WriteToDisk(registry) + err := registryHandler.WriteToDisk(registry) + ewc = vmextensionhelper.NewErrorWithClarification(utils.WriteToDiskError, err) + return ewc, err } err := registryHandler.WriteToDisk(registry) if err != nil { - return err + ewc = vmextensionhelper.NewErrorWithClarification(utils.WriteToDiskError, err) + return ewc, err } var commandToExecute string @@ -61,6 +67,7 @@ func (actionPlan *ActionPlan) executeHelper(registryHandler packageregistry.IPac commandToExecute = vmAppPackageCurrent.UpdateCommand default: errorMessageToReturn = errors.Errorf("Unexpected Action to perform encountered %v", act.actionToPerform) + ewc = vmextensionhelper.NewErrorWithClarification(utils.UnexpectedActionError, errorMessageToReturn) } if vmAppPackageCurrent.NumRebootsOccurred == MaxReboots { @@ -84,6 +91,7 @@ func (actionPlan *ActionPlan) executeHelper(registryHandler packageregistry.IPac errorMessageToReturn = errors.Errorf( "The application %v, version %v has been removed from the repository and cannot be installed. Please install a newer version of the application.", appName, version) + ewc = vmextensionhelper.NewErrorWithClarification(utils.DeletedAppVersionError, errorMessageToReturn) } else { // application is not marked as deleted downloadPath := vmAppPackageCurrent.GetWorkingDirectory(actionPlan.environment) @@ -91,6 +99,7 @@ func (actionPlan *ActionPlan) executeHelper(registryHandler packageregistry.IPac if err := os.MkdirAll(downloadPath, constants.FilePermissions_UserOnly_ReadWriteExecute); err != nil { errorMessageToReturn = extensionerrors.CombineErrors(errorMessageToReturn, errors.Wrapf(err, "failed to create download directory %s", downloadPath)) + ewc = vmextensionhelper.NewErrorWithClarification(utils.FailedToCreateDirectoryError, errorMessageToReturn) } // proceed only if there was no error in the previous operation if err == nil { @@ -99,6 +108,7 @@ func (actionPlan *ActionPlan) executeHelper(registryHandler packageregistry.IPac if err := actionPlan.hostGaCommunicator.DownloadPackage(actionPlan.logger, vmAppPackageCurrent.ApplicationName, downloadPackageFileName); err != nil { actionPlan.logger.Error("Failed to download package for application %v, version %v. Error: %v", appName, version, err.Error()) errorMessageToReturn = extensionerrors.CombineErrors(errorMessageToReturn, errors.Wrapf(err, "failed to download package file %s", downloadPackageFileName)) + ewc = vmextensionhelper.NewErrorWithClarification(utils.FailedToDownloadPackageError, errorMessageToReturn) } if err == nil { if packageFileChecksum, err := getMD5CheckSum(downloadPackageFileName); err == nil { @@ -114,6 +124,7 @@ func (actionPlan *ActionPlan) executeHelper(registryHandler packageregistry.IPac if err := actionPlan.hostGaCommunicator.DownloadConfig(actionPlan.logger, vmAppPackageCurrent.ApplicationName, downloadConfigFileName); err != nil { actionPlan.logger.Error("Failed to download config for application %v, version %v. Error: %v", appName, version, err.Error()) errorMessageToReturn = extensionerrors.CombineErrors(errorMessageToReturn, errors.Wrapf(err, "failed to download config file %s", downloadConfigFileName)) + ewc = vmextensionhelper.NewErrorWithClarification(utils.FailedToDownloadConfigError, errorMessageToReturn) } if err == nil { if configFileChecksum, err := getMD5CheckSum(downloadConfigFileName); err == nil { @@ -180,8 +191,11 @@ func (actionPlan *ActionPlan) executeHelper(registryHandler packageregistry.IPac case compSignal := <-completionSignal: if compSignal.err != nil { errorMessageToReturn = extensionerrors.CombineErrors(errorMessageToReturn, errors.Wrapf(compSignal.err, "Error executing command '%v'", commandToExecute)) + ewc = vmextensionhelper.NewErrorWithClarification(utils.CommandExecutionError, errorMessageToReturn) } else if compSignal.retCode != 0 { errorMessageToReturn = extensionerrors.CombineErrors(errorMessageToReturn, errors.Errorf("Command '%v' exited with non-zero error code", commandToExecute)) + ewc = vmextensionhelper.NewErrorWithClarification(utils.NonZeroExitCodeError, errorMessageToReturn) + } // Reset count if command successfully completed vmAppPackageCurrent.NumRebootsOccurred = 0 @@ -223,6 +237,9 @@ func (actionPlan *ActionPlan) executeHelper(registryHandler packageregistry.IPac // also cleanup directory deleteErr := os.RemoveAll(vmAppPackageCurrent.DownloadDir) errorMessageToReturn = extensionerrors.CombineErrors(errorMessageToReturn, deleteErr) + if deleteErr != nil { + ewc = vmextensionhelper.NewErrorWithClarification(utils.DeleteOperationError, deleteErr) + } } if errorMessageToReturn != nil { @@ -235,7 +252,7 @@ func (actionPlan *ActionPlan) executeHelper(registryHandler packageregistry.IPac err = registryHandler.WriteToDisk(registry) if err != nil { - return markCommandFailed(act.actionToPerform, commandToExecute, appName, version, err, eem) + return ewc, markCommandFailed(act.actionToPerform, commandToExecute, appName, version, err, eem) } if errorMessageToReturn == nil { @@ -245,7 +262,7 @@ func (actionPlan *ActionPlan) executeHelper(registryHandler packageregistry.IPac return } - return markCommandFailed(act.actionToPerform, commandToExecute, appName, version, errorMessageToReturn, eem) + return ewc, markCommandFailed(act.actionToPerform, commandToExecute, appName, version, errorMessageToReturn, eem) } func markCommandFailed(actionPerformed packageregistry.ActionEnum, commandToExecute string, appName string, version string, err error, eem *extensionevents.ExtensionEventManager) error { diff --git a/internal/actionplan/executehelper_test.go b/internal/actionplan/executehelper_test.go index 28b0906..2061746 100644 --- a/internal/actionplan/executehelper_test.go +++ b/internal/actionplan/executehelper_test.go @@ -147,23 +147,23 @@ func TestExecuteHelper(t *testing.T) { initTest(t) defer cleanTest() action := action{vmAppPackageCurrent, false, packageregistry.Install} - err := actionPlan.executeHelper(packageRegistry, commandHandler, packageregistry.CurrentPackageRegistry{}, &action, extensionEventManager) - assert.NoError(t, err) + err, _ := actionPlan.executeHelper(packageRegistry, commandHandler, packageregistry.CurrentPackageRegistry{}, &action, extensionEventManager) + assert.NoError(t, err.Err) assert.EqualValues(t, 1, mhgCommunicator.DownloadPackageCount, "download package count should be 1") assert.EqualValues(t, 1, mhgCommunicator.DownloadConfigCount, "download config count should be 1") action.actionToPerform = packageregistry.Remove - err = actionPlan.executeHelper(packageRegistry, commandHandler, packageregistry.CurrentPackageRegistry{}, &action, extensionEventManager) - assert.NoError(t, err) + err, _ = actionPlan.executeHelper(packageRegistry, commandHandler, packageregistry.CurrentPackageRegistry{}, &action, extensionEventManager) + assert.NoError(t, err.Err) assert.EqualValues(t, 1, mhgCommunicator.DownloadPackageCount, "download package count should be 1") assert.EqualValues(t, 1, mhgCommunicator.DownloadConfigCount, "download config count should be 1") assert.EqualValues(t, vmAppPackageCurrent.InstallCommand, commandHandler.Result[0].command, "1st command should be install") assert.EqualValues(t, vmAppPackageCurrent.RemoveCommand, commandHandler.Result[1].command, "2nd command should be remove") assert.Equal(t, 2, len(commandHandler.Result), "only 2 commands should be executed") - _, err = os.Stat(vmAppPackageCurrent.DownloadDir) + _, err.Err = os.Stat(vmAppPackageCurrent.DownloadDir) assert.Error(t, err, "downloadDir should be deleted") - _, ok := err.(*os.PathError) + _, ok := err.Err.(*os.PathError) assert.True(t, ok, "downloadDir should be deleted") } @@ -171,7 +171,7 @@ func TestDeletedApp(t *testing.T) { initTest(t) defer cleanTest() action := action{vmAppPackageDeleted, false, packageregistry.Install} - err := actionPlan.executeHelper(packageRegistry, commandHandler, packageregistry.CurrentPackageRegistry{}, &action, extensionEventManager) + err, _ := actionPlan.executeHelper(packageRegistry, commandHandler, packageregistry.CurrentPackageRegistry{}, &action, extensionEventManager) assert.Error(t, err, "The application test app, version 1.0.0 has been removed from the repository and cannot be installed. Please install a newer version of the application.") } diff --git a/main/main.go b/main/main.go index b057b5a..4525950 100644 --- a/main/main.go +++ b/main/main.go @@ -233,7 +233,20 @@ func customEnable(ext *vmextensionhelper.VMExtension, hostgaCommunicator hostgac } else { statusResult = status.StatusError } - err := utils.ReportStatus(ext.HandlerEnv, requestedSequenceNumber, statusResult, vmextensionhelper.EnableOperation.ToStatusName(), statusMessage) + + var err error + switch statusResult { + case status.StatusError: + ewc := executeError.GetErrorWithClarification() + if ewc != (vmextensionhelper.ErrorWithClarification{}) { + err = utils.ReportStatusWithError(ext.HandlerEnv, requestedSequenceNumber, vmextensionhelper.EnableOperation.ToStatusName(), ewc) + } else { + err = utils.ReportStatus(ext.HandlerEnv, requestedSequenceNumber, statusResult, vmextensionhelper.EnableOperation.ToStatusName(), statusMessage) + } + + default: + err = utils.ReportStatus(ext.HandlerEnv, requestedSequenceNumber, statusResult, vmextensionhelper.EnableOperation.ToStatusName(), statusMessage) + } if err != nil { errorMessage := fmt.Sprintf("Failed to save status file: %s", err.Error()) ext.ExtensionLogger.Error(errorMessage) diff --git a/main/main_test.go b/main/main_test.go index 9d9c2cc..2a61740 100644 --- a/main/main_test.go +++ b/main/main_test.go @@ -26,6 +26,7 @@ import ( handlersettings "github.com/Azure/azure-extension-platform/pkg/settings" "github.com/Azure/azure-extension-platform/pkg/status" "github.com/Azure/azure-extension-platform/vmextension" + vmextensionhelper "github.com/Azure/azure-extension-platform/vmextension" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -604,3 +605,196 @@ func createTestVMExtension(t *testing.T, settings interface{}) *vmextension.VMEx ExtensionEvents: eem, } } + +func Test_customEnable_ErrorWithClarification_EmptyStruct(t *testing.T) { + var ewc vmextensionhelper.ErrorWithClarification + + supportEwc := false + if ewc != (vmextensionhelper.ErrorWithClarification{}) { + supportEwc = true + } + + // For empty struct, supportEwc should be false + assert.False(t, supportEwc, "supportEwc should be false for empty ErrorWithClarification") +} + +func Test_customEnable_ErrorWithClarification_PopulatedStruct(t *testing.T) { + testError := errors.New("test error") + errorCode := utils.CommandExecutionError + ewc := vmextensionhelper.NewErrorWithClarification(errorCode, testError) + + supportEwc := false + if ewc != (vmextensionhelper.ErrorWithClarification{}) { + supportEwc = true + } + + // For populated struct, supportEwc should be true + assert.True(t, supportEwc, "supportEwc should be true for populated ErrorWithClarification") + assert.Equal(t, errorCode, ewc.ErrorCode, "Error code should match") + assert.Equal(t, testError.Error(), ewc.Err.Error(), "Error message should match") +} + +func Test_customEnable_ErrorWithClarification_ComparisonLogic(t *testing.T) { + testCases := []struct { + name string + errorCode int + errorMessage string + expectedSupportEwc bool + }{ + { + name: "Empty error clarification", + errorCode: utils.NoError, + errorMessage: "", + expectedSupportEwc: false, + }, + { + name: "Error code only", + errorCode: utils.CommandExecutionError, + errorMessage: "", + expectedSupportEwc: true, + }, + { + name: "Error message only", + errorCode: 0, + errorMessage: "test error", + expectedSupportEwc: true, + }, + { + name: "Both error code and message", + errorCode: utils.NonZeroExitCodeError, + errorMessage: "non-zero exit code", + expectedSupportEwc: true, + }, + { + name: "Negative error code", + errorCode: utils.WriteToDiskError, + errorMessage: "disk write failed", + expectedSupportEwc: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + var ewc vmextensionhelper.ErrorWithClarification + + if tc.errorCode != 0 || tc.errorMessage != "" { + var testError error + if tc.errorMessage != "" { + testError = errors.New(tc.errorMessage) + } + ewc = vmextensionhelper.NewErrorWithClarification(tc.errorCode, testError) + } + + supportEwc := false + if ewc != (vmextensionhelper.ErrorWithClarification{}) { + supportEwc = true + } + + assert.Equal(t, tc.expectedSupportEwc, supportEwc, + "supportEwc value should match expected for case: %s", tc.name) + }) + } +} + +func Test_customEnable_StatusErrorPath_WithClarification(t *testing.T) { + vmApplications := []extdeserialization.VmAppSetting{ + { + ApplicationName: "testapp", + TreatFailureAsDeploymentFailure: true, + }, + } + + ext := createTestVMExtension(t, vmApplications) + hostGaCommunicator := &NoopHostGaCommunicator{} + hostGaCommunicator.SetupVMAppInfo("testapp", "1.0", "install") + + executeError := &actionplan.ExecuteError{} + + // Deployment failure but no error clarification + + requestedSequenceNumber := *ext.CurrentSequenceNumber + 1 + statusMessage := "test error message" + + // Test empty clarification + ewc := executeError.GetErrorWithClarification() + supportEwc := false + if ewc != (vmextensionhelper.ErrorWithClarification{}) { + supportEwc = true + } + + // Should be false for empty clarification + assert.False(t, supportEwc, "testapp: supportEwc should be false when no error clarification is set") + + // Test with ReportStatus for empty clarification + err := utils.ReportStatus(ext.HandlerEnv, requestedSequenceNumber, status.StatusError, vmextensionhelper.EnableOperation.ToStatusName(), statusMessage) + assert.NoError(t, err, "testapp: ReportStatus should not return an error for empty clarification") + + // Verify status was written + statusType, err := utils.GetStatusType(ext.HandlerEnv, requestedSequenceNumber) + require.NoError(t, err, "testapp: GetStatusType should not return an error") + assert.Equal(t, status.StatusError, statusType, "testapp: status should be error") +} + +func Test_customEnable_StatusErrorPath_WithoutClarification(t *testing.T) { + vmApplications := []extdeserialization.VmAppSetting{ + { + ApplicationName: "testapp2", + TreatFailureAsDeploymentFailure: true, + }, + } + + ext := createTestVMExtension(t, vmApplications) + + // Create a test ExecuteError with error clarification set + executeError := &actionplan.ExecuteError{} + testError := errors.New("test deployment failed") + errorCode := utils.NonZeroExitCodeError + + // Simulate setting error clarification manually + executeError.SetCombinedExecuteErrors(testError) + + requestedSequenceNumber := *ext.CurrentSequenceNumber + 1 + + ewc := vmextensionhelper.NewErrorWithClarification(errorCode, testError) + + assert.True(t, (ewc != (vmextensionhelper.ErrorWithClarification{})), "testapp2: error clarification should not be empty") + + // Test with ReportStatusWithError for populated clarification + err := utils.ReportStatusWithError(ext.HandlerEnv, requestedSequenceNumber, vmextensionhelper.EnableOperation.ToStatusName(), ewc) + assert.NoError(t, err) + + // Verify status was written + statusType, err := utils.GetStatusType(ext.HandlerEnv, requestedSequenceNumber) + require.NoError(t, err, "testapp2: GetStatusType should not return an error") + assert.Equal(t, status.StatusError, statusType, "testapp2: status should be error") +} + +func Test_customEnable_StatusSuccess_NoErrorHandling(t *testing.T) { + vmApplications := []extdeserialization.VmAppSetting{ + { + ApplicationName: "successapp", + }, + } + + ext := createTestVMExtension(t, vmApplications) + hostGaCommunicator := &NoopHostGaCommunicator{} + hostGaCommunicator.SetupVMAppInfo("successapp", "1.0", "install") + + // Create a successful ExecuteError + executeError := &actionplan.ExecuteError{} + + requestedSequenceNumber := *ext.CurrentSequenceNumber + 1 + statusMessage := "Success" + + // Test the success case (no error with clarification) + if executeError.GetErrorIfDeploymentFailed() == nil { + // For success, we go to the default case in the switch statement + err := utils.ReportStatus(ext.HandlerEnv, requestedSequenceNumber, status.StatusSuccess, vmextensionhelper.EnableOperation.ToStatusName(), statusMessage) + assert.NoError(t, err, "ReportStatus should not return an error for successapp") + + // Verify status was written as success + statusType, err := utils.GetStatusType(ext.HandlerEnv, requestedSequenceNumber) + require.NoError(t, err, "Successapp: GetStatusType should not return an error") + assert.Equal(t, status.StatusSuccess, statusType) + } +} diff --git a/pkg/utils/errroclarification.go b/pkg/utils/errroclarification.go new file mode 100644 index 0000000..d399518 --- /dev/null +++ b/pkg/utils/errroclarification.go @@ -0,0 +1,15 @@ +package utils + +const ( + WriteToDiskError = -1 + UnexpectedActionError = -2 + FailedToCreateDirectoryError = -3 + FailedToDownloadPackageError = -4 + FailedToDownloadConfigError = -5 + DeleteOperationError = -6 + CommandExecutionError = 1 + DeletedAppVersionError = 2 + NonZeroExitCodeError = 3 + + NoError = 0 +) diff --git a/pkg/utils/status.go b/pkg/utils/status.go index 7f8a413..bdeef6e 100644 --- a/pkg/utils/status.go +++ b/pkg/utils/status.go @@ -8,6 +8,7 @@ import ( "github.com/Azure/azure-extension-platform/pkg/handlerenv" "github.com/Azure/azure-extension-platform/pkg/status" + vmextensionhelper "github.com/Azure/azure-extension-platform/vmextension" "github.com/pkg/errors" ) @@ -44,3 +45,13 @@ func ReportStatus(handlerEnv *handlerenv.HandlerEnvironment, requestedSequenceNu } return nil } + +func ReportStatusWithError(handlerEnv *handlerenv.HandlerEnvironment, requestedSequenceNumber uint, operationName string, ewc vmextensionhelper.ErrorWithClarification) error { + s := status.NewError(operationName, status.ErrorClarification{Code: ewc.ErrorCode, Message: ewc.Err.Error()}) + err := s.Save(handlerEnv.StatusFolder, requestedSequenceNumber) + if err != nil { + errorMsg := "failed to save handler status" + return &StatusSaveError{Err: errors.Wrap(err, errorMsg)} + } + return nil +}