Add option to propagate MissingSet faults in property.WaitForUpdates

Leaving PropagateMissing disabled by default; we don't want to change the behavior
for everything as it may surface benign faults in existing programs.

- Use PropagateMissing=true in task.Wait() function as failure to collect the
  Task info field must break out of the wait loop.

- vcsim: support override of per-session instance of the PropertyCollector singleton

- Add WaitForUpdates test for MissingSet faults

property/wait.go

@@ -20,13 +20,15 @@ import (
 	"context"
 
 	"github.com/vmware/govmomi/vim25/methods"
+	"github.com/vmware/govmomi/vim25/soap"
 	"github.com/vmware/govmomi/vim25/types"
 )
 
 // WaitFilter provides helpers to construct a types.CreateFilter for use with property.Wait
 type WaitFilter struct {
 	types.CreateFilter
-	Options *types.WaitOptions
+	Options          *types.WaitOptions
+	PropagateMissing bool
 }
 
 // Add a new ObjectSpec and PropertySpec to the WaitFilter
@@ -81,6 +83,8 @@ func Wait(ctx context.Context, c *Collector, obj types.ManagedObjectReference, p
 // The newly created collector is destroyed before this function returns (both
 // in case of success or error).
 //
+// By default, ObjectUpdate.MissingSet faults are not propagated to the returned error,
+// set WaitFilter.PropagateMissing=true to enable MissingSet fault propagation.
 func WaitForUpdates(ctx context.Context, c *Collector, filter *WaitFilter, f func([]types.ObjectUpdate) bool) error {
 	p, err := c.Create(ctx)
 	if err != nil {
@@ -125,6 +129,15 @@ func WaitForUpdates(ctx context.Context, c *Collector, filter *WaitFilter, f fun
 		req.Version = set.Version
 
 		for _, fs := range set.FilterSet {
+			if filter.PropagateMissing {
+				for i := range fs.ObjectSet {
+					for _, p := range fs.ObjectSet[i].MissingSet {
+						// Same behavior as mo.ObjectContentToType()
+						return soap.WrapVimFault(p.Fault.Fault)
+					}
+				}
+			}
+
 			if f(fs.ObjectSet) {
 				return nil
 			}

property/wait_test.go

@@ -0,0 +1,128 @@
+/*
+Copyright (c) 2019 VMware, Inc. All Rights Reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package property_test
+
+import (
+	"context"
+	"log"
+	"testing"
+
+	"github.com/vmware/govmomi"
+	"github.com/vmware/govmomi/object"
+	"github.com/vmware/govmomi/simulator"
+	"github.com/vmware/govmomi/vim25/methods"
+	"github.com/vmware/govmomi/vim25/soap"
+	"github.com/vmware/govmomi/vim25/types"
+)
+
+type PropertyCollector struct {
+	simulator.PropertyCollector
+}
+
+// CreatePropertyCollector overrides the vcsim impl to return this test's PC impl
+func (pc *PropertyCollector) CreatePropertyCollector(ctx *simulator.Context, c *types.CreatePropertyCollector) soap.HasFault {
+	return &methods.CreatePropertyCollectorBody{
+		Res: &types.CreatePropertyCollectorResponse{
+			Returnval: ctx.Session.Put(new(PropertyCollector)).Reference(),
+		},
+	}
+}
+
+// WaitForUpdatesEx overrides the vcsim impl to inject a fault via MissingSet
+func (pc *PropertyCollector) WaitForUpdatesEx(ctx *simulator.Context, r *types.WaitForUpdatesEx) soap.HasFault {
+	filter := ctx.Session.Get(pc.Filter[0]).(*simulator.PropertyFilter)
+
+	if r.Version != "" {
+		// Client should fail on the first response w/ MissingSet.
+		// This ensures we don't get into a tight loop if that doesn't happen.
+		select {}
+	}
+
+	return &methods.WaitForUpdatesExBody{
+		Res: &types.WaitForUpdatesExResponse{
+			Returnval: &types.UpdateSet{
+				Version: "-",
+				FilterSet: []types.PropertyFilterUpdate{{
+					Filter: filter.Reference(),
+					ObjectSet: []types.ObjectUpdate{{
+						Kind: types.ObjectUpdateKindEnter,
+						Obj:  filter.Spec.ObjectSet[0].Obj,
+						MissingSet: []types.MissingProperty{{
+							Path: "info",
+							Fault: types.LocalizedMethodFault{
+								Fault: new(types.NoPermission),
+							},
+						}},
+					}},
+				}},
+			},
+		},
+	}
+}
+
+// Test that task.Wait() propagates MissingSet errors
+func TestWaitPermissionFault(t *testing.T) {
+	ctx := context.Background()
+
+	model := simulator.ESX()
+
+	defer model.Remove()
+	err := model.Create()
+	if err != nil {
+		log.Fatal(err)
+	}
+
+	s := model.Service.NewServer()
+	defer s.Close()
+
+	c, _ := govmomi.NewClient(ctx, s.URL, true)
+
+	pc := new(PropertyCollector)
+	pc.Self = model.ServiceContent.PropertyCollector
+	simulator.Map.Put(pc)
+
+	dm := object.NewVirtualDiskManager(c.Client)
+
+	spec := &types.FileBackedVirtualDiskSpec{
+		VirtualDiskSpec: types.VirtualDiskSpec{
+			AdapterType: string(types.VirtualDiskAdapterTypeLsiLogic),
+			DiskType:    string(types.VirtualDiskTypeThin),
+		},
+		CapacityKb: 1024 * 1024,
+	}
+
+	name := "[LocalDS_0] disk1.vmdk"
+
+	task, err := dm.CreateVirtualDisk(ctx, name, nil, spec)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	err = task.Wait(ctx)
+	if err == nil {
+		t.Fatal("expected error")
+	}
+
+	if !soap.IsVimFault(err) {
+		t.Fatal("expected vim fault")
+	}
+
+	fault, ok := soap.ToVimFault(err).(*types.NoPermission)
+	if !ok {
+		t.Fatalf("unexpected vim fault: %T", fault)
+	}
+}

simulator/session_manager.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"fmt"
 	"net/http"
+	"reflect"
 	"strings"
 	"time"
 
@@ -365,7 +366,13 @@ func (s *Session) Get(ref types.ManagedObjectReference) mo.Reference {
 		return &m
 	case "PropertyCollector":
 		if ref == Map.content().PropertyCollector {
-			return s.Put(NewPropertyCollector(ref))
+			// Per-session instance of the PropertyCollector singleton.
+			// Using reflection here as PropertyCollector might be wrapped with a custom type.
+			obj = Map.Get(ref)
+			pc := reflect.New(reflect.TypeOf(obj).Elem())
+			obj = pc.Interface().(mo.Reference)
+			s.Registry.setReference(obj, ref)
+			return s.Put(obj)
 		}
 	}
 

task/wait.go

@@ -123,7 +123,18 @@ func Wait(ctx context.Context, ref types.ManagedObjectReference, pc *property.Co
 		defer close(cb.ch)
 	}
 
-	err := property.Wait(ctx, pc, ref, []string{"info"}, cb.fn)
+	filter := &property.WaitFilter{PropagateMissing: true}
+	filter.Add(ref, ref.Type, []string{"info"})
+
+	err := property.WaitForUpdates(ctx, pc, filter, func(updates []types.ObjectUpdate) bool {
+		for _, update := range updates {
+			if cb.fn(update.ChangeSet) {
+				return true
+			}
+		}
+
+		return false
+	})
 	if err != nil {
 		return nil, err
 	}