Repository: vmware/govmomi
Pull request #2984 (2022-11-05 05:50:16)

vcsim: Fix duplicated name check in CloneVM_Task
## Description

This PR fixes CloneVM_Task error handling so that cloning a vm with duplicated name fails

Closes: #2983

## 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
- [ ] Build related change

## How Has This Been Tested?

- [x] add unit tests
- [x] run test script (written in issue #2983 `To Reproduce`)

```bash
$ go run main.go
task failed: *types.DuplicateName

$ govc tasks -n 1
Task                                     Target                         Initiator                         Queued   Started Completed Result
CloneVm                                  vm-55                          vcsim                           13:52:45  13:52:45  13:52:45 error   [*types.DuplicateName]
```

## Checklist:

- [x] My code follows the `CONTRIBUTION`
  [guidelines](https://github.com/vmware/govmomi/blob/master/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/virtual_machine.go b/simulator/virtual_machine.go
index 8c256580..8deeceda 100644
--- a/simulator/virtual_machine.go
+++ b/simulator/virtual_machine.go
@@ -1901,6 +1901,12 @@ func (vm *VirtualMachine) CloneVMTask(ctx *Context, req *types.CloneVM_Task) soa
 		if pool == nil {
 			return nil, &types.InvalidArgument{InvalidProperty: "spec.location.pool"}
 		}
+		if obj := ctx.Map.FindByName(req.Name, folder.ChildEntity); obj != nil {
+			return nil, &types.DuplicateName{
+				Name:   req.Name,
+				Object: obj.Reference(),
+			}
+		}
 		config := types.VirtualMachineConfigSpec{
 			Name:    req.Name,
 			GuestId: vm.Config.GuestId,


Test patch:
diff --git a/govc/test/vm.bats b/govc/test/vm.bats
index a1c26bc9..d1ac1add 100755
--- a/govc/test/vm.bats
+++ b/govc/test/vm.bats
@@ -800,8 +800,11 @@ load test_helper
   run govc vm.clone -cluster DC0_C0 -vm "$vm" "$clone"
   assert_failure # already exists
 
+  run govc datastore.cp "$clone"/"$clone".vmx "$clone"/"$clone".vmx.copy
+  run govc vm.destroy "$clone"
+  run govc datastore.mv "$clone"/"$clone".vmx.copy "$clone"/"$clone".vmx # leave vmx file
   run govc vm.clone -force -vm "$vm" "$clone"
-  assert_success # clone vm with the same name
+  assert_success # clone vm with the same name vmx file
 
   vm=$(new_empty_vm)
   run govc vm.disk.create -vm "$vm" -thick -eager -size 10M -name "$vm/data.vmdk"
diff --git a/simulator/virtual_machine_test.go b/simulator/virtual_machine_test.go
index e659bc42..a983eddb 100644
--- a/simulator/virtual_machine_test.go
+++ b/simulator/virtual_machine_test.go
@@ -333,6 +333,77 @@ func TestCreateVmWithSpecialCharaters(t *testing.T) {
 	}
 }
 
+func TestCloneVm(t *testing.T) {
+	tests := []struct {
+		name   string
+		vmName string
+		config types.VirtualMachineCloneSpec
+		fail   bool
+	}{
+		{
+			"clone a vm",
+			"cloned-vm",
+			types.VirtualMachineCloneSpec{
+				Template: false,
+				PowerOn:  false,
+			},
+			false,
+		},
+		{
+			"vm name is duplicated",
+			"DC0_H0_VM0",
+			types.VirtualMachineCloneSpec{
+				Template: false,
+				PowerOn:  false,
+			},
+			true,
+		},
+	}
+
+	for _, test := range tests {
+		test := test // assign to local var since loop var is reused
+
+		t.Run(test.name, func(t *testing.T) {
+			m := VPX()
+			defer m.Remove()
+
+			Test(func(ctx context.Context, c *vim25.Client) {
+				finder := find.NewFinder(c, false)
+				dc, err := finder.DefaultDatacenter(ctx)
+				if err != nil {
+					t.Fatal(err)
+				}
+
+				folders, err := dc.Folders(ctx)
+				if err != nil {
+					t.Fatal(err)
+				}
+
+				vmFolder := folders.VmFolder
+
+				vmm := Map.Any("VirtualMachine").(*VirtualMachine)
+				vm := object.NewVirtualMachine(c, vmm.Reference())
+
+				task, err := vm.Clone(ctx, vmFolder, test.vmName, test.config)
+				if err != nil {
+					t.Fatal(err)
+				}
+
+				err = task.Wait(ctx)
+				if test.fail {
+					if err == nil {
+						t.Errorf("%s: expected error", test.name)
+					}
+				} else {
+					if err != nil {
+						t.Errorf("%s: %s", test.name, err)
+					}
+				}
+			}, m)
+		})
+	}
+}
+
 func TestReconfigVmDevice(t *testing.T) {
 	ctx := context.Background()