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 023f21a
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 23 deletions.
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 023f21a

Please sign in to comment.