Skip to content

Commit

Permalink
driver: handle non existing volumes and nodes
Browse files Browse the repository at this point in the history
According to the spec we should return a `NOT_FOUND` in various method.
We always assume the volume or node exist, however this might not the
case.

For example the user might delete the volume externally from the
UI or make a HTTP call to the DigitalOcean API to delete the volume.
Actions like this are outside the Kubernetes system, so the plugin needs
to handle these edge cases by signaling back that the resources don't
exist.

We return `NOT_FOUND` for any creation actions, such as creating a
volume, attaching a volume, formatting, etc.. Because we really need to
return an error as there is no way to recover from this

We don't return `NOT_FOUND` for any destroy actions, such as deleting,
detaching, unmounting, etc.. Because in all these cases it doesn't
matter if the volume doesn't exist. There is not much to do and assuming
it's "done" makes the overall system more stable
  • Loading branch information
fatih committed Aug 21, 2018
1 parent 07f32d9 commit 89c4115
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 23 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@
* Remove description to allow users to reuse volumes that were created by the
UI/API
[[GH-59]](https://github.com/digitalocean/csi-digitalocean/pull/59)
* Handle edge cases from external action, such as Volume deletion via UI more
gracefully. We're not very strict anymore in cases we don't need to be, but
we're also returning a better error in cases we need to be.
[[GH-60]](https://github.com/digitalocean/csi-digitalocean/pull/60)

## v0.1.3 - August 3rd 2018

Expand Down
41 changes: 41 additions & 0 deletions driver/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,22 @@ func (d *Driver) ControllerPublishVolume(ctx context.Context, req *csi.Controlle
})
ll.Info("controller publish volume called")

// check if volume exist before trying to attach it
_, resp, err := d.doClient.Storage.GetVolume(ctx, req.VolumeId)
if err != nil {
if resp != nil && resp.StatusCode == http.StatusNotFound {
return nil, status.Errorf(codes.NotFound, "volume %q not found", req.VolumeId)
}
}

// check if droplet exist before trying to attach the volume to the droplet
_, resp, err = d.doClient.Droplets.Get(ctx, dropletID)
if err != nil {
if resp != nil && resp.StatusCode == http.StatusNotFound {
return nil, status.Errorf(codes.NotFound, "droplet %q not found", dropletID)
}
}

action, resp, err := d.doClient.StorageActions.Attach(ctx, req.VolumeId, dropletID)
if err != nil {
// don't do anything if attached
Expand Down Expand Up @@ -224,6 +240,23 @@ func (d *Driver) ControllerUnpublishVolume(ctx context.Context, req *csi.Control
})
ll.Info("controller unpublish volume called")

// check if volume exist before trying to detach it
_, resp, err := d.doClient.Storage.GetVolume(ctx, req.VolumeId)
if err != nil {
if resp != nil && resp.StatusCode == http.StatusNotFound {
// assume it's detached
return &csi.ControllerUnpublishVolumeResponse{}, nil
}
}

// check if droplet exist before trying to detach the volume from the droplet
_, resp, err = d.doClient.Droplets.Get(ctx, dropletID)
if err != nil {
if resp != nil && resp.StatusCode == http.StatusNotFound {
return nil, status.Errorf(codes.NotFound, "droplet %q not found", dropletID)
}
}

action, resp, err := d.doClient.StorageActions.DetachByDropletID(ctx, req.VolumeId, dropletID)
if err != nil {
if (resp != nil && resp.StatusCode == http.StatusUnprocessableEntity) || strings.Contains(err.Error(), "Attachment not found") {
Expand Down Expand Up @@ -275,6 +308,14 @@ func (d *Driver) ValidateVolumeCapabilities(ctx context.Context, req *csi.Valida
})
ll.Info("validate volume capabilities called")

// check if volume exist before trying to validate it it
_, volResp, err := d.doClient.Storage.GetVolume(ctx, req.VolumeId)
if err != nil {
if volResp != nil && volResp.StatusCode == http.StatusNotFound {
return nil, status.Errorf(codes.NotFound, "volume %q not found", req.VolumeId)
}
}

hasSupport := func(mode csi.VolumeCapability_AccessMode_Mode) bool {
for _, m := range vcaps {
if mode == m.Mode {
Expand Down
44 changes: 21 additions & 23 deletions driver/node.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 89c4115

Please sign in to comment.