From 95aa2571dac212bfb067147a2bcf85b0eeb9afa6 Mon Sep 17 00:00:00 2001 From: akutz Date: Wed, 10 Jan 2024 08:14:51 -0600 Subject: [PATCH] api: WaitForUpdatesEx & DestroyPropertyFilter This patch introduces Ex variants for many of the property collector types/methods to take advantage of the remote API DestroyPropertyFilter. Now it is possible to use a single property collector for waiting for updates and remove previously added property filters. BREAKING: The semantics around the helper functions in the property package have changed. Please review any code that calls this package to ensure it is compatible with the new behaviors. --- find/finder.go | 2 +- govc/object/collect.go | 6 +- govc/object/find.go | 4 +- object/example_test.go | 2 +- object/task.go | 44 +++++- pbm/client_test.go | 2 +- pbm/simulator/simulator_test.go | 2 +- property/collector.go | 115 ++++++++++++++-- property/collector_test.go | 191 ++++++++++++++++++++++++++ property/example_test.go | 191 ++++++++++++++++++++++++++ property/filter.go | 164 ++++------------------ property/filter_test.go | 45 +----- property/match.go | 170 +++++++++++++++++++++++ property/match_test.go | 61 ++++++++ property/property_test.go | 21 +++ property/wait.go | 137 ++++++++++-------- property/wait_test.go | 139 +++++++++++++------ simulator/property_collector_test.go | 4 +- simulator/session_manager_test.go | 2 +- task/wait.go | 39 ++++-- vapi/namespace/simulator/simulator.go | 2 +- vapi/tags/example_test.go | 2 +- view/container_view.go | 16 +-- view/example_test.go | 4 +- 24 files changed, 1035 insertions(+), 330 deletions(-) create mode 100644 property/collector_test.go create mode 100644 property/match.go create mode 100644 property/match_test.go create mode 100644 property/property_test.go diff --git a/find/finder.go b/find/finder.go index 4830fc26e..887ddd5c7 100644 --- a/find/finder.go +++ b/find/finder.go @@ -835,7 +835,7 @@ func (f *Finder) networkByID(ctx context.Context, path string) (object.NetworkRe } defer v.Destroy(ctx) - filter := property.Filter{ + filter := property.Match{ "config.logicalSwitchUuid": path, "config.segmentId": path, } diff --git a/govc/object/collect.go b/govc/object/collect.go index d6f660dba..0689d3da5 100644 --- a/govc/object/collect.go +++ b/govc/object/collect.go @@ -53,7 +53,7 @@ type collect struct { kind kinds wait time.Duration - filter property.Filter + filter property.Match obj string } @@ -263,7 +263,7 @@ func (cmd *collect) match(update types.ObjectUpdate) bool { } for _, c := range update.ChangeSet { - if cmd.filter.MatchProperty(types.DynamicProperty{Name: c.Name, Val: c.Val}) { + if cmd.filter.Property(types.DynamicProperty{Name: c.Name, Val: c.Val}) { return true } } @@ -279,7 +279,7 @@ func (cmd *collect) toFilter(f *flag.FlagSet, props []string) ([]string, error) return props, nil } - cmd.filter = property.Filter{props[0][1:]: props[1]} + cmd.filter = property.Match{props[0][1:]: props[1]} return cmd.filter.Keys(), nil } diff --git a/govc/object/find.go b/govc/object/find.go index 57469d5d7..83adc98e5 100644 --- a/govc/object/find.go +++ b/govc/object/find.go @@ -165,7 +165,7 @@ Examples: } // rootMatch returns true if the root object path should be printed -func (cmd *find) rootMatch(ctx context.Context, root object.Reference, client *vim25.Client, filter property.Filter) bool { +func (cmd *find) rootMatch(ctx context.Context, root object.Reference, client *vim25.Client, filter property.Match) bool { ref := root.Reference() if !cmd.kind.wanted(ref.Type) { @@ -285,7 +285,7 @@ func (cmd *find) Run(ctx context.Context, f *flag.FlagSet) error { } } - filter := property.Filter{} + filter := property.Match{} if len(props)%2 != 0 { return flag.ErrHelp diff --git a/object/example_test.go b/object/example_test.go index 5b125c506..9b2b774eb 100644 --- a/object/example_test.go +++ b/object/example_test.go @@ -446,7 +446,7 @@ func ExampleCustomFieldsManager_Set() { } // filter used to find objects with "backup=true" - filter := property.Filter{"customValue": &types.CustomFieldStringValue{ + filter := property.Match{"customValue": &types.CustomFieldStringValue{ CustomFieldValue: types.CustomFieldValue{Key: field.Key}, Value: "true", }} diff --git a/object/task.go b/object/task.go index 088dd7f56..373407cc4 100644 --- a/object/task.go +++ b/object/task.go @@ -1,11 +1,11 @@ /* -Copyright (c) 2015 VMware, Inc. All Rights Reserved. +Copyright (c) 2015-2024 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 +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, @@ -18,6 +18,7 @@ package object import ( "context" + "fmt" "github.com/vmware/govmomi/property" "github.com/vmware/govmomi/task" @@ -43,18 +44,53 @@ func NewTask(c *vim25.Client, ref types.ManagedObjectReference) *Task { return &t } +// Deprecated: Please use WaitEx instead. func (t *Task) Wait(ctx context.Context) error { _, err := t.WaitForResult(ctx, nil) return err } -func (t *Task) WaitForResult(ctx context.Context, s ...progress.Sinker) (*types.TaskInfo, error) { +// Deprecated: Please use WaitForResultEx instead. +func (t *Task) WaitForResult(ctx context.Context, s ...progress.Sinker) (taskInfo *types.TaskInfo, result error) { + var pr progress.Sinker + if len(s) == 1 { + pr = s[0] + } + p, err := property.DefaultCollector(t.c).Create(ctx) + if err != nil { + return nil, err + } + + // Attempt to destroy the collector using the background context, as the + // specified context may have timed out or have been canceled. + defer func() { + if err := p.Destroy(context.Background()); err != nil { + if result == nil { + result = err + } else { + result = fmt.Errorf( + "destroy property collector failed with %s after failing to wait for updates: %w", + err, + result) + } + } + }() + + return task.WaitEx(ctx, t.Reference(), p, pr) +} + +func (t *Task) WaitEx(ctx context.Context) error { + _, err := t.WaitForResultEx(ctx, nil) + return err +} + +func (t *Task) WaitForResultEx(ctx context.Context, s ...progress.Sinker) (*types.TaskInfo, error) { var pr progress.Sinker if len(s) == 1 { pr = s[0] } p := property.DefaultCollector(t.c) - return task.Wait(ctx, t.Reference(), p, pr) + return task.WaitEx(ctx, t.Reference(), p, pr) } func (t *Task) Cancel(ctx context.Context) error { diff --git a/pbm/client_test.go b/pbm/client_test.go index feebbc4ab..a3666e946 100644 --- a/pbm/client_test.go +++ b/pbm/client_test.go @@ -125,7 +125,7 @@ func TestClient(t *testing.T) { } else { var cluster mo.ClusterComputeResource - err = v.RetrieveWithFilter(ctx, kind, []string{"datastore"}, &cluster, property.Filter{"name": clusterName}) + err = v.RetrieveWithFilter(ctx, kind, []string{"datastore"}, &cluster, property.Match{"name": clusterName}) if err != nil { t.Fatal(err) } diff --git a/pbm/simulator/simulator_test.go b/pbm/simulator/simulator_test.go index 7263c407d..1ff82f711 100644 --- a/pbm/simulator/simulator_test.go +++ b/pbm/simulator/simulator_test.go @@ -128,7 +128,7 @@ func TestSimulator(t *testing.T) { } else { var cluster mo.ClusterComputeResource - err = v.RetrieveWithFilter(ctx, kind, []string{"datastore"}, &cluster, property.Filter{"name": clusterName}) + err = v.RetrieveWithFilter(ctx, kind, []string{"datastore"}, &cluster, property.Match{"name": clusterName}) if err != nil { t.Fatal(err) } diff --git a/property/collector.go b/property/collector.go index 16bf22266..ec4267255 100644 --- a/property/collector.go +++ b/property/collector.go @@ -1,5 +1,5 @@ /* -Copyright (c) 2015-2023 VMware, Inc. All Rights Reserved. +Copyright (c) 2015-2024 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. @@ -19,6 +19,8 @@ package property import ( "context" "errors" + "fmt" + "sync" "github.com/vmware/govmomi/vim25" "github.com/vmware/govmomi/vim25/methods" @@ -27,11 +29,19 @@ import ( "github.com/vmware/govmomi/vim25/types" ) +// ErrConcurrentCollector is returned from WaitForUpdates, WaitForUpdatesEx, +// or CheckForUpdates if any of those calls are unable to obtain an exclusive +// lock for the property collector. +var ErrConcurrentCollector = fmt.Errorf( + "only one goroutine may invoke WaitForUpdates, WaitForUpdatesEx, " + + "or CheckForUpdates on a given PropertyCollector") + // Collector models the PropertyCollector managed object. // // For more information, see: // http://pubs.vmware.com/vsphere-60/index.jsp?topic=%2Fcom.vmware.wssdk.apiref.doc%2Fvmodl.query.PropertyCollector.html type Collector struct { + mu sync.Mutex roundTripper soap.RoundTripper reference types.ManagedObjectReference } @@ -46,7 +56,7 @@ func DefaultCollector(c *vim25.Client) *Collector { return &p } -func (p Collector) Reference() types.ManagedObjectReference { +func (p *Collector) Reference() types.ManagedObjectReference { return p.reference } @@ -85,18 +95,28 @@ func (p *Collector) Destroy(ctx context.Context) error { return nil } -func (p *Collector) CreateFilter(ctx context.Context, req types.CreateFilter) error { +func (p *Collector) CreateFilter(ctx context.Context, req types.CreateFilter) (*Filter, error) { req.This = p.Reference() - _, err := methods.CreateFilter(ctx, p.roundTripper, &req) + resp, err := methods.CreateFilter(ctx, p.roundTripper, &req) if err != nil { - return err + return nil, err } - return nil + return &Filter{roundTripper: p.roundTripper, reference: resp.Returnval}, nil } -func (p *Collector) WaitForUpdates(ctx context.Context, version string, opts ...*types.WaitOptions) (*types.UpdateSet, error) { +// Deprecated: Please use WaitForUpdatesEx instead. +func (p *Collector) WaitForUpdates( + ctx context.Context, + version string, + opts ...*types.WaitOptions) (*types.UpdateSet, error) { + + if !p.mu.TryLock() { + return nil, ErrConcurrentCollector + } + defer p.mu.Unlock() + req := types.WaitForUpdatesEx{ This: p.Reference(), Version: version, @@ -187,8 +207,15 @@ func (p *Collector) Retrieve(ctx context.Context, objs []types.ManagedObjectRefe return mo.LoadObjectContent(res.Returnval, dst) } -// RetrieveWithFilter populates dst as Retrieve does, but only for entities matching the given filter. -func (p *Collector) RetrieveWithFilter(ctx context.Context, objs []types.ManagedObjectReference, ps []string, dst interface{}, filter Filter) error { +// RetrieveWithFilter populates dst as Retrieve does, but only for entities +// that match the specified filter. +func (p *Collector) RetrieveWithFilter( + ctx context.Context, + objs []types.ManagedObjectReference, + ps []string, + dst interface{}, + filter Match) error { + if len(filter) == 0 { return p.Retrieve(ctx, objs, ps, dst) } @@ -200,7 +227,7 @@ func (p *Collector) RetrieveWithFilter(ctx context.Context, objs []types.Managed return err } - objs = filter.MatchObjectContent(content) + objs = filter.ObjectContent(content) if len(objs) == 0 { return nil @@ -214,3 +241,71 @@ func (p *Collector) RetrieveOne(ctx context.Context, obj types.ManagedObjectRefe var objs = []types.ManagedObjectReference{obj} return p.Retrieve(ctx, objs, ps, dst) } + +// WaitForUpdatesEx waits for any of the specified properties of the specified +// managed object to change. It calls the specified function for every update it +// receives. If this function returns false, it continues waiting for +// subsequent updates. If this function returns true, it stops waiting and +// returns. +// +// If the Context is canceled, a call to CancelWaitForUpdates() is made and its +// error value is returned. +// +// By default, ObjectUpdate.MissingSet faults are not propagated to the returned +// error, set WaitFilter.PropagateMissing=true to enable MissingSet fault +// propagation. +func (p *Collector) WaitForUpdatesEx( + ctx context.Context, + opts WaitOptions, + onUpdatesFn func([]types.ObjectUpdate) bool) error { + + if !p.mu.TryLock() { + return ErrConcurrentCollector + } + defer p.mu.Unlock() + + req := types.WaitForUpdatesEx{ + This: p.Reference(), + Options: opts.Options, + } + + for { + res, err := methods.WaitForUpdatesEx(ctx, p.roundTripper, &req) + if err != nil { + if ctx.Err() == context.Canceled { + return p.CancelWaitForUpdates(context.Background()) + } + return err + } + + set := res.Returnval + if set == nil { + if req.Options != nil && req.Options.MaxWaitSeconds != nil { + return nil // WaitOptions.MaxWaitSeconds exceeded + } + // Retry if the result came back empty + continue + } + + req.Version = set.Version + opts.Truncated = false + if set.Truncated != nil { + opts.Truncated = *set.Truncated + } + + for _, fs := range set.FilterSet { + if opts.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 onUpdatesFn(fs.ObjectSet) { + return nil + } + } + } +} diff --git a/property/collector_test.go b/property/collector_test.go new file mode 100644 index 000000000..00134c7fe --- /dev/null +++ b/property/collector_test.go @@ -0,0 +1,191 @@ +/* +Copyright (c) 2024-2024 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" + "testing" + "time" + + "github.com/vmware/govmomi/find" + "github.com/vmware/govmomi/object" + "github.com/vmware/govmomi/property" + "github.com/vmware/govmomi/simulator" + "github.com/vmware/govmomi/vim25" + "github.com/vmware/govmomi/vim25/types" +) + +func TestWaitForUpdatesEx(t *testing.T) { + model := simulator.VPX() + model.Datacenter = 1 + model.Cluster = 0 + model.Pool = 0 + model.Machine = 1 + model.Autostart = false + + simulator.Test(func(ctx context.Context, c *vim25.Client) { + // Set up the finder and get a VM. + finder := find.NewFinder(c, true) + datacenter, err := finder.DefaultDatacenter(ctx) + if err != nil { + t.Fatalf("default datacenter not found: %s", err) + } + finder.SetDatacenter(datacenter) + vmList, err := finder.VirtualMachineList(ctx, "*") + if len(vmList) == 0 { + t.Fatal("vmList == 0") + } + vm := vmList[0] + + pc, err := property.DefaultCollector(c).Create(ctx) + if err != nil { + t.Fatalf("failed to create new property collector: %s", err) + } + + // Start a goroutine to wait for power state changes to the VM. + chanResult := make(chan any) + cancelCtx, cancel := context.WithCancel(ctx) + defer cancel() + go func() { + defer close(chanResult) + if err := property.WaitForUpdatesEx( + cancelCtx, + pc, + &property.WaitFilter{ + CreateFilter: getDatacenterToVMFolderFilter(datacenter), + WaitOptions: property.WaitOptions{ + Options: &types.WaitOptions{ + MaxWaitSeconds: addrOf(int32(3)), + }, + }, + }, + func(updates []types.ObjectUpdate) bool { + return waitForPowerStateChanges( + cancelCtx, + vm, + chanResult, + updates, + types.VirtualMachinePowerStatePoweredOn) + }, + ); err != nil { + chanResult <- err + return + } + }() + + // Power on the VM to cause a property change. + if _, err := vm.PowerOn(ctx); err != nil { + t.Fatalf("error while powering on vm: %s", err) + } + + select { + case <-time.After(3 * time.Second): + t.Fatalf("timed out while waiting for property update") + case result := <-chanResult: + switch tResult := result.(type) { + case types.VirtualMachinePowerState: + if tResult != types.VirtualMachinePowerStatePoweredOn { + t.Fatalf("unexpected power state: %s", tResult) + } + case error: + t.Fatalf("error while waiting for updates: %s", tResult) + } + } + }, model) +} + +func waitForPowerStateChanges( + ctx context.Context, + vm *object.VirtualMachine, + chanResult chan any, + updates []types.ObjectUpdate, + expectedPowerState types.VirtualMachinePowerState) bool { + + for _, u := range updates { + if ctx.Err() != nil { + return false + } + if u.Obj != vm.Reference() { + continue + } + for _, cs := range u.ChangeSet { + if cs.Name == "runtime.powerState" { + if cs.Val == expectedPowerState { + select { + case <-ctx.Done(): + // No-op + default: + chanResult <- cs.Val + } + return true + } + } + } + } + return false +} + +func getDatacenterToVMFolderFilter(dc *object.Datacenter) types.CreateFilter { + // Define a wait filter that looks for updates to VM power + // states for VMs under the specified datacenter. + return types.CreateFilter{ + Spec: types.PropertyFilterSpec{ + ObjectSet: []types.ObjectSpec{ + { + Obj: dc.Reference(), + Skip: addrOf(true), + SelectSet: []types.BaseSelectionSpec{ + // Datacenter --> VM folder + &types.TraversalSpec{ + SelectionSpec: types.SelectionSpec{ + Name: "dcToVMFolder", + }, + Type: "Datacenter", + Path: "vmFolder", + SelectSet: []types.BaseSelectionSpec{ + &types.SelectionSpec{ + Name: "visitFolders", + }, + }, + }, + // Folder --> children (folder / VM) + &types.TraversalSpec{ + SelectionSpec: types.SelectionSpec{ + Name: "visitFolders", + }, + Type: "Folder", + // Folder --> children (folder / VM) + Path: "childEntity", + SelectSet: []types.BaseSelectionSpec{ + // Folder --> child folder + &types.SelectionSpec{ + Name: "visitFolders", + }, + }, + }, + }, + }, + }, + PropSet: []types.PropertySpec{ + { + Type: "VirtualMachine", + PathSet: []string{"runtime.powerState"}, + }, + }, + }, + } +} diff --git a/property/example_test.go b/property/example_test.go index 0c93dc8ce..507e74363 100644 --- a/property/example_test.go +++ b/property/example_test.go @@ -117,3 +117,194 @@ func ExampleWait() { // poweredOn // poweredOff } + +func ExampleCollector_WaitForUpdatesEx_addingRemovingPropertyFilters() { + model := simulator.VPX() + model.Datacenter = 1 + model.Cluster = 0 + model.Pool = 0 + model.Machine = 1 + model.Autostart = false + + simulator.Run(func(ctx context.Context, c *vim25.Client) error { + // Set up the finder and get a VM. + finder := find.NewFinder(c, true) + datacenter, err := finder.DefaultDatacenter(ctx) + if err != nil { + return fmt.Errorf("default datacenter not found: %w", err) + } + finder.SetDatacenter(datacenter) + vmList, err := finder.VirtualMachineList(ctx, "*") + if len(vmList) == 0 { + return fmt.Errorf("vmList == 0") + } + vm := vmList[0] + + pc, err := property.DefaultCollector(c).Create(ctx) + if err != nil { + return fmt.Errorf("failed to create new property collector: %w", err) + } + + // Start a goroutine to wait for power state changes to the VM. They + // should not be triggered as there is no property filter yet defined. + chanResult := make(chan any) + cancelCtx, cancel := context.WithCancel(ctx) + defer cancel() + go func() { + if err := pc.WaitForUpdatesEx( + cancelCtx, + property.WaitOptions{}, + func(updates []types.ObjectUpdate) bool { + return waitForPowerStateChanges( + cancelCtx, + vm, + chanResult, + updates, + types.VirtualMachinePowerStatePoweredOff) + }); err != nil { + + chanResult <- err + return + } + }() + + // Power on the VM to cause a property change. + if _, err := vm.PowerOn(ctx); err != nil { + return fmt.Errorf("error while powering on vm: %w", err) + } + + // The power change should be ignored. + select { + case <-time.After(3 * time.Second): + fmt.Println("poweredOn event not received") + case result := <-chanResult: + switch tResult := result.(type) { + case types.VirtualMachinePowerState: + return fmt.Errorf("update should not have been received without a property filter") + case error: + return fmt.Errorf("error while waiting for updates: %v", tResult) + } + } + + // Now create a property filter that will catch the update. + pf, err := pc.CreateFilter(ctx, getDatacenterToVMFolderFilter(datacenter)) + if err != nil { + return fmt.Errorf("failed to create dc2vm property filter: %w", err) + } + + // Power off the VM to cause a property change. + if _, err := vm.PowerOff(ctx); err != nil { + return fmt.Errorf("error while powering off vm: %w", err) + } + + // The power change should now be noticed. + select { + case <-time.After(3 * time.Second): + return fmt.Errorf("timed out while waiting for property update") + case result := <-chanResult: + switch tResult := result.(type) { + case types.VirtualMachinePowerState: + if tResult != types.VirtualMachinePowerStatePoweredOff { + return fmt.Errorf("unexpected power state: %v", tResult) + } + fmt.Println("poweredOff event received") + case error: + return fmt.Errorf("error while waiting for updates: %w", tResult) + } + } + + // Destroy the property filter and repeat, and the power change should + // once again be ignored. + if err := pf.Destroy(ctx); err != nil { + return fmt.Errorf("failed to destroy property filter: %w", err) + } + + // Power on the VM to cause a property change. + if _, err := vm.PowerOn(ctx); err != nil { + return fmt.Errorf("error while powering on vm: %w", err) + } + + // The power change should be ignored. + select { + case <-time.After(3 * time.Second): + fmt.Println("poweredOn event not received") + case result := <-chanResult: + switch tResult := result.(type) { + case types.VirtualMachinePowerState: + return fmt.Errorf("update should not have been received after property filter was destroyed") + case error: + return fmt.Errorf("error while waiting for updates: %v", tResult) + } + } + + return nil + }, model) + + // Output: + // poweredOn event not received + // poweredOff event received + // poweredOn event not received +} + +func ExampleCollector_WaitForUpdatesEx_errConcurrentCollector() { + simulator.Run(func(ctx context.Context, c *vim25.Client) error { + pc := property.DefaultCollector(c) + + waitOptions := property.WaitOptions{ + Options: &types.WaitOptions{ + MaxWaitSeconds: addrOf(int32(1)), + }, + } + + onUpdatesFn := func(_ []types.ObjectUpdate) bool { + return false + } + + waitForChanges := func(chanErr chan error) { + defer close(chanErr) + chanErr <- pc.WaitForUpdatesEx(ctx, waitOptions, onUpdatesFn) + } + + // Start two goroutines that wait for changes, but only one will begin + // waiting -- the other will return property.ErrConcurrentCollector. + chanErr1, chanErr2 := make(chan error), make(chan error) + go waitForChanges(chanErr1) + go waitForChanges(chanErr2) + + err1 := <-chanErr1 + err2 := <-chanErr2 + + if err1 == nil && err2 == nil { + return fmt.Errorf( + "one of the WaitForUpdate calls should have returned %s", + property.ErrConcurrentCollector) + } + + if err1 == property.ErrConcurrentCollector && + err2 == property.ErrConcurrentCollector { + + return fmt.Errorf( + "both of the WaitForUpdate calls returned %s", + property.ErrConcurrentCollector) + } + + fmt.Println("WaitForUpdatesEx call succeeded") + fmt.Println("WaitForUpdatesEx call returned ErrConcurrentCollector") + + // The third WaitForUpdatesEx call should be able to successfully obtain + // the lock since the other two calls are completed. + if err := pc.WaitForUpdatesEx(ctx, waitOptions, onUpdatesFn); err != nil { + return fmt.Errorf( + "unexpected error from third call to WaitForUpdatesEx: %s", err) + } + + fmt.Println("WaitForUpdatesEx call succeeded") + + return nil + }) + + // Output: + // WaitForUpdatesEx call succeeded + // WaitForUpdatesEx call returned ErrConcurrentCollector + // WaitForUpdatesEx call succeeded +} diff --git a/property/filter.go b/property/filter.go index 2287bbfd9..40cd718fa 100644 --- a/property/filter.go +++ b/property/filter.go @@ -1,5 +1,5 @@ /* -Copyright (c) 2017 VMware, Inc. All Rights Reserved. +Copyright (c) 2017-2024 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. @@ -17,152 +17,38 @@ limitations under the License. package property import ( - "fmt" - "path" - "reflect" - "strconv" - "strings" + "context" + "github.com/vmware/govmomi/vim25/methods" + "github.com/vmware/govmomi/vim25/soap" "github.com/vmware/govmomi/vim25/types" ) -// Filter provides methods for matching against types.DynamicProperty -type Filter map[string]types.AnyType - -// Keys returns the Filter map keys as a []string -func (f Filter) Keys() []string { - keys := make([]string, 0, len(f)) - - for key := range f { - keys = append(keys, key) - } - - return keys +// Filter models the Filter managed object. +// +// For more information, see: +// https://vdc-download.vmware.com/vmwb-repository/dcr-public/184bb3ba-6fa8-4574-a767-d0c96e2a38f4/ba9422ef-405c-47dd-8553-e11b619185b2/SDK/vsphere-ws/docs/ReferenceGuide/vmodl.query.PropertyCollector.Filter.html. +type Filter struct { + roundTripper soap.RoundTripper + reference types.ManagedObjectReference } -// MatchProperty returns true if a Filter entry matches the given prop. -func (f Filter) MatchProperty(prop types.DynamicProperty) bool { - if prop.Val == nil { - return false - } - match, ok := f[prop.Name] - if !ok { - return false - } - - if match == prop.Val { - return true - } - - ptype := reflect.TypeOf(prop.Val) - - if strings.HasPrefix(ptype.Name(), "ArrayOf") { - pval := reflect.ValueOf(prop.Val).Field(0) - - for i := 0; i < pval.Len(); i++ { - prop.Val = pval.Index(i).Interface() - - if f.MatchProperty(prop) { - return true - } - } - - return false - } - - if reflect.TypeOf(match) != ptype { - s, ok := match.(string) - if !ok { - return false - } - - // convert if we can - switch val := prop.Val.(type) { - case bool: - match, _ = strconv.ParseBool(s) - case int16: - x, _ := strconv.ParseInt(s, 10, 16) - match = int16(x) - case int32: - x, _ := strconv.ParseInt(s, 10, 32) - match = int32(x) - case int64: - match, _ = strconv.ParseInt(s, 10, 64) - case float32: - x, _ := strconv.ParseFloat(s, 32) - match = float32(x) - case float64: - match, _ = strconv.ParseFloat(s, 64) - case fmt.Stringer: - prop.Val = val.String() - case *types.CustomFieldStringValue: - prop.Val = fmt.Sprintf("%d:%s", val.Key, val.Value) - default: - if ptype.Kind() != reflect.String { - return false - } - // An enum type we can convert to a string type - prop.Val = reflect.ValueOf(prop.Val).String() - } - } - - switch pval := prop.Val.(type) { - case string: - s := match.(string) - if s == "*" { - return true // TODO: path.Match fails if s contains a '/' - } - m, _ := path.Match(s, pval) - return m - default: - return reflect.DeepEqual(match, pval) - } -} - -// MatchPropertyList returns true if all given props match the Filter. -func (f Filter) MatchPropertyList(props []types.DynamicProperty) bool { - for _, p := range props { - if !f.MatchProperty(p) { - return false - } - } - - return len(f) == len(props) // false if a property such as VM "guest" is unset -} - -// MatchObjectContent returns a list of ObjectContent.Obj where the ObjectContent.PropSet matches all properties the Filter. -func (f Filter) MatchObjectContent(objects []types.ObjectContent) []types.ManagedObjectReference { - var refs []types.ManagedObjectReference - - for _, o := range objects { - if f.MatchPropertyList(o.PropSet) { - refs = append(refs, o.Obj) - } - } - - return refs -} - -// MatchAnyPropertyList returns true if any given props match the Filter. -func (f Filter) MatchAnyPropertyList(props []types.DynamicProperty) bool { - for _, p := range props { - if f.MatchProperty(p) { - return true - } - } - - return false +func (f Filter) Reference() types.ManagedObjectReference { + return f.reference } -// MatchAnyObjectContent returns a list of ObjectContent.Obj where the ObjectContent.PropSet matches any property in the Filter. -func (f Filter) MatchAnyObjectContent(objects []types.ObjectContent) []types.ManagedObjectReference { - var refs []types.ManagedObjectReference +// Destroy destroys this filter. +// +// This operation can be called explicitly, or it can take place implicitly when +// the session that created the filter is closed. +func (f *Filter) Destroy(ctx context.Context) error { + if _, err := methods.DestroyPropertyFilter( + ctx, + f.roundTripper, + &types.DestroyPropertyFilter{This: f.Reference()}); err != nil { - for _, o := range objects { - if f.MatchAnyPropertyList(o.PropSet) { - refs = append(refs, o.Obj) - } + return err } - - return refs + f.reference = types.ManagedObjectReference{} + return nil } diff --git a/property/filter_test.go b/property/filter_test.go index c545e0d82..fbead88b6 100644 --- a/property/filter_test.go +++ b/property/filter_test.go @@ -14,47 +14,4 @@ See the License for the specific language governing permissions and limitations under the License. */ -package property - -import ( - "testing" - - "github.com/vmware/govmomi/vim25/types" -) - -func TestMatchProperty(t *testing.T) { - tests := []struct { - key string - val types.AnyType - pass types.AnyType - fail types.AnyType - }{ - {"string", "bar", "bar", "foo"}, - {"match", "foo.bar", "foo.*", "foobarbaz"}, - {"moref", types.ManagedObjectReference{Type: "HostSystem", Value: "foo"}, "HostSystem:foo", "bar"}, // implements fmt.Stringer - {"morefm", types.ManagedObjectReference{Type: "HostSystem", Value: "foo"}, "*foo", "bar"}, - {"morefs", types.ArrayOfManagedObjectReference{ManagedObjectReference: []types.ManagedObjectReference{{Type: "HostSystem", Value: "foo"}}}, "*foo", "bar"}, - {"enum", types.VirtualMachinePowerStatePoweredOn, "poweredOn", "poweredOff"}, - {"int16", int32(16), int32(16), int32(42)}, - {"int32", int32(32), int32(32), int32(42)}, - {"int32s", int32(32), "32", "42"}, - {"int64", int64(64), int64(64), int64(42)}, - {"int64s", int64(64), "64", "42"}, - {"float32", float32(32.32), float32(32.32), float32(42.0)}, - {"float32s", float32(32.32), "32.32", "42.0"}, - {"float64", float64(64.64), float64(64.64), float64(42.0)}, - {"float64s", float64(64.64), "64.64", "42.0"}, - } - - for _, test := range tests { - p := types.DynamicProperty{Name: test.key, Val: test.val} - - for match, value := range map[bool]types.AnyType{true: test.pass, false: test.fail} { - result := Filter{test.key: value}.MatchProperty(p) - - if result != match { - t.Errorf("%s: %t", test.key, result) - } - } - } -} +package property_test diff --git a/property/match.go b/property/match.go new file mode 100644 index 000000000..f3922296b --- /dev/null +++ b/property/match.go @@ -0,0 +1,170 @@ +/* +Copyright (c) 2017-2024 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 + +import ( + "fmt" + "path" + "reflect" + "strconv" + "strings" + + "github.com/vmware/govmomi/vim25/types" +) + +// Match provides methods for matching against types.DynamicProperty +type Match map[string]types.AnyType + +// Keys returns the Match map keys as a []string +func (m Match) Keys() []string { + keys := make([]string, 0, len(m)) + + for key := range m { + keys = append(keys, key) + } + + return keys +} + +// Property returns true if an entry matches the given prop. +func (m Match) Property(prop types.DynamicProperty) bool { + if prop.Val == nil { + return false + } + match, ok := m[prop.Name] + if !ok { + return false + } + + if match == prop.Val { + return true + } + + ptype := reflect.TypeOf(prop.Val) + + if strings.HasPrefix(ptype.Name(), "ArrayOf") { + pval := reflect.ValueOf(prop.Val).Field(0) + + for i := 0; i < pval.Len(); i++ { + prop.Val = pval.Index(i).Interface() + + if m.Property(prop) { + return true + } + } + + return false + } + + if reflect.TypeOf(match) != ptype { + s, ok := match.(string) + if !ok { + return false + } + + // convert if we can + switch val := prop.Val.(type) { + case bool: + match, _ = strconv.ParseBool(s) + case int16: + x, _ := strconv.ParseInt(s, 10, 16) + match = int16(x) + case int32: + x, _ := strconv.ParseInt(s, 10, 32) + match = int32(x) + case int64: + match, _ = strconv.ParseInt(s, 10, 64) + case float32: + x, _ := strconv.ParseFloat(s, 32) + match = float32(x) + case float64: + match, _ = strconv.ParseFloat(s, 64) + case fmt.Stringer: + prop.Val = val.String() + case *types.CustomFieldStringValue: + prop.Val = fmt.Sprintf("%d:%s", val.Key, val.Value) + default: + if ptype.Kind() != reflect.String { + return false + } + // An enum type we can convert to a string type + prop.Val = reflect.ValueOf(prop.Val).String() + } + } + + switch pval := prop.Val.(type) { + case string: + s := match.(string) + if s == "*" { + return true // TODO: path.Match fails if s contains a '/' + } + m, _ := path.Match(s, pval) + return m + default: + return reflect.DeepEqual(match, pval) + } +} + +// List returns true if all given props match. +func (m Match) List(props []types.DynamicProperty) bool { + for _, p := range props { + if !m.Property(p) { + return false + } + } + + return len(m) == len(props) // false if a property such as VM "guest" is unset +} + +// ObjectContent returns a list of ObjectContent.Obj where the +// ObjectContent.PropSet matches all properties the Filter. +func (m Match) ObjectContent(objects []types.ObjectContent) []types.ManagedObjectReference { + var refs []types.ManagedObjectReference + + for _, o := range objects { + if m.List(o.PropSet) { + refs = append(refs, o.Obj) + } + } + + return refs +} + +// AnyList returns true if any given props match. +func (m Match) AnyList(props []types.DynamicProperty) bool { + for _, p := range props { + if m.Property(p) { + return true + } + } + + return false +} + +// AnyObjectContent returns a list of ObjectContent.Obj where the +// ObjectContent.PropSet matches any property. +func (m Match) AnyObjectContent(objects []types.ObjectContent) []types.ManagedObjectReference { + var refs []types.ManagedObjectReference + + for _, o := range objects { + if m.AnyList(o.PropSet) { + refs = append(refs, o.Obj) + } + } + + return refs +} diff --git a/property/match_test.go b/property/match_test.go new file mode 100644 index 000000000..bfede34a6 --- /dev/null +++ b/property/match_test.go @@ -0,0 +1,61 @@ +/* +Copyright (c) 2024-2024 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 ( + "testing" + + "github.com/vmware/govmomi/property" + "github.com/vmware/govmomi/vim25/types" +) + +func TestMatchProperty(t *testing.T) { + tests := []struct { + key string + val types.AnyType + pass types.AnyType + fail types.AnyType + }{ + {"string", "bar", "bar", "foo"}, + {"match", "foo.bar", "foo.*", "foobarbaz"}, + {"moref", types.ManagedObjectReference{Type: "HostSystem", Value: "foo"}, "HostSystem:foo", "bar"}, // implements fmt.Stringer + {"morefm", types.ManagedObjectReference{Type: "HostSystem", Value: "foo"}, "*foo", "bar"}, + {"morefs", types.ArrayOfManagedObjectReference{ManagedObjectReference: []types.ManagedObjectReference{{Type: "HostSystem", Value: "foo"}}}, "*foo", "bar"}, + {"enum", types.VirtualMachinePowerStatePoweredOn, "poweredOn", "poweredOff"}, + {"int16", int32(16), int32(16), int32(42)}, + {"int32", int32(32), int32(32), int32(42)}, + {"int32s", int32(32), "32", "42"}, + {"int64", int64(64), int64(64), int64(42)}, + {"int64s", int64(64), "64", "42"}, + {"float32", float32(32.32), float32(32.32), float32(42.0)}, + {"float32s", float32(32.32), "32.32", "42.0"}, + {"float64", float64(64.64), float64(64.64), float64(42.0)}, + {"float64s", float64(64.64), "64.64", "42.0"}, + } + + for _, test := range tests { + p := types.DynamicProperty{Name: test.key, Val: test.val} + + for match, value := range map[bool]types.AnyType{true: test.pass, false: test.fail} { + result := property.Match{test.key: value}.Property(p) + + if result != match { + t.Errorf("%s: %t", test.key, result) + } + } + } +} diff --git a/property/property_test.go b/property/property_test.go new file mode 100644 index 000000000..d132f2109 --- /dev/null +++ b/property/property_test.go @@ -0,0 +1,21 @@ +/* +Copyright (c) 2024-2024 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 + +func addrOf[T any](t T) *T { + return &t +} diff --git a/property/wait.go b/property/wait.go index fbb680771..6b173afed 100644 --- a/property/wait.go +++ b/property/wait.go @@ -1,5 +1,5 @@ /* -Copyright (c) 2015-2017 VMware, Inc. All Rights Reserved. +Copyright (c) 2015-2024 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. @@ -18,20 +18,25 @@ package property import ( "context" + "fmt" - "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 +// WaitOptions defines options for a property collector's WaitForUpdatesEx +// method. +type WaitOptions struct { Options *types.WaitOptions PropagateMissing bool Truncated bool } +// WaitFilter provides helpers to construct a types.CreateFilter for use with property.Wait +type WaitFilter struct { + types.CreateFilter + WaitOptions +} + // Add a new ObjectSpec and PropertySpec to the WaitFilter func (f *WaitFilter) Add(obj types.ManagedObjectReference, kind string, ps []string, set ...types.BaseSelectionSpec) *WaitFilter { spec := types.ObjectSpec{ @@ -70,8 +75,8 @@ func Wait(ctx context.Context, c *Collector, obj types.ManagedObjectReference, p }) } -// WaitForUpdates waits for any of the specified properties of the specified managed -// object to change. It calls the specified function for every update it +// WaitForUpdates waits for any of the specified properties of the specified +// managed object to change. It calls the specified function for every update it // receives. If this function returns false, it continues waiting for // subsequent updates. If this function returns true, it stops waiting and // returns. @@ -80,14 +85,24 @@ func Wait(ctx context.Context, c *Collector, obj types.ManagedObjectReference, p // creates a new property collector and calls CreateFilter. A new property // collector is required because filters can only be added, not removed. // -// If the Context is canceled, a call to CancelWaitForUpdates() is made and its error value is returned. -// The newly created collector is destroyed before this function returns (both -// in case of success or error). +// If the Context is canceled, a call to CancelWaitForUpdates() is made and its +// error value is returned. 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) +// By default, ObjectUpdate.MissingSet faults are not propagated to the returned +// error, set WaitFilter.PropagateMissing=true to enable MissingSet fault +// propagation. +// +// Deprecated: Please consider using WaitForUpdatesEx instead, as it does not +// create a new property collector, instead it destroys the property filter +// after the expected update is received. +func WaitForUpdates( + ctx context.Context, + c *Collector, + filter *WaitFilter, + onUpdatesFn func([]types.ObjectUpdate) bool) (result error) { + + pc, err := c.Create(ctx) if err != nil { return err } @@ -95,57 +110,65 @@ func WaitForUpdates(ctx context.Context, c *Collector, filter *WaitFilter, f fun // Attempt to destroy the collector using the background context, as the // specified context may have timed out or have been canceled. defer func() { - _ = p.Destroy(context.Background()) + if err := pc.Destroy(context.Background()); err != nil { + if result == nil { + result = err + } else { + result = fmt.Errorf( + "destroy property collector failed with %s after failing to wait for updates: %w", + err, + result) + } + } }() - err = p.CreateFilter(ctx, filter.CreateFilter) - if err != nil { + // Create a property filter for the property collector. + if _, err := pc.CreateFilter(ctx, filter.CreateFilter); err != nil { return err } - req := types.WaitForUpdatesEx{ - This: p.Reference(), - Options: filter.Options, - } + return pc.WaitForUpdatesEx(ctx, filter.WaitOptions, onUpdatesFn) +} - for { - res, err := methods.WaitForUpdatesEx(ctx, p.roundTripper, &req) - if err != nil { - if ctx.Err() == context.Canceled { - werr := p.CancelWaitForUpdates(context.Background()) - return werr - } - return err - } +// WaitForUpdates waits for any of the specified properties of the specified +// managed object to change. It calls the specified function for every update it +// receives. If this function returns false, it continues waiting for +// subsequent updates. If this function returns true, it stops waiting and +// returns. +// +// If the Context is canceled, a call to CancelWaitForUpdates() is made and its +// error value is returned. +// +// By default, ObjectUpdate.MissingSet faults are not propagated to the returned +// error, set WaitFilter.PropagateMissing=true to enable MissingSet fault +// propagation. +func WaitForUpdatesEx( + ctx context.Context, + pc *Collector, + filter *WaitFilter, + onUpdatesFn func([]types.ObjectUpdate) bool) (result error) { + + // Create a property filter for the property collector. + pf, err := pc.CreateFilter(ctx, filter.CreateFilter) + if err != nil { + return err + } - set := res.Returnval - if set == nil { - if req.Options != nil && req.Options.MaxWaitSeconds != nil { - return nil // WaitOptions.MaxWaitSeconds exceeded + // Destroy the filter using the background context, as the specified context + // may have timed out or have been canceled. + defer func() { + if err := pf.Destroy(context.Background()); err != nil { + if result == nil { + result = err + } else { + result = fmt.Errorf( + "destroy property filter failed with %s after failing to wait for updates: %w", + err, + result) } - // Retry if the result came back empty - continue - } - - req.Version = set.Version - filter.Truncated = false - if set.Truncated != nil { - filter.Truncated = *set.Truncated } - 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 - } - } - } + return pc.WaitForUpdatesEx(ctx, filter.WaitOptions, onUpdatesFn) } diff --git a/property/wait_test.go b/property/wait_test.go index d438fe201..0f80ca595 100644 --- a/property/wait_test.go +++ b/property/wait_test.go @@ -1,5 +1,5 @@ /* -Copyright (c) 2019 VMware, Inc. All Rights Reserved. +Copyright (c) 2019-2024 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. @@ -29,53 +29,61 @@ import ( "github.com/vmware/govmomi/vim25/types" ) -type PropertyCollector struct { - simulator.PropertyCollector -} +// Test that task.Wait() propagates MissingSet errors +func TestWaitPermissionFault(t *testing.T) { + ctx := context.Background() -// 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(), + 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(propCollForWaitForPermsTest) + 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, } -} -// 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) + name := "[LocalDS_0] disk1.vmdk" - 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 {} + task, err := dm.CreateVirtualDisk(ctx, name, nil, spec) + if err != nil { + t.Fatal(err) } - 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), - }, - }}, - }}, - }}, - }, - }, + 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) } } -// Test that task.Wait() propagates MissingSet errors -func TestWaitPermissionFault(t *testing.T) { +// Test that task.WaitEx() propagates MissingSet errors +func TestWaitExPermissionFault(t *testing.T) { ctx := context.Background() model := simulator.ESX() @@ -91,7 +99,7 @@ func TestWaitPermissionFault(t *testing.T) { c, _ := govmomi.NewClient(ctx, s.URL, true) - pc := new(PropertyCollector) + pc := new(propCollForWaitForPermsTest) pc.Self = model.ServiceContent.PropertyCollector simulator.Map.Put(pc) @@ -112,7 +120,7 @@ func TestWaitPermissionFault(t *testing.T) { t.Fatal(err) } - err = task.Wait(ctx) + err = task.WaitEx(ctx) if err == nil { t.Fatal("expected error") } @@ -126,3 +134,54 @@ func TestWaitPermissionFault(t *testing.T) { t.Fatalf("unexpected vim fault: %T", fault) } } + +type propCollForWaitForPermsTest struct { + simulator.PropertyCollector +} + +// CreatePropertyCollector overrides the vcsim impl to return this test's PC impl +func (pc *propCollForWaitForPermsTest) CreatePropertyCollector( + ctx *simulator.Context, + c *types.CreatePropertyCollector) soap.HasFault { + + return &methods.CreatePropertyCollectorBody{ + Res: &types.CreatePropertyCollectorResponse{ + Returnval: ctx.Session.Put(new(propCollForWaitForPermsTest)).Reference(), + }, + } +} + +// WaitForUpdatesEx overrides the vcsim impl to inject a fault via MissingSet +func (pc *propCollForWaitForPermsTest) 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), + }, + }}, + }}, + }}, + }, + }, + } +} diff --git a/simulator/property_collector_test.go b/simulator/property_collector_test.go index 9e400d1e8..0b8ebce48 100644 --- a/simulator/property_collector_test.go +++ b/simulator/property_collector_test.go @@ -498,7 +498,7 @@ func TestWaitForUpdatesOneUpdateCalculation(t *testing.T) { types.VirtualMachinePowerStatePoweredOn: vm.PowerOff, } - err = pc.CreateFilter(ctx, filter.CreateFilter) + _, err = pc.CreateFilter(ctx, filter.CreateFilter) if err != nil { t.Fatal(err) } @@ -1495,7 +1495,7 @@ func TestPropertyCollectorSession(t *testing.T) { // aka issue-923 pc := property.DefaultCollector(c.Client) filter := new(property.WaitFilter).Add(c.ServiceContent.RootFolder, "Folder", []string{"name"}) - if err = pc.CreateFilter(ctx, filter.CreateFilter); err != nil { + if _, err = pc.CreateFilter(ctx, filter.CreateFilter); err != nil { t.Fatal(err) } diff --git a/simulator/session_manager_test.go b/simulator/session_manager_test.go index 4057dd2a7..a5484d2b3 100644 --- a/simulator/session_manager_test.go +++ b/simulator/session_manager_test.go @@ -332,7 +332,7 @@ func TestSessionManagerPropertyCollector(t *testing.T) { PartialUpdates: true, } - if err = pc.CreateFilter(ctx, filter); err != nil { + if _, err = pc.CreateFilter(ctx, filter); err != nil { t.Fatal(err) } diff --git a/task/wait.go b/task/wait.go index d52458e66..cae84d530 100644 --- a/task/wait.go +++ b/task/wait.go @@ -1,5 +1,5 @@ /* -Copyright (c) 2015-2023 VMware, Inc. All Rights Reserved. +Copyright (c) 2015-2024 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. @@ -99,7 +99,7 @@ func (t *taskCallback) fn(pc []types.PropertyChange) bool { } } -// Wait waits for a task to finish with either success or failure. It does so +// WaitEx waits for a task to finish with either success or failure. It does so // by waiting for the "info" property of task managed object to change. The // function returns when it finds the task in the "success" or "error" state. // In the former case, the return value is nil. In the latter case the return @@ -113,7 +113,12 @@ func (t *taskCallback) fn(pc []types.PropertyChange) bool { // The detail for the progress update is set to an empty string. If the task // finishes in the error state, the error instance is passed through as well. // Note that this error is the same error that is returned by this function. -func Wait(ctx context.Context, ref types.ManagedObjectReference, pc *property.Collector, s progress.Sinker) (*types.TaskInfo, error) { +func WaitEx( + ctx context.Context, + ref types.ManagedObjectReference, + pc *property.Collector, + s progress.Sinker) (*types.TaskInfo, error) { + cb := &taskCallback{} // Include progress sink if specified @@ -122,19 +127,29 @@ func Wait(ctx context.Context, ref types.ManagedObjectReference, pc *property.Co defer close(cb.ch) } - filter := &property.WaitFilter{PropagateMissing: true} + filter := &property.WaitFilter{ + WaitOptions: property.WaitOptions{ + 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 + if err := property.WaitForUpdatesEx( + ctx, + pc, + filter, + func(updates []types.ObjectUpdate) bool { + for _, update := range updates { + // Only look at updates for the expected task object. + if update.Obj == ref { + if cb.fn(update.ChangeSet) { + return true + } + } } - } + return false + }); err != nil { - return false - }) - if err != nil { return nil, err } diff --git a/vapi/namespace/simulator/simulator.go b/vapi/namespace/simulator/simulator.go index 55382fb80..f568675d4 100644 --- a/vapi/namespace/simulator/simulator.go +++ b/vapi/namespace/simulator/simulator.go @@ -84,7 +84,7 @@ func enabledClusters(c *govmomi.Client) ([]types.ManagedObjectReference, error) } defer func() { _ = v.Destroy(ctx) }() - return v.Find(ctx, kind, property.Filter{"name": "WCP-*"}) + return v.Find(ctx, kind, property.Match{"name": "WCP-*"}) } func (h *Handler) clusters(w http.ResponseWriter, r *http.Request) { diff --git a/vapi/tags/example_test.go b/vapi/tags/example_test.go index 81b42fdd6..0506d1356 100644 --- a/vapi/tags/example_test.go +++ b/vapi/tags/example_test.go @@ -91,7 +91,7 @@ func ExampleManager_GetAttachedTagsOnObjects() { log.Fatal(err) } - vms, err := v.Find(ctx, nil, property.Filter{}) // List all VMs in the inventory + vms, err := v.Find(ctx, nil, property.Match{}) // List all VMs in the inventory if err != nil { return err } diff --git a/view/container_view.go b/view/container_view.go index 39041c41f..356994035 100644 --- a/view/container_view.go +++ b/view/container_view.go @@ -91,7 +91,7 @@ func (v ContainerView) Retrieve(ctx context.Context, kind []string, ps []string, } // RetrieveWithFilter populates dst as Retrieve does, but only for entities matching the given filter. -func (v ContainerView) RetrieveWithFilter(ctx context.Context, kind []string, ps []string, dst interface{}, filter property.Filter) error { +func (v ContainerView) RetrieveWithFilter(ctx context.Context, kind []string, ps []string, dst interface{}, filter property.Match) error { if len(filter) == 0 { return v.Retrieve(ctx, kind, ps, dst) } @@ -103,7 +103,7 @@ func (v ContainerView) RetrieveWithFilter(ctx context.Context, kind []string, ps return err } - objs := filter.MatchObjectContent(content) + objs := filter.ObjectContent(content) pc := property.DefaultCollector(v.Client()) @@ -111,10 +111,10 @@ func (v ContainerView) RetrieveWithFilter(ctx context.Context, kind []string, ps } // Find returns object references for entities of type kind, matching the given filter. -func (v ContainerView) Find(ctx context.Context, kind []string, filter property.Filter) ([]types.ManagedObjectReference, error) { +func (v ContainerView) Find(ctx context.Context, kind []string, filter property.Match) ([]types.ManagedObjectReference, error) { if len(filter) == 0 { // Ensure we have at least 1 filter to avoid retrieving all properties. - filter = property.Filter{"name": "*"} + filter = property.Match{"name": "*"} } var content []types.ObjectContent @@ -124,14 +124,14 @@ func (v ContainerView) Find(ctx context.Context, kind []string, filter property. return nil, err } - return filter.MatchObjectContent(content), nil + return filter.ObjectContent(content), nil } // FindAny returns object references for entities of type kind, matching any property the given filter. -func (v ContainerView) FindAny(ctx context.Context, kind []string, filter property.Filter) ([]types.ManagedObjectReference, error) { +func (v ContainerView) FindAny(ctx context.Context, kind []string, filter property.Match) ([]types.ManagedObjectReference, error) { if len(filter) == 0 { // Ensure we have at least 1 filter to avoid retrieving all properties. - filter = property.Filter{"name": "*"} + filter = property.Match{"name": "*"} } var content []types.ObjectContent @@ -141,5 +141,5 @@ func (v ContainerView) FindAny(ctx context.Context, kind []string, filter proper return nil, err } - return filter.MatchAnyObjectContent(content), nil + return filter.AnyObjectContent(content), nil } diff --git a/view/example_test.go b/view/example_test.go index 18353831a..98e829448 100644 --- a/view/example_test.go +++ b/view/example_test.go @@ -117,7 +117,7 @@ func ExampleContainerView_RetrieveWithFilter() { var vms []mo.VirtualMachine var names []string - err = v.RetrieveWithFilter(ctx, kind, []string{"name"}, &vms, property.Filter{"name": "*_VM1"}) + err = v.RetrieveWithFilter(ctx, kind, []string{"name"}, &vms, property.Match{"name": "*_VM1"}) if err != nil { return err } @@ -154,7 +154,7 @@ func ExampleContainerView_Find() { log.Fatal(err) } - vms, err := v.Find(ctx, kind, property.Filter{}) + vms, err := v.Find(ctx, kind, property.Match{}) if err != nil { return err }