Skip to content

Commit

Permalink
Refactor to use Volume IDs and remove ambiguity
Browse files Browse the repository at this point in the history
  • Loading branch information
Jamie Hannaford committed May 2, 2017
1 parent 8f6df26 commit ba33b16
Show file tree
Hide file tree
Showing 6 changed files with 231 additions and 300 deletions.
192 changes: 74 additions & 118 deletions pkg/cloudprovider/providers/openstack/openstack_volumes.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,13 @@ import (
volumes_v1 "github.com/gophercloud/gophercloud/openstack/blockstorage/v1/volumes"
volumes_v2 "github.com/gophercloud/gophercloud/openstack/blockstorage/v2/volumes"
"github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/volumeattach"
"github.com/gophercloud/gophercloud/pagination"

"github.com/golang/glog"
)

type volumeService interface {
createVolume(opts VolumeCreateOpts) (string, error)
getVolume(diskName string) (Volume, error)
createVolume(opts VolumeCreateOpts) (string, error) // Returns VolumeID
getVolume(volumeID string) (Volume, error)
deleteVolume(volumeName string) error
}

Expand Down Expand Up @@ -108,102 +107,67 @@ func (volumes *VolumesV2) createVolume(opts VolumeCreateOpts) (string, error) {
return vol.ID, nil
}

func (volumes *VolumesV1) getVolume(diskName string) (Volume, error) {
var volume_v1 volumes_v1.Volume
var volume Volume
err := volumes_v1.List(volumes.blockstorage, nil).EachPage(func(page pagination.Page) (bool, error) {
vols, err := volumes_v1.ExtractVolumes(page)
if err != nil {
glog.Errorf("Failed to extract volumes: %v", err)
return false, err
} else {
for _, v := range vols {
glog.V(4).Infof("%s %s %v", v.ID, v.Name, v.Attachments)
if v.Name == diskName || strings.Contains(v.ID, diskName) {
volume_v1 = v
return true, nil
}
}
}
// if it reached here then no disk with the given name was found.
errmsg := fmt.Sprintf("Unable to find disk: %s", diskName)
return false, errors.New(errmsg)
})
func (volumes *VolumesV1) getVolume(volumeID string) (Volume, error) {
volumeV1, err := volumes_v1.Get(volumes.blockstorage, volumeID).Extract()
if err != nil {
glog.Errorf("Error occurred getting volume: %s", diskName)
return volume, err
glog.Errorf("Error occurred getting volume by ID: %s", volumeID)
return Volume{}, err
}

volume.ID = volume_v1.ID
volume.Name = volume_v1.Name
volume.Status = volume_v1.Status
volume := Volume{
ID: volumeV1.ID,
Name: volumeV1.Name,
Status: volumeV1.Status,
}

if len(volume_v1.Attachments) > 0 && volume_v1.Attachments[0]["server_id"] != nil {
volume.AttachedServerId = volume_v1.Attachments[0]["server_id"].(string)
volume.AttachedDevice = volume_v1.Attachments[0]["device"].(string)
if len(volumeV1.Attachments) > 0 && volumeV1.Attachments[0]["server_id"] != nil {
volume.AttachedServerId = volumeV1.Attachments[0]["server_id"].(string)
volume.AttachedDevice = volumeV1.Attachments[0]["device"].(string)
}

return volume, nil
}

func (volumes *VolumesV2) getVolume(diskName string) (Volume, error) {
var volume_v2 volumes_v2.Volume
var volume Volume
err := volumes_v2.List(volumes.blockstorage, nil).EachPage(func(page pagination.Page) (bool, error) {
vols, err := volumes_v2.ExtractVolumes(page)
if err != nil {
glog.Errorf("Failed to extract volumes: %v", err)
return false, err
} else {
for _, v := range vols {
glog.V(4).Infof("%s %s %v", v.ID, v.Name, v.Attachments)
if v.Name == diskName || strings.Contains(v.ID, diskName) {
volume_v2 = v
return true, nil
}
}
}
// if it reached here then no disk with the given name was found.
errmsg := fmt.Sprintf("Unable to find disk: %s", diskName)
return false, errors.New(errmsg)
})
func (volumes *VolumesV2) getVolume(volumeID string) (Volume, error) {
volumeV2, err := volumes_v2.Get(volumes.blockstorage, volumeID).Extract()
if err != nil {
glog.Errorf("Error occurred getting volume: %s", diskName)
return volume, err
glog.Errorf("Error occurred getting volume by ID: %s", volumeID)
return Volume{}, err
}

volume.ID = volume_v2.ID
volume.Name = volume_v2.Name
volume.Status = volume_v2.Status
volume := Volume{
ID: volumeV2.ID,
Name: volumeV2.Name,
Status: volumeV2.Status,
}

if len(volume_v2.Attachments) > 0 {
volume.AttachedServerId = volume_v2.Attachments[0].ServerID
volume.AttachedDevice = volume_v2.Attachments[0].Device
if len(volumeV2.Attachments) > 0 {
volume.AttachedServerId = volumeV2.Attachments[0].ServerID
volume.AttachedDevice = volumeV2.Attachments[0].Device
}

return volume, nil
}

func (volumes *VolumesV1) deleteVolume(volumeName string) error {

err := volumes_v1.Delete(volumes.blockstorage, volumeName).ExtractErr()
func (volumes *VolumesV1) deleteVolume(volumeID string) error {
err := volumes_v1.Delete(volumes.blockstorage, volumeID).ExtractErr()
if err != nil {
glog.Errorf("Cannot delete volume %s: %v", volumeName, err)
glog.Errorf("Cannot delete volume %s: %v", volumeID, err)
}
return err
}

func (volumes *VolumesV2) deleteVolume(volumeName string) error {
err := volumes_v2.Delete(volumes.blockstorage, volumeName).ExtractErr()
func (volumes *VolumesV2) deleteVolume(volumeID string) error {
err := volumes_v2.Delete(volumes.blockstorage, volumeID).ExtractErr()
if err != nil {
glog.Errorf("Cannot delete volume %s: %v", volumeName, err)
glog.Errorf("Cannot delete volume %s: %v", volumeID, err)
}
return err
}

// Attaches given cinder volume to the compute running kubelet
func (os *OpenStack) AttachDisk(instanceID string, diskName string) (string, error) {
volume, err := os.getVolume(diskName)
func (os *OpenStack) AttachDisk(instanceID, volumeID string) (string, error) {
volume, err := os.getVolume(volumeID)
if err != nil {
return "", err
}
Expand All @@ -217,11 +181,11 @@ func (os *OpenStack) AttachDisk(instanceID string, diskName string) (string, err

if volume.AttachedServerId != "" {
if instanceID == volume.AttachedServerId {
glog.V(4).Infof("Disk: %q is already attached to compute: %q", diskName, instanceID)
glog.V(4).Infof("Disk %s is already attached to instance %s", volumeID, instanceID)
return volume.ID, nil
}
glog.V(2).Infof("Disk %q is attached to a different compute (%q), detaching", diskName, volume.AttachedServerId)
err = os.DetachDisk(volume.AttachedServerId, diskName)
glog.V(2).Infof("Disk %s is attached to a different instance (%s), detaching", volumeID, volume.AttachedServerId)
err = os.DetachDisk(volume.AttachedServerId, volumeID)
if err != nil {
return "", err
}
Expand All @@ -232,16 +196,16 @@ func (os *OpenStack) AttachDisk(instanceID string, diskName string) (string, err
VolumeID: volume.ID,
}).Extract()
if err != nil {
glog.Errorf("Failed to attach %s volume to %s compute: %v", diskName, instanceID, err)
glog.Errorf("Failed to attach %s volume to %s compute: %v", volumeID, instanceID, err)
return "", err
}
glog.V(2).Infof("Successfully attached %s volume to %s compute", diskName, instanceID)
glog.V(2).Infof("Successfully attached %s volume to %s compute", volumeID, instanceID)
return volume.ID, nil
}

// Detaches given cinder volume from the compute running kubelet
func (os *OpenStack) DetachDisk(instanceID string, partialDiskId string) error {
volume, err := os.getVolume(partialDiskId)
// DetachDisk detaches given cinder volume from the compute running kubelet
func (os *OpenStack) DetachDisk(instanceID, volumeID string) error {
volume, err := os.getVolume(volumeID)
if err != nil {
return err
}
Expand Down Expand Up @@ -270,26 +234,24 @@ func (os *OpenStack) DetachDisk(instanceID string, partialDiskId string) error {
return nil
}

// Takes a partial/full disk id or diskname
func (os *OpenStack) getVolume(diskName string) (Volume, error) {

// Retrieves Volume by its ID.
func (os *OpenStack) getVolume(volumeID string) (Volume, error) {
volumes, err := os.volumeService("")
if err != nil || volumes == nil {
glog.Errorf("Unable to initialize cinder client for region: %s", os.region)
return Volume{}, err
}

return volumes.getVolume(diskName)
return volumes.getVolume(volumeID)
}

// Create a volume of given size (in GiB)
func (os *OpenStack) CreateVolume(name string, size int, vtype, availability string, tags *map[string]string) (volumeName string, err error) {

func (os *OpenStack) CreateVolume(name string, size int, vtype, availability string, tags *map[string]string) (string, error) {
volumes, err := os.volumeService("")
if err != nil || volumes == nil {
glog.Errorf("Unable to initialize cinder client for region: %s", os.region)
return "", err
}

opts := VolumeCreateOpts{
Name: name,
Size: size,
Expand All @@ -299,27 +261,27 @@ func (os *OpenStack) CreateVolume(name string, size int, vtype, availability str
if tags != nil {
opts.Metadata = *tags
}
volume_id, err := volumes.createVolume(opts)
volumeID, err := volumes.createVolume(opts)

if err != nil {
glog.Errorf("Failed to create a %d GB volume: %v", size, err)
return "", err
}

glog.Infof("Created volume %v", volume_id)
return volume_id, nil
glog.Infof("Created volume with ID %s", volumeID)
return volumeID, nil
}

// GetDevicePath returns the path of an attached block storage volume, specified by its id.
func (os *OpenStack) GetDevicePath(diskId string) string {
func (os *OpenStack) GetDevicePath(volumeID string) string {
// Build a list of candidate device paths
candidateDeviceNodes := []string{
// KVM
fmt.Sprintf("virtio-%s", diskId[:20]),
fmt.Sprintf("virtio-%s", volumeID[:20]),
// KVM virtio-scsi
fmt.Sprintf("scsi-0QEMU_QEMU_HARDDISK_%s", diskId[:20]),
fmt.Sprintf("scsi-0QEMU_QEMU_HARDDISK_%s", volumeID[:20]),
// ESXi
fmt.Sprintf("wwn-0x%s", strings.Replace(diskId, "-", "", -1)),
fmt.Sprintf("wwn-0x%s", strings.Replace(volumeID, "-", "", -1)),
}

files, _ := ioutil.ReadDir("/dev/disk/by-id/")
Expand All @@ -333,17 +295,17 @@ func (os *OpenStack) GetDevicePath(diskId string) string {
}
}

glog.Warningf("Failed to find device for the diskid: %q\n", diskId)
glog.Warningf("Failed to find device for the volumeID: %q\n", volumeID)
return ""
}

func (os *OpenStack) DeleteVolume(volumeName string) error {
used, err := os.diskIsUsed(volumeName)
func (os *OpenStack) DeleteVolume(volumeID string) error {
used, err := os.diskIsUsed(volumeID)
if err != nil {
return err
}
if used {
msg := fmt.Sprintf("Cannot delete the volume %q, it's still attached to a node", volumeName)
msg := fmt.Sprintf("Cannot delete the volume %q, it's still attached to a node", volumeID)
return k8s_volume.NewDeletedVolumeInUseError(msg)
}

Expand All @@ -353,19 +315,19 @@ func (os *OpenStack) DeleteVolume(volumeName string) error {
return err
}

err = volumes.deleteVolume(volumeName)
err = volumes.deleteVolume(volumeID)
if err != nil {
glog.Errorf("Cannot delete volume %s: %v", volumeName, err)
glog.Errorf("Cannot delete volume %s: %v", volumeID, err)
}
return nil

}

// Get device path of attached volume to the compute running kubelet, as known by cinder
func (os *OpenStack) GetAttachmentDiskPath(instanceID string, diskName string) (string, error) {
func (os *OpenStack) GetAttachmentDiskPath(instanceID, volumeID string) (string, error) {
// See issue #33128 - Cinder does not always tell you the right device path, as such
// we must only use this value as a last resort.
volume, err := os.getVolume(diskName)
volume, err := os.getVolume(volumeID)
if err != nil {
return "", err
}
Expand All @@ -375,47 +337,41 @@ func (os *OpenStack) GetAttachmentDiskPath(instanceID string, diskName string) (
// see http://developer.openstack.org/api-ref-blockstorage-v1.html
return volume.AttachedDevice, nil
} else {
errMsg := fmt.Sprintf("Disk %q is attached to a different compute: %q, should be detached before proceeding", diskName, volume.AttachedServerId)
errMsg := fmt.Sprintf("Disk %q is attached to a different compute: %q, should be detached before proceeding", volumeID, volume.AttachedServerId)
glog.Errorf(errMsg)
return "", errors.New(errMsg)
}
}
return "", fmt.Errorf("volume %s is not attached to %s", diskName, instanceID)
return "", fmt.Errorf("volume %s is not attached to %s", volumeID, instanceID)
}

// query if a volume is attached to a compute instance
func (os *OpenStack) DiskIsAttached(diskName, instanceID string) (bool, error) {
volume, err := os.getVolume(diskName)
func (os *OpenStack) DiskIsAttached(instanceID, volumeID string) (bool, error) {
volume, err := os.getVolume(volumeID)
if err != nil {
return false, err
}

if instanceID == volume.AttachedServerId {
return true, nil
}
return false, nil
return instanceID == volume.AttachedServerId, nil
}

// query if a list of volumes are attached to a compute instance
func (os *OpenStack) DisksAreAttached(diskNames []string, instanceID string) (map[string]bool, error) {
func (os *OpenStack) DisksAreAttached(instanceID string, volumeIDs []string) (map[string]bool, error) {
attached := make(map[string]bool)
for _, diskName := range diskNames {
is_attached, _ := os.DiskIsAttached(diskName, instanceID)
attached[diskName] = is_attached
for _, volumeID := range volumeIDs {
isAttached, _ := os.DiskIsAttached(instanceID, volumeID)
attached[volumeID] = isAttached
}
return attached, nil
}

// diskIsUsed returns true a disk is attached to any node.
func (os *OpenStack) diskIsUsed(diskName string) (bool, error) {
volume, err := os.getVolume(diskName)
func (os *OpenStack) diskIsUsed(volumeID string) (bool, error) {
volume, err := os.getVolume(volumeID)
if err != nil {
return false, err
}
if volume.AttachedServerId != "" {
return true, nil
}
return false, nil
return volume.AttachedServerId != "", nil
}

// query if we should trust the cinder provide deviceName, See issue #33128
Expand Down
Loading

0 comments on commit ba33b16

Please sign in to comment.