Merge pull request #38423 from vmware/panicAttackvmwarerelease-master

Automatic merge from submit-queue (batch tested with PRs 34002, 38535, 37330, 38522, 38423)

Fix panic in vSphere cloud provider

Currently vSphere Cloud Provider triggers panic in controller-manager pod kubernetes. This is because it queries for the cluster name from the VC. We have eliminated that code from the vSphere cloud provider. 

Fixes #36295

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.
+// Returns the name of the VM on which this code is running.
-// This is done by searching for the name of virtual machine by current IP.
 // 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
 	}
-
+	return vm.Name, nil
-	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
 }
 
 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
+	return nil, false
-}
-
-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
 }
 
 // Routes returns a false since the interface is not supported for vSphere.

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,
+		cfg: &cfg,
-		clusterName: failureZone,
 	}
 
-	z, ok := vs.Zones()
+	_, ok := vs.Zones()
-	if !ok {
+	if ok {
-		t.Fatalf("Zones() returned false")
+		t.Fatalf("Zones() returned true")
-	}
-
-	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)
 	}
 }