fix: vcsim: re-parent children in ResourcePool.Destroy

Fixes #3396
vmware · Apr 2, 2024 · dcf063c · dcf063c
1 parent 89a6a4c
commit dcf063c
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 1 deletion.
diff --git 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
@@ -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
@@ -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