Repository: vmware/govmomi
Pull request #3574 (2024-10-04 02:32:10)

vcsim: handle traversal spec object updates
Handle the case when an object used for the traversal itself is updated (e.g. ListView).


Patch:
diff --git a/simulator/property_collector.go b/simulator/property_collector.go
index 95d703da..4fee683e 100644
--- a/simulator/property_collector.go
+++ b/simulator/property_collector.go
@@ -518,7 +518,7 @@ func (rr *retrieveResult) selectSet(ctx *Context, obj reflect.Value, s []types.B
 	return nil
 }
 
-func (pc *PropertyCollector) collect(ctx *Context, r *types.RetrievePropertiesEx) (*types.RetrieveResult, types.BaseMethodFault) {
+func collect(ctx *Context, r *types.RetrievePropertiesEx) (*types.RetrieveResult, types.BaseMethodFault) {
 	var refs []types.ManagedObjectReference
 
 	rr := &retrieveResult{
@@ -574,6 +574,8 @@ func (pc *PropertyCollector) CreateFilter(ctx *Context, c *types.CreateFilter) s
 		Returnval: filter.Self,
 	}
 
+	ctx.Map.AddHandler(filter)
+
 	return body
 }
 
@@ -666,7 +668,7 @@ func (pc *PropertyCollector) ContinueRetrievePropertiesEx(ctx *Context, r *types
 func (pc *PropertyCollector) RetrievePropertiesEx(ctx *Context, r *types.RetrievePropertiesEx) soap.HasFault {
 	body := &methods.RetrievePropertiesExBody{}
 
-	res, fault := pc.collect(ctx, r)
+	res, fault := collect(ctx, r)
 
 	if fault != nil {
 		switch fault.(type) {
@@ -782,10 +784,7 @@ func (pc *PropertyCollector) apply(ctx *Context, update *types.UpdateSet) types.
 	for _, ref := range pc.Filter {
 		filter := ctx.Session.Get(ref).(*PropertyFilter)
 
-		r := &types.RetrievePropertiesEx{}
-		r.SpecSet = append(r.SpecSet, filter.Spec)
-
-		res, fault := pc.collect(ctx, r)
+		res, fault := filter.collect(ctx)
 		if fault != nil {
 			return fault
 		}
@@ -951,6 +950,7 @@ func (pc *PropertyCollector) WaitForUpdatesEx(ctx *Context, r *types.WaitForUpda
 
 			for _, f := range pc.Filter {
 				filter := ctx.Session.Get(f).(*PropertyFilter)
+				filter.update(ctx)
 				fu := types.PropertyFilterUpdate{Filter: f}
 
 				for _, update := range updates {
diff --git a/simulator/property_filter.go b/simulator/property_filter.go
index 8871b179..98100608 100644
--- a/simulator/property_filter.go
+++ b/simulator/property_filter.go
@@ -31,8 +31,29 @@ type PropertyFilter struct {
 
 	pc   *PropertyCollector
 	refs map[types.ManagedObjectReference]struct{}
+	sync bool
 }
 
+func (f *PropertyFilter) UpdateObject(o mo.Reference, changes []types.PropertyChange) {
+	// A PropertyFilter's traversal spec is "applied" on the initial call to WaitForUpdates,
+	// with matching objects tracked in the `refs` field.
+	// New and deleted objects matching the filter are accounted for within PropertyCollector.
+	// But when an object used for the traversal itself is updated (e.g. ListView),
+	// we need to update the tracked `refs` on the next call to WaitForUpdates.
+	ref := o.Reference()
+
+	for _, set := range f.Spec.ObjectSet {
+		if set.Obj == ref && len(set.SelectSet) != 0 {
+			f.sync = true
+			break
+		}
+	}
+}
+
+func (_ *PropertyFilter) PutObject(_ mo.Reference) {}
+
+func (_ *PropertyFilter) RemoveObject(_ *Context, _ types.ManagedObjectReference) {}
+
 func (f *PropertyFilter) DestroyPropertyFilter(ctx *Context, c *types.DestroyPropertyFilter) soap.HasFault {
 	body := &methods.DestroyPropertyFilterBody{}
 
@@ -45,6 +66,21 @@ func (f *PropertyFilter) DestroyPropertyFilter(ctx *Context, c *types.DestroyPro
 	return body
 }
 
+func (f *PropertyFilter) collect(ctx *Context) (*types.RetrieveResult, types.BaseMethodFault) {
+	req := &types.RetrievePropertiesEx{
+		SpecSet: []types.PropertyFilterSpec{f.Spec},
+	}
+	return collect(ctx, req)
+}
+
+func (f *PropertyFilter) update(ctx *Context) {
+	if f.sync {
+		f.sync = false
+		clear(f.refs)
+		_, _ = f.collect(ctx)
+	}
+}
+
 // matches returns true if the change matches one of the filter Spec.PropSet
 func (f *PropertyFilter) matches(ctx *Context, ref types.ManagedObjectReference, change *types.PropertyChange) bool {
 	var kind reflect.Type
diff --git a/simulator/view_manager.go b/simulator/view_manager.go
index 94e1d526..0ed58926 100644
--- a/simulator/view_manager.go
+++ b/simulator/view_manager.go
@@ -275,9 +275,6 @@ func (v *ListView) ModifyListView(ctx *Context, req *types.ModifyListView) soap.
 
 	for _, ref := range req.Remove {
 		RemoveReference(&v.View, ref)
-		if ctx.Map.Get(ref) == nil {
-			body.Res.Returnval = append(body.Res.Returnval, ref)
-		}
 	}
 
 	if len(req.Remove) != 0 || len(req.Add) != 0 {


Test patch:
diff --git a/simulator/property_collector_test.go b/simulator/property_collector_test.go
index edf14bc1..40c4fc03 100644
--- a/simulator/property_collector_test.go
+++ b/simulator/property_collector_test.go
@@ -822,8 +822,6 @@ func TestPropertyCollectorInvalidSpecName(t *testing.T) {
 	obj := Map.Put(new(Folder))
 	folderPutChild(SpoofContext(), &obj.(*Folder).Folder, new(Folder))
 
-	pc := &PropertyCollector{}
-
 	req := types.RetrievePropertiesEx{
 		SpecSet: []types.PropertyFilterSpec{
 			{
@@ -853,7 +851,7 @@ func TestPropertyCollectorInvalidSpecName(t *testing.T) {
 		},
 	}
 
-	_, err := pc.collect(SpoofContext(), &req)
+	_, err := collect(SpoofContext(), &req)
 	if err == nil {
 		t.Fatal("expected error")
 	}
diff --git a/simulator/view_manager_test.go b/simulator/view_manager_test.go
index 8d3d68ba..e3181dcc 100644
--- a/simulator/view_manager_test.go
+++ b/simulator/view_manager_test.go
@@ -222,5 +222,14 @@ func TestListViewModify(t *testing.T) {
 		if len(refs) != 1 {
 			t.Errorf("unresolved refs=%s", refs)
 		}
+
+		refs, err = list.Remove(ctx, add)
+		if err != nil {
+			log.Fatal(err)
+		}
+
+		if len(refs) != 0 {
+			t.Errorf("unresolved refs=%s", refs)
+		}
 	})
 }
diff --git a/view/example_test.go b/view/example_test.go
index 8922ce11..edf807e8 100644
--- a/view/example_test.go
+++ b/view/example_test.go
@@ -22,6 +22,7 @@ import (
 	"log"
 	"sort"
 	"sync"
+	"time"
 
 	"github.com/vmware/govmomi/find"
 	"github.com/vmware/govmomi/object"
@@ -29,6 +30,7 @@ import (
 	"github.com/vmware/govmomi/simulator"
 	"github.com/vmware/govmomi/view"
 	"github.com/vmware/govmomi/vim25"
+	"github.com/vmware/govmomi/vim25/methods"
 	"github.com/vmware/govmomi/vim25/mo"
 	"github.com/vmware/govmomi/vim25/types"
 )
@@ -247,3 +249,181 @@ func ExampleListView_tasks() {
 	})
 	// Output: success=4
 }
+
+// viewOfVM returns a spec to traverse a ListView of ContainerView, for tracking VM updates.
+func viewOfVM(ref types.ManagedObjectReference, pathSet []string) types.CreateFilter {
+	return types.CreateFilter{
+		Spec: types.PropertyFilterSpec{
+			ObjectSet: []types.ObjectSpec{{
+				Obj:  ref,
+				Skip: types.NewBool(true),
+				SelectSet: []types.BaseSelectionSpec{
+					// ListView --> ContainerView
+					&types.TraversalSpec{
+						Type: "ListView",
+						Path: "view",
+						SelectSet: []types.BaseSelectionSpec{
+							&types.SelectionSpec{
+								Name: "visitViews",
+							},
+						},
+					},
+					// ContainerView --> VM
+					&types.TraversalSpec{
+						SelectionSpec: types.SelectionSpec{
+							Name: "visitViews",
+						},
+						Type: "ContainerView",
+						Path: "view",
+					},
+				},
+			}},
+			PropSet: []types.PropertySpec{{
+				Type:    "VirtualMachine",
+				PathSet: pathSet,
+			}},
+		},
+	}
+}
+
+// Example of using WaitForUpdates with a ListView of ContainerView.
+// Each container root is a Cluster view of VirtualMachines.
+// Modifying the ListView changes which VirtualMachine updates are returned by WaitForUpdates.
+func ExampleListView_ofContainerView() {
+	model := simulator.VPX()
+	model.Cluster = 3
+
+	simulator.Run(func(ctx context.Context, c *vim25.Client) error {
+		m := view.NewManager(c)
+
+		kind := []string{"ClusterComputeResource"}
+		root, err := m.CreateContainerView(ctx, c.ServiceContent.RootFolder, kind, true)
+		if err != nil {
+			return err
+		}
+
+		clusters, err := root.Find(ctx, kind, property.Match{})
+		if err != nil {
+			return err
+		}
+
+		list, err := m.CreateListView(ctx, nil)
+		if err != nil {
+			return err
+		}
+
+		kind = []string{"VirtualMachine"}
+		views := make(map[types.ManagedObjectReference]*view.ContainerView)
+
+		for _, cluster := range clusters {
+			cv, err := m.CreateContainerView(ctx, cluster, kind, true)
+			if err != nil {
+				return err
+			}
+
+			_, err = list.Add(ctx, []types.ManagedObjectReference{cv.Reference()})
+
+			views[cluster] = cv
+		}
+
+		pc := property.DefaultCollector(c)
+
+		pathSet := []string{"config.extraConfig"}
+		pf, err := pc.CreateFilter(ctx, viewOfVM(list.Reference(), pathSet))
+		if err != nil {
+			return err
+		}
+		defer pf.Destroy(ctx)
+
+		// MaxWaitSeconds value of 0 causes WaitForUpdatesEx to do one update calculation and return any results.
+		req := types.WaitForUpdatesEx{
+			This: pc.Reference(),
+			Options: &types.WaitOptions{
+				MaxWaitSeconds: types.NewInt32(0),
+			},
+		}
+
+		updates := func() ([]types.ObjectUpdate, error) {
+			res, err := methods.WaitForUpdatesEx(ctx, c.Client, &req)
+			if err != nil {
+				return nil, err
+			}
+
+			set := res.Returnval
+
+			if set == nil {
+				return nil, nil
+			}
+
+			req.Version = set.Version
+
+			if len(set.FilterSet) == 0 {
+				return nil, nil
+			}
+
+			return set.FilterSet[0].ObjectSet, nil
+		}
+
+		set, err := updates()
+		if err != nil {
+			return err
+		}
+
+		all := make([]types.ManagedObjectReference, len(set))
+		for i := range set {
+			all[i] = set[i].Obj
+		}
+
+		fmt.Printf("Initial updates=%d\n", len(all))
+
+		reconfig := func() {
+			spec := types.VirtualMachineConfigSpec{
+				ExtraConfig: []types.BaseOptionValue{
+					&types.OptionValue{Key: "time", Value: time.Now().String()},
+				},
+			}
+			// Change state of all VMs in all Clusters
+			for _, ref := range all {
+				task, err := object.NewVirtualMachine(c, ref).Reconfigure(ctx, spec)
+				if err != nil {
+					panic(err)
+				}
+				if err = task.Wait(ctx); err != nil {
+					panic(err)
+				}
+			}
+
+			set, err := updates()
+			if err != nil {
+				panic(err)
+			}
+
+			fmt.Printf("Reconfig updates=%d\n", len(set))
+		}
+
+		reconfig()
+
+		cluster := views[clusters[0]].Reference()
+		_, err = list.Remove(ctx, []types.ManagedObjectReference{cluster})
+		if err != nil {
+			return err
+		}
+
+		reconfig()
+
+		_, err = list.Add(ctx, []types.ManagedObjectReference{cluster})
+		if err != nil {
+			return err
+		}
+
+		reconfig()
+
+		return nil
+	}, model)
+
+	// Output:
+	// Initial updates=6
+	// Reconfig updates=6
+	// Reconfig updates=4
+	// Reconfig updates=6
+}