From 125ce7232722a9e7cdf813f54dd7834e52d8be38 Mon Sep 17 00:00:00 2001 From: Doug MacEachern Date: Tue, 20 Feb 2018 10:22:01 -0800 Subject: [PATCH 1/2] vSphere: Minimize property collection via Finder The 'All' parameter of the 'NewFinder' function controls property collection while searching the inventory. When 'All' is set to 'false', Finder collects the minimal set of object properties required to search inventory. When 'All' is set to 'true', Finder collects *all* object properties, which are *not* required to search inventory. Setting 'All' to 'true' is only useful when inspecting all properties of an object, such as by certain govc commands when the '-json' or '-dump' flags are specified. Changing All=false in VCP minimizes the SOAP payload size and marshalling required on both sides, without impacting any functionality. --- .../providers/vsphere/vclib/datacenter.go | 4 +- .../providers/vsphere/vclib/utils.go | 2 +- .../providers/vsphere/vclib/utils_test.go | 71 +++++++++++++++++++ test/e2e/storage/vsphere/vsphere.go | 17 ++--- test/e2e/storage/vsphere/vsphere_utils.go | 7 +- 5 files changed, 87 insertions(+), 14 deletions(-) create mode 100644 pkg/cloudprovider/providers/vsphere/vclib/utils_test.go diff --git a/pkg/cloudprovider/providers/vsphere/vclib/datacenter.go b/pkg/cloudprovider/providers/vsphere/vclib/datacenter.go index 8b0a10e9a92..d62695b5c5a 100644 --- a/pkg/cloudprovider/providers/vsphere/vclib/datacenter.go +++ b/pkg/cloudprovider/providers/vsphere/vclib/datacenter.go @@ -39,7 +39,7 @@ type Datacenter struct { // GetDatacenter returns the DataCenter Object for the given datacenterPath // If datacenter is located in a folder, include full path to datacenter else just provide the datacenter name func GetDatacenter(ctx context.Context, connection *VSphereConnection, datacenterPath string) (*Datacenter, error) { - finder := find.NewFinder(connection.GoVmomiClient.Client, true) + finder := find.NewFinder(connection.GoVmomiClient.Client, false) datacenter, err := finder.Datacenter(ctx, datacenterPath) if err != nil { glog.Errorf("Failed to find the datacenter: %s. err: %+v", datacenterPath, err) @@ -52,7 +52,7 @@ func GetDatacenter(ctx context.Context, connection *VSphereConnection, datacente // GetAllDatacenter returns all the DataCenter Objects func GetAllDatacenter(ctx context.Context, connection *VSphereConnection) ([]*Datacenter, error) { var dc []*Datacenter - finder := find.NewFinder(connection.GoVmomiClient.Client, true) + finder := find.NewFinder(connection.GoVmomiClient.Client, false) datacenters, err := finder.DatacenterList(ctx, "*") if err != nil { glog.Errorf("Failed to find the datacenter. err: %+v", err) diff --git a/pkg/cloudprovider/providers/vsphere/vclib/utils.go b/pkg/cloudprovider/providers/vsphere/vclib/utils.go index d449e5fe905..0704948b802 100644 --- a/pkg/cloudprovider/providers/vsphere/vclib/utils.go +++ b/pkg/cloudprovider/providers/vsphere/vclib/utils.go @@ -46,7 +46,7 @@ func IsNotFound(err error) bool { } func getFinder(dc *Datacenter) *find.Finder { - finder := find.NewFinder(dc.Client(), true) + finder := find.NewFinder(dc.Client(), false) finder.SetDatacenter(dc.Datacenter) return finder } diff --git a/pkg/cloudprovider/providers/vsphere/vclib/utils_test.go b/pkg/cloudprovider/providers/vsphere/vclib/utils_test.go new file mode 100644 index 00000000000..ab4bd89df42 --- /dev/null +++ b/pkg/cloudprovider/providers/vsphere/vclib/utils_test.go @@ -0,0 +1,71 @@ +/* +Copyright 2018 The Kubernetes Authors. + +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 vclib + +import ( + "context" + "testing" + + "github.com/vmware/govmomi" + "github.com/vmware/govmomi/simulator" +) + +func TestUtils(t *testing.T) { + ctx := context.Background() + + model := simulator.VPX() + // Child folder "F0" will be created under the root folder and datacenter folders, + // and all resources are created within the "F0" child folders. + model.Folder = 1 + + defer model.Remove() + err := model.Create() + if err != nil { + t.Fatal(err) + } + + s := model.Service.NewServer() + defer s.Close() + + c, err := govmomi.NewClient(ctx, s.URL, true) + if err != nil { + t.Fatal(err) + } + + vc := &VSphereConnection{GoVmomiClient: c} + + dc, err := GetDatacenter(ctx, vc, testDefaultDatacenter) + if err != nil { + t.Error(err) + } + + finder := getFinder(dc) + datastores, err := finder.DatastoreList(ctx, "*") + if err != nil { + t.Fatal(err) + } + + count := model.Count() + if count.Datastore != len(datastores) { + t.Errorf("got %d Datastores, expected: %d", len(datastores), count.Datastore) + } + + _, err = finder.Datastore(ctx, testNameNotFound) + if !IsNotFound(err) { + t.Errorf("unexpected error: %s", err) + } +} diff --git a/test/e2e/storage/vsphere/vsphere.go b/test/e2e/storage/vsphere/vsphere.go index b9cb5925b6e..d5c52055997 100644 --- a/test/e2e/storage/vsphere/vsphere.go +++ b/test/e2e/storage/vsphere/vsphere.go @@ -18,6 +18,11 @@ package vsphere import ( "fmt" + "path/filepath" + "strconv" + "strings" + "time" + "github.com/vmware/govmomi" "github.com/vmware/govmomi/find" "github.com/vmware/govmomi/object" @@ -25,10 +30,6 @@ import ( "github.com/vmware/govmomi/vim25/types" "golang.org/x/net/context" "k8s.io/kubernetes/test/e2e/framework" - "path/filepath" - "strconv" - "strings" - "time" ) const ( @@ -57,7 +58,7 @@ type VolumeOptions struct { // GetDatacenter returns the DataCenter Object for the given datacenterPath func (vs *VSphere) GetDatacenter(ctx context.Context, datacenterPath string) (*object.Datacenter, error) { Connect(ctx, vs) - finder := find.NewFinder(vs.Client.Client, true) + finder := find.NewFinder(vs.Client.Client, false) return finder.Datacenter(ctx, datacenterPath) } @@ -70,7 +71,7 @@ func (vs *VSphere) GetDatacenterFromObjectReference(ctx context.Context, dc obje // GetAllDatacenter returns all the DataCenter Objects func (vs *VSphere) GetAllDatacenter(ctx context.Context) ([]*object.Datacenter, error) { Connect(ctx, vs) - finder := find.NewFinder(vs.Client.Client, true) + finder := find.NewFinder(vs.Client.Client, false) return finder.DatacenterList(ctx, "*") } @@ -88,7 +89,7 @@ func (vs *VSphere) GetVMByUUID(ctx context.Context, vmUUID string, dc object.Ref func (vs *VSphere) GetFolderByPath(ctx context.Context, dc object.Reference, folderPath string) (vmFolderMor types.ManagedObjectReference, err error) { Connect(ctx, vs) datacenter := object.NewDatacenter(vs.Client.Client, dc.Reference()) - finder := find.NewFinder(datacenter.Client(), true) + finder := find.NewFinder(datacenter.Client(), false) finder.SetDatacenter(datacenter) vmFolder, err := finder.Folder(ctx, folderPath) if err != nil { @@ -113,7 +114,7 @@ func (vs *VSphere) CreateVolume(volumeOptions *VolumeOptions, dataCenterRef type return "", fmt.Errorf("datacenter is nil") } vs.initVolumeOptions(volumeOptions) - finder := find.NewFinder(datacenter.Client(), true) + finder := find.NewFinder(datacenter.Client(), false) finder.SetDatacenter(datacenter) ds, err := finder.Datastore(ctx, volumeOptions.Datastore) if err != nil { diff --git a/test/e2e/storage/vsphere/vsphere_utils.go b/test/e2e/storage/vsphere/vsphere_utils.go index 8cea902ca51..468d240b25e 100644 --- a/test/e2e/storage/vsphere/vsphere_utils.go +++ b/test/e2e/storage/vsphere/vsphere_utils.go @@ -41,10 +41,11 @@ import ( "k8s.io/kubernetes/test/e2e/framework" "k8s.io/kubernetes/test/e2e/storage/utils" - "github.com/vmware/govmomi/find" - vimtypes "github.com/vmware/govmomi/vim25/types" "regexp" "strings" + + "github.com/vmware/govmomi/find" + vimtypes "github.com/vmware/govmomi/vim25/types" ) const ( @@ -657,7 +658,7 @@ func registerNodeVM(nodeName, workingDir, vmxFilePath string, rpool *object.Reso framework.Logf("Registering node VM %s with vmx file path %s", nodeName, vmxFilePath) nodeInfo := TestContext.NodeMapper.GetNodeInfo(nodeName) - finder := find.NewFinder(nodeInfo.VSphere.Client.Client, true) + finder := find.NewFinder(nodeInfo.VSphere.Client.Client, false) vmFolder, err := finder.FolderOrDefault(ctx, workingDir) Expect(err).NotTo(HaveOccurred()) From c90e33dda485e63c34fe5cb7032534ec0db8c1ab Mon Sep 17 00:00:00 2001 From: Doug MacEachern Date: Thu, 22 Feb 2018 15:17:01 -0800 Subject: [PATCH 2/2] update bazel: adds new vclib test --- pkg/cloudprovider/providers/vsphere/vclib/BUILD | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/cloudprovider/providers/vsphere/vclib/BUILD b/pkg/cloudprovider/providers/vsphere/vclib/BUILD index f1a11cecbbc..3feb1e43131 100644 --- a/pkg/cloudprovider/providers/vsphere/vclib/BUILD +++ b/pkg/cloudprovider/providers/vsphere/vclib/BUILD @@ -63,6 +63,7 @@ go_test( "datacenter_test.go", "datastore_test.go", "folder_test.go", + "utils_test.go", "virtualmachine_test.go", ], embed = [":go_default_library"],