Resource suggestion fixes for vic-machine

- Support vic-machine create targeting Datacenter within an inventory folder.

- Simplify compute-resource suggestions.
  Removes quite a bit of code, which also did not work when clusters/hosts were within inventory folders.
  Some of the removed code was working around the old govmomi Finder "list mode" limitations.

- Add vcsim based tests for vic-machine validation against multiple resources of all types,
  within inventory folders.  These tests also cover most of the error paths.

- Add LicenseManager support to vcsim

Fixes vmware#4203

cmd/vic-machine/common/compute.go

@@ -39,7 +39,7 @@ func (c *Compute) ComputeFlagsNoName() []cli.Flag {
 		cli.StringFlag{
 			Name:        "compute-resource, r",
 			Value:       "",
-			Usage:       "Compute resource path, e.g. myCluster/Resources/myRP. Default to <default cluster>/Resources",
+			Usage:       "Compute resource path, e.g. myCluster",
 			Destination: &c.ComputeResourcePath,
 		},
 	}

lib/install/validate/compute.go

@@ -15,18 +15,13 @@
 package validate
 
 import (
-	"fmt"
-	"path"
-	"path/filepath"
-	"sort"
-	"strings"
-
 	"context"
 
 	log "github.com/Sirupsen/logrus"
 
 	"github.com/vmware/govmomi/find"
 	"github.com/vmware/govmomi/object"
+	"github.com/vmware/govmomi/vim25/mo"
 	"github.com/vmware/vic/lib/config"
 	"github.com/vmware/vic/lib/install/data"
 	"github.com/vmware/vic/pkg/errors"
@@ -36,41 +31,14 @@ import (
 func (v *Validator) compute(ctx context.Context, input *data.Data, conf *config.VirtualContainerHostConfigSpec) {
 	defer trace.End(trace.Begin(""))
 
-	// Compute
-
-	// compute resource looks like <toplevel>/<sub/path>
-	// this should map to /datacenter-name/host/<toplevel>/Resources/<sub/path>
-	// we need to validate that <toplevel> exists and then that the combined path exists.
+	// ComputeResourcePath should resolve to a ComputeResource, ClusterComputeResource or ResourcePool
 
 	pool, err := v.ResourcePoolHelper(ctx, input.ComputeResourcePath)
 	v.NoteIssue(err)
 	if pool == nil {
 		return
 	}
 
-	// stash the pool for later use
-	v.ResourcePoolPath = pool.InventoryPath
-
-	// some hoops for while we're still using session package
-	v.Session.Pool = pool
-	v.Session.PoolPath = pool.InventoryPath
-	v.Session.ClusterPath = v.inventoryPathToCluster(pool.InventoryPath)
-
-	clusters, err := v.Session.Finder.ComputeResourceList(v.Context, v.Session.ClusterPath)
-	if err != nil {
-		log.Errorf("Unable to acquire reference to cluster %q: %s", path.Base(v.Session.ClusterPath), err)
-		v.NoteIssue(err)
-		return
-	}
-
-	if len(clusters) != 1 {
-		err := fmt.Errorf("Unable to acquire unambiguous reference to cluster %q", path.Base(v.Session.ClusterPath))
-		log.Error(err)
-		v.NoteIssue(err)
-	}
-
-	v.Session.Cluster = clusters[0]
-
 	// TODO: for vApp creation assert that the name doesn't exist
 	// TODO: for RP creation assert whatever we decide about the pool - most likely that it's empty
 }
@@ -81,172 +49,79 @@ func (v *Validator) ResourcePoolHelper(ctx context.Context, path string) (*objec
 	defer trace.End(trace.Begin(path))
 
 	// if compute-resource is unspecified is there a default
-	if path == "" || path == "/" {
+	if path == "" {
 		if v.Session.Pool != nil {
 			log.Debugf("Using default resource pool for compute resource: %q", v.Session.Pool.InventoryPath)
 			return v.Session.Pool, nil
 		}
 
 		// if no path specified and no default available the show all
-		v.suggestComputeResource("*")
+		v.suggestComputeResource()
 		return nil, errors.New("No unambiguous default compute resource available: --compute-resource must be specified")
 	}
 
-	ipath := v.computePathToInventoryPath(path)
-	log.Debugf("Converted original path %q to %q", path, ipath)
-
-	// first try the path directly without any processing
-	pools, err := v.Session.Finder.ResourcePoolList(ctx, path)
+	pool, err := v.Session.Finder.ResourcePool(ctx, path)
 	if err != nil {
 		log.Debugf("Failed to look up compute resource as absolute path %q: %s", path, err)
 		if _, ok := err.(*find.NotFoundError); !ok {
 			// we return err directly here so we can check the type
 			return nil, err
 		}
-
-		// if it starts with datacenter then we know it's absolute and invalid
-		if strings.HasPrefix(path, "/"+v.Session.DatacenterPath) {
-			v.suggestComputeResource(path)
-			return nil, err
-		}
 	}
 
-	if len(pools) == 0 {
-		// assume it's a cluster specifier - that's the formal case, e.g. /cluster/resource/pool
-		// not /cluster/Resources/resource/pool
-		// everything from now on will use this assumption
+	var compute *object.ComputeResource
 
-		pools, err = v.Session.Finder.ResourcePoolList(ctx, ipath)
+	if pool == nil {
+		// check if its a ComputeResource or ClusterComputeResource
+
+		compute, err = v.Session.Finder.ComputeResource(ctx, path)
 		if err != nil {
-			log.Debugf("failed to look up compute resource as cluster path %q: %s", ipath, err)
-			if _, ok := err.(*find.NotFoundError); !ok {
-				// we return err directly here so we can check the type
-				return nil, err
+			if _, ok := err.(*find.NotFoundError); ok {
+				v.suggestComputeResource()
 			}
-		}
-	}
-
-	if len(pools) == 1 {
-		log.Debugf("Selected compute resource %q", pools[0].InventoryPath)
-		return pools[0], nil
-	}
-
-	// both cases we want to suggest options
-	v.suggestComputeResource(ipath)
-
-	if len(pools) == 0 {
-		log.Debugf("no such compute resource %q", path)
-		// we return err directly here so we can check the type
-		return nil, err
-	}
-
-	// TODO: error about required disabmiguation and list entries in nets
-	return nil, errors.New("ambiguous compute resource " + path)
-}
-
-func (v *Validator) suggestComputeResource(path string) {
-	defer trace.End(trace.Begin(path))
-
-	log.Infof("Suggesting valid values for --compute-resource based on %q", path)
-
-	// allow us to work on inventory paths
-	path = v.computePathToInventoryPath(path)
 
-	var matches []string
-	for matches = nil; matches == nil; matches = v.findValidPool(path) {
-		// back up the path until we find a pool
-		newpath := filepath.Dir(path)
-		if newpath == "." {
-			// filepath.Dir returns . which has no meaning for us
-			newpath = "/"
-		}
-		if newpath == path {
-			break
+			return nil, err
 		}
-		path = newpath
-	}
-
-	if matches == nil {
-		// Backing all the way up didn't help
-		log.Info("Failed to find resource pool in the provided path, showing all top level resource pools.")
-		matches = v.findValidPool("*")
-	}
 
-	if matches != nil {
-		// we've collected recommendations - displayname
-		log.Info("Suggested values for --compute-resource:")
-		for _, p := range matches {
-			log.Infof("  %q", v.inventoryPathToComputePath(p))
+		// Use the default pool
+		pool, err = compute.ResourcePool(ctx)
+		if err != nil {
+			return nil, err
 		}
-		return
-	}
-
-	log.Info("No resource pools found")
-}
-
-func (v *Validator) findValidPool(path string) []string {
-	defer trace.End(trace.Begin(path))
-
-	// list pools in path
-	matches := v.listResourcePools(path)
-	if matches != nil {
-		sort.Strings(matches)
-		return matches
-	}
-
-	// no pools in path, but if path is cluster, list pools in cluster
-	clusters, err := v.Session.Finder.ComputeResourceList(v.Context, path)
-	if len(clusters) == 0 {
-		// not a cluster
-		log.Debugf("Path %q does not identify a cluster (or clusters) or the list could not be obtained: %s", path, err)
-		return nil
-	}
+	} else {
+		// TODO: add an object.ResourcePool.Owner method (see compute.ResourcePool.GetCluster)
+		var p mo.ResourcePool
 
-	if len(clusters) > 1 {
-		log.Debugf("Suggesting clusters as there are multiple matches")
-		matches = make([]string, len(clusters))
-		for i, c := range clusters {
-			matches[i] = c.InventoryPath
+		if err = pool.Properties(ctx, pool.Reference(), []string{"owner"}, &p); err != nil {
+			log.Errorf("Unable to get cluster of resource pool %s: %s", pool.Name(), err)
+			return nil, err
 		}
-		sort.Strings(matches)
-		return matches
-	}
 
-	log.Debugf("Suggesting pools for cluster %q", clusters[0].Name())
-	matches = v.listResourcePools(fmt.Sprintf("%s/Resources/*", clusters[0].InventoryPath))
-	if matches == nil {
-		// no child pools so recommend cluster directly
-		return []string{clusters[0].InventoryPath}
+		compute = object.NewComputeResource(pool.Client(), p.Owner)
 	}
 
-	return matches
-}
-
-func (v *Validator) listResourcePools(path string) []string {
-	defer trace.End(trace.Begin(path))
-
-	pools, err := v.Session.Finder.ResourcePoolList(v.Context, path+"/*")
-	if err != nil {
-		log.Debugf("Unable to list pools for %q: %s", path, err)
-		return nil
-	}
+	// stash the pool for later use
+	v.ResourcePoolPath = pool.InventoryPath
 
-	if len(pools) == 0 {
-		return nil
-	}
+	// some hoops for while we're still using session package
+	v.Session.Pool = pool
+	v.Session.PoolPath = pool.InventoryPath
 
-	matches := make([]string, len(pools))
-	for i, p := range pools {
-		matches[i] = p.InventoryPath
-	}
+	v.Session.Cluster = compute
+	v.Session.ClusterPath = compute.InventoryPath
 
-	return matches
+	return pool, nil
 }
 
-func (v *Validator) GetResourcePool(input *data.Data) (*object.ResourcePool, error) {
+func (v *Validator) suggestComputeResource() {
 	defer trace.End(trace.Begin(""))
 
-	return v.ResourcePoolHelper(v.Context, input.ComputeResourcePath)
+	compute, _ := v.Session.Finder.ComputeResourceList(v.Context, "*")
+
+	log.Info("Suggested values for --compute-resource:")
+	for _, c := range compute {
+		log.Infof("  %q", c.Name())
+	}
 }
 
 func (v *Validator) ValidateCompute(ctx context.Context, input *data.Data, computeRequired bool) (*config.VirtualContainerHostConfigSpec, error) {

lib/install/validate/storage.go

@@ -37,7 +37,7 @@ func (v *Validator) storage(ctx context.Context, input *data.Data, conf *config.
 	// Image Store
 	imageDSpath, ds, err := v.DatastoreHelper(ctx, input.ImageDatastorePath, "", "--image-store")
 
-	if imageDSpath == nil {
+	if err != nil {
 		v.NoteIssue(err)
 		return
 	}
@@ -47,7 +47,6 @@ func (v *Validator) storage(ctx context.Context, input *data.Data, conf *config.
 		imageDSpath.Path = input.DisplayName
 	}
 
-	v.NoteIssue(err)
 	if ds != nil {
 		v.SetDatastore(ds, imageDSpath)
 		conf.AddImageStore(imageDSpath)