6-10-List failure on VC

[mhagen-vmware]

Ran the command:
bin/vic-machine-linux ls --target xxxx.xxx.vmware.com/ --thumbprint=xxx --user xxx --password=xxx --compute-resource cls

Mar 8 2017 18:39:26.874-06:00 INFO ### Listing VCHs ####
 Mar 8 2017 18:39:27.043-06:00 INFO Validating target 
Mar 8 2017 18:39:27.043-06:00 INFO Validating compute resource
 Mar 8 2017 18:39:27.055-06:00 INFO Suggesting valid values for --compute-resource based on "cls" 
Mar 8 2017 18:39:27.057-06:00 INFO Failed to find resource pool in the provided path, showing all top level resource pools.
 Mar 8 2017 18:39:27.060-06:00 INFO Suggested values for --compute-resource: 
Mar 8 2017 18:39:27.060-06:00 INFO "cls"
 Mar 8 2017 18:39:27.060-06:00 ERROR -------------------- 
Mar 8 2017 18:39:27.060-06:00 ERROR resource pool 'cls' not found 
Mar 8 2017 18:39:27.060-06:00 ERROR List cannot continue - compute resource validation failed: validation of configuration failed
 Mar 8 2017 18:39:27.060-06:00 ERROR --------------------
 Mar 8 2017 18:39:27.060-06:00 ERROR vic-machine-linux ls failed: list failed

Weird error, it suggests cls, but at the same time indicates cls is not valid, only difference being single quotes vs double quotes?

[mhagen-vmware]

This test was testing the trailing slash specifically, but it had the exact same error without the trailing slash in the previous test also.

[dougm]

Likely related to the folder work that still needs finishing: #3817 #3975

[dougm]

Possible workaround: try with a path instead of just the name, for example:

--compute-resource ./cls
--compute-resource host/cls
--compute-resource /dc1/host/cls

[hickeng]

It's likely that the problem relates to the following:

Mar  9 2017 22:39:30.211Z DEBUG Path is treated as absolute given datacenter prefix ""
Mar  9 2017 22:39:30.211Z DEBUG [ END ] [github.com/vmware/vic/lib/install/validate.(*Validator).computePathToInventoryPath:596] [174.209µs] cluster1
Mar  9 2017 22:39:30.211Z DEBUG Converted original path "cluster1" to "cluster1"

We are identifying the compute path as an absolute inventory path when it is not, due to it having a prefix matching the specified datacenter.
#3557 added a path that allows omission of the datacenter qualifier specifically for vic-machine ls, but doesn't add corresponding checks when we test in validator:

	if strings.HasPrefix(path, v.DatacenterPath) {
		log.Debugf("Path is treated as absolute given datacenter prefix %q", v.DatacenterPath)
		return path
	}

[gigawhitlocks]

Verified @dougm 's workaround:

~/go/src/github.com/vmware/vic master*
❯ ./bin/vic-machine-linux ls --target 192.168.218.147 --user root  --password xxx --thumbprint=8B:CA:90:49:AD:3D:91:0A:8B:46:33:B1:1B:36:8F:A2:87:5C:13:81 --compute-resource=ib-aus-office-97.localdomain
Mar  9 2017 17:43:24.629-06:00 INFO  ### Listing VCHs ####
Mar  9 2017 17:43:24.711-06:00 INFO  Validating target
Mar  9 2017 17:43:24.721-06:00 INFO  Validating compute resource
Mar  9 2017 17:43:24.750-06:00 INFO  Suggesting valid values for --compute-resource based on "ib-aus-office-97.localdomain"
Mar  9 2017 17:43:24.754-06:00 INFO  Failed to find resource pool in the provided path, showing all top level resource pools.
Mar  9 2017 17:43:24.760-06:00 INFO  Suggested values for --compute-resource:
Mar  9 2017 17:43:24.760-06:00 INFO    "ib-aus-office-97.localdomain"
Mar  9 2017 17:43:24.760-06:00 ERROR --------------------
Mar  9 2017 17:43:24.760-06:00 ERROR resource pool 'ib-aus-office-97.localdomain' not found
Mar  9 2017 17:43:24.760-06:00 ERROR List cannot continue - compute resource validation failed: validation of configuration failed
Mar  9 2017 17:43:24.760-06:00 ERROR --------------------
Mar  9 2017 17:43:24.760-06:00 ERROR vic-machine-linux ls failed: list failed

~/go/src/github.com/vmware/vic master*
❯ ./bin/vic-machine-linux ls --target 192.168.218.147 --user root  --password xxx --thumbprint=8B:CA:90:49:AD:3D:91:0A:8B:46:33:B1:1B:36:8F:A2:87:5C:13:81 --compute-resource=./ib-aus-office-97.localdomain
Mar  9 2017 17:44:16.203-06:00 INFO  ### Listing VCHs ####
Mar  9 2017 17:44:16.299-06:00 INFO  Validating target
Mar  9 2017 17:44:16.318-06:00 INFO  Validating compute resource

ID        PATH                                                              NAME           VERSION                 UPGRADE STATUS
8         /ha-datacenter/host/ib-aus-office-97.localdomain/Resources        ian-vch        v0.9.0-3-95110a3        Up to date

[hickeng]

Adding a datacenter qualifier to target also works. I've not been able to figure out a small change that allows us to construct a functional inventory path that we can pass to finder without knowing the datacenter.

I've been using this patch to sidestep the treating as abs path issue, and to avoid requiring the user specifying a trailing /

index 568b52a..4dfcb2c 100644
--- a/lib/install/validate/compute.go
+++ b/lib/install/validate/compute.go
@@ -93,6 +93,11 @@ func (v *Validator) ResourcePoolHelper(ctx context.Context, path string) (*objec
        ipath := v.computePathToInventoryPath(path)
        log.Debugf("Converted original path %q to %q", path, ipath)

+       // ensure there's a trailing "/"
+       if !strings.HasSuffix(path, "/") {
+               path += "/"
+       }
+
        // first try the path directly without any processing
        pools, err := v.Session.Finder.ResourcePoolList(ctx, path)
        if err != nil {
@@ -103,7 +108,7 @@ func (v *Validator) ResourcePoolHelper(ctx context.Context, path string) (*objec
                }

                // if it starts with datacenter then we know it's absolute and invalid
-               if strings.HasPrefix(path, "/"+v.Session.DatacenterPath) {
+               if v.Session.DatacenterPath != "" && strings.HasPrefix(path, "/"+v.Session.DatacenterPath) {
                        v.suggestComputeResource(path)
                        return nil, err
                }
diff --git a/lib/install/validate/validator.go b/lib/install/validate/validator.go
index 5c0e6fb..cd83116 100644
--- a/lib/install/validate/validator.go
+++ b/lib/install/validate/validator.go
@@ -596,7 +596,7 @@ func (v *Validator) computePathToInventoryPath(path string) string {
        defer trace.End(trace.Begin(path))

        // if it opens with the datacenter prefix the assume it's an absolute
-       if strings.HasPrefix(path, v.DatacenterPath) {
+       if v.DatacenterPath != "" && strings.HasPrefix(path, v.DatacenterPath) {
                log.Debugf("Path is treated as absolute given datacenter prefix %q", v.DatacenterPath)
                return path
        }
@@ -619,6 +619,10 @@ func (v *Validator) computePathToInventoryPath(path string) string {
                }
                if len(pElem) > 1 {
                        parts = append(parts, pElem[1])
+               } else {
+                       // ensure there's a trailing slash after the components are joined
+                       // seems to be needed for govmomi finder
+                       parts = append(parts, "")
                }
        } else if path != "" {
                // for ESX, first element is a pool
@@ -632,7 +636,7 @@ func (v *Validator) inventoryPathToComputePath(path string) string {
        defer trace.End(trace.Begin(path))

        // sanity check datacenter
-       if !strings.HasPrefix(path, v.DatacenterPath) {
+       if v.DatacenterPath != "" && !strings.HasPrefix(path, v.DatacenterPath) {
                log.Debugf("Expected path to be within target datacenter %q: %q", v.DatacenterPath, path)
                v.NoteIssue(errors.New("inventory path was not in datacenter scope"))
                return ""

[gigawhitlocks]

TIL github has a diff choice for syntax highlighting patches!

Anyway I'm more than happy to implement a regression test for this issue while putting in your fix @hickeng -- it also strikes me as something that it might be helpful to break into a helper function so that if we need to find all occurrences of this test in the future we can use guru instead of trying to find the instances manually.

I think the bit that you added that ensures there's a trailing slash will also resolve #3975

[mdubya66]

We will prioritize this for future sprints. thanks.

[hickeng]

@gigawhitlocks the diff I supplied DOESN'T handle cluster style paths such as /cluster1/my/resource/pool when there's no datacenter specified. I've not found a way of supplying a datacenter wildcard.
One approach would be to reject anything except a single path element for compute-resource (e.g. mypool is okay, my/pool is not) in the case where there's no datacenter qualifier.

The datacenter should be required for create/delete/inspect as it's the toplevel namespace. It's only ls where empty is a concern.

[stuclem]

Added the following to the release notes:

---

• Deployment fails with a list failed error when you specify resources by name rather than by path. #4203
  Deployment of a VCH fails with an error about failing to find resources that you specified by name in the --compute-resource option. However, vic-machine suggests the resource that you specified as a valid resource.

   INFO Validating compute resource
   INFO Suggesting valid values for --compute-resource based on "cls" 
   INFO Failed to find resource pool in the provided path, showing all top level resource pools.
   INFO Suggested values for --compute-resource: 
   INFO "cls"ERROR resource pool 'cls' not found 
   ERROR List cannot continue - compute resource validation failed: validation of configuration failed
   vic-machine-linux ls failed: list failed
  

  Workaround: Specify the full path to the resource rather than just the resource name.

---

@mhagen-vmware does this describe the issue accurately?

[mhagen-vmware]

@stuclem lgtm

[stuclem]

Thanks @mhagen-vmware. Removing kind/note.

[dougm]

@jzt where are you at with this? I'm working on #3975 which is likely related, but start starting to get into the vic-machine part. I'll end up changing lib/install/management/finder.go at least, you might be on the same path.

Opened #4338 , Finder enhancement that may help in your case too.

[jzt]

@dougm I have a working solution that I don't think is particularly ideal, as it apparently should not work. :) I spoke with @hickeng and he mentioned that your govmomi changes should drastically simplify the suggestions code. I was going to wait for your merge (which I suspect will happen soon) before deciding whether or not I should put up a PR yet. Judging by what I'm seeing/hearing, I should probably try to incorporate your finder changes to get a solution that functions as expected.

[dougm]

@jzt ok Finder change is merged, ls change is in review. Lemme know if I can help with this one.