From 5e376fe5f9ec03aaac0f0a1bb8ea4d52cea6826e Mon Sep 17 00:00:00 2001 From: Ritesh H Shukla Date: Fri, 9 Dec 2016 07:33:10 +0000 Subject: [PATCH] Fix panic in vSphere cloud provider. Fixes #36295 --- .../providers/vsphere/vsphere.go | 57 ++++--------------- .../providers/vsphere/vsphere_test.go | 23 ++------ 2 files changed, 16 insertions(+), 64 deletions(-) diff --git a/pkg/cloudprovider/providers/vsphere/vsphere.go b/pkg/cloudprovider/providers/vsphere/vsphere.go index e80556540f8..77e4d9fbd25 100644 --- a/pkg/cloudprovider/providers/vsphere/vsphere.go +++ b/pkg/cloudprovider/providers/vsphere/vsphere.go @@ -100,8 +100,6 @@ type VSphere struct { cfg *VSphereConfig // InstanceID of the server where this VSphere object is instantiated. localInstanceID string - // Cluster that VirtualMachine belongs to - clusterName string } type VSphereConfig struct { @@ -201,17 +199,16 @@ func init() { }) } -// Returns the name of the VM and its Cluster on which this code is running. -// This is done by searching for the name of virtual machine by current IP. +// Returns the name of the VM on which this code is running. // Prerequisite: this code assumes VMWare vmtools or open-vm-tools to be installed in the VM. -func readInstance(client *govmomi.Client, cfg *VSphereConfig) (string, string, error) { +func getVMName(client *govmomi.Client, cfg *VSphereConfig) (string, error) { addrs, err := net.InterfaceAddrs() if err != nil { - return "", "", err + return "", err } if len(addrs) == 0 { - return "", "", fmt.Errorf("unable to retrieve Instance ID") + return "", fmt.Errorf("unable to retrieve Instance ID") } // Create context @@ -224,7 +221,7 @@ func readInstance(client *govmomi.Client, cfg *VSphereConfig) (string, string, e // Fetch and set data center dc, err := f.Datacenter(ctx, cfg.Global.Datacenter) if err != nil { - return "", "", err + return "", err } f.SetDatacenter(dc) @@ -234,7 +231,7 @@ func readInstance(client *govmomi.Client, cfg *VSphereConfig) (string, string, e for _, v := range addrs { ip, _, err := net.ParseCIDR(v.String()) if err != nil { - return "", "", fmt.Errorf("unable to parse cidr from ip") + return "", fmt.Errorf("unable to parse cidr from ip") } // Finds a virtual machine or host by IP address. @@ -244,33 +241,15 @@ func readInstance(client *govmomi.Client, cfg *VSphereConfig) (string, string, e } } if svm == nil { - return "", "", fmt.Errorf("unable to retrieve vm reference from vSphere") + return "", fmt.Errorf("unable to retrieve vm reference from vSphere") } var vm mo.VirtualMachine err = s.Properties(ctx, svm.Reference(), []string{"name", "resourcePool"}, &vm) if err != nil { - return "", "", err + return "", err } - - var cluster string - if vm.ResourcePool != nil { - // Extract the Cluster Name if VM belongs to a ResourcePool - var rp mo.ResourcePool - err = s.Properties(ctx, *vm.ResourcePool, []string{"parent"}, &rp) - if err == nil { - var ccr mo.ComputeResource - err = s.Properties(ctx, *rp.Parent, []string{"name"}, &ccr) - if err == nil { - cluster = ccr.Name - } else { - glog.Warningf("VM %s, does not belong to a vSphere Cluster, will not have FailureDomain label", vm.Name) - } - } else { - glog.Warningf("VM %s, does not belong to a vSphere Cluster, will not have FailureDomain label", vm.Name) - } - } - return vm.Name, cluster, nil + return vm.Name, nil } func newVSphere(cfg VSphereConfig) (*VSphere, error) { @@ -292,7 +271,7 @@ func newVSphere(cfg VSphereConfig) (*VSphere, error) { return nil, err } - id, cluster, err := readInstance(c, &cfg) + id, err := getVMName(c, &cfg) if err != nil { return nil, err } @@ -301,7 +280,6 @@ func newVSphere(cfg VSphereConfig) (*VSphere, error) { client: c, cfg: &cfg, localInstanceID: id, - clusterName: cluster, } runtime.SetFinalizer(&vs, logout) @@ -634,20 +612,9 @@ func (vs *VSphere) LoadBalancer() (cloudprovider.LoadBalancer, bool) { // Zones returns an implementation of Zones for Google vSphere. func (vs *VSphere) Zones() (cloudprovider.Zones, bool) { - glog.V(1).Info("Claiming to support Zones") + glog.V(1).Info("The vSphere cloud provider does not support zones") - return vs, true -} - -func (vs *VSphere) GetZone() (cloudprovider.Zone, error) { - glog.V(1).Infof("Current datacenter is %v, cluster is %v", vs.cfg.Global.Datacenter, vs.clusterName) - - // The clusterName is determined from the VirtualMachine ManagedObjectReference during init - // If the VM is not created within a Cluster, this will return empty-string - return cloudprovider.Zone{ - Region: vs.cfg.Global.Datacenter, - FailureDomain: vs.clusterName, - }, nil + return nil, false } // Routes returns a false since the interface is not supported for vSphere. diff --git a/pkg/cloudprovider/providers/vsphere/vsphere_test.go b/pkg/cloudprovider/providers/vsphere/vsphere_test.go index 6e84da1880c..fb9c1a8c07f 100644 --- a/pkg/cloudprovider/providers/vsphere/vsphere_test.go +++ b/pkg/cloudprovider/providers/vsphere/vsphere_test.go @@ -128,30 +128,15 @@ func TestVSphereLogin(t *testing.T) { func TestZones(t *testing.T) { cfg := VSphereConfig{} cfg.Global.Datacenter = "myDatacenter" - failureZone := "myCluster" // Create vSphere configuration object vs := VSphere{ - cfg: &cfg, - clusterName: failureZone, + cfg: &cfg, } - z, ok := vs.Zones() - if !ok { - t.Fatalf("Zones() returned false") - } - - zone, err := z.GetZone() - if err != nil { - t.Fatalf("GetZone() returned error: %s", err) - } - - if zone.Region != vs.cfg.Global.Datacenter { - t.Fatalf("GetZone() returned wrong region (%s)", zone.Region) - } - - if zone.FailureDomain != failureZone { - t.Fatalf("GetZone() returned wrong Failure Zone (%s)", zone.FailureDomain) + _, ok := vs.Zones() + if ok { + t.Fatalf("Zones() returned true") } }