Find datastores in folders

Update the vsphereclient Datastores method to return datastores under foldes under the Datastores root folder. We don't keep track of the inventory path of the datastore - when the name of the datastore is used for creating a new vm it's looked up on the compute resource, so the inventory path isn't needed.
sdg0919 · May 4, 2020 · ee7e2ad · ee7e2ad
1 parent bddec69
commit ee7e2ad
Show file tree

Hide file tree

Showing 9 changed files with 77 additions and 23 deletions.
diff --git a/provider/vsphere/client.go b/provider/vsphere/client.go
@@ -24,7 +24,7 @@ type Client interface {
 	ComputeResources(context.Context) ([]*mo.ComputeResource, error)
 	ResourcePools(context.Context, string) ([]*object.ResourcePool, error)
 	CreateVirtualMachine(context.Context, vsphereclient.CreateVirtualMachineParams) (*mo.VirtualMachine, error)
-	Datastores(context.Context) ([]*mo.Datastore, error)
+	Datastores(context.Context) ([]mo.Datastore, error)
 	DeleteDatastoreFile(context.Context, string) error
 	DestroyVMFolder(context.Context, string) error
 	EnsureVMFolder(context.Context, string, string) (*object.Folder, error)

diff --git a/provider/vsphere/environ.go b/provider/vsphere/environ.go
@@ -236,13 +236,13 @@ func (env *sessionEnviron) getVMFolder() string {
 	return env.environ.cloud.Credential.Attributes()[credAttrVMFolder]
 }
 
-func (env *sessionEnviron) accessibleDatastores(ctx callcontext.ProviderCallContext) ([]*mo.Datastore, error) {
+func (env *sessionEnviron) accessibleDatastores(ctx callcontext.ProviderCallContext) ([]mo.Datastore, error) {
 	datastores, err := env.client.Datastores(env.ctx)
 	if err != nil {
 		HandleCredentialError(err, env, ctx)
 		return nil, errors.Trace(err)
 	}
-	var results []*mo.Datastore
+	var results []mo.Datastore
 	for _, ds := range datastores {
 		if !ds.Summary.Accessible {
 			continue

diff --git a/provider/vsphere/environ_policy_test.go b/provider/vsphere/environ_policy_test.go
@@ -112,7 +112,7 @@ func (s *environPolSuite) TestPrecheckInstanceChecksConstraintZones(c *gc.C) {
 }
 
 func (s *environPolSuite) TestPrecheckInstanceChecksConstraintDatastore(c *gc.C) {
-	s.client.datastores = []*mo.Datastore{{
+	s.client.datastores = []mo.Datastore{{
 		ManagedEntity: mo.ManagedEntity{Name: "foo"},
 	}, {
 		ManagedEntity: mo.ManagedEntity{Name: "bar"},

diff --git a/provider/vsphere/environ_test.go b/provider/vsphere/environ_test.go
@@ -72,7 +72,7 @@ func (s *environSuite) TestDestroy(c *gc.C) {
 }
 
 func (s *environSuite) TestDestroyController(c *gc.C) {
-	s.client.datastores = []*mo.Datastore{{
+	s.client.datastores = []mo.Datastore{{
 		ManagedEntity: mo.ManagedEntity{Name: "foo"},
 	}, {
 		ManagedEntity: mo.ManagedEntity{Name: "bar"},

diff --git a/provider/vsphere/internal/vsphereclient/client.go b/provider/vsphere/internal/vsphereclient/client.go
@@ -258,8 +258,8 @@ func (c *Client) ComputeResources(ctx context.Context) ([]*mo.ComputeResource, e
 }
 
 // Datastores returns list of all datastores in the system.
-func (c *Client) Datastores(ctx context.Context) ([]*mo.Datastore, error) {
-	_, datacenter, err := c.finder(ctx)
+func (c *Client) Datastores(ctx context.Context) ([]mo.Datastore, error) {
+	finder, datacenter, err := c.finder(ctx)
 	if err != nil {
 		return nil, errors.Trace(err)
 	}
@@ -268,17 +268,27 @@ func (c *Client) Datastores(ctx context.Context) ([]*mo.Datastore, error) {
 		return nil, errors.Trace(err)
 	}
 
-	es, err := c.lister(folders.DatastoreFolder.Reference()).List(ctx)
+	dsPath := path.Join(folders.DatastoreFolder.InventoryPath, "...")
+	c.logger.Tracef("listing datastores under %q", dsPath)
+	items, err := finder.DatastoreList(ctx, dsPath)
 	if err != nil {
+		if _, ok := err.(*find.NotFoundError); ok {
+			c.logger.Debugf("no datastores for path %q", dsPath)
+			return nil, nil
+		}
 		return nil, errors.Trace(err)
 	}
 
-	var datastores []*mo.Datastore
-	for _, e := range es {
-		switch o := e.Object.(type) {
-		case mo.Datastore:
-			datastores = append(datastores, &o)
-		}
+	refs := make([]types.ManagedObjectReference, len(items))
+	for i, item := range items {
+		c.logger.Tracef("%s", item.InventoryPath)
+		refs[i] = item.Reference()
+	}
+
+	var datastores []mo.Datastore
+	err = c.client.Retrieve(ctx, refs, nil, &datastores)
+	if err != nil {
+		return nil, errors.Annotate(err, "retrieving datastore details")
 	}
 	return datastores, nil
 }

diff --git a/provider/vsphere/internal/vsphereclient/client_test.go b/provider/vsphere/internal/vsphereclient/client_test.go
@@ -816,7 +816,9 @@ func (s *clientSuite) TestDatastores(c *gc.C) {
 		retrievePropertiesStubCall("FakeRootFolder"),
 		retrievePropertiesStubCall("FakeRootFolder"),
 		retrievePropertiesStubCall("FakeDatacenter"),
+		makeStubCall("FindByInventoryPath", "FakeSearchIndex", "/dc0/datastore"),
 		retrievePropertiesStubCall("FakeDatastoreFolder"),
+		retrievePropertiesStubCall("FakeDatastore1", "FakeDatastore2"),
 	})
 
 	c.Assert(result, gc.HasLen, 2)

diff --git a/provider/vsphere/internal/vsphereclient/mock_test.go b/provider/vsphere/internal/vsphereclient/mock_test.go
@@ -252,12 +252,24 @@ func (r *mockRoundTripper) RoundTrip(ctx context.Context, req, res soap.HasFault
 		req := req.(*methods.FindByInventoryPathBody).Req
 		r.MethodCall(r, "FindByInventoryPath", req.This.Value, req.InventoryPath)
 		logger.Debugf("FindByInventoryPath ref: %q, path: %q", req.This.Value, req.InventoryPath)
-		res.Res = &types.FindByInventoryPathResponse{
-			Returnval: &types.ManagedObjectReference{
-				Type:  "ComputeResource",
-				Value: "z0",
-			},
+		var findResponse *types.FindByInventoryPathResponse
+		if req.InventoryPath == "/dc0/datastore" {
+			findResponse = &types.FindByInventoryPathResponse{
+				Returnval: &types.ManagedObjectReference{
+					Type:  "Folder",
+					Value: "FakeDatastoreFolder",
+				},
+			}
+
+		} else {
+			findResponse = &types.FindByInventoryPathResponse{
+				Returnval: &types.ManagedObjectReference{
+					Type:  "ComputeResource",
+					Value: "z0",
+				},
+			}
 		}
+		res.Res = findResponse
 	case *methods.MarkAsTemplateBody:
 		req := req.(*methods.MarkAsTemplateBody).Req
 		r.MethodCall(r, "MarkAsTemplate", req.This.Value)

diff --git a/provider/vsphere/mock_test.go b/provider/vsphere/mock_test.go
@@ -37,7 +37,7 @@ type mockClient struct {
 	resourcePools         map[string][]*object.ResourcePool
 	createdVirtualMachine *mo.VirtualMachine
 	virtualMachines       []*mo.VirtualMachine
-	datastores            []*mo.Datastore
+	datastores            []mo.Datastore
 	vmFolder              *object.Folder
 	hasPrivilege          bool
 }
@@ -70,7 +70,7 @@ func (c *mockClient) CreateVirtualMachine(ctx context.Context, args vsphereclien
 	return c.createdVirtualMachine, c.NextErr()
 }
 
-func (c *mockClient) Datastores(ctx context.Context) ([]*mo.Datastore, error) {
+func (c *mockClient) Datastores(ctx context.Context) ([]mo.Datastore, error) {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 	c.MethodCall(c, "Datastores", ctx)

diff --git a/provider/vsphere/mocks/client_mock.go b/provider/vsphere/mocks/client_mock.go