From 10c22fd1a61074b402207f440837624da90820ed Mon Sep 17 00:00:00 2001 From: Doug MacEachern Date: Thu, 28 May 2020 11:23:59 -0700 Subject: [PATCH 1/2] govc: support raw object references in import.ova NetworkMapping vCenter doesn't allow duplicate port group names within the network folder when creating via the UI or the AddDVPortgroupTask() API. However, NSXt 3.0 can do so via other (unknown) means. With this condition, govc would fail to map the given network name to its managed object reference. To workaround this, a raw MO ref can now be used in the NetworkMapping, instead of the Network name or inventory path. Related history: 223168f0 * Add NetworkMapping section to importx options. e3c3cd0a * Populate network mapping from ovf envelope (#546) Fixes #1996 --- govc/importx/ovf.go | 52 +++++++++++++++++++++++++++---------------- govc/test/import.bats | 37 ++++++++++++++++++++++++++++++ simulator/dvs.go | 13 +++++++---- 3 files changed, 79 insertions(+), 23 deletions(-) diff --git a/govc/importx/ovf.go b/govc/importx/ovf.go index ed7570e92..9672389f6 100644 --- a/govc/importx/ovf.go +++ b/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() diff --git a/govc/test/import.bats b/govc/test/import.bats index 8a9a84cd0..839c26b88 100755 --- a/govc/test/import.bats +++ b/govc/test/import.bats @@ -148,3 +148,40 @@ 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 + + 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 +} diff --git a/simulator/dvs.go b/simulator/dvs.go index bc61b3d91..d40ae9250 100644 --- a/simulator/dvs.go +++ b/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(), + } } } From 7cdad997ab5e78fb1ab86d12db1dfd8f6620e004 Mon Sep 17 00:00:00 2001 From: Doug MacEachern Date: Fri, 29 May 2020 05:38:47 -0700 Subject: [PATCH 2/2] Finder: support DistributedVirtualSwitch traversal While the native vSphere SearchIndex and ContainerView APIs don't support inventory paths with this structure, it is useful for use with NSXt3 where portgroup may not be unique within the same network inventory folder. --- govc/test/import.bats | 5 ++++ govc/test/network.bats | 16 +++++++++++ govc/vm/network/add.go | 1 + list/lister.go | 65 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 87 insertions(+) diff --git a/govc/test/import.bats b/govc/test/import.bats index 839c26b88..d0f56c392 100755 --- a/govc/test/import.bats +++ b/govc/test/import.bats @@ -178,6 +178,11 @@ load test_helper 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") diff --git a/govc/test/network.bats b/govc/test/network.bats index 655f0699b..a5155dd45 100755 --- a/govc/test/network.bats +++ b/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" { diff --git a/govc/vm/network/add.go b/govc/vm/network/add.go index 66f0aa1f4..9f4c5d8f4 100644 --- a/govc/vm/network/add.go +++ b/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-*` } diff --git a/list/lister.go b/list/lister.go index 2ee32e6bc..9a4caed68 100644 --- a/list/lister.go +++ b/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,