Merge pull request #10143 from hashicorp/do_9951

builder/vsphere: skip iso download if hashed file is already present on remote datastore

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) {
+}

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)
+			}
+		}
+	}
+}

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
 	}

builder/vsphere/driver/datastore_mock.go

@@ -7,6 +7,10 @@ import (
 
 type DatastoreMock struct {
 	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 {
+	if ds.NameReturn == "" {
 		return "datastore-mock"
+	}
+	return ds.NameReturn
 }
 
 func (ds *DatastoreMock) Reference() types.ManagedObjectReference {

builder/vsphere/iso/builder.go

@@ -40,7 +40,8 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack
 		&common.StepConnect{
 			Config: &b.config.ConnectConfig,
 		},
-		&packerCommon.StepDownload{
+		&common.StepDownload{
+			DownloadStep: &packerCommon.StepDownload{
 				Checksum:    b.config.ISOChecksum,
 				Description: "ISO",
 				Extension:   b.config.TargetExtension,
@@ -48,6 +49,11 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack
 				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,
 			Label: b.config.CDConfig.CDLabel,

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)