Skip to content

Update status on enable #82

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hbuinguyen2025 wants to merge 17 commits into
base: master
Choose a base branch
from
Open

Update status on enable #82

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
1b2ddb5
Update the logic for `shouldReportStatus`
Mar 4, 2026
c9e4372
Fixed test setup and cleanup
Mar 5, 2026
d6d50b9
Added unit tests for the updated shouldReportStatus function
Mar 5, 2026
4f743d6
Update deprecated references of ioutil in main_test.go
Mar 5, 2026
00b373e
Edit code comment
Mar 9, 2026
586085d
Added custom error for hostgacommunicator
Mar 13, 2026
adefc61
Update status computation
Mar 13, 2026
469251a
Update reportStatusFunc to reportStatusWrapper
Mar 19, 2026
e306815
Fix statusMessage
Mar 19, 2026
c4b7178
Fix unit test
Mar 19, 2026
cf17000
Remove check for new requested sequence without vm app update
Mar 20, 2026
e9473c0
Update to use requestedSeqNumber
Mar 20, 2026
5e90dd1
Add a check for when status file path is a directory too
Mar 20, 2026
69300b4
Update tests for hostgacommunicator to use newly defined error
Mar 20, 2026
b175aa4
Create custom DownloadPackageError
Mar 20, 2026
9729754
Added custom error DownloadConfigError
Mar 20, 2026
f7786fc
Replace the Rename with Copy when backing up status file
Mar 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 20 additions & 16 deletions main/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,21 +214,12 @@ func customEnable(ext *vmextensionhelper.VMExtension, hostgaCommunicator hostgac
return errors.Wrapf(err, "Could not get package registry")
}

// write success status if requested sequence number is newer
shouldReportStatus := false

if ext.CurrentSequenceNumber == nil || requestedSequenceNumber > *ext.CurrentSequenceNumber {
shouldReportStatus = true
} else if requestedSequenceNumber == *ext.CurrentSequenceNumber {
statusType, err := utils.GetStatusType(ext.HandlerEnv, requestedSequenceNumber)
if err != nil || strings.EqualFold(string(statusType), string(status.StatusTransitioning)) {
// either something is wrong with the status file
// or its a transitioning status file
// overwrite it in either case
shouldReportStatus = true
}
}
if shouldReportStatus {
// The status file needs to be updated whenever there is some VM App actions because
// all VM Apps are always processed. It also needed to be updated if the status is
// Transitioning even if there's no VM App actions to do. The only time update isn't
// needed is when VM is rebooted and there are no changes to the desired packages,
// and no VM App re-run after rebooting.
if shouldReportStatus(ext, requestedSequenceNumber, vmAppResults) {
var statusResult status.StatusType
statusMessage := getStatusMessage(currentPackageRegistry.GetPackageCollection(), executeError, result)
if executeError.GetErrorIfDeploymentFailed() == nil { // treatFailureAsDeploymentFailure
Expand Down Expand Up @@ -258,7 +249,20 @@ func customEnable(ext *vmextensionhelper.VMExtension, hostgaCommunicator hostgac
return nil
}

// Callback indicating the extension is being removed
func shouldReportStatus(ext *vmextensionhelper.VMExtension, requestedSequenceNumber uint, vmAppResults *actionplan.PackageOperationResults) bool {
// Report status if any VM App operations were executed
if vmAppResults != nil && len(*vmAppResults) > 0 {
return true
}
// Also report status if the current status is Transitioning (to update it to Success/Error)
// or if there is something wrong with the status file
statusType, err := utils.GetStatusType(ext.HandlerEnv, requestedSequenceNumber)
if err != nil || strings.EqualFold(string(statusType), string(status.StatusTransitioning)) {
return true
}
return false
}

func vmAppUninstallCallback(ext *vmextensionhelper.VMExtension) error {
ext.ExtensionEvents.LogInformationalEvent("Uninstalling", "VmApplications extension - removing all applications for uninstall")
hostGaCommunicator := hostgacommunicator.HostGaCommunicator{}
Expand Down
89 changes: 77 additions & 12 deletions main/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"encoding/json"
"errors"
"fmt"
"io/ioutil"
"os"
"path"
"path/filepath"
Expand Down Expand Up @@ -73,15 +72,15 @@ func nopLog() *logging.ExtensionLogger {

var maintestdir string

func TestMain(m *testing.M) {
testdir, err := ioutil.TempDir("", "maintest")
func setupTest(t *testing.T) {
testdir, err := os.MkdirTemp("", "maintest")
if err != nil {
return
t.Fatalf("Failed to create temp dir: %v", err)
}

err = os.MkdirAll(testdir, constants.FilePermissions_UserOnly_ReadWriteExecute)
if err != nil {
return
t.Fatalf("Failed to create test dir: %v", err)
}

setSequenceNumberFunc = func(extName, extVersion string, seqNo uint) error {
Expand All @@ -90,32 +89,36 @@ func TestMain(m *testing.M) {
}

maintestdir = testdir
exitVal := m.Run()
os.RemoveAll(maintestdir)

os.Exit(exitVal)
t.Cleanup(func() {
os.RemoveAll(maintestdir)
})
}

func Test_settingsFailToInit(t *testing.T) {
setupTest(t)
ExtensionVersion = ""
defer resetExtensionVersion()
err := getExtensionAndRun([]string{"vm-application-manager", "enable"})
require.Error(t, err)
}

func Test_failToCreateExtension(t *testing.T) {
setupTest(t)
// This will fail automatically because Guest Agent hasn't set the required sequence numbers
err := getExtensionAndRun([]string{"vm-application-manager", "enable"})
require.Error(t, err)
}

func Test_getVMPackageData_noSettings(t *testing.T) {
setupTest(t)
ext := createTestVMExtension(t, nil)
err := customEnable(ext, noopHostGaCommunicator, 0)
require.Error(t, err)
}

func Test_getVMPackageData_cannotDeserialize(t *testing.T) {
setupTest(t)
vmPackages := "yabasnarfle {}"

ext := createTestVMExtension(t, vmPackages)
Expand All @@ -124,6 +127,7 @@ func Test_getVMPackageData_cannotDeserialize(t *testing.T) {
}

func Test_getVMPackageData_noApplications(t *testing.T) {
setupTest(t)
vmApplications := []extdeserialization.VmAppSetting{}

ext := createTestVMExtension(t, vmApplications)
Expand All @@ -132,6 +136,7 @@ func Test_getVMPackageData_noApplications(t *testing.T) {
}

func Test_getVMPackageData_valid(t *testing.T) {
setupTest(t)
order := 1
vmApplications := []extdeserialization.VmAppSetting{
{
Expand All @@ -148,6 +153,7 @@ func Test_getVMPackageData_valid(t *testing.T) {
}

func Test_getVMAppProtectedSettings_valid(t *testing.T) {
setupTest(t)
order := 1
actions := extdeserialization.ActionSetting{
ActionName: "logging",
Expand Down Expand Up @@ -181,6 +187,7 @@ func Test_getVMAppProtectedSettings_valid(t *testing.T) {
}

func Test_getVMAppProtectedSettings_valid_no_custom_actions(t *testing.T) {
setupTest(t)
order := 1

appSettings := extdeserialization.VmAppSetting{
Expand All @@ -201,6 +208,7 @@ func Test_getVMAppProtectedSettings_valid_no_custom_actions(t *testing.T) {
}

func Test_getVMPackageData_noVersion(t *testing.T) {
setupTest(t)
order := 1
vmApplications := []extdeserialization.VmAppSetting{
{
Expand All @@ -217,6 +225,7 @@ func Test_getVMPackageData_noVersion(t *testing.T) {
}

func Test_GetApplicationMetadataWithInvalidRebootBehavior_DefaultsToNone(t *testing.T) {
setupTest(t)
order := 1
vmApplications := []extdeserialization.VmAppSetting{
{
Expand Down Expand Up @@ -247,6 +256,7 @@ func Test_GetApplicationMetadataWithInvalidRebootBehavior_DefaultsToNone(t *test
}

func Test_getVMPackageDataCustomAction_valid(t *testing.T) {
setupTest(t)
order := 1
actions := extdeserialization.ActionSetting{
ActionName: "Action1",
Expand Down Expand Up @@ -285,7 +295,7 @@ func Test_getVMPackageDataCustomAction_valid(t *testing.T) {
require.Contains(t, currentpackages[vmApplications[0].ApplicationName].Result, actionplan.Success)
// test contents of the status file
statusFilePath := filepath.Join(ext.HandlerEnv.StatusFolder, fmt.Sprintf("%d.status", requestedSequenceNumber))
fileBytes, err := ioutil.ReadFile(statusFilePath)
fileBytes, err := os.ReadFile(statusFilePath)
require.NoError(t, err)
statusReport := status.StatusReport{}
err = json.Unmarshal(fileBytes, &statusReport)
Expand All @@ -309,6 +319,7 @@ func Test_getVMPackageDataCustomAction_valid(t *testing.T) {
}

func Test_getVMPackageDataCustomAction_CriticalError(t *testing.T) {
setupTest(t)
order := 1
actions := extdeserialization.ActionSetting{
ActionName: "Action1",
Expand All @@ -333,6 +344,7 @@ func Test_getVMPackageDataCustomAction_CriticalError(t *testing.T) {
}

func Test_getVMPackageData_noApplicationName(t *testing.T) {
setupTest(t)
order := 1
vmApplications := []extdeserialization.VmAppSetting{
{
Expand All @@ -349,6 +361,7 @@ func Test_getVMPackageData_noApplicationName(t *testing.T) {
}

func Test_main_statusIsWrittenForCriticalErrors(t *testing.T) {
setupTest(t)
order := 1
vmApplications := []extdeserialization.VmAppSetting{
{
Expand All @@ -372,7 +385,7 @@ func Test_main_statusIsWrittenForCriticalErrors(t *testing.T) {
err := getExtensionAndRun([]string{"vm-application-manager", vmextension.EnableOperation.ToString()})
require.NoError(t, err)
statusFilePath := filepath.Join(ext.HandlerEnv.StatusFolder, fmt.Sprintf("%d.status", requestedSequenceNumber))
fileBytes, err := ioutil.ReadFile(statusFilePath)
fileBytes, err := os.ReadFile(statusFilePath)
require.NoError(t, err)
fileString := string(fileBytes)
require.Contains(t, fileString, vmextension.EnableOperation.ToStatusName())
Expand All @@ -384,6 +397,7 @@ func Test_main_statusIsWrittenForCriticalErrors(t *testing.T) {
}

func Test_main_statusIsNotWrittenForFileLockErrors(t *testing.T) {
setupTest(t)
order := 1
vmApplications := []extdeserialization.VmAppSetting{
{
Expand Down Expand Up @@ -417,6 +431,7 @@ func Test_main_statusIsNotWrittenForFileLockErrors(t *testing.T) {
}

func Test_main_nothingToProcess_noStatusUpdate(t *testing.T) {
setupTest(t)
vmApplications := []extdeserialization.VmAppSetting{}
ext := createTestVMExtension(t, vmApplications)

Expand All @@ -434,6 +449,7 @@ func Test_main_nothingToProcess_noStatusUpdate(t *testing.T) {
}

func Test_main_transitioningStatusIsUpdated(t *testing.T) {
setupTest(t)
vmApplications := []extdeserialization.VmAppSetting{}
ext := createTestVMExtension(t, vmApplications)

Expand All @@ -451,22 +467,68 @@ func Test_main_transitioningStatusIsUpdated(t *testing.T) {
}

func Test_main_nothingToProcess_withStatus(t *testing.T) {
setupTest(t)
vmApplications := []extdeserialization.VmAppSetting{}
ext := createTestVMExtension(t, vmApplications)
hostGaCommunicator := NoopHostGaCommunicator{}
requestedSequenceNumber := *ext.CurrentSequenceNumber + 1
err := customEnable(ext, &hostGaCommunicator, requestedSequenceNumber)
require.NoError(t, err)
statusFilePath := filepath.Join(ext.HandlerEnv.StatusFolder, fmt.Sprintf("%d.status", requestedSequenceNumber))
fileBytes, err := ioutil.ReadFile(statusFilePath)
fileBytes, err := os.ReadFile(statusFilePath)
require.NoError(t, err)
fileString := string(fileBytes)
require.Contains(t, fileString, vmextension.EnableOperation.ToStatusName())
require.Contains(t, fileString, status.StatusSuccess)
require.Equal(t, requestedSequenceNumber, currentSequenceNumber)
}

// Test that shouldReportStatus returns true when vmAppResults has elements,
// even if there's an existing error status file
func Test_shouldReportStatus_withVmAppResults_existingErrorStatus(t *testing.T) {
setupTest(t)
vmApplications := []extdeserialization.VmAppSetting{}
ext := createTestVMExtension(t, vmApplications)
requestedSequenceNumber := *ext.CurrentSequenceNumber

// Pre-create a status file with error status
err := utils.ReportStatus(ext.HandlerEnv, requestedSequenceNumber, status.StatusError, vmextension.EnableOperation.ToStatusName(), "previous failure")
require.NoError(t, err)

// Create vmAppResults with one result (simulating an app was executed)
vmAppResults := &actionplan.PackageOperationResults{
{PackageName: "testapp", Operation: "install", Result: actionplan.Success},
}

// shouldReportStatus should return true because vmAppResults has elements
result := shouldReportStatus(ext, requestedSequenceNumber, vmAppResults)
require.True(t, result, "shouldReportStatus should return true when vmAppResults has elements")
}

// Test that shouldReportStatus returns true when vmAppResults has elements,
// even if there's an existing success status file
func Test_shouldReportStatus_withVmAppResults_existingSuccessStatus(t *testing.T) {
setupTest(t)
vmApplications := []extdeserialization.VmAppSetting{}
ext := createTestVMExtension(t, vmApplications)
requestedSequenceNumber := *ext.CurrentSequenceNumber

// Pre-create a status file with success status
err := utils.ReportStatus(ext.HandlerEnv, requestedSequenceNumber, status.StatusSuccess, vmextension.EnableOperation.ToStatusName(), "previous success")
require.NoError(t, err)

// Create vmAppResults with one result (simulating an app was executed)
vmAppResults := &actionplan.PackageOperationResults{
{PackageName: "testapp", Operation: "install", Result: "Error: command failed"},
}

// shouldReportStatus should return true because vmAppResults has elements
result := shouldReportStatus(ext, requestedSequenceNumber, vmAppResults)
require.True(t, result, "shouldReportStatus should return true when vmAppResults has elements")
}

func Test_uninstall_cannotCreatePackageRegistry(t *testing.T) {
setupTest(t)
vmApplications := []extdeserialization.VmAppSetting{}
ext := createTestVMExtension(t, vmApplications)
hostGaCommunicator := NoopHostGaCommunicator{}
Expand All @@ -480,13 +542,14 @@ func Test_uninstall_cannotCreatePackageRegistry(t *testing.T) {
}

func Test_uninstall_cannotReadPackageRegistry(t *testing.T) {
setupTest(t)
vmApplications := []extdeserialization.VmAppSetting{}
ext := createTestVMExtension(t, vmApplications)
hostGaCommunicator := NoopHostGaCommunicator{}

// Write an invalid registry so we can't create a package registry
appRegistryFilePath := path.Join(ext.HandlerEnv.ConfigFolder, packageregistry.LocalApplicationRegistryFileName)
ioutil.WriteFile(appRegistryFilePath, []byte("}"), 0644)
os.WriteFile(appRegistryFilePath, []byte("}"), 0644)
defer os.Remove(appRegistryFilePath)

err := doVmAppUninstallCallback(ext, &hostGaCommunicator)
Expand All @@ -495,6 +558,7 @@ func Test_uninstall_cannotReadPackageRegistry(t *testing.T) {
}

func Test_uninstall_noAppsToUninstall(t *testing.T) {
setupTest(t)
vmApplications := []extdeserialization.VmAppSetting{}
ext := createTestVMExtension(t, vmApplications)
hostGaCommunicator := NoopHostGaCommunicator{}
Expand Down Expand Up @@ -536,6 +600,7 @@ func Test_uninstall_noAppsToUninstall(t *testing.T) {
}

func Test_uninstall_uninstallApps(t *testing.T) {
setupTest(t)
vmApplications := []extdeserialization.VmAppSetting{}
ext := createTestVMExtension(t, vmApplications)
hostGaCommunicator := NoopHostGaCommunicator{}
Expand Down