From e3d30d0059b4ed4311817a6b055d4b0c9b94e98f Mon Sep 17 00:00:00 2001 From: Vishnu Karthik Ravindran Date: Mon, 7 Oct 2024 19:55:32 -0400 Subject: [PATCH] Fail windows update when installed version does not match cr: https://code.amazon.com/reviews/CR-153091737 --- agent/update/processor/processor.go | 9 +++-- agent/update/processor/processor_test.go | 37 ++++++------------- agent/update/processor/progress.go | 4 +- agent/update/processor/progress_test.go | 1 - agent/updateutil/updateconstants/constants.go | 5 +-- agent/updateutil/updateutil_windows.go | 8 +--- agent/updateutil/updateutil_windows_test.go | 22 ----------- 7 files changed, 21 insertions(+), 65 deletions(-) diff --git a/agent/update/processor/processor.go b/agent/update/processor/processor.go index a5e5ad9d6..37689fdd9 100644 --- a/agent/update/processor/processor.go +++ b/agent/update/processor/processor.go @@ -578,11 +578,14 @@ func verifyInstallation(mgr *updateManager, log log.T, updateDetail *UpdateDetai } if !isRollback { - // reset the sub status as it is used in the succeeded function now - mgr.subStatus = "" // initiate rollback when agent installation is not complete if versionInstalledErrCode := mgr.util.VerifyInstalledVersion(log, version); versionInstalledErrCode != "" { - mgr.subStatus = string(versionInstalledErrCode) + message := updateutil.BuildMessage(nil, + "failed to identify installed target agent version: %v", + updateDetail.TargetVersion) + updateDetail.AppendError(log, message) + // we will receive only 1 error code - ErrorInstTargetVersionNotFoundViaReg + return mgr.failed(updateDetail, log, versionInstalledErrCode, message, false) } log.Infof("%v is running", updateDetail.PackageName) return mgr.succeeded(updateDetail, log) diff --git a/agent/update/processor/processor_test.go b/agent/update/processor/processor_test.go index e5a49da50..d105d2297 100644 --- a/agent/update/processor/processor_test.go +++ b/agent/update/processor/processor_test.go @@ -1684,30 +1684,11 @@ func TestVerifyInstallation_VersionCheck_Success(t *testing.T) { return nil } - // action - err := verifyInstallation(updater.mgr, logger, updateDetail, false) - - // assert - assert.NoError(t, err) - assert.False(t, isRollbackCalled) - assert.Equal(t, updateDetail.State, Completed) - assert.Equal(t, updateDetail.Result, contracts.ResultStatusSuccess) - assert.Equal(t, updater.mgr.subStatus, "") -} - -func TestVerifyInstallation_VersionCheck_Failed_WMI(t *testing.T) { - // setup - var logger = logmocks.NewMockLog() - control := &stubControl{serviceIsRunning: true, versionErrorCode: updateconstants.ErrorInstTargetVersionNotFoundViaWMIC} - updater := createUpdaterStubs(control) - updateDetail := createUpdateDetail(Installed) - isRollbackCalled := false - - updater.mgr.rollback = func(mgr *updateManager, log log.T, updateDetail *UpdateDetail) (err error) { - isRollbackCalled = true + finalize_error_code := "" + updater.mgr.finalize = func(mgr *updateManager, updateDetail *UpdateDetail, errorCode string) (err error) { + finalize_error_code = errorCode return nil } - // action err := verifyInstallation(updater.mgr, logger, updateDetail, false) @@ -1716,7 +1697,7 @@ func TestVerifyInstallation_VersionCheck_Failed_WMI(t *testing.T) { assert.False(t, isRollbackCalled) assert.Equal(t, updateDetail.State, Completed) assert.Equal(t, updateDetail.Result, contracts.ResultStatusSuccess) - assert.Equal(t, updater.mgr.subStatus, string(updateconstants.ErrorInstTargetVersionNotFoundViaWMIC)) + assert.Equal(t, string(""), finalize_error_code) } func TestVerifyInstallation_VersionCheck_Failed_Reg(t *testing.T) { @@ -1731,7 +1712,11 @@ func TestVerifyInstallation_VersionCheck_Failed_Reg(t *testing.T) { isRollbackCalled = true return nil } - + finalize_error_code := "" + updater.mgr.finalize = func(mgr *updateManager, updateDetail *UpdateDetail, errorCode string) (err error) { + finalize_error_code = errorCode + return nil + } // action err := verifyInstallation(updater.mgr, logger, updateDetail, false) @@ -1739,8 +1724,8 @@ func TestVerifyInstallation_VersionCheck_Failed_Reg(t *testing.T) { assert.NoError(t, err) assert.False(t, isRollbackCalled) assert.Equal(t, updateDetail.State, Completed) - assert.Equal(t, updateDetail.Result, contracts.ResultStatusSuccess) - assert.Equal(t, updater.mgr.subStatus, string(updateconstants.ErrorInstTargetVersionNotFoundViaReg)) + assert.Equal(t, updateDetail.Result, contracts.ResultStatusFailed) + assert.Equal(t, string(updateconstants.ErrorInstTargetVersionNotFoundViaReg), finalize_error_code) } func TestVerifyInstallationCannotStartAgent(t *testing.T) { diff --git a/agent/update/processor/progress.go b/agent/update/processor/progress.go index d78fc650b..ddfef83bc 100644 --- a/agent/update/processor/progress.go +++ b/agent/update/processor/progress.go @@ -96,8 +96,8 @@ func (u *updateManager) succeeded(updateDetail *UpdateDetail, log logPkg.T) (err log.WriteEvent( logger.AgentUpdateResultMessage, updateDetail.SourceVersion, - PrepareHealthStatus(updateDetail, u.subStatus, updateDetail.TargetVersion)) - return u.finalize(u, updateDetail, u.subStatus) + PrepareHealthStatus(updateDetail, "", updateDetail.TargetVersion)) + return u.finalize(u, updateDetail, "") } // failed sets update to failed with error messages diff --git a/agent/update/processor/progress_test.go b/agent/update/processor/progress_test.go index 7fffaa6bc..16f972d3e 100644 --- a/agent/update/processor/progress_test.go +++ b/agent/update/processor/progress_test.go @@ -13,7 +13,6 @@ // Package processor contains the methods for update ssm agent. // It also provides methods for sendReply and updateInstanceInfo - //go:build e2e // +build e2e diff --git a/agent/updateutil/updateconstants/constants.go b/agent/updateutil/updateconstants/constants.go index 0b84b3c1f..a20604663 100644 --- a/agent/updateutil/updateconstants/constants.go +++ b/agent/updateutil/updateconstants/constants.go @@ -281,10 +281,7 @@ const ( ErrorCannotStartService ErrorCode = "ErrorCannotStartService" // ErrorInstTargetVersionNotFoundViaReg represents that the target agent version could not be found using Registry - ErrorInstTargetVersionNotFoundViaReg ErrorCode = "InstTgtVerNotFoundViaReg" - - // ErrorInstTargetVersionNotFoundViaWMIC represents that the target agent version could not be found using WMIC - ErrorInstTargetVersionNotFoundViaWMIC ErrorCode = "InstTgtVerNotFoundViaWMIC" + ErrorInstTargetVersionNotFoundViaReg ErrorCode = "ErrorInstTargetVersionNotFoundViaReg" // ErrorCannotStopService represents Cannot stop Ec2Config service ErrorCannotStopService ErrorCode = "ErrorCannotStopService" diff --git a/agent/updateutil/updateutil_windows.go b/agent/updateutil/updateutil_windows.go index 6d30c8be5..8604087e2 100644 --- a/agent/updateutil/updateutil_windows.go +++ b/agent/updateutil/updateutil_windows.go @@ -52,7 +52,6 @@ var ( deleteFile = fileutil.DeleteFile fileWrite = fileutil.WriteIntoFileWithPermissions getVersionThroughRegistryKeyRef = getVersionThroughRegistryKey - getVersionThroughWMIRef = getVersionThroughWMI ) type UpdatePluginRunState struct { @@ -339,12 +338,7 @@ func verifyVersion(log log.T, targetVersion string) updateconstants.ErrorCode { log.Infof("Verifying Agent version using Registry") registryCurrentAgentVersion := getVersionThroughRegistryKeyRef(log) if targetVersion == registryCurrentAgentVersion { - log.Infof("Verifying Agent version using WMI query") - wmiCurrentAgentVersion := getVersionThroughWMIRef(log) - if targetVersion == wmiCurrentAgentVersion { - return "" // return blank when success - } - return updateconstants.ErrorInstTargetVersionNotFoundViaWMIC + return "" // success code } return updateconstants.ErrorInstTargetVersionNotFoundViaReg } diff --git a/agent/updateutil/updateutil_windows_test.go b/agent/updateutil/updateutil_windows_test.go index 74b1ea5b1..d122de442 100644 --- a/agent/updateutil/updateutil_windows_test.go +++ b/agent/updateutil/updateutil_windows_test.go @@ -127,35 +127,16 @@ func TestVerifyVersion_Success(t *testing.T) { getVersionThroughRegistryKeyRef = func(log logPkg.T) string { return expectedVersion } - getVersionThroughWMIRef = func(log logPkg.T) string { - return expectedVersion - } logMock := log.NewMockLog() errCode := verifyVersion(logMock, expectedVersion) assert.Equal(t, updateconstants.ErrorCode(""), errCode) } -func TestVerifyVersion_Failed_WMI(t *testing.T) { - expectedVersion := "3.2.0.0" - getVersionThroughRegistryKeyRef = func(log logPkg.T) string { - return expectedVersion - } - getVersionThroughWMIRef = func(log logPkg.T) string { - return "" - } - logMock := log.NewMockLog() - errCode := verifyVersion(logMock, expectedVersion) - assert.Equal(t, updateconstants.ErrorInstTargetVersionNotFoundViaWMIC, errCode) -} - func TestVerifyVersion_Failed_Registry(t *testing.T) { expectedVersion := "3.2.0.0" getVersionThroughRegistryKeyRef = func(log logPkg.T) string { return "" } - getVersionThroughWMIRef = func(log logPkg.T) string { - return expectedVersion - } logMock := log.NewMockLog() errCode := verifyVersion(logMock, expectedVersion) assert.Equal(t, updateconstants.ErrorInstTargetVersionNotFoundViaReg, errCode) @@ -165,9 +146,6 @@ func TestVerifyVersion_Failed_Both(t *testing.T) { getVersionThroughRegistryKeyRef = func(log logPkg.T) string { return "" } - getVersionThroughWMIRef = func(log logPkg.T) string { - return "" - } logMock := log.NewMockLog() expectedVersion := "3.2.0.0" errCode := verifyVersion(logMock, expectedVersion)