Repository: vmware/govmomi
Pull request #1924 (2020-04-10 18:20:17)

vcsim: validate DVS membership
When adding a DVPG backed network to a VM, vCenter requires that all hosts in the ResourcePool
are connected to the DVS.


Patch:
diff --git a/govc/vm/clone.go b/govc/vm/clone.go
index acd3ea1a..df2a42e3 100644
--- a/govc/vm/clone.go
+++ b/govc/vm/clone.go
@@ -117,7 +117,7 @@ func (cmd *clone) Usage() string {
 }
 
 func (cmd *clone) Description() string {
-	return `Clone VM or template to NAME.
+	return `Clone VM to NAME.
 
 Examples:
   govc vm.clone -vm template-vm new-vm
@@ -126,8 +126,7 @@ Examples:
   govc vm.clone -vm template-vm -link -snapshot s-name new-vm
   govc vm.clone -vm template-vm -cluster cluster1 new-vm # use compute cluster placement
   govc vm.clone -vm template-vm -datastore-cluster dscluster new-vm # use datastore cluster placement
-  govc vm.clone -vm template-vm -snapshot $(govc snapshot.tree -vm template-vm -C) new-vm
-  govc vm.clone -vm template-vm -template new-template # clone a VM template`
+  govc vm.clone -vm template-vm -snapshot $(govc snapshot.tree -vm template-vm -C) new-vm`
 }
 
 func (cmd *clone) Process(ctx context.Context) error {
@@ -262,10 +261,6 @@ func (cmd *clone) Run(ctx context.Context, f *flag.FlagSet) error {
 		}
 	}
 
-	if cmd.template {
-		return nil
-	}
-
 	if cmd.on {
 		task, err := vm.PowerOn(ctx)
 		if err != nil {
diff --git a/simulator/virtual_machine.go b/simulator/virtual_machine.go
index 9434c034..00733cb6 100644
--- a/simulator/virtual_machine.go
+++ b/simulator/virtual_machine.go
@@ -949,6 +949,47 @@ func getDiskSize(disk *types.VirtualDisk) int64 {
 	return disk.CapacityInBytes
 }
 
+func (vm *VirtualMachine) validateSwitchMembers(id string) types.BaseMethodFault {
+	var dswitch *DistributedVirtualSwitch
+
+	var find func(types.ManagedObjectReference)
+	find = func(child types.ManagedObjectReference) {
+		s, ok := Map.Get(child).(*DistributedVirtualSwitch)
+		if ok && s.Uuid == id {
+			dswitch = s
+			return
+		}
+		walk(Map.Get(child), find)
+	}
+	f := Map.getEntityDatacenter(vm).NetworkFolder
+	walk(Map.Get(f), find) // search in NetworkFolder and any sub folders
+
+	if dswitch == nil {
+		log.Printf("DVS %s cannot be found", id)
+		return new(types.NotFound)
+	}
+
+	h := Map.Get(*vm.Runtime.Host).(*HostSystem)
+	c := hostParent(&h.HostSystem)
+	isMember := func(val types.ManagedObjectReference) bool {
+		for _, mem := range dswitch.Summary.HostMember {
+			if mem == val {
+				return true
+			}
+		}
+		log.Printf("%s is not a member of VDS %s", h.Name, dswitch.Name)
+		return false
+	}
+
+	for _, ref := range c.Host {
+		if !isMember(ref) {
+			return &types.InvalidArgument{InvalidProperty: "spec.deviceChange.device.port.switchUuid"}
+		}
+	}
+
+	return nil
+}
+
 func (vm *VirtualMachine) configureDevice(devices object.VirtualDeviceList, spec *types.VirtualDeviceConfigSpec) types.BaseMethodFault {
 	device := spec.Device
 	d := device.GetVirtualDevice()
@@ -988,6 +1029,9 @@ func (vm *VirtualMachine) configureDevice(devices object.VirtualDeviceList, spec
 			summary = fmt.Sprintf("DVSwitch: %s", b.Port.SwitchUuid)
 			net.Type = "DistributedVirtualPortgroup"
 			net.Value = b.Port.PortgroupKey
+			if err := vm.validateSwitchMembers(b.Port.SwitchUuid); err != nil {
+				return err
+			}
 		}
 
 		Map.Update(vm, []types.PropertyChange{


Test patch:
diff --git a/simulator/virtual_machine_fault_test.go b/simulator/virtual_machine_fault_test.go
new file mode 100644
index 00000000..45571a1c
--- /dev/null
+++ b/simulator/virtual_machine_fault_test.go
@@ -0,0 +1,124 @@
+/*
+Copyright (c) 2020 VMware, Inc. All Rights Reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package simulator_test
+
+import (
+	"context"
+	"testing"
+
+	"github.com/vmware/govmomi/find"
+	"github.com/vmware/govmomi/object"
+	"github.com/vmware/govmomi/simulator"
+	"github.com/vmware/govmomi/task"
+	"github.com/vmware/govmomi/vim25"
+	"github.com/vmware/govmomi/vim25/mo"
+	"github.com/vmware/govmomi/vim25/types"
+)
+
+func TestSwitchMembers(t *testing.T) {
+	simulator.Test(func(ctx context.Context, c *vim25.Client) {
+		finder := find.NewFinder(c)
+		dvs, _ := finder.Network(ctx, "DVS0")
+		pg, _ := finder.Network(ctx, "DC0_DVPG0")
+		vm, _ := finder.VirtualMachine(ctx, "DC0_C0_RP0_VM0")
+		host, _ := vm.HostSystem(ctx)
+
+		// The proper way to remove a host from is with dvs.Reconfigure + types.ConfigSpecOperationRemove,
+		// but that would fail here with ResourceInUse. And creating a new DVS + PG is cumbersome,
+		// so just force the removal of host from the DVS + PG, which has the same effect on vm.AddDevice().
+		dswitch := simulator.Map.Get(dvs.Reference()).(*simulator.DistributedVirtualSwitch)
+		portgrp := simulator.Map.Get(pg.Reference()).(*simulator.DistributedVirtualPortgroup)
+		simulator.RemoveReference(&dswitch.Summary.HostMember, host.Reference())
+		simulator.RemoveReference(&portgrp.Host, host.Reference())
+
+		backing, _ := pg.EthernetCardBackingInfo(ctx)
+		nic, _ := object.EthernetCardTypes().CreateEthernetCard("", backing)
+
+		err := vm.AddDevice(ctx, nic)
+		if err == nil {
+			t.Fatal("expected error")
+		}
+
+		fault := err.(task.Error).Fault()
+		invalid, ok := fault.(*types.InvalidArgument)
+		if !ok {
+			t.Fatalf("unexpected fault=%T", fault)
+		}
+
+		if invalid.InvalidProperty != "spec.deviceChange.device.port.switchUuid" {
+			t.Errorf("unexpected property=%s", invalid.InvalidProperty)
+		}
+	})
+}
+
+func TestMultiSwitchMembers(t *testing.T) {
+	model := simulator.VPX()
+	model.PortgroupNSX = 1
+
+	simulator.Test(func(ctx context.Context, c *vim25.Client) {
+		finder := find.NewFinder(c)
+		dc, _ := finder.Datacenter(ctx, "DC0")
+		f, _ := dc.Folders(ctx)
+		vm, _ := finder.VirtualMachine(ctx, "DC0_C0_RP0_VM0")
+		pg0, _ := finder.Network(ctx, "DC0_NSXPG0")
+
+		var props0 mo.DistributedVirtualPortgroup
+		_ = dc.Properties(ctx, pg0.Reference(), []string{"config"}, &props0)
+
+		// Create a 2nd DVS "DVS1" with no hosts attached and 1 PG "DC0_NSXPG1",
+		// using the same LogicalSwitchUuid as DC0_NSXPG0
+		_, _ = f.NetworkFolder.CreateDVS(ctx, types.DVSCreateSpec{
+			ConfigSpec: &types.VMwareDVSConfigSpec{
+				DVSConfigSpec: types.DVSConfigSpec{
+					Name: "DVS1",
+				},
+			},
+		})
+		dvs, _ := finder.Network(ctx, "DVS1")
+		dswitch := dvs.(*object.DistributedVirtualSwitch)
+		_, _ = dswitch.AddPortgroup(ctx, []types.DVPortgroupConfigSpec{{
+			Name:              "DC0_NSXPG1",
+			LogicalSwitchUuid: props0.Config.LogicalSwitchUuid,
+		}})
+		pg1, _ := finder.Network(ctx, "DC0_NSXPG1")
+
+		backing, _ := pg1.EthernetCardBackingInfo(ctx)
+		nic, _ := object.EthernetCardTypes().CreateEthernetCard("", backing)
+
+		err := vm.AddDevice(ctx, nic)
+		if err == nil {
+			t.Fatal("expected error")
+		}
+
+		fault := err.(task.Error).Fault()
+		invalid, ok := fault.(*types.InvalidArgument)
+		if !ok {
+			t.Fatalf("unexpected fault=%T", fault)
+		}
+
+		if invalid.InvalidProperty != "spec.deviceChange.device.port.switchUuid" {
+			t.Errorf("unexpected property=%s", invalid.InvalidProperty)
+		}
+
+		var props1 mo.DistributedVirtualPortgroup
+		_ = dc.Properties(ctx, pg1.Reference(), []string{"config"}, &props1)
+
+		if props0.Config.LogicalSwitchUuid != props1.Config.LogicalSwitchUuid {
+			t.Error("LogicalSwitchUuid should be the same")
+		}
+	}, model)
+}