Fix test for vsphere vmfolder change;

sdg0919 · Feb 10, 2020 · 2dbf785 · 2dbf785
1 parent b9a3423
commit 2dbf785
Show file tree

Hide file tree

Showing 8 changed files with 62 additions and 35 deletions.
diff --git a/provider/vsphere/environ_broker_test.go b/provider/vsphere/environ_broker_test.go
@@ -351,7 +351,7 @@ func (s *environBrokerSuite) TestStartInstanceFailsWithAvailabilityZone(c *gc.C)
 	_, err := s.env.StartInstance(s.callCtx, startInstArgs)
 	c.Assert(err, gc.Not(jc.Satisfies), environs.IsAvailabilityZoneIndependent)
 
-	s.client.CheckCallNames(c, "ComputeResources", "ResourcePools", "ResourcePools", "CreateVirtualMachine", "Close")
+	s.client.CheckCallNames(c, "ComputeResources", "ResourcePools", "ResourcePools", "CreateVirtualMachine", "FindFolder", "Close")
 	createVMCall1 := s.client.Calls()[3]
 	createVMArgs1 := createVMCall1.Args[1].(vsphereclient.CreateVirtualMachineParams)
 	c.Assert(createVMArgs1.ComputeResource, jc.DeepEquals, s.client.computeResources[0])
@@ -401,12 +401,9 @@ func (s *environBrokerSuite) TestStopInstances(c *gc.C) {
 	c.Assert(err, jc.ErrorIsNil)
 
 	var paths []string
-	s.client.CheckCallNames(c, "RemoveVirtualMachines", "RemoveVirtualMachines", "Close")
-	for i := 0; i < 2; i++ {
-		args := s.client.Calls()[i].Args
-		paths = append(paths, args[1].(string))
-	}
-
+	s.client.CheckCallNames(c, "RemoveVirtualMachines", "FindFolder", "RemoveVirtualMachines", "FindFolder", "Close")
+	paths = append(paths, s.client.Calls()[0].Args[1].(string))
+	paths = append(paths, s.client.Calls()[2].Args[1].(string))
 	// NOTE(axw) we must use SameContents, not DeepEquals, because
 	// we run the RemoveVirtualMachines calls concurrently.
 	c.Assert(paths, jc.SameContents, []string{
@@ -419,18 +416,18 @@ func (s *environBrokerSuite) TestStopInstancesOneFailure(c *gc.C) {
 	s.client.SetErrors(errors.New("bah"))
 	err := s.env.StopInstances(s.callCtx, "vm-0", "vm-1")
 
-	s.client.CheckCallNames(c, "RemoveVirtualMachines", "RemoveVirtualMachines", "Close")
+	s.client.CheckCallNames(c, "RemoveVirtualMachines", "FindFolder", "RemoveVirtualMachines", "FindFolder", "Close")
 	vmName := path.Base(s.client.Calls()[0].Args[1].(string))
 	c.Assert(err, gc.ErrorMatches, fmt.Sprintf("failed to stop instance %s: bah", vmName))
 }
 
 func (s *environBrokerSuite) TestStopInstancesMultipleFailures(c *gc.C) {
 	err1 := errors.New("bah")
 	err2 := errors.New("bleh")
-	s.client.SetErrors(err1, err2)
+	s.client.SetErrors(err1, nil, err2, nil)
 	err := s.env.StopInstances(s.callCtx, "vm-0", "vm-1")
 
-	s.client.CheckCallNames(c, "RemoveVirtualMachines", "RemoveVirtualMachines", "Close")
+	s.client.CheckCallNames(c, "RemoveVirtualMachines", "FindFolder", "RemoveVirtualMachines", "FindFolder", "Close")
 	vmName1 := path.Base(s.client.Calls()[0].Args[1].(string))
 	if vmName1 == "vm-1" {
 		err1, err2 = err2, err1

diff --git a/provider/vsphere/environ_test.go b/provider/vsphere/environ_test.go
@@ -43,11 +43,11 @@ func (s *environSuite) TestBootstrap(c *gc.C) {
 	// We dial a connection before calling calling Bootstrap,
 	// in order to create the VM folder.
 	s.dialStub.CheckCallNames(c, "Dial")
-	s.client.CheckCallNames(c, "EnsureVMFolder", "Close")
+	s.client.CheckCallNames(c, "EnsureVMFolder", "FindFolder", "Close")
 	ensureVMFolderCall := s.client.Calls()[0]
-	c.Assert(ensureVMFolderCall.Args, gc.HasLen, 2)
+	c.Assert(ensureVMFolderCall.Args, gc.HasLen, 3)
 	c.Assert(ensureVMFolderCall.Args[0], gc.Implements, new(context.Context))
-	c.Assert(ensureVMFolderCall.Args[1], gc.Equals,
+	c.Assert(ensureVMFolderCall.Args[2], gc.Equals,
 		`Juju Controller (deadbeef-1bad-500d-9000-4b1d0d06f00d)/Model "testmodel" (2d02eeac-9dbb-11e4-89d3-123b93f75cba)`,
 	)
 }
@@ -62,7 +62,7 @@ func (s *environSuite) TestDestroy(c *gc.C) {
 	err := s.env.Destroy(s.callCtx)
 	c.Assert(err, jc.ErrorIsNil)
 	c.Assert(destroyCalled, jc.IsTrue)
-	s.client.CheckCallNames(c, "DestroyVMFolder", "Close")
+	s.client.CheckCallNames(c, "DestroyVMFolder", "FindFolder", "Close")
 	destroyVMFolderCall := s.client.Calls()[0]
 	c.Assert(destroyVMFolderCall.Args, gc.HasLen, 2)
 	c.Assert(destroyVMFolderCall.Args[0], gc.Implements, new(context.Context))
@@ -98,7 +98,7 @@ func (s *environSuite) TestDestroyController(c *gc.C) {
 
 	s.dialStub.CheckCallNames(c, "Dial")
 	s.client.CheckCallNames(c,
-		"DestroyVMFolder", "RemoveVirtualMachines", "DestroyVMFolder",
+		"DestroyVMFolder", "FindFolder", "RemoveVirtualMachines", "DestroyVMFolder",
 		"Datastores", "DeleteDatastoreFile", "DeleteDatastoreFile",
 		"Close",
 	)
@@ -110,24 +110,24 @@ func (s *environSuite) TestDestroyController(c *gc.C) {
 		`Juju Controller (*)/Model "testmodel" (2d02eeac-9dbb-11e4-89d3-123b93f75cba)`,
 	)
 
-	removeVirtualMachinesCall := s.client.Calls()[1]
+	removeVirtualMachinesCall := s.client.Calls()[2]
 	c.Assert(removeVirtualMachinesCall.Args, gc.HasLen, 2)
 	c.Assert(removeVirtualMachinesCall.Args[0], gc.Implements, new(context.Context))
 	c.Assert(removeVirtualMachinesCall.Args[1], gc.Equals,
 		`Juju Controller (foo)/Model "*" (*)/*`,
 	)
 
-	destroyControllerVMFolderCall := s.client.Calls()[2]
+	destroyControllerVMFolderCall := s.client.Calls()[3]
 	c.Assert(destroyControllerVMFolderCall.Args, gc.HasLen, 2)
 	c.Assert(destroyControllerVMFolderCall.Args[0], gc.Implements, new(context.Context))
 	c.Assert(destroyControllerVMFolderCall.Args[1], gc.Equals, `Juju Controller (foo)`)
 
-	deleteDatastoreFileCall1 := s.client.Calls()[4]
+	deleteDatastoreFileCall1 := s.client.Calls()[5]
 	c.Assert(deleteDatastoreFileCall1.Args, gc.HasLen, 2)
 	c.Assert(deleteDatastoreFileCall1.Args[0], gc.Implements, new(context.Context))
 	c.Assert(deleteDatastoreFileCall1.Args[1], gc.Equals, "[bar] juju-vmdks/foo")
 
-	deleteDatastoreFileCall2 := s.client.Calls()[5]
+	deleteDatastoreFileCall2 := s.client.Calls()[6]
 	c.Assert(deleteDatastoreFileCall2.Args, gc.HasLen, 2)
 	c.Assert(deleteDatastoreFileCall2.Args[0], gc.Implements, new(context.Context))
 	c.Assert(deleteDatastoreFileCall2.Args[1], gc.Equals, "[baz] juju-vmdks/foo")
@@ -138,7 +138,7 @@ func (s *environSuite) TestAdoptResources(c *gc.C) {
 	c.Assert(err, jc.ErrorIsNil)
 
 	s.dialStub.CheckCallNames(c, "Dial")
-	s.client.CheckCallNames(c, "MoveVMFolderInto", "Close")
+	s.client.CheckCallNames(c, "MoveVMFolderInto", "FindFolder", "Close")
 	moveVMFolderIntoCall := s.client.Calls()[0]
 	c.Assert(moveVMFolderIntoCall.Args, gc.HasLen, 3)
 	c.Assert(moveVMFolderIntoCall.Args[0], gc.Implements, new(context.Context))

diff --git a/provider/vsphere/fixture_test.go b/provider/vsphere/fixture_test.go
@@ -8,6 +8,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 
+	"github.com/juju/errors"
 	"github.com/juju/testing"
 	jc "github.com/juju/testing/checkers"
 	"github.com/vmware/govmomi/vim25/soap"
@@ -136,7 +137,7 @@ func AssertInvalidatesCredential(c *gc.C, client *mockClient, f func(context.Pro
 		Detail: struct {
 			Fault types.AnyType `xml:",any,typeattr"`
 		}{Fault: types.NoPermission{}},
-	}))
+	}), errors.New("find folder failed"))
 	var called bool
 	ctx := &context.CloudCallContext{
 		InvalidateCredentialFunc: func(string) error {

diff --git a/provider/vsphere/internal/vsphereclient/client.go b/provider/vsphere/internal/vsphereclient/client.go
@@ -319,13 +319,16 @@ func (c *Client) EnsureVMFolder(ctx context.Context, credAttrFolder string, fold
 
 	createFolder := func(parent *object.Folder, name string) (*object.Folder, error) {
 		getFolder := func() (*object.Folder, error) {
-			return finder.Folder(ctx, path.Join(parent.InventoryPath, name))
+			fd, err := finder.Folder(ctx, path.Join(parent.InventoryPath, name))
+			if err != nil {
+				return nil, errors.Trace(err)
+			}
+			return fd, nil
 		}
-		_, err := parent.CreateFolder(ctx, name)
+		fd, err := parent.CreateFolder(ctx, name)
 		if err == nil {
-			return getFolder()
+			return fd, nil
 		}
-		c.logger.Criticalf("EnsureVMFolder createFolder parent.InventoryPath %q, name %q, err %+v", parent.InventoryPath, name, err)
 		if soap.IsSoapFault(err) {
 			switch soap.ToSoapFault(err).VimFault().(type) {
 			case types.DuplicateName:

diff --git a/provider/vsphere/internal/vsphereclient/client_test.go b/provider/vsphere/internal/vsphereclient/client_test.go
@@ -237,6 +237,15 @@ func (s *clientSuite) SetUpTest(c *gc.C) {
 				{Name: "name", Val: "foo"},
 			},
 		}},
+		"FakeVMDKSFolder": {{
+			Obj: types.ManagedObjectReference{
+				Type:  "Folder",
+				Value: "FakeVmFolder",
+			},
+			PropSet: []types.DynamicProperty{
+				{Name: "name", Val: "juju-vmdks"},
+			},
+		}},
 		"FakeControllerVmFolder": {{
 			Obj: types.ManagedObjectReference{
 				Type:  "Folder",
@@ -638,6 +647,8 @@ func (s *clientSuite) TestEnsureVMFolder(c *gc.C) {
 	c.Assert(folder, gc.NotNil)
 
 	s.roundTripper.CheckCalls(c, []testing.StubCall{
+		retrievePropertiesStubCall("FakeRootFolder"),
+		retrievePropertiesStubCall("FakeRootFolder"),
 		retrievePropertiesStubCall("FakeRootFolder"),
 		retrievePropertiesStubCall("FakeRootFolder"),
 		retrievePropertiesStubCall("FakeDatacenter"),
@@ -660,6 +671,9 @@ func (s *clientSuite) TestMoveVMFolderInto(c *gc.C) {
 		retrievePropertiesStubCall("FakeVmFolder"),
 		retrievePropertiesStubCall("FakeHostFolder"),
 		retrievePropertiesStubCall("FakeRootFolder"),
+		retrievePropertiesStubCall("FakeRootFolder"),
+		retrievePropertiesStubCall("FakeDatacenter"),
+		retrievePropertiesStubCall("FakeRootFolder"),
 		retrievePropertiesStubCall("FakeDatacenter"),
 		retrievePropertiesStubCall("FakeVmFolder"),
 		retrievePropertiesStubCall("FakeControllerVmFolder"),

diff --git a/provider/vsphere/internal/vsphereclient/createvm_test.go b/provider/vsphere/internal/vsphereclient/createvm_test.go
@@ -78,6 +78,8 @@ func (s *clientSuite) TestCreateVirtualMachine(c *gc.C) {
 	templateCisp := baseCisp()
 	templateCisp.EntityName = vmTemplateName(args)
 	s.roundTripper.CheckCalls(c, []testing.StubCall{
+		retrievePropertiesStubCall("FakeRootFolder"),
+		retrievePropertiesStubCall("FakeRootFolder"),
 		retrievePropertiesStubCall("FakeRootFolder"),
 		retrievePropertiesStubCall("FakeRootFolder"),
 		retrievePropertiesStubCall("FakeDatacenter"),
@@ -89,17 +91,28 @@ func (s *clientSuite) TestCreateVirtualMachine(c *gc.C) {
 		retrievePropertiesStubCall("FakeDatastore1", "FakeDatastore2"),
 		retrievePropertiesStubCall("FakeRootFolder"),
 		retrievePropertiesStubCall("FakeRootFolder"),
+		retrievePropertiesStubCall("FakeRootFolder"),
+		retrievePropertiesStubCall("FakeRootFolder"),
+		retrievePropertiesStubCall("FakeDatacenter"),
+		retrievePropertiesStubCall("FakeRootFolder"),
 		retrievePropertiesStubCall("FakeDatacenter"),
 		retrievePropertiesStubCall("FakeVmFolder"),
-		retrievePropertiesStubCall("FakeVmFolder"),
+		retrievePropertiesStubCall("FakeControllerVmFolder"),
+		retrievePropertiesStubCall("FakeHostFolder"),
 		{"CreateImportSpec", []interface{}{
 			UbuntuOVF,
 			types.ManagedObjectReference{Type: "Datastore", Value: "FakeDatastore2"},
 			templateCisp,
 		}},
 		retrievePropertiesStubCall("FakeRootFolder"),
 		retrievePropertiesStubCall("FakeRootFolder"),
+		retrievePropertiesStubCall("FakeRootFolder"),
+		retrievePropertiesStubCall("FakeRootFolder"),
 		retrievePropertiesStubCall("FakeDatacenter"),
+		retrievePropertiesStubCall("FakeRootFolder"),
+		retrievePropertiesStubCall("FakeDatacenter"),
+		retrievePropertiesStubCall("FakeVmFolder"),
+		retrievePropertiesStubCall("FakeHostFolder"),
 		{"CreateFolder", []interface{}{"juju-vmdks"}},
 		{"CreateFolder", []interface{}{"ctrl"}},
 		{"CreateFolder", []interface{}{"xenial"}},
@@ -161,7 +174,7 @@ func (s *clientSuite) TestCreateVirtualMachineNoDiskUUID(c *gc.C) {
 	_, err := client.CreateVirtualMachine(context.Background(), args)
 	c.Assert(err, jc.ErrorIsNil)
 
-	s.roundTripper.CheckCall(c, 31, "CloneVM_Task", "vm-0", &types.VirtualMachineConfigSpec{
+	s.roundTripper.CheckCall(c, 44, "CloneVM_Task", "vm-0", &types.VirtualMachineConfigSpec{
 		ExtraConfig: []types.BaseOptionValue{
 			&types.OptionValue{Key: "k", Value: "v"},
 		},
@@ -200,13 +213,13 @@ func (s *clientSuite) TestCreateVirtualMachineDatastoreSpecified(c *gc.C) {
 	cisp := baseCisp()
 	cisp.EntityName = vmTemplateName(args)
 	s.roundTripper.CheckCall(
-		c, 14, "CreateImportSpec", UbuntuOVF,
+		c, 21, "CreateImportSpec", UbuntuOVF,
 		types.ManagedObjectReference{Type: "Datastore", Value: "FakeDatastore1"},
 		cisp,
 	)
 
 	s.roundTripper.CheckCall(
-		c, 31, "CloneVM_Task", "vm-0", &types.VirtualMachineConfigSpec{
+		c, 44, "CloneVM_Task", "vm-0", &types.VirtualMachineConfigSpec{
 			ExtraConfig: []types.BaseOptionValue{
 				&types.OptionValue{Key: "k", Value: "v"},
 			},
@@ -331,12 +344,12 @@ func (s *clientSuite) TestCreateVirtualMachineMultipleNetworksSpecifiedFirstDefa
 		},
 	}
 
-	s.roundTripper.CheckCall(c, 21, "ImportVApp", &types.VirtualMachineImportSpec{
+	s.roundTripper.CheckCall(c, 34, "ImportVApp", &types.VirtualMachineImportSpec{
 		ConfigSpec: types.VirtualMachineConfigSpec{
 			Name: "vm-name",
 		},
 	})
-	s.roundTripper.CheckCall(c, 31, "CloneVM_Task", "vm-0", &types.VirtualMachineConfigSpec{
+	s.roundTripper.CheckCall(c, 44, "CloneVM_Task", "vm-0", &types.VirtualMachineConfigSpec{
 		ExtraConfig: []types.BaseOptionValue{
 			&types.OptionValue{Key: "k", Value: "v"},
 		},
@@ -391,12 +404,12 @@ func (s *clientSuite) TestCreateVirtualMachineNetworkSpecifiedDVPortgroup(c *gc.
 	}
 
 	retrieveDVSCall := retrievePropertiesStubCall("dvs-0")
-	s.roundTripper.CheckCall(c, 30, retrieveDVSCall.FuncName, retrieveDVSCall.Args...)
+	s.roundTripper.CheckCall(c, 43, retrieveDVSCall.FuncName, retrieveDVSCall.Args...)
 
 	// When the external network is a distributed virtual portgroup,
 	// we must make an additional RetrieveProperties call to fetch
 	// the DVS's UUID. This bumps the ImportVApp position by one.
-	s.roundTripper.CheckCall(c, 32, "CloneVM_Task", "vm-0", &types.VirtualMachineConfigSpec{
+	s.roundTripper.CheckCall(c, 45, "CloneVM_Task", "vm-0", &types.VirtualMachineConfigSpec{
 		ExtraConfig: []types.BaseOptionValue{
 			&types.OptionValue{Key: "k", Value: "v"},
 		},

diff --git a/provider/vsphere/internal/vsphereclient/mock_test.go b/provider/vsphere/internal/vsphereclient/mock_test.go
@@ -90,7 +90,6 @@ func (r *mockRoundTripper) RoundTrip(ctx context.Context, req, res soap.HasFault
 	if r.roundTrip != nil {
 		return r.roundTrip(ctx, req, res)
 	}
-
 	switch res := res.(type) {
 	case *methods.RetrievePropertiesBody:
 		req := req.(*methods.RetrievePropertiesBody).Req

diff --git a/provider/vsphere/upgrades_test.go b/provider/vsphere/upgrades_test.go
@@ -84,7 +84,7 @@ func (s *environUpgradeSuite) TestEnvironUpgradeOperationModelFolders(c *gc.C) {
 	s.client.CheckCallNames(c, "EnsureVMFolder", "VirtualMachines", "MoveVMsInto", "Close")
 	ensureVMFolderCall := s.client.Calls()[0]
 	moveVMsIntoCall := s.client.Calls()[2]
-	c.Assert(ensureVMFolderCall.Args[1], gc.Equals,
+	c.Assert(ensureVMFolderCall.Args[2], gc.Equals,
 		`Juju Controller (foo)/Model "testmodel" (2d02eeac-9dbb-11e4-89d3-123b93f75cba)`)
 	c.Assert(moveVMsIntoCall.Args[1], gc.Equals,
 		`Juju Controller (foo)/Model "testmodel" (2d02eeac-9dbb-11e4-89d3-123b93f75cba)`)