cloudfoundry / bosh-softlayer-cpi-release

An external BOSH CPI for the SoftLayer cloud written in Golang
Apache License 2.0
14 stars 20 forks source link

No disk found should exit successfully when delete_disk #313

Closed gu-bin closed 5 years ago

gu-bin commented 5 years ago

Currently CPI returns error if the disk doesn't exist. Should exit successfully

cf-gitbot commented 5 years ago

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/165409409

The labels on this github issue will be updated when the story is started.

gu-bin commented 5 years ago

Add the following code to delete_disk.go and add corresponding unit tests.

    // Find the disk
    _, err := dd.diskService.Find(diskCID.Int())
    if err != nil {
        if _, ok := err.(api.CloudError); ok {
            return nil, nil
        }
        return nil, bosherr.WrapErrorf(err, "Finding disk '%s' to vm '%s", diskCID)
    }
edwardstudy commented 5 years ago

It's not a SL API error, we can refactor method softlayerClient.GetBlockVolumeDetailsBySoftLayerAccount:

func (c *ClientManager) GetBlockVolumeDetailsBySoftLayerAccount(volumeId int, mask string) (datatypes.Network_Storage, bool, error) {
    if mask == "" {
        mask = VOLUME_DETAIL_MASK
    }

    volumes, err := c.AccountService.Mask(mask).Filter(filter.Path("iscsiNetworkStorage.id").Eq(volumeId).Build()).GetIscsiNetworkStorage()
    if err != nil {
        return datatypes.Network_Storage{}, false, err
    }

        // If volumes are empty, return empty results
    if len(volumes) == 0 {
        return datatypes.Network_Storage{}, false, nil
    }
    if len(volumes) > 1 {
        return datatypes.Network_Storage{}, false, bosherr.Errorf("Exist more than one volume with id %d", volumeId)
    }

    return volumes[0], true, nil
}
LiYa-Xu commented 5 years ago

@gu-bin @edwardstudy please help review the PR :https://github.com/cloudfoundry/bosh-softlayer-cpi-release/pull/314

gu-bin commented 5 years ago

PR merged