From dcf063cd2ab5d0a98619477ae82e138ace4dae2e Mon Sep 17 00:00:00 2001 From: Doug MacEachern Date: Tue, 2 Apr 2024 11:45:58 -0700 Subject: [PATCH] fix: vcsim: re-parent children in ResourcePool.Destroy Fixes #3396 --- govc/test/vcsim.bats | 11 +++++++++++ simulator/finder_test.go | 34 ++++++++++++++++++++++++++++++++++ simulator/resource_pool.go | 6 +++++- 3 files changed, 50 insertions(+), 1 deletion(-) diff --git a/govc/test/vcsim.bats b/govc/test/vcsim.bats index 6f2f2f2fd..3828fda57 100755 --- a/govc/test/vcsim.bats +++ b/govc/test/vcsim.bats @@ -239,6 +239,17 @@ EOF wait $pid } +@test "vcsim issue #3396" { + vcsim_env + + govc pool.create /DC0/host/DC0_C0/Resources/foo + govc pool.create /DC0/host/DC0_C0/Resources/foo/bar + govc find -l /DC0/host/DC0_C0/Resources + govc pool.destroy /DC0/host/DC0_C0/Resources/foo + # prior to the fix, "bar"'s Parent was still "foo", causing the following to hang + govc find -l /DC0/host/DC0_C0/Resources +} + @test "vcsim run container" { require_docker diff --git a/simulator/finder_test.go b/simulator/finder_test.go index 3a45121d3..9e856c1dd 100644 --- a/simulator/finder_test.go +++ b/simulator/finder_test.go @@ -26,6 +26,7 @@ import ( "github.com/vmware/govmomi" "github.com/vmware/govmomi/find" "github.com/vmware/govmomi/object" + "github.com/vmware/govmomi/vim25" "github.com/vmware/govmomi/vim25/types" ) @@ -273,3 +274,36 @@ func TestFinderDefaultHostVPX(t *testing.T) { t.Errorf("unexpected error type=%T", err) } } + +func TestFinderDestroyedParentResourcePool(t *testing.T) { + Test(func(ctx context.Context, c *vim25.Client) { + finder := find.NewFinder(c) + + rp, err := finder.ResourcePool(ctx, "/DC0/host/DC0_C0/Resources") + if err != nil { + t.Fatal(err) + } + + foo, err := rp.Create(ctx, "foo", types.DefaultResourceConfigSpec()) + if err != nil { + t.Fatal(err) + } + + bar, err := foo.Create(ctx, "bar", types.DefaultResourceConfigSpec()) + if err != nil { + t.Fatal(err) + } + + task, err := foo.Destroy(ctx) + if err != nil { + t.Fatal(err) + } + if err := task.WaitEx(ctx); err != nil { + t.Fatal(err) + } + + if _, err := finder.Element(ctx, bar.Reference()); err != nil { + t.Fatal(err) + } + }) +} diff --git a/simulator/resource_pool.go b/simulator/resource_pool.go index eeab93af7..d34fa5b88 100644 --- a/simulator/resource_pool.go +++ b/simulator/resource_pool.go @@ -483,7 +483,11 @@ func (p *ResourcePool) DestroyTask(ctx *Context, req *types.Destroy_Task) soap.H RemoveReference(&parent.ResourcePool, req.This) // The grandchildren become children of the parent (rp) - parent.ResourcePool = append(parent.ResourcePool, p.ResourcePool.ResourcePool...) + for _, ref := range p.ResourcePool.ResourcePool { + child := ctx.Map.Get(ref).(*ResourcePool) + ctx.WithLock(child, func() { child.Parent = &parent.Self }) + parent.ResourcePool = append(parent.ResourcePool, ref) + } }) // And VMs move to the parent