From 796c40f89b4611b59ab2c96f2e8270105436934a Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Tue, 20 Oct 2020 14:49:27 -0700 Subject: [PATCH] builder/vsphere: skip iso download if hashed file is already present in remote packer_cache --- builder/vsphere/common/step_download.go | 68 ++++++++++++++++ builder/vsphere/common/step_download_test.go | 85 ++++++++++++++++++++ builder/vsphere/common/step_remote_upload.go | 20 +++-- builder/vsphere/driver/datastore_mock.go | 13 ++- builder/vsphere/iso/builder.go | 20 +++-- common/step_download.go | 13 ++- 6 files changed, 199 insertions(+), 20 deletions(-) create mode 100644 builder/vsphere/common/step_download.go create mode 100644 builder/vsphere/common/step_download_test.go diff --git a/builder/vsphere/common/step_download.go b/builder/vsphere/common/step_download.go new file mode 100644 index 000000000..25b980356 --- /dev/null +++ b/builder/vsphere/common/step_download.go @@ -0,0 +1,68 @@ +package common + +import ( + "context" + "fmt" + "net/url" + + "github.com/hashicorp/packer/builder/vsphere/driver" + "github.com/hashicorp/packer/helper/multistep" + "github.com/hashicorp/packer/packer" +) + +// Defining this interface ensures that we use the common step download, or the +// mock created to test this wrapper +type DownloadStep interface { + Run(context.Context, multistep.StateBag) multistep.StepAction + Cleanup(multistep.StateBag) + UseSourceToFindCacheTarget(source string) (*url.URL, string, error) +} + +// VSphere has a specialized need -- before we waste time downloading an iso, +// we need to check whether that iso already exists on the remote datastore. +// if it does, we skip the download. This wrapping-step still uses the common +// StepDownload but only if the image isn't already present on the datastore. +type StepDownload struct { + DownloadStep DownloadStep + // These keys are VSphere-specific and used to check the remote datastore. + Url []string + ResultKey string + Datastore string + Host string +} + +func (s *StepDownload) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction { + driver := state.Get("driver").(driver.Driver) + ui := state.Get("ui").(packer.Ui) + + // Check whether iso is present on remote datastore. + ds, err := driver.FindDatastore(s.Datastore, s.Host) + if err != nil { + state.Put("error", fmt.Errorf("datastore doesn't exist: %v", err)) + return multistep.ActionHalt + } + + // loop over URLs to see if any are already present. If they are, store that + // one instate and continue + for _, source := range s.Url { + _, targetPath, err := s.DownloadStep.UseSourceToFindCacheTarget(source) + if err != nil { + state.Put("error", fmt.Errorf("Error getting target path: %s", err)) + return multistep.ActionHalt + } + _, remotePath, _, _ := GetRemoteDirectoryAndPath(targetPath, ds) + + if exists := ds.FileExists(remotePath); exists { + ui.Say(fmt.Sprintf("File %s already uploaded; continuing", targetPath)) + state.Put(s.ResultKey, targetPath) + return multistep.ActionContinue + } + } + + // ISO is not present on datastore, so we need to download, then upload it. + // Pass through to the common download step. + return s.DownloadStep.Run(ctx, state) +} + +func (s *StepDownload) Cleanup(state multistep.StateBag) { +} diff --git a/builder/vsphere/common/step_download_test.go b/builder/vsphere/common/step_download_test.go new file mode 100644 index 000000000..631006655 --- /dev/null +++ b/builder/vsphere/common/step_download_test.go @@ -0,0 +1,85 @@ +package common + +import ( + "context" + "net/url" + "testing" + + "github.com/hashicorp/packer/builder/vsphere/driver" + "github.com/hashicorp/packer/helper/multistep" +) + +/// create mock step +type MockDownloadStep struct { + RunCalled bool +} + +func (s *MockDownloadStep) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction { + s.RunCalled = true + return multistep.ActionContinue +} + +func (s *MockDownloadStep) Cleanup(state multistep.StateBag) {} + +func (s *MockDownloadStep) UseSourceToFindCacheTarget(source string) (*url.URL, string, error) { + return nil, "sometarget", nil +} + +/// start tests +func downloadStepState(exists bool) *multistep.BasicStateBag { + state := basicStateBag(nil) + dsMock := &driver.DatastoreMock{ + FileExistsReturn: exists, + } + driverMock := &driver.DriverMock{ + DatastoreMock: dsMock, + } + state.Put("driver", driverMock) + return state +} + +func TestStepDownload_Run(t *testing.T) { + testcases := []struct { + name string + filePresent bool + expectedAction multistep.StepAction + expectInternalStepCalled bool + errMessage string + }{ + { + name: "Remote iso present; download shouldn't be called", + filePresent: true, + expectedAction: multistep.ActionContinue, + expectInternalStepCalled: false, + errMessage: "", + }, + { + name: "Remote iso not present; download should be called", + filePresent: false, + expectedAction: multistep.ActionContinue, + expectInternalStepCalled: true, + errMessage: "", + }, + } + for _, tc := range testcases { + internalStep := &MockDownloadStep{} + state := downloadStepState(tc.filePresent) + step := &StepDownload{ + DownloadStep: internalStep, + Url: []string{"https://path/to/fake-url.iso"}, + Datastore: "datastore-mock", + Host: "fake-host", + } + stepAction := step.Run(context.TODO(), state) + if stepAction != tc.expectedAction { + t.Fatalf("%s: Recieved wrong step action; step exists, should return early.", tc.name) + } + if tc.expectInternalStepCalled != internalStep.RunCalled { + if tc.expectInternalStepCalled { + t.Fatalf("%s: Expected internal download step to be called", tc.name) + } else { + t.Fatalf("%s: Expected internal download step not to be called", tc.name) + } + } + } +} diff --git a/builder/vsphere/common/step_remote_upload.go b/builder/vsphere/common/step_remote_upload.go index bfabaeb67..522ae5a96 100644 --- a/builder/vsphere/common/step_remote_upload.go +++ b/builder/vsphere/common/step_remote_upload.go @@ -42,24 +42,30 @@ func (s *StepRemoteUpload) Run(_ context.Context, state multistep.StateBag) mult return multistep.ActionContinue } +func GetRemoteDirectoryAndPath(path string, ds driver.Datastore) (string, string, string, string) { + filename := filepath.Base(path) + remotePath := fmt.Sprintf("packer_cache/%s", filename) + remoteDirectory := fmt.Sprintf("[%s] packer_cache/", ds.Name()) + fullRemotePath := fmt.Sprintf("%s/%s", remoteDirectory, filename) + + return filename, remotePath, remoteDirectory, fullRemotePath + +} func (s *StepRemoteUpload) uploadFile(path string, d driver.Driver, ui packer.Ui) (string, error) { ds, err := d.FindDatastore(s.Datastore, s.Host) if err != nil { return "", fmt.Errorf("datastore doesn't exist: %v", err) } - filename := filepath.Base(path) - remotePath := fmt.Sprintf("packer_cache/%s", filename) - remoteDirectory := fmt.Sprintf("[%s] packer_cache/", ds.Name()) - fullRemotePath := fmt.Sprintf("%s/%s", remoteDirectory, filename) - - ui.Say(fmt.Sprintf("Uploading %s to %s", filename, remotePath)) + filename, remotePath, remoteDirectory, fullRemotePath := GetRemoteDirectoryAndPath(path, ds) if exists := ds.FileExists(remotePath); exists == true { - ui.Say(fmt.Sprintf("File %s already uploaded; continuing", filename)) + ui.Say(fmt.Sprintf("File %s already exists; skipping upload.", fullRemotePath)) return fullRemotePath, nil } + ui.Say(fmt.Sprintf("Uploading %s to %s", filename, remotePath)) + if err := ds.MakeDirectory(remoteDirectory); err != nil { return "", err } diff --git a/builder/vsphere/driver/datastore_mock.go b/builder/vsphere/driver/datastore_mock.go index b85c95f5b..889dd254a 100644 --- a/builder/vsphere/driver/datastore_mock.go +++ b/builder/vsphere/driver/datastore_mock.go @@ -6,7 +6,11 @@ import ( ) type DatastoreMock struct { - FileExistsCalled bool + FileExistsCalled bool + FileExistsReturn bool + + NameReturn string + MakeDirectoryCalled bool ResolvePathCalled bool @@ -30,11 +34,14 @@ func (ds *DatastoreMock) Info(params ...string) (*mo.Datastore, error) { func (ds *DatastoreMock) FileExists(path string) bool { ds.FileExistsCalled = true - return false + return ds.FileExistsReturn } func (ds *DatastoreMock) Name() string { - return "datastore-mock" + if ds.NameReturn == "" { + return "datastore-mock" + } + return ds.NameReturn } func (ds *DatastoreMock) Reference() types.ManagedObjectReference { diff --git a/builder/vsphere/iso/builder.go b/builder/vsphere/iso/builder.go index e3fc9ec46..a7f6f29cb 100644 --- a/builder/vsphere/iso/builder.go +++ b/builder/vsphere/iso/builder.go @@ -40,13 +40,19 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack &common.StepConnect{ Config: &b.config.ConnectConfig, }, - &packerCommon.StepDownload{ - Checksum: b.config.ISOChecksum, - Description: "ISO", - Extension: b.config.TargetExtension, - ResultKey: "iso_path", - TargetPath: b.config.TargetPath, - Url: b.config.ISOUrls, + &common.StepDownload{ + DownloadStep: &packerCommon.StepDownload{ + Checksum: b.config.ISOChecksum, + Description: "ISO", + Extension: b.config.TargetExtension, + ResultKey: "iso_path", + TargetPath: b.config.TargetPath, + Url: b.config.ISOUrls, + }, + Url: b.config.ISOUrls, + ResultKey: "iso_path", + Datastore: b.config.Datastore, + Host: b.config.Host, }, &packerCommon.StepCreateCD{ Files: b.config.CDConfig.CDFiles, diff --git a/common/step_download.go b/common/step_download.go index 48e93baf0..f2f0ca177 100644 --- a/common/step_download.go +++ b/common/step_download.go @@ -108,10 +108,10 @@ func (s *StepDownload) Run(ctx context.Context, state multistep.StateBag) multis return multistep.ActionHalt } -func (s *StepDownload) download(ctx context.Context, ui packer.Ui, source string) (string, error) { +func (s *StepDownload) UseSourceToFindCacheTarget(source string) (*url.URL, string, error) { u, err := parseSourceURL(source) if err != nil { - return "", fmt.Errorf("url parse: %s", err) + return nil, "", fmt.Errorf("url parse: %s", err) } if checksum := u.Query().Get("checksum"); checksum != "" { s.Checksum = checksum @@ -142,7 +142,7 @@ func (s *StepDownload) download(ctx context.Context, ui packer.Ui, source string } targetPath, err = packer.CachePath(targetPath) if err != nil { - return "", fmt.Errorf("CachePath: %s", err) + return nil, "", fmt.Errorf("CachePath: %s", err) } } else if filepath.Ext(targetPath) == "" { // When an absolute path is provided @@ -157,7 +157,14 @@ func (s *StepDownload) download(ctx context.Context, ui packer.Ui, source string targetPath += ".iso" } } + return u, targetPath, nil +} +func (s *StepDownload) download(ctx context.Context, ui packer.Ui, source string) (string, error) { + u, targetPath, err := s.UseSourceToFindCacheTarget(source) + if err != nil { + return "", err + } lockFile := targetPath + ".lock" log.Printf("Acquiring lock for: %s (%s)", u.String(), lockFile)