Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Adding SCSI controller type filter for vSphere disk attach #27496

Merged
merged 1 commit into from
Jun 23, 2016
Merged

Adding SCSI controller type filter for vSphere disk attach #27496

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
66 changes: 50 additions & 16 deletions pkg/cloudprovider/providers/vsphere/vsphere.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -70,7 +78,6 @@ type VSphereConfig struct {
PublicNetwork string `gcfg:"public-network"`
}
Disk struct {
DiskController string `dcfg:"diskcontroller"`
SCSIControllerType string `dcfg:"scsicontrollertype"`
}
}
Expand Down Expand Up @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd love to see a vSphere doc why other scsi controllers don't support hot plug.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the problem is when you detach the device from the controller(of type lsilogic) the removal from the /dev path is not happening as expected. thats the reason why I added the driver type filter.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The information about how to mitigate this error should be returned to the user. Users typically have very poor visibility into logs.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added it to the returned error.

return nil, errors.New("Controller type not supported. Please configure 'lsilogic-sas' OR 'pvscsi'")
}
vs := VSphere{
cfg: &cfg,
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not calling FindDiskController?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FindDiskController can only find based on the controller( either scsi or ide). there is no govmomi api call to get scsi based on its driver type. so I have to add getSCSIController() to filter VM devices based on the particular scsi driver type.


var newSCSICreated = false
var newSCSIController types.BaseVirtualDevice
diskController, err := vmDevices.FindDiskController(vs.cfg.Disk.DiskController)
if err != nil {
// create a scsi controller if there is not one
newSCSIController, err := vmDevices.CreateSCSIController(vs.cfg.Disk.SCSIControllerType)
// 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 {
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()
Expand All @@ -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
Expand All @@ -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)
}
Expand Down Expand Up @@ -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()

Expand Down
1 change: 1 addition & 0 deletions pkg/cloudprovider/providers/vsphere/vsphere_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
14 changes: 0 additions & 14 deletions pkg/volume/vsphere_volume/vsphere_volume_util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why no more probe?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with these driver types the rescan is done automatically. so removed the code

if err == nil {
Expand Down Expand Up @@ -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 {
Expand Down