Repository: vmware/govmomi
Pull request #2586 (2021-09-13 20:29:40)

vcsim: Fix PowerOnMultiVMTask() to return per-VM tasks
## Description

Return array of tasks (one per VM) as result in the task returned
for multi-VM PowerOn operation for datacenter. This enables API caller
to be able to wait on actual VM PowerOn at per VM level.

Closes: #2584

## Type of change

Please mark options that are relevant:

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to
  not work as expected)
- [ ] This change requires a documentation update

## How Has This Been Tested?

Added a new unit test to verify the change to datacenter PowerOn task implementation code works fine.
The new test can be individually run as:
```
$ go test -run TestDatacenterPowerOnMultiVMs -failfast -v github.com/vmware/govmomi/simulator
=== RUN   TestDatacenterPowerOnMultiVMs
--- PASS: TestDatacenterPowerOnMultiVMs (0.05s)
PASS
ok  	github.com/vmware/govmomi/simulator	0.057s
```

- [x] TestDatacenterPowerOnMultiVMs
Finds 2 VMs in VPX simulator inventory (under same DC), powers them off first individually, invokes DC PowerOnVM API (single call for 2 VMs), verifies the results are expected (returned task has sub-tasks per-VM, waiting on each sub-task works).

Running all `simulator` unit tests gave clean run:
```
$ go test github.com/vmware/govmomi/simulator
ok  	github.com/vmware/govmomi/simulator	8.562s
```

**Test Configuration**:
* Toolchain: go version go1.16.2b7 linux/amd64
* SDK:
* (add more if needed)

## Checklist:

- [x] My code follows the CONTRIBUTION [guidelines](./../CONTRIBUTING.md) of
  this project
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [x] I have added tests that prove my fix is effective or that my feature works
- [x] New and existing unit tests pass locally with my changes
- [ ] Any dependent changes have been merged


Patch:
diff --git a/simulator/datacenter.go b/simulator/datacenter.go
index e4294175..1ab6df47 100644
--- a/simulator/datacenter.go
+++ b/simulator/datacenter.go
@@ -153,14 +153,25 @@ func (dc *Datacenter) PowerOnMultiVMTask(ctx *Context, req *types.PowerOnMultiVM
 			return nil, new(types.NotImplemented)
 		}
 
+		// Return per-VM tasks, structured as:
+		// thisTask.result - DC level task
+		//    +- []Attempted
+		//        +- subTask.result - VM level powerOn task result
+		//        +- ...
+		res := types.ClusterPowerOnVmResult{}
+		res.Attempted = []types.ClusterAttemptedVmInfo{}
+
 		for _, ref := range req.Vm {
 			vm := Map.Get(ref).(*VirtualMachine)
+			// NOTE: Simulator does not actually perform any specific host-level placement
+			// (equivalent to vSphere DRS).
 			ctx.WithLock(vm, func() {
-				vm.PowerOnVMTask(ctx, &types.PowerOnVM_Task{})
+				vmTaskBody := vm.PowerOnVMTask(ctx, &types.PowerOnVM_Task{}).(*methods.PowerOnVM_TaskBody)
+				res.Attempted = append(res.Attempted, types.ClusterAttemptedVmInfo{Vm: ref, Task: &vmTaskBody.Res.Returnval})
 			})
 		}
 
-		return nil, nil
+		return res, nil
 	})
 
 	return &methods.PowerOnMultiVM_TaskBody{


Test patch:
diff --git a/simulator/datacenter_test.go b/simulator/datacenter_test.go
index f6639843..c6899548 100644
--- a/simulator/datacenter_test.go
+++ b/simulator/datacenter_test.go
@@ -17,8 +17,12 @@ limitations under the License.
 package simulator
 
 import (
+	"context"
 	"testing"
 
+	"github.com/vmware/govmomi"
+	"github.com/vmware/govmomi/find"
+	"github.com/vmware/govmomi/object"
 	"github.com/vmware/govmomi/simulator/esx"
 	"github.com/vmware/govmomi/simulator/vpx"
 	"github.com/vmware/govmomi/vim25/mo"
@@ -28,11 +32,11 @@ import (
 func TestDatacenterCreateFolders(t *testing.T) {
 	// For this test we only want the RootFolder, 1 Datacenter and its child folders
 	models := []Model{
-		Model{
+		{
 			ServiceContent: esx.ServiceContent,
 			RootFolder:     esx.RootFolder,
 		},
-		Model{
+		{
 			ServiceContent: vpx.ServiceContent,
 			RootFolder:     vpx.RootFolder,
 			Datacenter:     1,
@@ -83,3 +87,86 @@ func TestDatacenterCreateFolders(t *testing.T) {
 		}
 	}
 }
+
+func TestDatacenterPowerOnMultiVMs(t *testing.T) {
+	ctx := context.Background()
+
+	m := VPX()
+	defer m.Remove()
+	err := m.Create()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	s := m.Service.NewServer()
+	defer s.Close()
+
+	c, err := govmomi.NewClient(ctx, s.URL, true)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	finder := find.NewFinder(c.Client, false)
+	dc, err := finder.DefaultDatacenter(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	finder.SetDatacenter(dc)
+
+	vms, err := finder.VirtualMachineList(ctx, "*")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Default inventory could change in future to many VMs, ensure we have
+	// at least these many VMs to test.
+	numTestVMs := 2
+	if len(vms) < numTestVMs {
+		t.Fatalf("Need at least %v VMs in a datacenter for this test", numTestVMs)
+	}
+	testVMs := []types.ManagedObjectReference{}
+	for _, vm := range vms[:numTestVMs] {
+		testVMs = append(testVMs, vm.Reference())
+	}
+
+	// Ensure VMs are powered off first before testing multi-VM power-on.
+	for _, vm := range vms[:numTestVMs] {
+		task, err := vm.PowerOff(ctx)
+		if err != nil {
+			t.Fatal(err)
+		}
+		err = task.Wait(ctx)
+		if err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	dcTask, err := dc.PowerOnVM(ctx, testVMs)
+	if err != nil {
+		t.Fatal(err)
+	}
+	info, err := dcTask.WaitForResult(ctx, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	switch dcResult := info.Result.(type) {
+	case types.ClusterPowerOnVmResult:
+		if len(dcResult.Attempted) != len(testVMs) {
+			t.Fatalf("Unexpected per-vm tasks in results, found %v, expected %v",
+				len(dcResult.Attempted), len(testVMs))
+		}
+		for i, vmResult := range dcResult.Attempted {
+			if vmResult.Task == nil {
+				t.Fatalf("Found per-vm task nil for VM #%v", i)
+			}
+			vmTask := object.NewTask(c.Client, *vmResult.Task)
+			err := vmTask.Wait(ctx)
+			if err != nil {
+				t.Fatalf("%v", err)
+			}
+		}
+	default:
+		t.Fatalf("Unexpected result type %T returned for DC PowerOnMultiVM", dcResult)
+	}
+}