Repository: vmware/govmomi
Pull request #2657 (2021-11-12 13:57:10)

fix: Distribute vcsim VMs across resource pools
This fix aligns the behavior of vcsim with the documentation for the
`pool` and `machine` model flags.

When no child pools are created, i.e. `pool` is not specified, VMs will
be deployed in the root resource pool of a cluster, i.e. `RP0`. The VMs
will also be prefixed with the corresponding resource pool. In order to
not break standalone host behavior, VMs deployed on a standalone host
will not carry a resource pool identifier in their name. See the linked
issue for deployment examples and naming.

The total number of VMs deployed in a VPX model is the result of the
number of datacenters, clusters and resource pools.

BREAKING:

The name of virtual machines deployed in `vcsim` in a cluster (and
optionally child resource pools) has changed to include the
corresponding resource pool name. VM names deployed to standalone hosts
in `vcsim` are not changed.

Closes: #2047
Signed-off-by: Michael Gasch <mgasch@vmware.com>

## Description

Please include a summary of the change and which issue is fixed. Please also
include relevant motivation and context. List any dependencies that are required
for this change.

Closes: #(issue-number)

## 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)
- [x] Breaking change (fix or feature that would cause existing functionality to
  not work as expected)
- [x] This change requires a documentation update
- [ ] Build related change

## How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide
instructions so we can reproduce. If applicable, please also list any relevant
details for your test configuration.

- [x] make test passes
- [x] make govc-test passes

## 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
- [x] 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

embano1: Note unit tests failing due to regression in #2654
embano1: Example outputs:

## `vcsim` (defaults)
```json
"DC0_H0_VM0"
{
  "Type": "ResourcePool",
  "Value": "resgroup-22"
}
"DC0_H0_VM1"
{
  "Type": "ResourcePool",
  "Value": "resgroup-22"
}
"DC0_C0_RP0_VM0"
{
  "Type": "ResourcePool",
  "Value": "resgroup-26"
}
"DC0_C0_RP0_VM1"
{
  "Type": "ResourcePool",
  "Value": "resgroup-26"
}
```

## `vcsim -dc 1 -pool 1 -cluster 1 -vm 2`

```json
"DC0_H0_VM0"
{
  "Type": "ResourcePool",
  "Value": "resgroup-22"
}
"DC0_H0_VM1"
{
  "Type": "ResourcePool",
  "Value": "resgroup-22"
}
"DC0_C0_RP1_VM0"
{
  "Type": "ResourcePool",
  "Value": "resgroup-52"
}
"DC0_C0_RP1_VM1"
{
  "Type": "ResourcePool",
  "Value": "resgroup-52"
}
```

## `vcsim -dc 2 -cluster 1 -vm 2`

```json
"DC0_H0_VM0"
{
  "Type": "ResourcePool",
  "Value": "resgroup-22"
}
"DC0_H0_VM1"
{
  "Type": "ResourcePool",
  "Value": "resgroup-22"
}
"DC0_C0_RP0_VM0"
{
  "Type": "ResourcePool",
  "Value": "resgroup-26"
}
"DC0_C0_RP0_VM1"
{
  "Type": "ResourcePool",
  "Value": "resgroup-26"
}
"DC1_H0_VM0"
{
  "Type": "ResourcePool",
  "Value": "resgroup-72"
}
"DC1_H0_VM1"
{
  "Type": "ResourcePool",
  "Value": "resgroup-72"
}
"DC1_C0_RP0_VM0"
{
  "Type": "ResourcePool",
  "Value": "resgroup-76"
}
"DC1_C0_RP0_VM1"
{
  "Type": "ResourcePool",
  "Value": "resgroup-76"
}
```

## `vcsim -dc 1 -pool 2 -cluster 2 -vm 2`

```json
"DC0_H0_VM0"
{
  "Type": "ResourcePool",
  "Value": "resgroup-22"
}
"DC0_H0_VM1"
{
  "Type": "ResourcePool",
  "Value": "resgroup-22"
}
"DC0_C0_RP1_VM0"
{
  "Type": "ResourcePool",
  "Value": "resgroup-52"
}
"DC0_C0_RP1_VM1"
{
  "Type": "ResourcePool",
  "Value": "resgroup-52"
}
"DC0_C0_RP2_VM0"
{
  "Type": "ResourcePool",
  "Value": "resgroup-53"
}
"DC0_C0_RP2_VM1"
{
  "Type": "ResourcePool",
  "Value": "resgroup-53"
}
"DC0_C1_RP1_VM0"
{
  "Type": "ResourcePool",
  "Value": "resgroup-81"
}
"DC0_C1_RP1_VM1"
{
  "Type": "ResourcePool",
  "Value": "resgroup-81"
}
"DC0_C1_RP2_VM0"
{
  "Type": "ResourcePool",
  "Value": "resgroup-82"
}
"DC0_C1_RP2_VM1"
{
  "Type": "ResourcePool",
  "Value": "resgroup-82"
}
```

Patch:
diff --git a/simulator/model.go b/simulator/model.go
index 3083677b..594ed137 100644
--- a/simulator/model.go
+++ b/simulator/model.go
@@ -107,6 +107,7 @@ type Model struct {
 	// For example: /DC0/host/DC0_C0/Resources
 	// The root ResourcePool is named "RP0" within other object names.
 	// When Model.Pool is set to 1 or higher, this creates child ResourcePools under the root pool.
+	// Note that this flag is not effective on standalone hosts.
 	// For example: /DC0/host/DC0_C0/Resources/DC0_C0_RP1
 	// Name prefix: RP, vcsim flag: -pool
 	Pool int
@@ -117,7 +118,12 @@ type Model struct {
 	// Name prefix: LocalDS, vcsim flag: -ds
 	Datastore int
 
-	// Machine specifies the number of VirtualMachine entities to create per ResourcePool
+	// Machine specifies the number of VirtualMachine entities to create per
+	// ResourcePool. If the pool flag is specified, the specified number of virtual
+	// machines will be deployed to each child pool and prefixed with the child
+	// resource pool name. Otherwise they are deployed into the root resource pool,
+	// prefixed with RP0. On standalone hosts, machines are always deployed into the
+	// root resource pool without any prefix.
 	// Name prefix: VM, vcsim flag: -vm
 	Machine int
 
@@ -731,19 +737,24 @@ func (m *Model) Create() error {
 				}
 			}
 
-			pool, err := cluster.ResourcePool(ctx)
+			rootRP, err := cluster.ResourcePool(ctx)
 			if err != nil {
 				return err
 			}
 
 			prefix := clusterName + "_RP"
 
-			addMachine(prefix+"0", nil, pool, folders)
+			// put VMs in cluster RP if no child RP(s) configured
+			if m.Pool == 0 {
+				addMachine(prefix+"0", nil, rootRP, folders)
+			}
 
-			for npool := 1; npool <= m.Pool; npool++ {
+			// create child RP(s) with VMs
+			for childRP := 1; childRP <= m.Pool; childRP++ {
 				spec := types.DefaultResourceConfigSpec()
 
-				_, err = pool.Create(ctx, m.fmtName(prefix, npool), spec)
+				p, err := rootRP.Create(ctx, m.fmtName(prefix, childRP), spec)
+				addMachine(m.fmtName(prefix, childRP), nil, p, folders)
 				if err != nil {
 					return err
 				}
@@ -756,7 +767,7 @@ func (m *Model) Create() error {
 				vspec := NewVAppConfigSpec()
 				name := m.fmtName(prefix, napp)
 
-				vapp, err := pool.CreateVApp(ctx, name, rspec, vspec, nil)
+				vapp, err := rootRP.CreateVApp(ctx, name, rspec, vspec, nil)
 				if err != nil {
 					return err
 				}


Test patch:
diff --git a/simulator/finder_test.go b/simulator/finder_test.go
index 8a1c4f56..edd6977f 100644
--- a/simulator/finder_test.go
+++ b/simulator/finder_test.go
@@ -74,9 +74,9 @@ func TestFinderVPX(t *testing.T) {
 		{"DatacenterList", "/F0/*", 1},
 		{"DatacenterList", "/DC0", 1},
 		{"VirtualMachineList", "/DC0/vm/*", (m.Host + m.Cluster) * m.Machine},
-		{"VirtualMachineList", "F0/DC1_C0_RP0_VM0", 1},
-		{"VirtualMachineList", "./DC1_C0_RP0_VM0", 0},
-		{"VirtualMachineList", "DC1_C0_RP0_VM0", 1}, // find . -type VirtualMachine -name DC1_C0_RP0_VM0
+		{"VirtualMachineList", "F0/DC1_C0_RP1_VM0", 1},
+		{"VirtualMachineList", "./DC1_C0_RP1_VM0", 0},
+		{"VirtualMachineList", "DC1_C0_RP1_VM0", 1}, // find . -type VirtualMachine -name DC1_C0_RP0_VM0
 		{"VirtualAppList", "/DC0/vm/*", 0},
 		{"DatastoreList", "/DC0/datastore/*", m.Datastore},
 		{"DatastoreList", "./*", 0},
@@ -123,7 +123,7 @@ func TestFinderVPX(t *testing.T) {
 		{"DatacenterList", "/*", m.Datacenter - m.Folder},
 		{"DatacenterList", "/*/*", m.Folder},
 		{"DatastoreList", "/F1/DC2/datastore/F1/LocalDS_0", 1},
-		{"VirtualMachineList", "DC1_C0_RP0_VM0", 0}, // TODO: recurse all Datacenters?
+		{"VirtualMachineList", "DC1_C0_RP1_VM0", 0}, // TODO: recurse all Datacenters?
 	}
 
 	f := reflect.ValueOf(finder)
diff --git a/simulator/model_test.go b/simulator/model_test.go
index 1503049a..db7eca4c 100644
--- a/simulator/model_test.go
+++ b/simulator/model_test.go
@@ -26,7 +26,7 @@ func compareModel(t *testing.T, m *Model) {
 	count := m.Count()
 
 	hosts := (m.Host + (m.ClusterHost * m.Cluster)) * m.Datacenter
-	vms := ((m.Host + m.Cluster) * m.Datacenter) * m.Machine
+	vms := ((m.Host + m.Cluster + m.Pool) * m.Datacenter) * m.Machine
 	// child pools + root pools
 	pools := (m.Pool * m.Cluster * m.Datacenter) + (m.Host+m.Cluster)*m.Datacenter
 	// root folder + Datacenter folders {host,vm,datastore,network} + top-level folders
diff --git a/simulator/search_index_test.go b/simulator/search_index_test.go
index 942220d1..0cac8dde 100644
--- a/simulator/search_index_test.go
+++ b/simulator/search_index_test.go
@@ -160,9 +160,9 @@ func TestSearchIndexFindChild(t *testing.T) {
 		// Datacenter -> host Folder -> Cluster -> ResourcePool -> ResourcePool
 		{"DC0", "host", "DC0_C0", "Resources", "DC0_C0_RP1"},
 		// Datacenter -> host Folder -> Cluster -> ResourcePool -> VirtualMachine
-		{"DC0", "host", "DC0_C0", "Resources", "DC0_C0_RP0_VM0"},
+		{"DC0", "host", "DC0_C0", "Resources", "DC0_C0_RP1", "DC0_C0_RP1_VM0"},
 		// Datacenter -> vm Folder -> VirtualMachine
-		{"DC0", "vm", "DC0_C0_RP0_VM0"},
+		{"DC0", "vm", "DC0_C0_RP1_VM0"},
 	}
 
 	root := c.ServiceContent.RootFolder