provisioner/powershell: Update remote clean up logic

* Add retry logic so that the provisioner will retry if it fails to upload/execute because of some restart provisioner step
* Add a testConfigWithSkipClean for testing that the provisioner executes the correct commands
* Add a test case for toggling the "skip_clean" config option

provisioner/powershell/provisioner.go

@@ -73,6 +73,8 @@ type Config struct {
 
 	ExecutionPolicy ExecutionPolicy `mapstructure:"execution_policy"`
 
+	remoteCleanUpScriptPath string
+
 	ctx interpolate.Context
 }
 
@@ -155,6 +157,8 @@ func (p *Provisioner) Prepare(raws ...interface{}) error {
 		p.config.Vars = make([]string, 0)
 	}
 
+	p.config.remoteCleanUpScriptPath = fmt.Sprintf(`c:/Windows/Temp/packer-cleanup-%s.ps1`, uuid.TimeOrderedUUID())
+
 	var errs error
 	if p.config.Script != "" && len(p.config.Scripts) > 0 {
 		errs = packer.MultiErrorAppend(errs,
@@ -249,6 +253,8 @@ func (p *Provisioner) Provision(ctx context.Context, ui packer.Ui, comm packer.C
 		defer os.Remove(temp)
 	}
 
+	// every provisioner run will only have one env var script file so lets add it first
+	uploadedScripts := []string{p.config.RemoteEnvVarPath}
 	for _, path := range scripts {
 		ui.Say(fmt.Sprintf("Provisioning with powershell script: %s", path))
 
@@ -295,50 +301,57 @@ func (p *Provisioner) Provision(ctx context.Context, ui packer.Ui, comm packer.C
 		// Close the original file since we copied it
 		f.Close()
 
-		log.Printf("%s returned with exit code %d", p.config.RemotePath, cmd.ExitStatus())
+		// Record every other uploaded script file so we can clean it up later
+		uploadedScripts = append(uploadedScripts, p.config.RemotePath)
 
+		log.Printf("%s returned with exit code %d", p.config.RemotePath, cmd.ExitStatus())
 		if err := p.config.ValidExitCode(cmd.ExitStatus()); err != nil {
 			return err
 		}
+	}
 
-		if !p.config.SkipClean {
-			files := []string{p.config.RemotePath, p.config.RemoteEnvVarPath}
-			command, err := p.cleanUpRemoteFilesCommand(files...)
-			if err != nil {
-				log.Printf("failed to prepare packer cleanup script")
-			}
+	if p.config.SkipClean {
+		return nil
+	}
 
-			cmd = &packer.RemoteCmd{Command: command}
-			if err := cmd.RunWithUi(ctx, comm, ui); err != nil {
-				log.Printf("failed to clean up temporary files")
-			}
+	err := retry.Config{StartTimeout: p.config.StartRetryTimeout}.Run(ctx, func(ctx context.Context) error {
+		command, err := p.createRemoteCleanUpCommand(uploadedScripts)
+		if err != nil {
+			log.Printf("failed to create a remote cleanup script: %s", err)
+			return err
 		}
 
+		cmd := &packer.RemoteCmd{Command: command}
+		return cmd.RunWithUi(ctx, comm, ui)
+	})
+	if err != nil {
+		log.Printf("failed to clean up temporary files: %s", strings.Join(uploadedScripts, ","))
 	}
 
 	return nil
 }
 
-func (p *Provisioner) cleanUpRemoteFilesCommand(files ...string) (string, error) {
-	if len(files) == 0 {
-		return "", fmt.Errorf("no files provided for cleanup")
+// createRemoteCleanUpCommand will generated a powershell script that will remove remote files;
+// returning a command that can be executed remotely to do the cleanup.
+func (p *Provisioner) createRemoteCleanUpCommand(remoteFiles []string) (string, error) {
+	if len(remoteFiles) == 0 {
+		return "", fmt.Errorf("no remoteFiles provided for cleanup")
 	}
 
 	var b strings.Builder
-	baseDir := filepath.Dir(p.config.RemotePath)
-	uploadPath := filepath.Join(baseDir, fmt.Sprintf("packer-cleanup-%s.ps1", uuid.TimeOrderedUUID()))
 	// This script should self destruct.
-	files = append(files, uploadPath)
-	for _, filename := range files {
+	remotePath := p.config.remoteCleanUpScriptPath
+	remoteFiles = append(remoteFiles, remotePath)
+	for _, filename := range remoteFiles {
 		fmt.Fprintf(&b, "Remove-Item %s\n", filename)
 	}
 
-	if err := p.communicator.Upload(uploadPath, strings.NewReader(b.String()), nil); err != nil {
-		log.Printf("packer clean up script %q failed to upload: %s", uploadPath, err)
+	if err := p.communicator.Upload(remotePath, strings.NewReader(b.String()), nil); err != nil {
+		return "", fmt.Errorf("clean up script %q failed to upload: %s", remotePath, err)
 	}
 
 	data := map[string]string{
-		"Path": uploadPath,
+		"Path": remotePath,
 		"Vars": p.config.RemoteEnvVarPath,
 	}
 	p.config.ctx.Data = data

provisioner/powershell/provisioner_test.go

@@ -15,16 +15,6 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func testConfig() map[string]interface{} {
-	return map[string]interface{}{
-		"inline": []interface{}{"foo", "bar"},
-	}
-}
-
-func init() {
-	//log.SetOutput(ioutil.Discard)
-}
-
 func TestProvisionerPrepare_extractScript(t *testing.T) {
 	config := testConfig()
 	p := new(Provisioner)
@@ -335,11 +325,6 @@ func testUi() *packer.BasicUi {
 	}
 }
 
-func testObjects() (packer.Ui, packer.Communicator) {
-	ui := testUi()
-	return ui, new(packer.MockCommunicator)
-}
-
 func TestProvisionerProvision_ValidExitCodes(t *testing.T) {
 	config := testConfig()
 	delete(config, "inline")
@@ -387,7 +372,8 @@ func TestProvisionerProvision_InvalidExitCodes(t *testing.T) {
 }
 
 func TestProvisionerProvision_Inline(t *testing.T) {
-	config := testConfig()
+	// skip_clean is set to true otherwise the last command executed by the provisioner is the cleanup.
+	config := testConfigWithSkipClean()
 	delete(config, "inline")
 
 	// Defaults provided by Packer
@@ -400,7 +386,7 @@ func TestProvisionerProvision_Inline(t *testing.T) {
 	p.config.PackerBuildName = "vmware"
 	p.config.PackerBuilderType = "iso"
 	comm := new(packer.MockCommunicator)
-	p.Prepare(config)
+	_ = p.Prepare(config)
 	err := p.Provision(context.Background(), ui, comm, make(map[string]interface{}))
 	if err != nil {
 		t.Fatal("should not have error")
@@ -439,7 +425,8 @@ func TestProvisionerProvision_Scripts(t *testing.T) {
 	defer os.Remove(tempFile.Name())
 	defer tempFile.Close()
 
-	config := testConfig()
+	// skip_clean is set to true otherwise the last command executed by the provisioner is the cleanup.
+	config := testConfigWithSkipClean()
 	delete(config, "inline")
 	config["scripts"] = []string{tempFile.Name()}
 	config["packer_build_name"] = "foobuild"
@@ -465,11 +452,12 @@ func TestProvisionerProvision_Scripts(t *testing.T) {
 
 func TestProvisionerProvision_ScriptsWithEnvVars(t *testing.T) {
 	tempFile, _ := ioutil.TempFile("", "packer")
-	config := testConfig()
 	ui := testUi()
 	defer os.Remove(tempFile.Name())
 	defer tempFile.Close()
 
+	// skip_clean is set to true otherwise the last command executed by the provisioner is the cleanup.
+	config := testConfigWithSkipClean()
 	delete(config, "inline")
 
 	config["scripts"] = []string{tempFile.Name()}
@@ -499,10 +487,56 @@ func TestProvisionerProvision_ScriptsWithEnvVars(t *testing.T) {
 	}
 }
 
-func TestProvisionerProvision_UISlurp(t *testing.T) {
-	// UI should be called n times
+func TestProvisionerProvision_SkipClean(t *testing.T) {
+	tempFile, _ := ioutil.TempFile("", "packer")
+	defer func() {
+		tempFile.Close()
+		os.Remove(tempFile.Name())
+	}()
 
-	// UI should receive following messages / output
+	config := map[string]interface{}{
+		"scripts":     []string{tempFile.Name()},
+		"remote_path": "c:/Windows/Temp/script.ps1",
+	}
+
+	tt := []struct {
+		SkipClean                bool
+		LastExecutedCommandRegex string
+	}{
+		{
+			SkipClean:                true,
+			LastExecutedCommandRegex: `powershell -executionpolicy bypass "& { if \(Test-Path variable:global:ProgressPreference\){set-variable -name variable:global:ProgressPreference -value 'SilentlyContinue'};\. c:/Windows/Temp/packer-ps-env-vars-[[:alnum:]]{8}-[[:alnum:]]{4}-[[:alnum:]]{4}-[[:alnum:]]{4}-[[:alnum:]]{12}\.ps1; &'c:/Windows/Temp/script.ps1'; exit \$LastExitCode }"`,
+		},
+		{
+			SkipClean:                false,
+			LastExecutedCommandRegex: `powershell -executionpolicy bypass "& { if \(Test-Path variable:global:ProgressPreference\){set-variable -name variable:global:ProgressPreference -value 'SilentlyContinue'};\. c:/Windows/Temp/packer-ps-env-vars-[[:alnum:]]{8}-[[:alnum:]]{4}-[[:alnum:]]{4}-[[:alnum:]]{4}-[[:alnum:]]{12}\.ps1; &'c:/Windows/Temp/packer-cleanup-[[:alnum:]]{8}-[[:alnum:]]{4}-[[:alnum:]]{4}-[[:alnum:]]{4}-[[:alnum:]]{12}\.ps1'; exit \$LastExitCode }"`,
+		},
+	}
+
+	for _, tc := range tt {
+		tc := tc
+		p := new(Provisioner)
+		ui := testUi()
+		comm := new(packer.MockCommunicator)
+
+		config["skip_clean"] = tc.SkipClean
+		if err := p.Prepare(config); err != nil {
+			t.Fatalf("failed to prepare config when SkipClean is %t: %s", tc.SkipClean, err)
+		}
+		err := p.Provision(context.Background(), ui, comm, make(map[string]interface{}))
+		if err != nil {
+			t.Fatal("should not have error")
+		}
+
+		// When SkipClean is false the last executed command should be the clean up command;
+		// otherwise it will be the execution command for the provisioning script.
+		cmd := comm.StartCmd.Command
+		re := regexp.MustCompile(tc.LastExecutedCommandRegex)
+		matched := re.MatchString(cmd)
+		if !matched {
+			t.Fatalf(`Got unexpected command when SkipClean is %t: %s`, tc.SkipClean, cmd)
+		}
+	}
 }
 
 func TestProvisionerProvision_UploadFails(t *testing.T) {
@@ -771,3 +805,16 @@ func TestCancel(t *testing.T) {
 	// Don't actually call Cancel() as it performs an os.Exit(0)
 	// which kills the 'go test' tool
 }
+
+func testConfig() map[string]interface{} {
+	return map[string]interface{}{
+		"inline": []interface{}{"foo", "bar"},
+	}
+}
+
+func testConfigWithSkipClean() map[string]interface{} {
+	return map[string]interface{}{
+		"inline":     []interface{}{"foo", "bar"},
+		"skip_clean": true,
+	}
+}