Fix accidental VMApp removal when extension update fails. Also migrate VMApp download directory on extension update.

.gitignore

@@ -13,3 +13,4 @@ bundle
 extension-launcher*
 internal/customactionplan/testdir/
 licenses
+main.exe

internal/packageregistry/packageregistry.go

@@ -5,7 +5,6 @@ package packageregistry
 
 import (
 	"encoding/json"
-	"io/ioutil"
 	"os"
 	"path"
 	"time"
@@ -168,7 +167,7 @@ func (self *PackageRegistry) GetExistingPackages() (CurrentPackageRegistry, erro
 	_, err := os.Stat(localApplicationRegistryFilePath)
 	if err == nil {
 		// The file exists
-		fileBytes, err := ioutil.ReadFile(localApplicationRegistryFilePath)
+		fileBytes, err := os.ReadFile(localApplicationRegistryFilePath)
 		if err != nil {
 			return currentPackageRegistry, err
 		}
@@ -210,7 +209,7 @@ func (self *PackageRegistry) WriteToDisk(packageRegistry CurrentPackageRegistry)
 		return err
 	}
 
-	err = ioutil.WriteFile(regFile, bytes, constants.FilePermissions_UserOnly_ReadWrite)
+	err = os.WriteFile(regFile, bytes, constants.FilePermissions_UserOnly_ReadWrite)
 	self.logger.Info("Wrote package registry to %v", regFile)
 
 	if doesBackupFileExist {

main/main.go

@@ -124,7 +124,7 @@ func getVMExtension() (*vmextensionhelper.VMExtension, error) {
 		return nil, err
 	}
 
-	ii.UninstallCallback = vmAppUninstallCallback
+	ii.UninstallCallback = nil // no need to do any special handling on uninstall, so we can set the callback to nil
 	ii.UpdateCallback = vmAppUpdateCallback
 	ii.LogFileNamePattern = "VmAppExt_%v.log"
 
@@ -257,42 +257,3 @@ func customEnable(ext *vmextensionhelper.VMExtension, hostgaCommunicator hostgac
 
 	return nil
 }
-
-// Callback indicating the extension is being removed
-func vmAppUninstallCallback(ext *vmextensionhelper.VMExtension) error {
-	ext.ExtensionEvents.LogInformationalEvent("Uninstalling", "VmApplications extension - removing all applications for uninstall")
-	hostGaCommunicator := hostgacommunicator.HostGaCommunicator{}
-	err := doVmAppUninstallCallback(ext, &hostGaCommunicator)
-	if err == nil {
-		ext.ExtensionEvents.LogInformationalEvent("Completed", "VmApplications extension uninstalled. Result=Success")
-	} else {
-		ext.ExtensionEvents.LogInformationalEvent(
-			"Completed",
-			fmt.Sprintf("VmApplications extension uninstall finished. Result=Failure;Reason=%v", err.Error()))
-	}
-	return err
-}
-
-func doVmAppUninstallCallback(ext *vmextensionhelper.VMExtension, hostGaCommunicator hostgacommunicator.IHostGaCommunicator) error {
-	packageRegistry, err := packageregistry.New(ext.ExtensionLogger, ext.HandlerEnv, filelockTimeoutDuration)
-	if err != nil {
-		return errors.Wrapf(err, "Could not create package registry")
-	}
-	defer packageRegistry.Close()
-
-	currentPackageRegistry, err := packageRegistry.GetExistingPackages()
-	if err != nil {
-		return errors.Wrapf(err, "Could not read current package registry")
-	}
-
-	// Create an empty incoming collection so we'll create an action plan to remove all applications
-	emptyIncomingCollection := make(packageregistry.VMAppPackageIncomingCollection, 0)
-
-	actionPlan := actionplan.New(currentPackageRegistry, emptyIncomingCollection, ext.HandlerEnv, hostGaCommunicator, ext.ExtensionLogger)
-	commandHandler := commandhandler.CommandHandler{}
-
-	// Removing applications is best effort, so even if there are errors here, we ignore them
-	_, _ = actionPlan.Execute(packageRegistry, ext.ExtensionEvents, &commandHandler)
-
-	return nil
-}

main/main_test.go

@@ -29,7 +29,6 @@ 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"
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
@@ -466,84 +465,6 @@ func Test_main_nothingToProcess_withStatus(t *testing.T) {
 	require.Equal(t, requestedSequenceNumber, currentSequenceNumber)
 }
 
-func Test_uninstall_cannotCreatePackageRegistry(t *testing.T) {
-	vmApplications := []extdeserialization.VmAppSetting{}
-	ext := createTestVMExtension(t, vmApplications)
-	hostGaCommunicator := NoopHostGaCommunicator{}
-
-	// Set the config folder to an invalid path so we can't create a package registry
-	ext.HandlerEnv.ConfigFolder = "/yabaflarg/flarpaglarp"
-
-	err := doVmAppUninstallCallback(ext, &hostGaCommunicator)
-	require.Error(t, err)
-	require.EqualError(t, err, cannotCreatePackageRegistryError)
-}
-
-func Test_uninstall_cannotReadPackageRegistry(t *testing.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)
-	defer os.Remove(appRegistryFilePath)
-
-	err := doVmAppUninstallCallback(ext, &hostGaCommunicator)
-	require.Error(t, err)
-	require.EqualError(t, err, "Could not read current package registry: invalid character '}' looking for beginning of value")
-}
-
-func Test_uninstall_noAppsToUninstall(t *testing.T) {
-	vmApplications := []extdeserialization.VmAppSetting{}
-	ext := createTestVMExtension(t, vmApplications)
-	hostGaCommunicator := NoopHostGaCommunicator{}
-
-	package1 := path.Join(ext.HandlerEnv.ConfigFolder, "package1")
-	package2 := path.Join(ext.HandlerEnv.ConfigFolder, "package2")
-	package1Quotes := fmt.Sprintf("\"%v\"", package1)
-	package2Quotes := fmt.Sprintf("\"%v\"", package2)
-
-	// Create a package registry where the remove commands will write their respective files
-	reg := packageregistry.CurrentPackageRegistry{"package1": &packageregistry.VMAppPackageCurrent{
-		ApplicationName:    "package1",
-		DirectDownloadOnly: false,
-		InstallCommand:     "dontcare",
-		RemoveCommand:      "echo moein > " + package1Quotes,
-		UpdateCommand:      "dontcare",
-		Version:            "1.2.3.1",
-	}, "package2": &packageregistry.VMAppPackageCurrent{
-		ApplicationName:    "package2",
-		DirectDownloadOnly: true,
-		InstallCommand:     "dontcare",
-		RemoveCommand:      "echo moein > " + package2Quotes,
-		UpdateCommand:      "dontcare",
-		Version:            "1.2.3.2",
-	}}
-
-	pkgHndlr, err := packageregistry.New(nopLog(), ext.HandlerEnv, time.Second)
-	assert.NoError(t, err, "operation should not throw error")
-	err = pkgHndlr.WriteToDisk(reg)
-	assert.NoError(t, err, "Should be able to write package registry to disk")
-	pkgHndlr.Close()
-
-	err = doVmAppUninstallCallback(ext, &hostGaCommunicator)
-	require.NoError(t, err)
-
-	// Verify we removed both apps, which deleted the files
-	require.True(t, fileExists(package1), "First application was not removed")
-	require.True(t, fileExists(package2), "Second application was not removed")
-}
-
-func Test_uninstall_uninstallApps(t *testing.T) {
-	vmApplications := []extdeserialization.VmAppSetting{}
-	ext := createTestVMExtension(t, vmApplications)
-	hostGaCommunicator := NoopHostGaCommunicator{}
-
-	err := doVmAppUninstallCallback(ext, &hostGaCommunicator)
-	require.NoError(t, err)
-}
-
 func fileExists(filePath string) bool {
 	if _, err := os.Stat(filePath); errors.Is(err, os.ErrNotExist) {
 		return false