Merge pull request #7188 from hashicorp/dedupe_step_output_dir

deduplicate step_output_dir and move to common folder

builder/hyperv/common/step_output_dir_test.go

@@ -1,146 +0,0 @@
-package common
-
-import (
-	"context"
-	"os"
-	"testing"
-
-	"github.com/hashicorp/packer/helper/multistep"
-)
-
-func TestStepOutputDir_imp(t *testing.T) {
-	var _ multistep.Step = new(StepOutputDir)
-}
-
-func TestStepOutputDir_Default(t *testing.T) {
-	state := testState(t)
-	step := new(StepOutputDir)
-
-	step.Path = genTestDirPath("packerHypervOutput")
-
-	// Test the run
-	if action := step.Run(context.Background(), state); action != multistep.ActionContinue {
-		t.Fatalf("Bad action: %v", action)
-	}
-	if _, ok := state.GetOk("error"); ok {
-		t.Fatal("Should NOT have error")
-	}
-
-	// The directory should have been created
-	if _, err := os.Stat(step.Path); err != nil {
-		t.Fatal("Should have created output directory")
-	}
-
-	// Remove the directory created due to test
-	err := os.RemoveAll(step.Path)
-	if err != nil {
-		t.Fatalf("Error encountered removing directory created by test: %s", err)
-	}
-}
-
-func TestStepOutputDir_DirectoryAlreadyExistsNoForce(t *testing.T) {
-	state := testState(t)
-	step := new(StepOutputDir)
-
-	step.Path = genTestDirPath("packerHypervOutput")
-
-	// Create the directory so that we can test
-	err := os.Mkdir(step.Path, 0755)
-	if err != nil {
-		t.Fatal("Test failed to create directory for test of Cancel and Cleanup")
-	}
-	defer os.RemoveAll(step.Path) // Ensure we clean up if something goes wrong
-
-	step.Force = false // Default
-	// Test the run
-	if action := step.Run(context.Background(), state); action != multistep.ActionHalt {
-		t.Fatalf("Should halt when directory exists and 'Force' is false. Bad action: %v", action)
-	}
-	if _, ok := state.GetOk("error"); !ok {
-		t.Fatal("Should error when directory exists and 'Force' is false")
-	}
-}
-
-func TestStepOutputDir_DirectoryAlreadyExistsForce(t *testing.T) {
-	state := testState(t)
-	step := new(StepOutputDir)
-
-	step.Path = genTestDirPath("packerHypervOutput")
-
-	// Create the directory so that we can test
-	err := os.Mkdir(step.Path, 0755)
-	if err != nil {
-		t.Fatal("Test failed to create directory for test of Cancel and Cleanup")
-	}
-	defer os.RemoveAll(step.Path) // Ensure we clean up if something goes wrong
-
-	step.Force = true // User specified that existing directory and contents should be discarded
-	if action := step.Run(context.Background(), state); action != multistep.ActionContinue {
-		t.Fatalf("Should complete when directory exists and 'Force' is true. Bad action: %v", action)
-	}
-	if _, ok := state.GetOk("error"); ok {
-		t.Fatalf("Should NOT error when directory exists and 'Force' is true: %s", err)
-	}
-}
-
-func TestStepOutputDir_CleanupBuildCancelled(t *testing.T) {
-	state := testState(t)
-	step := new(StepOutputDir)
-
-	step.Path = genTestDirPath("packerHypervOutput")
-
-	// Create the directory so that we can test the cleanup
-	err := os.Mkdir(step.Path, 0755)
-	if err != nil {
-		t.Fatal("Test failed to create directory for test of Cancel and Cleanup")
-	}
-	defer os.RemoveAll(step.Path) // Ensure we clean up if something goes wrong
-
-	// 'Cancel' the build
-	state.Put(multistep.StateCancelled, true)
-
-	// Ensure the directory isn't removed if the cleanup flag is false
-	step.cleanup = false
-	step.Cleanup(state)
-	if _, err := os.Stat(step.Path); err != nil {
-		t.Fatal("Output dir should NOT be removed if on 'Cancel' if cleanup flag is unset/false")
-	}
-
-	// Ensure the directory is removed if the cleanup flag is true
-	step.cleanup = true
-	step.Cleanup(state)
-	if _, err := os.Stat(step.Path); err == nil {
-		t.Fatalf("Output directory should NOT exist after 'Cancel' and Cleanup: %s", step.Path)
-	}
-}
-
-func TestStepOutputDir_CleanupBuildHalted(t *testing.T) {
-	state := testState(t)
-	step := new(StepOutputDir)
-
-	step.Path = genTestDirPath("packerHypervOutput")
-
-	// Create the directory so that we can test the cleanup
-	err := os.Mkdir(step.Path, 0755)
-	if err != nil {
-		t.Fatal("Test failed to create directory for test of Cancel and Cleanup")
-	}
-	defer os.RemoveAll(step.Path) // Ensure we clean up if something goes wrong
-
-	// 'Halt' the build and test the directory is removed
-	state.Put(multistep.StateHalted, true)
-
-	// Ensure the directory isn't removed if the cleanup flag is false
-	step.cleanup = false
-	step.Cleanup(state)
-	if _, err := os.Stat(step.Path); err != nil {
-		t.Fatal("Output dir should NOT be removed if on 'Halt' if cleanup flag is unset/false")
-	}
-
-	// Ensure the directory is removed if the cleanup flag is true
-	step.cleanup = true
-	step.Cleanup(state)
-	if _, err := os.Stat(step.Path); err == nil {
-		t.Fatalf("Output directory should NOT exist after 'Halt' and Cleanup: %s", step.Path)
-	}
-}

builder/hyperv/iso/builder.go

@@ -376,7 +376,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 		&hypervcommon.StepCreateBuildDir{
 			TempPath: b.config.TempPath,
 		},
-		&hypervcommon.StepOutputDir{
+		&common.StepOutputDir{
 			Force: b.config.PackerForce,
 			Path:  b.config.OutputDir,
 		},

builder/hyperv/vmcx/builder.go

@@ -392,7 +392,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 		&hypervcommon.StepCreateBuildDir{
 			TempPath: b.config.TempPath,
 		},
-		&hypervcommon.StepOutputDir{
+		&common.StepOutputDir{
 			Force: b.config.PackerForce,
 			Path:  b.config.OutputDir,
 		},

builder/virtualbox/common/step_output_dir.go

@@ -1,86 +0,0 @@
-package common
-
-import (
-	"context"
-	"fmt"
-	"log"
-	"os"
-	"path/filepath"
-	"time"
-
-	"github.com/hashicorp/packer/helper/multistep"
-	"github.com/hashicorp/packer/packer"
-)
-
-// StepOutputDir sets up the output directory by creating it if it does
-// not exist, deleting it if it does exist and we're forcing, and cleaning
-// it up when we're done with it.
-type StepOutputDir struct {
-	Force bool
-	Path  string
-
-	cleanup bool
-}
-
-func (s *StepOutputDir) Run(_ context.Context, state multistep.StateBag) multistep.StepAction {
-	ui := state.Get("ui").(packer.Ui)
-
-	if _, err := os.Stat(s.Path); err == nil {
-		if !s.Force {
-			err := fmt.Errorf(
-				"Output directory exists: %s\n\n"+
-					"Use the force flag to delete it prior to building.",
-				s.Path)
-			state.Put("error", err)
-			return multistep.ActionHalt
-		}
-
-		ui.Say("Deleting previous output directory...")
-		os.RemoveAll(s.Path)
-	}
-
-	// Enable cleanup
-	s.cleanup = true
-
-	// Create the directory
-	if err := os.MkdirAll(s.Path, 0755); err != nil {
-		state.Put("error", err)
-		return multistep.ActionHalt
-	}
-
-	// Make sure we can write in the directory
-	f, err := os.Create(filepath.Join(s.Path, "_packer_perm_check"))
-	if err != nil {
-		err = fmt.Errorf("Couldn't write to output directory: %s", err)
-		state.Put("error", err)
-		return multistep.ActionHalt
-	}
-	f.Close()
-	os.Remove(f.Name())
-
-	return multistep.ActionContinue
-}
-
-func (s *StepOutputDir) Cleanup(state multistep.StateBag) {
-	if !s.cleanup {
-		return
-	}
-
-	_, cancelled := state.GetOk(multistep.StateCancelled)
-	_, halted := state.GetOk(multistep.StateHalted)
-
-	if cancelled || halted {
-		ui := state.Get("ui").(packer.Ui)
-
-		ui.Say("Deleting output directory...")
-		for i := 0; i < 5; i++ {
-			err := os.RemoveAll(s.Path)
-			if err == nil {
-				break
-			}
-
-			log.Printf("Error removing output dir: %s", err)
-			time.Sleep(2 * time.Second)
-		}
-	}
-}

builder/virtualbox/iso/builder.go

@@ -209,7 +209,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 			TargetPath:   b.config.TargetPath,
 			Url:          b.config.ISOUrls,
 		},
-		&vboxcommon.StepOutputDir{
+		&common.StepOutputDir{
 			Force: b.config.PackerForce,
 			Path:  b.config.OutputDir,
 		},

builder/virtualbox/ovf/builder.go

@@ -50,7 +50,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 
 	// Build the steps.
 	steps := []multistep.Step{
-		&vboxcommon.StepOutputDir{
+		&common.StepOutputDir{
 			Force: b.config.PackerForce,
 			Path:  b.config.OutputDir,
 		},

common/step_output_dir_test.go

@@ -1,14 +1,25 @@
 package common
 
 import (
+	"bytes"
 	"context"
 	"io/ioutil"
 	"os"
 	"testing"
 
 	"github.com/hashicorp/packer/helper/multistep"
+	"github.com/hashicorp/packer/packer"
 )
 
+func testState(t *testing.T) multistep.StateBag {
+	state := new(multistep.BasicStateBag)
+	state.Put("ui", &packer.BasicUi{
+		Reader: new(bytes.Buffer),
+		Writer: new(bytes.Buffer),
+	})
+	return state
+}
+
 func testStepOutputDir(t *testing.T) *StepOutputDir {
 	td, err := ioutil.TempDir("", "packer")
 	if err != nil {