Skip to content

Commit ad24cb4

Browse files
committedNov 21, 2024··
fix: Property Collector updates should be triggered on the empty FilterSet
The PropertyCollector update closure passed to WaitForUpdatesEx is only triggered when a non-empty FilterSet occurs. This is an issue when side effects need to be triggered inside the update function such as notification mechanisms that an update was acknowledged. It should be the choice of the update closure to determine whether to process the empty FilterSet or not. Closes #3631 Signed-off-by: Brian Rak <brian.rak@broadcom.com>
1 parent 8101bae commit ad24cb4

File tree

2 files changed

+70
-2
lines changed

2 files changed

+70
-2
lines changed
 

‎property/collector.go

+11-2
Original file line numberDiff line numberDiff line change
@@ -264,9 +264,12 @@ func (p *Collector) RetrieveOne(ctx context.Context, obj types.ManagedObjectRefe
264264

265265
// WaitForUpdatesEx waits for any of the specified properties of the specified
266266
// managed object to change. It calls the specified function for every update it
267-
// receives. If this function returns false, it continues waiting for
267+
// receives an update - including the empty filter set, which can occur if no
268+
// objects are eligible for updates.
269+
//
270+
// If this function returns false, it continues waiting for
268271
// subsequent updates. If this function returns true, it stops waiting and
269-
// returns.
272+
// returns upon receiving the first non-empty filter set.
270273
//
271274
// If the Context is canceled, a call to CancelWaitForUpdates() is made and its
272275
// error value is returned.
@@ -313,6 +316,12 @@ func (p *Collector) WaitForUpdatesEx(
313316
opts.Truncated = *set.Truncated
314317
}
315318

319+
if len(set.FilterSet) == 0 {
320+
// Trigger callbacks in case callers need to be notified
321+
// of the empty filter set.
322+
_ = onUpdatesFn(make([]types.ObjectUpdate, 0))
323+
}
324+
316325
for _, fs := range set.FilterSet {
317326
if opts.PropagateMissing {
318327
for i := range fs.ObjectSet {

‎property/collector_test.go

+59
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package property_test
1818

1919
import (
2020
"context"
21+
"fmt"
2122
"testing"
2223
"time"
2324

@@ -110,6 +111,64 @@ func TestWaitForUpdatesEx(t *testing.T) {
110111
}, model)
111112
}
112113

114+
func TestWaitForUpdatesExEmptyUpdateSet(t *testing.T) {
115+
model := simulator.VPX()
116+
model.Datacenter = 1
117+
model.Cluster = 0
118+
model.Pool = 0
119+
model.Machine = 0
120+
model.Autostart = false
121+
122+
simulator.Test(func(ctx context.Context, c *vim25.Client) {
123+
// Set up the finder and get a VM.
124+
finder := find.NewFinder(c, true)
125+
datacenter, err := finder.DefaultDatacenter(ctx)
126+
if err != nil {
127+
t.Fatalf("default datacenter not found: %s", err.Error())
128+
}
129+
finder.SetDatacenter(datacenter)
130+
vmList, err := finder.VirtualMachineList(ctx, "*")
131+
if len(vmList) != 0 {
132+
t.Fatalf("vmList != 0")
133+
}
134+
135+
pc, err := property.DefaultCollector(c).Create(ctx)
136+
if err != nil {
137+
t.Fatalf("failed to create new property collector: %s", err.Error())
138+
}
139+
140+
// Start a goroutine to wait for updates on any VMs.
141+
// Since there are no VMs in the filter set, we expect to
142+
// receive an empty update set.
143+
chanResult := make(chan error)
144+
cancelCtx, cancel := context.WithCancel(ctx)
145+
go func() {
146+
defer close(chanResult)
147+
_ = pc.WaitForUpdatesEx(
148+
cancelCtx,
149+
&property.WaitOptions{},
150+
func(updates []types.ObjectUpdate) bool {
151+
var err error
152+
if len(updates) > 0 {
153+
err = fmt.Errorf("unexpected update")
154+
}
155+
chanResult <- err
156+
cancel()
157+
return true
158+
})
159+
}()
160+
161+
select {
162+
case <-ctx.Done():
163+
t.Fatalf("timed out while waiting for updates")
164+
case err := <-chanResult:
165+
if err != nil {
166+
t.Fatalf("error while waiting for updates: %s", err.Error())
167+
}
168+
}
169+
}, model)
170+
}
171+
113172
func TestRetrievePropertiesOneAtATime(t *testing.T) {
114173
model := simulator.VPX()
115174
model.Datacenter = 1

0 commit comments

Comments
 (0)
Please sign in to comment.