From 97a791389a3dc2406e20e1e6e50449b59d659b66 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Fri, 11 Jan 2019 15:06:36 -0800 Subject: [PATCH] deduplicate step_output_dir and move to common folder --- builder/hyperv/common/step_output_dir_test.go | 146 ------------------ builder/hyperv/iso/builder.go | 2 +- builder/hyperv/vmcx/builder.go | 2 +- builder/virtualbox/common/step_output_dir.go | 86 ----------- builder/virtualbox/iso/builder.go | 2 +- builder/virtualbox/ovf/builder.go | 2 +- .../common => common}/step_output_dir.go | 0 .../common => common}/step_output_dir_test.go | 11 ++ 8 files changed, 15 insertions(+), 236 deletions(-) delete mode 100644 builder/hyperv/common/step_output_dir_test.go delete mode 100644 builder/virtualbox/common/step_output_dir.go rename {builder/hyperv/common => common}/step_output_dir.go (100%) rename {builder/virtualbox/common => common}/step_output_dir_test.go (91%) diff --git a/builder/hyperv/common/step_output_dir_test.go b/builder/hyperv/common/step_output_dir_test.go deleted file mode 100644 index 5933a5fe9..000000000 --- a/builder/hyperv/common/step_output_dir_test.go +++ /dev/null @@ -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) - } -} diff --git a/builder/hyperv/iso/builder.go b/builder/hyperv/iso/builder.go index 7fccf0634..08a010268 100644 --- a/builder/hyperv/iso/builder.go +++ b/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, }, diff --git a/builder/hyperv/vmcx/builder.go b/builder/hyperv/vmcx/builder.go index d3eb5f261..13b733ba2 100644 --- a/builder/hyperv/vmcx/builder.go +++ b/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, }, diff --git a/builder/virtualbox/common/step_output_dir.go b/builder/virtualbox/common/step_output_dir.go deleted file mode 100644 index fc39d08b2..000000000 --- a/builder/virtualbox/common/step_output_dir.go +++ /dev/null @@ -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) - } - } -} diff --git a/builder/virtualbox/iso/builder.go b/builder/virtualbox/iso/builder.go index 603a36aa8..55aa602a9 100644 --- a/builder/virtualbox/iso/builder.go +++ b/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, }, diff --git a/builder/virtualbox/ovf/builder.go b/builder/virtualbox/ovf/builder.go index c87a16cb0..60142728f 100644 --- a/builder/virtualbox/ovf/builder.go +++ b/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, }, diff --git a/builder/hyperv/common/step_output_dir.go b/common/step_output_dir.go similarity index 100% rename from builder/hyperv/common/step_output_dir.go rename to common/step_output_dir.go diff --git a/builder/virtualbox/common/step_output_dir_test.go b/common/step_output_dir_test.go similarity index 91% rename from builder/virtualbox/common/step_output_dir_test.go rename to common/step_output_dir_test.go index 766154ca9..197977151 100644 --- a/builder/virtualbox/common/step_output_dir_test.go +++ b/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 {