Merge pull request #27496 from hpcloud/hpe/vsphere-scsidriver

Automatic merge from submit-queue

Adding SCSI controller type filter for vSphere disk attach

Hot plug of disks to a SCSI controller of type lsilogic doesn't work as expected. When a device is detached from the controller, it fails to remove the device from the /dev path which makes the subsequent attaches to the node to fail. With scsi controller types lsilogic-sas or paravirtual this seems to work well. This patch filters the existing controller for these types, and if it doesn't find one, it creates a new controller for disk attach.

This PR is dependent on https://github.com/kubernetes/kubernetes/pull/26658 (1st commit) also targeting this for 1.3

pkg/cloudprovider/providers/vsphere/vsphere.go

@@ -37,12 +37,20 @@ import (
 	"github.com/golang/glog"
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/cloudprovider"
+	"k8s.io/kubernetes/pkg/util/runtime"
 )
 
 const ProviderName = "vsphere"
 const ActivePowerState = "poweredOn"
 const DefaultDiskController = "scsi"
-const DefaultSCSIControllerType = "lsilogic"
+const DefaultSCSIControllerType = "lsilogic-sas"
+
+// Controller types that are currently supported for hot attach of disks
+// lsilogic driver type is currently not supported because,when a device gets detached
+// it fails to remove the device from the /dev path (which should be manually done)
+// making the subsequent attaches to the node to fail.
+// TODO: Add support for lsilogic driver type
+var supportedSCSIControllerType = []string{"lsilogic-sas", "pvscsi"}
 
 var ErrNoDiskUUIDFound = errors.New("No disk UUID found")
 var ErrNoDiskIDFound = errors.New("No vSphere disk ID found")
@@ -70,7 +78,6 @@ type VSphereConfig struct {
 		PublicNetwork string `gcfg:"public-network"`
 	}
 	Disk struct {
-		DiskController     string `dcfg:"diskcontroller"`
 		SCSIControllerType string `dcfg:"scsicontrollertype"`
 	}
 }
@@ -146,11 +153,11 @@ func newVSphere(cfg VSphereConfig) (*VSphere, error) {
 		return nil, err
 	}
 
-	if cfg.Disk.DiskController == "" {
-		cfg.Disk.DiskController = DefaultDiskController
-	}
 	if cfg.Disk.SCSIControllerType == "" {
 		cfg.Disk.SCSIControllerType = DefaultSCSIControllerType
+	} else if !checkControllerSupported(cfg.Disk.SCSIControllerType) {
+		glog.Errorf("%v is not a supported SCSI Controller type. Please configure 'lsilogic-sas' OR 'pvscsi'", cfg.Disk.SCSIControllerType)
+		return nil, errors.New("Controller type not supported. Please configure 'lsilogic-sas' OR 'pvscsi'")
 	}
 	vs := VSphere{
 		cfg:             &cfg,
@@ -159,6 +166,15 @@ func newVSphere(cfg VSphereConfig) (*VSphere, error) {
 	return &vs, nil
 }
 
+func checkControllerSupported(ctrlType string) bool {
+	for _, c := range supportedSCSIControllerType {
+		if ctrlType == c {
+			return true
+		}
+	}
+	return false
+}
+
 func vsphereLogin(cfg *VSphereConfig, ctx context.Context) (*govmomi.Client, error) {
 
 	// Parse URL from string
@@ -274,7 +290,7 @@ func (i *Instances) List(filter string) ([]string, error) {
 		return nil, err
 	}
 
-	glog.V(3).Infof("found %s instances matching %s: %s",
+	glog.V(3).Infof("Found %s instances matching %s: %s",
 		len(vmList), filter, vmList)
 
 	return vmList, nil
@@ -518,15 +534,18 @@ func (vs *VSphere) AttachDisk(vmDiskPath string, nodeName string) (diskID string
 		return "", "", err
 	}
 
-	// find SCSI controller to attach the disk
-	var newSCSICreated bool = false
+	var diskControllerType = vs.cfg.Disk.SCSIControllerType
+	// find SCSI controller of particular type from VM devices
+	var diskController = getSCSIController(vmDevices, diskControllerType)
+
+	var newSCSICreated = false
 	var newSCSIController types.BaseVirtualDevice
-	diskController, err := vmDevices.FindDiskController(vs.cfg.Disk.DiskController)
+	// creating a scsi controller as there is none found of controller type defined
+	if diskController == nil {
+		glog.V(4).Infof("Creating a SCSI controller of %v type", diskControllerType)
+		newSCSIController, err := vmDevices.CreateSCSIController(diskControllerType)
 		if err != nil {
-		// create a scsi controller if there is not one
-		newSCSIController, err := vmDevices.CreateSCSIController(vs.cfg.Disk.SCSIControllerType)
-		if err != nil {
-			glog.V(3).Infof("cannot create new SCSI controller - %v", err)
+			runtime.HandleError(fmt.Errorf("error creating new SCSI controller: %v", err))
 			return "", "", err
 		}
 		configNewSCSIController := newSCSIController.(types.BaseVirtualSCSIController).GetVirtualSCSIController()
@@ -551,8 +570,10 @@ func (vs *VSphere) AttachDisk(vmDiskPath string, nodeName string) (diskID string
 			//cannot cleanup if there is no device list
 			return "", "", err
 		}
-		if diskController, err = vmDevices.FindDiskController(vs.cfg.Disk.DiskController); err != nil {
-			glog.V(3).Infof("cannot find disk controller - %v", err)
+
+		diskController = getSCSIController(vmDevices, vs.cfg.Disk.SCSIControllerType)
+		if diskController == nil {
+			glog.Errorf("cannot find SCSI controller in VM - %v", err)
 			// attempt clean up of scsi controller
 			cleanUpController(newSCSIController, vmDevices, vm, ctx)
 			return "", "", err
@@ -567,7 +588,7 @@ func (vs *VSphere) AttachDisk(vmDiskPath string, nodeName string) (diskID string
 	// Attach disk to the VM
 	err = vm.AddDevice(ctx, disk)
 	if err != nil {
-		glog.V(3).Infof("cannot attach disk to the vm - %v", err)
+		glog.Errorf("cannot attach disk to the vm - %v", err)
 		if newSCSICreated {
 			cleanUpController(newSCSIController, vmDevices, vm, ctx)
 		}
@@ -606,6 +627,19 @@ func (vs *VSphere) AttachDisk(vmDiskPath string, nodeName string) (diskID string
 	return deviceName, diskUUID, nil
 }
 
+func getSCSIController(vmDevices object.VirtualDeviceList, scsiType string) *types.VirtualController {
+	// get virtual scsi controller of passed argument type
+	for _, device := range vmDevices {
+		devType := vmDevices.Type(device)
+		if devType == scsiType {
+			if c, ok := device.(types.BaseVirtualController); ok {
+				return c.GetVirtualController()
+			}
+		}
+	}
+	return nil
+}
+
 func getVirtualDiskUUID(newDevice types.BaseVirtualDevice) (string, error) {
 	vd := newDevice.GetVirtualDevice()
 

pkg/cloudprovider/providers/vsphere/vsphere_test.go

@@ -37,6 +37,7 @@ func configFromEnv() (cfg VSphereConfig, ok bool) {
 	cfg.Global.Datacenter = os.Getenv("VSPHERE_DATACENTER")
 	cfg.Network.PublicNetwork = os.Getenv("VSPHERE_PUBLIC_NETWORK")
 	cfg.Global.Datastore = os.Getenv("VSPHERE_DATASTORE")
+	cfg.Disk.SCSIControllerType = os.Getenv("VSPHERE_SCSICONTROLLER_TYPE")
 	if os.Getenv("VSPHERE_INSECURE") != "" {
 		InsecureFlag, err = strconv.ParseBool(os.Getenv("VSPHERE_INSECURE"))
 	} else {

pkg/volume/vsphere_volume/vsphere_volume_util.go

@@ -68,8 +68,6 @@ func (util *VsphereDiskUtil) AttachDisk(vm *vsphereVolumeMounter, globalPDPath s
 	numTries := 0
 	for {
 		devicePath = verifyDevicePath(diskUUID)
-		// probe the attached vol so that symlink in /dev/disk/by-id is created
-		scsiHostRescan()
 
 		_, err := os.Stat(devicePath)
 		if err == nil {
@@ -107,18 +105,6 @@ func (util *VsphereDiskUtil) AttachDisk(vm *vsphereVolumeMounter, globalPDPath s
 	return nil
 }
 
-// rescan scsi bus
-func scsiHostRescan() {
-	scsi_path := "/sys/class/scsi_host/"
-	if dirs, err := ioutil.ReadDir(scsi_path); err == nil {
-		for _, f := range dirs {
-			name := scsi_path + f.Name() + "/scan"
-			data := []byte("- - -")
-			ioutil.WriteFile(name, data, 0666)
-		}
-	}
-}
-
 func verifyDevicePath(diskUUID string) string {
 	files, _ := ioutil.ReadDir("/dev/disk/by-id/")
 	for _, f := range files {