Fix finder inconsistencies in vsphere input (#6245)

Gopkg.lock

@@ -1177,7 +1177,6 @@
   name = "github.com/vmware/govmomi"
   packages = [
     ".",
-    "find",
     "list",
     "nfc",
     "object",

plugins/inputs/vsphere/finder.go

@@ -16,6 +16,8 @@ var childTypes map[string][]string
 
 var addFields map[string][]string
 
+var containers map[string]interface{}
+
 // Finder allows callers to find resources in vCenter given a query string.
 type Finder struct {
 	client *Client
@@ -29,11 +31,6 @@ type ResourceFilter struct {
 	paths   []string
 }
 
-type nameAndRef struct {
-	name string
-	ref  types.ManagedObjectReference
-}
-
 // FindAll returns the union of resources found given the supplied resource type and paths.
 func (f *Finder) FindAll(ctx context.Context, resType string, paths []string, dst interface{}) error {
 	for _, p := range paths {
@@ -87,15 +84,18 @@ func (f *Finder) descend(ctx context.Context, root types.ManagedObjectReference,
 	var content []types.ObjectContent
 
 	fields := []string{"name"}
+	recurse := tokens[pos]["name"] == "**"
+
+	types := ct
 	if isLeaf {
-		// Special case: The last token is a recursive wildcard, so we can grab everything
+		if af, ok := addFields[resType]; ok {
-		// recursively in a single call.
+			fields = append(fields, af...)
-		if tokens[pos]["name"] == "**" {
+		}
+		if recurse {
+			// Special case: The last token is a recursive wildcard, so we can grab everything
+			// recursively in a single call.
 			v2, err := m.CreateContainerView(ctx, root, []string{resType}, true)
 			defer v2.Destroy(ctx)
-			if af, ok := addFields[resType]; ok {
-				fields = append(fields, af...)
-			}
 			err = v2.Retrieve(ctx, []string{resType}, fields, &content)
 			if err != nil {
 				return err
@@ -105,23 +105,16 @@ func (f *Finder) descend(ctx context.Context, root types.ManagedObjectReference,
 			}
 			return nil
 		}
-
+		types = []string{resType} // Only load wanted object type at leaf level
-		if af, ok := addFields[resType]; ok {
+	}
-			fields = append(fields, af...)
+	err = v.Retrieve(ctx, types, fields, &content)
-		}
+	if err != nil {
-		err = v.Retrieve(ctx, []string{resType}, fields, &content)
+		return err
-		if err != nil {
-			return err
-		}
-	} else {
-		err = v.Retrieve(ctx, ct, fields, &content)
-		if err != nil {
-			return err
-		}
 	}
 
+	rerunAsLeaf := false
 	for _, c := range content {
-		if !tokens[pos].MatchPropertyList(c.PropSet[:1]) {
+		if !matchName(tokens[pos], c.PropSet) {
 			continue
 		}
 
@@ -137,44 +130,45 @@ func (f *Finder) descend(ctx context.Context, root types.ManagedObjectReference,
 		}
 
 		// Deal with recursive wildcards (**)
-		inc := 1 // Normally we advance one token.
+		var inc int
-		if tokens[pos]["name"] == "**" {
+		if recurse {
-			if isLeaf {
+			inc = 0 // By default, we stay on this token
-				inc = 0 // Can't advance past last token, so keep descending the tree
+			if !isLeaf {
-			} else {
+				// Lookahead to next token.
-				// Lookahead to next token. If it matches this child, we are out of
+				if matchName(tokens[pos+1], c.PropSet) {
-				// the recursive wildcard handling and we can advance TWO tokens ahead, since
+					// Are we looking ahead at a leaf node that has the wanted type?
-				// the token that ended the recursive wildcard mode is now consumed.
+					// Rerun the entire level as a leaf. This is needed since all properties aren't loaded
-				if tokens[pos+1].MatchPropertyList(c.PropSet) {
+					// when we're processing non-leaf nodes.
-					if pos < len(tokens)-2 {
+					if pos == len(tokens)-2 {
+						if c.Obj.Type == resType {
+							rerunAsLeaf = true
+							continue
+						}
+					} else if _, ok := containers[c.Obj.Type]; ok {
+						// Tokens match and we're looking ahead at a container type that's not a leaf
+						// Consume this token and the next.
 						inc = 2
-					} else {
-						// We found match and it's at a leaf! Grab it!
-						objs[c.Obj.String()] = c
-						continue
 					}
-				} else {
-					// We didn't break out of recursicve wildcard mode yet, so stay on this token.
-					inc = 0
-
 				}
 			}
+		} else {
+			// The normal case: Advance to next token before descending
+			inc = 1
 		}
 		err := f.descend(ctx, c.Obj, resType, tokens, pos+inc, objs)
 		if err != nil {
 			return err
 		}
 	}
-	return nil
-}
 
-func nameFromObjectContent(o types.ObjectContent) string {
+	if rerunAsLeaf {
-	for _, p := range o.PropSet {
+		// We're at a "pseudo leaf", i.e. we looked ahead a token and found that this level contains leaf nodes.
-		if p.Name == "name" {
+		// Rerun the entire level as a leaf to get those nodes. This will only be executed when pos is one token
-			return p.Val.(string)
+		// before the last, to pos+1 will always point to a leaf token.
-		}
+		return f.descend(ctx, root, resType, tokens, pos+1, objs)
 	}
-	return "<unknown>"
+
+	return nil
 }
 
 func objectContentToTypedArray(objs map[string]types.ObjectContent, dst interface{}) error {
@@ -214,11 +208,20 @@ func (r *ResourceFilter) FindAll(ctx context.Context, dst interface{}) error {
 	return r.finder.FindAll(ctx, r.resType, r.paths, dst)
 }
 
+func matchName(f property.Filter, props []types.DynamicProperty) bool {
+	for _, prop := range props {
+		if prop.Name == "name" {
+			return f.MatchProperty(prop)
+		}
+	}
+	return false
+}
+
 func init() {
 	childTypes = map[string][]string{
 		"HostSystem":             {"VirtualMachine"},
-		"ComputeResource":        {"HostSystem", "ResourcePool"},
+		"ComputeResource":        {"HostSystem", "ResourcePool", "VirtualApp"},
-		"ClusterComputeResource": {"HostSystem", "ResourcePool"},
+		"ClusterComputeResource": {"HostSystem", "ResourcePool", "VirtualApp"},
 		"Datacenter":             {"Folder"},
 		"Folder": {
 			"Folder",
@@ -238,4 +241,13 @@ func init() {
 		"ClusterComputeResource": {"parent"},
 		"Datacenter":             {"parent"},
 	}
+
+	containers = map[string]interface{}{
+		"HostSystem":      nil,
+		"ComputeResource": nil,
+		"Datacenter":      nil,
+		"ResourcePool":    nil,
+		"Folder":          nil,
+		"VirtualApp":      nil,
+	}
 }

plugins/inputs/vsphere/vsphere_test.go

@@ -155,9 +155,13 @@ func defaultVSphere() *VSphere {
 	}
 }
 
-func createSim() (*simulator.Model, *simulator.Server, error) {
+func createSim(folders int) (*simulator.Model, *simulator.Server, error) {
 	model := simulator.VPX()
 
+	model.Folder = folders
+	model.Datacenter = 2
+	//model.App = 1
+
 	err := model.Create()
 	if err != nil {
 		return nil, nil, err
@@ -262,7 +266,7 @@ func TestMaxQuery(t *testing.T) {
 	if unsafe.Sizeof(i) < 8 {
 		return
 	}
-	m, s, err := createSim()
+	m, s, err := createSim(0)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -298,6 +302,20 @@ func TestMaxQuery(t *testing.T) {
 	c2.close()
 }
 
+func testLookupVM(ctx context.Context, t *testing.T, f *Finder, path string, expected int, expectedName string) {
+	poweredOn := types.VirtualMachinePowerState("poweredOn")
+	var vm []mo.VirtualMachine
+	err := f.Find(ctx, "VirtualMachine", path, &vm)
+	require.NoError(t, err)
+	require.Equal(t, expected, len(vm))
+	if expectedName != "" {
+		require.Equal(t, expectedName, vm[0].Name)
+	}
+	for _, v := range vm {
+		require.Equal(t, poweredOn, v.Runtime.PowerState)
+	}
+}
+
 func TestFinder(t *testing.T) {
 	// Don't run test on 32-bit machines due to bug in simulator.
 	// https://github.com/vmware/govmomi/issues/1330
@@ -306,7 +324,7 @@ func TestFinder(t *testing.T) {
 		return
 	}
 
-	m, s, err := createSim()
+	m, s, err := createSim(0)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -320,13 +338,13 @@ func TestFinder(t *testing.T) {
 
 	f := Finder{c}
 
-	dc := []mo.Datacenter{}
+	var dc []mo.Datacenter
 	err = f.Find(ctx, "Datacenter", "/DC0", &dc)
 	require.NoError(t, err)
 	require.Equal(t, 1, len(dc))
 	require.Equal(t, "DC0", dc[0].Name)
 
-	host := []mo.HostSystem{}
+	var host []mo.HostSystem
 	err = f.Find(ctx, "HostSystem", "/DC0/host/DC0_H0/DC0_H0", &host)
 	require.NoError(t, err)
 	require.Equal(t, 1, len(host))
@@ -343,67 +361,64 @@ func TestFinder(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, 3, len(host))
 
-	vm := []mo.VirtualMachine{}
+	var vm []mo.VirtualMachine
-	err = f.Find(ctx, "VirtualMachine", "/DC0/vm/DC0_H0_VM0", &vm)
+	testLookupVM(ctx, t, &f, "/DC0/vm/DC0_H0_VM0", 1, "")
-	require.NoError(t, err)
+	testLookupVM(ctx, t, &f, "/DC0/vm/DC0_C0*", 2, "")
-	require.Equal(t, 1, len(dc))
+	testLookupVM(ctx, t, &f, "/DC0/*/DC0_H0_VM0", 1, "DC0_H0_VM0")
-	require.Equal(t, "DC0_H0_VM0", vm[0].Name)
+	testLookupVM(ctx, t, &f, "/DC0/*/DC0_H0_*", 2, "")
-
+	testLookupVM(ctx, t, &f, "/DC0/**/DC0_H0_VM*", 2, "")
-	vm = []mo.VirtualMachine{}
+	testLookupVM(ctx, t, &f, "/DC0/**", 4, "")
-	err = f.Find(ctx, "VirtualMachine", "/DC0/vm/DC0_C0*", &vm)
+	testLookupVM(ctx, t, &f, "/DC1/**", 4, "")
-	require.NoError(t, err)
+	testLookupVM(ctx, t, &f, "/**", 8, "")
-	require.Equal(t, 1, len(dc))
+	testLookupVM(ctx, t, &f, "/**/vm/**", 8, "")
-
+	testLookupVM(ctx, t, &f, "/*/host/**/*DC*", 8, "")
-	vm = []mo.VirtualMachine{}
+	testLookupVM(ctx, t, &f, "/*/host/**/*DC*VM*", 8, "")
-	err = f.Find(ctx, "VirtualMachine", "/DC0/*/DC0_H0_VM0", &vm)
+	testLookupVM(ctx, t, &f, "/*/host/**/*DC*/*/*DC*", 4, "")
-	require.NoError(t, err)
-	require.Equal(t, 1, len(dc))
-	require.Equal(t, "DC0_H0_VM0", vm[0].Name)
-
-	vm = []mo.VirtualMachine{}
-	err = f.Find(ctx, "VirtualMachine", "/DC0/*/DC0_H0_*", &vm)
-	require.NoError(t, err)
-	require.Equal(t, 2, len(vm))
-
-	vm = []mo.VirtualMachine{}
-	err = f.Find(ctx, "VirtualMachine", "/DC0/**/DC0_H0_VM*", &vm)
-	require.NoError(t, err)
-	require.Equal(t, 2, len(vm))
-
-	vm = []mo.VirtualMachine{}
-	err = f.Find(ctx, "VirtualMachine", "/DC0/**", &vm)
-	require.NoError(t, err)
-	require.Equal(t, 4, len(vm))
-
-	vm = []mo.VirtualMachine{}
-	err = f.Find(ctx, "VirtualMachine", "/**", &vm)
-	require.NoError(t, err)
-	require.Equal(t, 4, len(vm))
-
-	vm = []mo.VirtualMachine{}
-	err = f.Find(ctx, "VirtualMachine", "/**/DC0_H0_VM*", &vm)
-	require.NoError(t, err)
-	require.Equal(t, 2, len(vm))
-
-	vm = []mo.VirtualMachine{}
-	err = f.Find(ctx, "VirtualMachine", "/DC0/**/DC0_H0_VM*", &vm)
-	require.NoError(t, err)
-	require.Equal(t, 2, len(vm))
-
-	vm = []mo.VirtualMachine{}
-	err = f.Find(ctx, "VirtualMachine", "/**/vm/**", &vm)
-	require.NoError(t, err)
-	require.Equal(t, 4, len(vm))
 
 	vm = []mo.VirtualMachine{}
 	err = f.FindAll(ctx, "VirtualMachine", []string{"/DC0/vm/DC0_H0*", "/DC0/vm/DC0_C0*"}, &vm)
 	require.NoError(t, err)
 	require.Equal(t, 4, len(vm))
 
-	vm = []mo.VirtualMachine{}
+}
-	err = f.FindAll(ctx, "VirtualMachine", []string{"/**"}, &vm)
+
+func TestFolders(t *testing.T) {
+	// Don't run test on 32-bit machines due to bug in simulator.
+	// https://github.com/vmware/govmomi/issues/1330
+	var i int
+	if unsafe.Sizeof(i) < 8 {
+		return
+	}
+
+	m, s, err := createSim(1)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer m.Remove()
+	defer s.Close()
+
+	v := defaultVSphere()
+	ctx := context.Background()
+
+	c, err := NewClient(ctx, s.URL, v)
+
+	f := Finder{c}
+
+	var folder []mo.Folder
+	err = f.Find(ctx, "Folder", "/F0", &folder)
 	require.NoError(t, err)
-	require.Equal(t, 4, len(vm))
+	require.Equal(t, 1, len(folder))
+	require.Equal(t, "F0", folder[0].Name)
+
+	var dc []mo.Datacenter
+	err = f.Find(ctx, "Datacenter", "/F0/DC1", &dc)
+	require.NoError(t, err)
+	require.Equal(t, 1, len(dc))
+	require.Equal(t, "DC1", dc[0].Name)
+
+	testLookupVM(ctx, t, &f, "/F0/DC0/vm/**/F*", 0, "")
+	testLookupVM(ctx, t, &f, "/F0/DC1/vm/**/F*/*VM*", 4, "")
+	testLookupVM(ctx, t, &f, "/F0/DC1/vm/**/F*/**", 4, "")
 }
 
 func TestAll(t *testing.T) {
@@ -414,7 +429,7 @@ func TestAll(t *testing.T) {
 		return
 	}
 
-	m, s, err := createSim()
+	m, s, err := createSim(0)
 	if err != nil {
 		t.Fatal(err)
 	}