From 345049b663d637b71db78a65111f14ee044df368 Mon Sep 17 00:00:00 2001 From: Alvaro Aleman Date: Sun, 20 Jan 2019 00:54:14 +0100 Subject: [PATCH] Fix PV allocation on non-English vSphere --- pkg/cloudprovider/providers/vsphere/BUILD | 4 ++ .../providers/vsphere/vsphere_util.go | 21 ++++-- .../providers/vsphere/vsphere_util_test.go | 72 +++++++++++++++++++ 3 files changed, 93 insertions(+), 4 deletions(-) create mode 100644 pkg/cloudprovider/providers/vsphere/vsphere_util_test.go diff --git a/pkg/cloudprovider/providers/vsphere/BUILD b/pkg/cloudprovider/providers/vsphere/BUILD index 6e3034f86d0..2831ceec49c 100644 --- a/pkg/cloudprovider/providers/vsphere/BUILD +++ b/pkg/cloudprovider/providers/vsphere/BUILD @@ -31,6 +31,8 @@ go_library( "//vendor/github.com/vmware/govmomi/vapi/tags:go_default_library", "//vendor/github.com/vmware/govmomi/vim25:go_default_library", "//vendor/github.com/vmware/govmomi/vim25/mo:go_default_library", + "//vendor/github.com/vmware/govmomi/vim25/soap:go_default_library", + "//vendor/github.com/vmware/govmomi/vim25/types:go_default_library", "//vendor/gopkg.in/gcfg.v1:go_default_library", "//vendor/k8s.io/klog:go_default_library", ], @@ -41,6 +43,7 @@ go_test( srcs = [ "credentialmanager_test.go", "vsphere_test.go", + "vsphere_util_test.go", ], embed = [":go_default_library"], deps = [ @@ -55,6 +58,7 @@ go_test( "//staging/src/k8s.io/client-go/informers:go_default_library", "//staging/src/k8s.io/client-go/kubernetes/fake:go_default_library", "//staging/src/k8s.io/cloud-provider:go_default_library", + "//vendor/github.com/vmware/govmomi:go_default_library", "//vendor/github.com/vmware/govmomi/lookup/simulator:go_default_library", "//vendor/github.com/vmware/govmomi/property:go_default_library", "//vendor/github.com/vmware/govmomi/simulator:go_default_library", diff --git a/pkg/cloudprovider/providers/vsphere/vsphere_util.go b/pkg/cloudprovider/providers/vsphere/vsphere_util.go index 04f241a89d3..4fa4850b3e6 100644 --- a/pkg/cloudprovider/providers/vsphere/vsphere_util.go +++ b/pkg/cloudprovider/providers/vsphere/vsphere_util.go @@ -23,12 +23,13 @@ import ( "io/ioutil" "os" "path/filepath" - "regexp" "strings" "time" "github.com/vmware/govmomi/vim25" "github.com/vmware/govmomi/vim25/mo" + "github.com/vmware/govmomi/vim25/soap" + "github.com/vmware/govmomi/vim25/types" "k8s.io/klog" "k8s.io/api/core/v1" @@ -325,10 +326,9 @@ func getcanonicalVolumePath(ctx context.Context, dc *vclib.Datacenter, volumePat // Querying a non-existent dummy disk on the datastore folder. // It would fail and return an folder ID in the error message. _, err := dc.GetVirtualDiskPage83Data(ctx, dummyDiskVolPath) + canonicalVolumePath, err = getPathFromFileNotFound(err) if err != nil { - re := regexp.MustCompile("File (.*?) was not found") - match := re.FindStringSubmatch(err.Error()) - canonicalVolumePath = match[1] + return "", fmt.Errorf("failed to get path from dummy request: %v", err) } } diskPath := vclib.GetPathFromVMDiskPath(canonicalVolumePath) @@ -342,6 +342,19 @@ func getcanonicalVolumePath(ctx context.Context, dc *vclib.Datacenter, volumePat return canonicalVolumePath, nil } +// getPathFromFileNotFound returns the path from a fileNotFound error +func getPathFromFileNotFound(err error) (string, error) { + if soap.IsSoapFault(err) { + fault := soap.ToSoapFault(err) + f, ok := fault.VimFault().(types.FileNotFound) + if !ok { + return "", fmt.Errorf("%v is not a FileNotFound error", err) + } + return f.File, nil + } + return "", fmt.Errorf("%v is not a soap fault", err) +} + func setdatastoreFolderIDMap( datastoreFolderIDMap map[string]map[string]string, datastore string, diff --git a/pkg/cloudprovider/providers/vsphere/vsphere_util_test.go b/pkg/cloudprovider/providers/vsphere/vsphere_util_test.go new file mode 100644 index 00000000000..702e1108e7e --- /dev/null +++ b/pkg/cloudprovider/providers/vsphere/vsphere_util_test.go @@ -0,0 +1,72 @@ +/* +Copyright 2019 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 vsphere + +import ( + "context" + "fmt" + "testing" + + "k8s.io/kubernetes/pkg/cloudprovider/providers/vsphere/vclib" + + "github.com/vmware/govmomi" + "github.com/vmware/govmomi/simulator" +) + +func TestGetPathFromFileNotFound(t *testing.T) { + ctx := context.Background() + + // vCenter model + initial set of objects (cluster, hosts, VMs, network, datastore, etc) + model := simulator.VPX() + + 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 := &vclib.VSphereConnection{Client: c.Client} + + dc, err := vclib.GetDatacenter(ctx, vc, vclib.TestDefaultDatacenter) + if err != nil { + t.Errorf("failed to get datacenter: %v", err) + } + + requestDiskPath := fmt.Sprintf("[%s] %s", vclib.TestDefaultDatastore, DummyDiskName) + _, err = dc.GetVirtualDiskPage83Data(ctx, requestDiskPath) + if err == nil { + t.Error("expected error when calling GetVirtualDiskPage83Data") + } + + _, err = getPathFromFileNotFound(err) + if err != nil { + t.Errorf("expected err to be nil but was %v", err) + } + + _, err = getPathFromFileNotFound(nil) + if err == nil { + t.Errorf("expected err when calling getPathFromFileNotFound with nil err") + } +}