Merge pull request #1997 from dougm/issue-1996

govc: support raw object references in import.ova NetworkMapping

Author: dougm

govc/importx/ovf.go

@@ -23,6 +23,7 @@ import (
 	"fmt"
 	"path"
 
+	"github.com/vmware/govmomi/find"
 	"github.com/vmware/govmomi/govc/cli"
 	"github.com/vmware/govmomi/govc/flags"
 	"github.com/vmware/govmomi/nfc"
@@ -163,34 +164,42 @@ func (cmd *ovfx) Map(op []Property) (p []types.KeyValue) {
 	return
 }
 
-func (cmd *ovfx) NetworkMap(e *ovf.Envelope) (p []types.OvfNetworkMapping) {
+func (cmd *ovfx) NetworkMap(e *ovf.Envelope) ([]types.OvfNetworkMapping, error) {
 	ctx := context.TODO()
 	finder, err := cmd.DatastoreFlag.Finder()
 	if err != nil {
-		return
+		return nil, err
 	}
 
-	networks := map[string]string{}
-
-	if e.Network != nil {
-		for _, net := range e.Network.Networks {
-			networks[net.Name] = net.Name
+	var nmap []types.OvfNetworkMapping
+	for _, m := range cmd.Options.NetworkMapping {
+		if m.Network == "" {
+			continue // Not set, let vSphere choose the default network
 		}
-	}
 
-	for _, net := range cmd.Options.NetworkMapping {
-		networks[net.Name] = net.Network
-	}
+		var ref types.ManagedObjectReference
 
-	for src, dst := range networks {
-		if net, err := finder.Network(ctx, dst); err == nil {
-			p = append(p, types.OvfNetworkMapping{
-				Name:    src,
-				Network: net.Reference(),
-			})
+		net, err := finder.Network(ctx, m.Network)
+		if err != nil {
+			switch err.(type) {
+			case *find.NotFoundError:
+				if !ref.FromString(m.Network) {
+					return nil, err
+				} // else this is a raw MO ref
+			default:
+				return nil, err
+			}
+		} else {
+			ref = net.Reference()
 		}
+
+		nmap = append(nmap, types.OvfNetworkMapping{
+			Name:    m.Name,
+			Network: ref,
+		})
 	}
-	return
+
+	return nmap, err
 }
 
 func (cmd *ovfx) Import(fpath string) (*types.ManagedObjectReference, error) {
@@ -224,6 +233,11 @@ func (cmd *ovfx) Import(fpath string) (*types.ManagedObjectReference, error) {
 		name = cmd.Name
 	}
 
+	nmap, err := cmd.NetworkMap(e)
+	if err != nil {
+		return nil, err
+	}
+
 	cisp := types.OvfCreateImportSpecParams{
 		DiskProvisioning:   cmd.Options.DiskProvisioning,
 		EntityName:         name,
@@ -233,7 +247,7 @@ func (cmd *ovfx) Import(fpath string) (*types.ManagedObjectReference, error) {
 			DeploymentOption: cmd.Options.Deployment,
 			Locale:           "US"},
 		PropertyMapping: cmd.Map(cmd.Options.PropertyMapping),
-		NetworkMapping:  cmd.NetworkMap(e),
+		NetworkMapping:  nmap,
 	}
 
 	host, err := cmd.HostSystemIfSpecified()

govc/test/import.bats

@@ -148,3 +148,45 @@ load test_helper
   run govc import.vmdk -force "$GOVC_TEST_VMDK_SRC" "$name"
   assert_success # exists, but -force was used
 }
+
+@test "import duplicate dvpg names" {
+  vcsim_env
+
+  run govc dvs.create DVS1 # DVS0 already exists
+  assert_success
+
+  run govc dvs.portgroup.add -dvs DVS0 -type ephemeral NSX-dvpg
+  assert_success
+
+  run govc dvs.portgroup.add -dvs DVS1 -type ephemeral NSX-dvpg
+  assert_success
+
+  ovf="$GOVC_IMAGES/$TTYLINUX_NAME.ovf"
+
+  spec=$(govc import.spec "$GOVC_IMAGES/$TTYLINUX_NAME.ovf")
+
+  run govc import.ovf -name ttylinux -options - "$ovf" <<<"$spec"
+  assert_success # no network specified
+
+  options=$(jq ".NetworkMapping[].Network = \"enoent\"" <<<"$spec")
+
+  run govc import.ovf -options - "$ovf" <<<"$options"
+  assert_failure # network not found
+
+  options=$(jq ".NetworkMapping[].Network = \"NSX-dvpg\"" <<<"$spec")
+
+  run govc import.ovf -options - "$ovf" <<<"$options"
+  assert_failure # 2 networks have the same name
+
+  options=$(jq ".NetworkMapping[].Network = \"DVS0/NSX-dvpg\"" <<<"$spec")
+
+  run govc import.ovf -name ttylinux2 -options - "$ovf" <<<"$options"
+  assert_success # switch_name/portgroup_name is unique
+
+  switch=$(govc find -i network -name DVS0)
+  id=$(govc find -i network -config.distributedVirtualSwitch "$switch" -name NSX-dvpg)
+  options=$(jq ".NetworkMapping[].Network = \"$id\"" <<<"$spec")
+
+  run govc import.ovf -options - "$ovf" <<<"$options"
+  assert_success # using raw MO id
+}

govc/test/network.bats

@@ -38,6 +38,22 @@ load test_helper
 
   run govc device.remove -vm $vm $eth0
   assert_failure "govc: device '$eth0' not found"
+
+  # Test PG's with the same name
+  run govc dvs.create DVS1 # DVS0 already exists
+  assert_success
+
+  run govc dvs.portgroup.add -dvs DVS0 -type ephemeral NSX-dvpg
+  assert_success
+
+  run govc dvs.portgroup.add -dvs DVS1 -type ephemeral NSX-dvpg
+  assert_success
+
+  run govc vm.network.add -vm $vm -net NSX-dvpg
+  assert_failure # resolves to multiple networks
+
+  run govc vm.network.add -vm $vm -net DVS0/NSX-dvpg
+  assert_success # switch_name/portgroup_name is unique
 }
 
 @test "network change backing" {

govc/vm/network/add.go

@@ -47,6 +47,7 @@ func (cmd *add) Description() string {
 
 Examples:
   govc vm.network.add -vm $vm -net "VM Network" -net.adapter e1000e
+  govc vm.network.add -vm $vm -net SwitchName/PortgroupName
   govc device.info -vm $vm ethernet-*`
 }
 

list/lister.go

@@ -165,6 +165,8 @@ func (l Lister) List(ctx context.Context) ([]Element, error) {
 		return l.ListHostSystem(ctx)
 	case "VirtualApp":
 		return l.ListVirtualApp(ctx)
+	case "VmwareDistributedVirtualSwitch", "DistributedVirtualSwitch":
+		return l.ListDistributedVirtualSwitch(ctx)
 	default:
 		return nil, fmt.Errorf("cannot traverse type " + l.Reference.Type)
 	}
@@ -497,6 +499,69 @@ func (l Lister) ListHostSystem(ctx context.Context) ([]Element, error) {
 	return es, nil
 }
 
+func (l Lister) ListDistributedVirtualSwitch(ctx context.Context) ([]Element, error) {
+	ospec := types.ObjectSpec{
+		Obj:  l.Reference,
+		Skip: types.NewBool(true),
+	}
+
+	fields := []string{
+		"portgroup",
+	}
+
+	for _, f := range fields {
+		tspec := types.TraversalSpec{
+			Path: f,
+			Skip: types.NewBool(false),
+			Type: "DistributedVirtualSwitch",
+		}
+
+		ospec.SelectSet = append(ospec.SelectSet, &tspec)
+	}
+
+	childTypes := []string{
+		"DistributedVirtualPortgroup",
+	}
+
+	var pspecs []types.PropertySpec
+	for _, t := range childTypes {
+		pspec := types.PropertySpec{
+			Type: t,
+		}
+
+		if l.All {
+			pspec.All = types.NewBool(true)
+		} else {
+			pspec.PathSet = []string{"name"}
+		}
+
+		pspecs = append(pspecs, pspec)
+	}
+
+	req := types.RetrieveProperties{
+		SpecSet: []types.PropertyFilterSpec{
+			{
+				ObjectSet: []types.ObjectSpec{ospec},
+				PropSet:   pspecs,
+			},
+		},
+	}
+
+	var dst []interface{}
+
+	err := l.retrieveProperties(ctx, req, &dst)
+	if err != nil {
+		return nil, err
+	}
+
+	es := []Element{}
+	for _, v := range dst {
+		es = append(es, ToElement(v.(mo.Reference), l.Prefix))
+	}
+
+	return es, nil
+}
+
 func (l Lister) ListVirtualApp(ctx context.Context) ([]Element, error) {
 	ospec := types.ObjectSpec{
 		Obj:  l.Reference,

simulator/dvs.go

@@ -18,6 +18,7 @@ package simulator
 
 import (
 	"strconv"
+	"strings"
 
 	"github.com/vmware/govmomi/vim25/methods"
 	"github.com/vmware/govmomi/vim25/mo"
@@ -41,10 +42,14 @@ func (s *DistributedVirtualSwitch) AddDVPortgroupTask(ctx *Context, c *types.Add
 			pg.Name = spec.Name
 			pg.Entity().Name = pg.Name
 
-			if obj := Map.FindByName(pg.Name, f.ChildEntity); obj != nil {
-				return nil, &types.DuplicateName{
-					Name:   pg.Name,
-					Object: obj.Reference(),
+			// Standard AddDVPortgroupTask() doesn't allow duplicate names, but NSX 3.0 does create some DVPGs with the same name.
+			// Allow duplicate names using this prefix so we can reproduce and test this condition.
+			if !strings.HasPrefix(pg.Name, "NSX-") {
+				if obj := Map.FindByName(pg.Name, f.ChildEntity); obj != nil {
+					return nil, &types.DuplicateName{
+						Name:   pg.Name,
+						Object: obj.Reference(),
+					}
 				}
 			}