From fa12113eaf07483c8c3ae2640b60e8389e0ae536 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Thu, 25 Oct 2018 15:07:07 -0700 Subject: [PATCH] remove unused NewLocalArtifact consolidate artifacts entirely remove local artifact object and get artifacts in the vmx builder the same way we do in the vmware iso builder --- builder/vmware/common/artifact.go | 92 +++++++++---------- builder/vmware/common/artifact_test.go | 36 -------- builder/vmware/common/output_dir.go | 1 + builder/vmware/common/remote_artifact.go | 39 -------- builder/vmware/common/remote_artifact_test.go | 15 --- builder/vmware/common/step_export.go | 4 +- builder/vmware/iso/builder.go | 23 +---- builder/vmware/vmx/builder.go | 22 +---- 8 files changed, 51 insertions(+), 181 deletions(-) delete mode 100644 builder/vmware/common/remote_artifact.go delete mode 100644 builder/vmware/common/remote_artifact_test.go diff --git a/builder/vmware/common/artifact.go b/builder/vmware/common/artifact.go index bab8478ae..729dc0e17 100644 --- a/builder/vmware/common/artifact.go +++ b/builder/vmware/common/artifact.go @@ -2,17 +2,17 @@ package common import ( "fmt" - "os" - "path/filepath" + "strconv" + "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" ) -// BuilderId for the local artifacts -const BuilderId = "mitchellh.vmware" -const BuilderIdESX = "mitchellh.vmware-esx" - const ( + // BuilderId for the local artifacts + BuilderId = "mitchellh.vmware" + BuilderIdESX = "mitchellh.vmware-esx" + ArtifactConfFormat = "artifact.conf.format" ArtifactConfKeepRegistered = "artifact.conf.keep_registered" ArtifactConfSkipExport = "artifact.conf.skip_export" @@ -23,52 +23,11 @@ const ( type artifact struct { builderId string id string - dir string + dir OutputDir f []string config map[string]string } -// NewLocalArtifact returns a VMware artifact containing the files -// in the given directory. -// NewLocalArtifact returns a VMware artifact containing the files -// in the given directory. -func NewLocalArtifact(id string, dir string) (packer.Artifact, error) { - files := make([]string, 0, 5) - visit := func(path string, info os.FileInfo, err error) error { - if err != nil { - return err - } - if !info.IsDir() { - files = append(files, path) - } - return nil - } - - if err := filepath.Walk(dir, visit); err != nil { - return nil, err - } - - return &artifact{ - builderId: id, - dir: dir, - f: files, - }, nil -} - -func NewArtifact(vmname string, dir OutputDir, files []string, config map[string]string, esxi bool) (packer.Artifact, error) { - builderID := BuilderId - if esxi { - builderID = BuilderIdESX - } - - return &artifact{ - builderId: builderID, - id: vmname, - dir: dir.String(), - f: files, - }, nil -} - func (a *artifact) BuilderId() string { return a.builderId } @@ -90,5 +49,40 @@ func (a *artifact) State(name string) interface{} { } func (a *artifact) Destroy() error { - return os.RemoveAll(a.dir) + return a.dir.RemoveAll() +} + +func NewArtifact(remoteType string, format string, exportOutputPath string, vmName string, skipExport bool, keepRegistered bool, state multistep.StateBag) (packer.Artifact, error) { + var files []string + var dir OutputDir + var err error + if remoteType != "" && format != "" && !skipExport { + dir = new(LocalOutputDir) + dir.SetOutputDir(exportOutputPath) + files, err = dir.ListFiles() + } else { + files, err = state.Get("dir").(OutputDir).ListFiles() + } + if err != nil { + return nil, err + } + + // Set the proper builder ID + builderId := BuilderId + if remoteType != "" { + builderId = BuilderIdESX + } + + config := make(map[string]string) + config[ArtifactConfKeepRegistered] = strconv.FormatBool(keepRegistered) + config[ArtifactConfFormat] = format + config[ArtifactConfSkipExport] = strconv.FormatBool(skipExport) + + return &artifact{ + builderId: builderId, + id: vmName, + dir: dir, + f: files, + config: config, + }, nil } diff --git a/builder/vmware/common/artifact_test.go b/builder/vmware/common/artifact_test.go index c2bbac5dc..dd2c64b74 100644 --- a/builder/vmware/common/artifact_test.go +++ b/builder/vmware/common/artifact_test.go @@ -1,9 +1,6 @@ package common import ( - "io/ioutil" - "os" - "path/filepath" "testing" "github.com/hashicorp/packer/packer" @@ -12,36 +9,3 @@ import ( func TestLocalArtifact_impl(t *testing.T) { var _ packer.Artifact = new(artifact) } - -func TestNewLocalArtifact(t *testing.T) { - td, err := ioutil.TempDir("", "packer") - if err != nil { - t.Fatalf("err: %s", err) - } - defer os.RemoveAll(td) - - err = ioutil.WriteFile(filepath.Join(td, "a"), []byte("foo"), 0644) - if err != nil { - t.Fatalf("err: %s", err) - } - - if err := os.Mkdir(filepath.Join(td, "b"), 0755); err != nil { - t.Fatalf("err: %s", err) - } - dir := new(LocalOutputDir) - dir.SetOutputDir(td) - files, err := dir.ListFiles() - - config := make(map[string]string) - a, err := NewArtifact("vm1", dir, files, config, false) - - if a.BuilderId() != BuilderId { - t.Fatalf("bad: %#v", a.BuilderId()) - } - if a.Id() != "vm1" { - t.Fatalf("bad: %#v", a.Id()) - } - if len(a.Files()) != 1 { - t.Fatalf("should length 1: %d", len(a.Files())) - } -} diff --git a/builder/vmware/common/output_dir.go b/builder/vmware/common/output_dir.go index 8af513a19..85b690f31 100644 --- a/builder/vmware/common/output_dir.go +++ b/builder/vmware/common/output_dir.go @@ -4,6 +4,7 @@ package common // of the output directory for VMware-based products. The abstraction is made // so that the output directory can be properly made on remote (ESXi) based // VMware products as well as local. +// For remote builds, OutputDir interface is satisfied by the ESX5Driver. type OutputDir interface { DirExists() (bool, error) ListFiles() ([]string, error) diff --git a/builder/vmware/common/remote_artifact.go b/builder/vmware/common/remote_artifact.go deleted file mode 100644 index 4cc1b2a2f..000000000 --- a/builder/vmware/common/remote_artifact.go +++ /dev/null @@ -1,39 +0,0 @@ -package common - -import ( - "fmt" -) - -// Artifact is the result of running the VMware builder, namely a set -// of files associated with the resulting machine. -type RemoteArtifact struct { - builderId string - id string - dir OutputDir - f []string - config map[string]string -} - -func (a *RemoteArtifact) BuilderId() string { - return a.builderId -} - -func (a *RemoteArtifact) Files() []string { - return a.f -} - -func (a *RemoteArtifact) Id() string { - return a.id -} - -func (a *RemoteArtifact) String() string { - return fmt.Sprintf("VM files in directory: %s", a.dir) -} - -func (a *RemoteArtifact) State(name string) interface{} { - return a.config[name] -} - -func (a *RemoteArtifact) Destroy() error { - return a.dir.RemoveAll() -} diff --git a/builder/vmware/common/remote_artifact_test.go b/builder/vmware/common/remote_artifact_test.go deleted file mode 100644 index 292a87b54..000000000 --- a/builder/vmware/common/remote_artifact_test.go +++ /dev/null @@ -1,15 +0,0 @@ -package common - -import ( - "testing" - - "github.com/hashicorp/packer/packer" -) - -func TestArtifact_Impl(t *testing.T) { - var raw interface{} - raw = &RemoteArtifact{} - if _, ok := raw.(packer.Artifact); !ok { - t.Fatal("Artifact must be a proper artifact") - } -} diff --git a/builder/vmware/common/step_export.go b/builder/vmware/common/step_export.go index fb891151a..088541d0d 100644 --- a/builder/vmware/common/step_export.go +++ b/builder/vmware/common/step_export.go @@ -26,7 +26,7 @@ type StepExport struct { OutputDir string } -func (s *StepExport) generateArgs(c *DriverConfig, outputPath string, hidePassword bool) []string { +func (s *StepExport) generateArgs(c *DriverConfig, displayName string, outputPath string, hidePassword bool) []string { password := url.QueryEscape(c.RemotePassword) if hidePassword { password = "****" @@ -36,7 +36,7 @@ func (s *StepExport) generateArgs(c *DriverConfig, outputPath string, hidePasswo "--skipManifestCheck", "-tt=" + s.Format, - "vi://" + c.RemoteUser + ":" + password + "@" + c.RemoteHost + "/" + s.VMName, + "vi://" + c.RemoteUser + ":" + password + "@" + c.RemoteHost + "/" + displayName, outputPath, } return append(s.OVFToolOptions, args...) diff --git a/builder/vmware/iso/builder.go b/builder/vmware/iso/builder.go index 2b410533f..9ec662dca 100644 --- a/builder/vmware/iso/builder.go +++ b/builder/vmware/iso/builder.go @@ -6,7 +6,6 @@ import ( "io/ioutil" "log" "os" - "strconv" "time" vmwcommon "github.com/hashicorp/packer/builder/vmware/common" @@ -247,8 +246,6 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe dir = new(vmwcommon.LocalOutputDir) } - exportOutputPath := b.config.OutputDir - if b.config.RemoteType != "" { b.config.OutputDir = b.config.VMName } @@ -394,24 +391,8 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe } // Compile the artifact list - var files []string - if b.config.RemoteType != "" && b.config.Format != "" && !b.config.SkipExport { - dir = new(vmwcommon.LocalOutputDir) - dir.SetOutputDir(exportOutputPath) - files, err = dir.ListFiles() - } else { - files, err = state.Get("dir").(vmwcommon.OutputDir).ListFiles() - } - if err != nil { - return nil, err - } - - config := make(map[string]string) - config[vmwcommon.ArtifactConfKeepRegistered] = strconv.FormatBool(b.config.KeepRegistered) - config[vmwcommon.ArtifactConfFormat] = b.config.Format - config[vmwcommon.ArtifactConfSkipExport] = strconv.FormatBool(b.config.SkipExport) - - return vmwcommon.NewArtifact(b.config.VMName, dir, files, config, b.config.RemoteType != "") + return vmwcommon.NewArtifact(b.config.RemoteType, b.config.Format, b.config.OutputDir, + b.config.VMName, b.config.SkipExport, b.config.KeepRegistered, state) } func (b *Builder) Cancel() { diff --git a/builder/vmware/vmx/builder.go b/builder/vmware/vmx/builder.go index 1bd470f37..4f0a5258c 100644 --- a/builder/vmware/vmx/builder.go +++ b/builder/vmware/vmx/builder.go @@ -4,7 +4,6 @@ import ( "errors" "fmt" "log" - "strconv" "time" vmwcommon "github.com/hashicorp/packer/builder/vmware/common" @@ -180,25 +179,10 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe if _, ok := state.GetOk(multistep.StateHalted); ok { return nil, errors.New("Build was halted.") } - // Compile the artifact list - var files []string - if b.config.RemoteType != "" && b.config.Format != "" { - dir = new(vmwcommon.LocalOutputDir) - dir.SetOutputDir(b.config.OutputDir) - files, err = dir.ListFiles() - } else { - files, err = state.Get("dir").(vmwcommon.OutputDir).ListFiles() - } - if err != nil { - return nil, err - } - - config := make(map[string]string) - config[vmwcommon.ArtifactConfKeepRegistered] = strconv.FormatBool(b.config.KeepRegistered) - config[vmwcommon.ArtifactConfFormat] = b.config.Format - config[vmwcommon.ArtifactConfSkipExport] = strconv.FormatBool(b.config.SkipExport) - return vmwcommon.NewArtifact(b.config.VMName, dir, files, config, b.config.RemoteType != "") + // Artifact + return vmwcommon.NewArtifact(b.config.RemoteType, b.config.Format, b.config.OutputDir, + b.config.VMName, b.config.SkipExport, b.config.KeepRegistered, state) } // Cancel.