diff --git a/api/host_volumes.go b/api/host_volumes.go index 576e2fbd6..591cee1d2 100644 --- a/api/host_volumes.go +++ b/api/host_volumes.go @@ -177,6 +177,8 @@ type HostVolumeDeleteRequest struct { ID string } +type HostVolumeDeleteResponse struct{} + // Create forwards to client agents so a host volume can be created on those // hosts, and registers the volume with Nomad servers. func (hv *HostVolumes) Create(req *HostVolumeCreateRequest, opts *WriteOptions) (*HostVolumeCreateResponse, *WriteMeta, error) { @@ -236,11 +238,12 @@ func (hv *HostVolumes) List(req *HostVolumeListRequest, opts *QueryOptions) ([]* } // Delete deletes a host volume -func (hv *HostVolumes) Delete(req *HostVolumeDeleteRequest, opts *WriteOptions) (*WriteMeta, error) { +func (hv *HostVolumes) Delete(req *HostVolumeDeleteRequest, opts *WriteOptions) (*HostVolumeDeleteResponse, *WriteMeta, error) { + var resp *HostVolumeDeleteResponse path, err := url.JoinPath("/v1/volume/host/", url.PathEscape(req.ID)) if err != nil { - return nil, err + return nil, nil, err } - wm, err := hv.client.delete(path, nil, nil, opts) - return wm, err + wm, err := hv.client.delete(path, nil, resp, opts) + return resp, wm, err } diff --git a/client/hostvolumemanager/host_volume_plugin.go b/client/hostvolumemanager/host_volume_plugin.go index c51336b41..364201176 100644 --- a/client/hostvolumemanager/host_volume_plugin.go +++ b/client/hostvolumemanager/host_volume_plugin.go @@ -52,12 +52,20 @@ type PluginFingerprint struct { Version *version.Version `json:"version"` } -// HostVolumePluginCreateResponse gets stored on the volume in server state. -// Plugins are expected to respond to 'create' calls with json that -// unmarshals to this struct. +// HostVolumePluginCreateResponse returns values to the server that may be shown +// to the user or stored in server state. Plugins are expected to respond to +// 'create' calls with json that unmarshals to this struct. type HostVolumePluginCreateResponse struct { Path string `json:"path"` SizeBytes int64 `json:"bytes"` + Error string `json:"error"` +} + +// HostVolumePluginDeleteResponse returns values to the server that may be shown +// to the user or stored in server state. Plugins are expected to respond to +// 'delete' calls with json that unmarshals to this struct. +type HostVolumePluginDeleteResponse struct { + Error string `json:"error"` } const HostVolumePluginMkdirID = "mkdir" @@ -255,13 +263,21 @@ func (p *HostVolumePluginExternal) Create(ctx context.Context, fmt.Sprintf("%s=%s", EnvParameters, params), } + var pluginResp HostVolumePluginCreateResponse log := p.log.With("volume_name", req.Name, "volume_id", req.ID) stdout, _, err := p.runPlugin(ctx, log, "create", envVars) if err != nil { - return nil, fmt.Errorf("error creating volume %q with plugin %q: %w", req.ID, p.ID, err) + jsonErr := json.Unmarshal(stdout, &pluginResp) + if jsonErr != nil { + // if we got an error, we can't actually count on getting JSON, so + // optimistically look for it and return the original error + // otherwise + return nil, fmt.Errorf( + "error creating volume %q with plugin %q: %w", req.ID, p.ID, err) + } + return nil, fmt.Errorf("error creating volume %q with plugin %q: %w: %s", + req.ID, p.ID, err, pluginResp.Error) } - - var pluginResp HostVolumePluginCreateResponse err = json.Unmarshal(stdout, &pluginResp) if err != nil { // note: if a plugin does not return valid json, a volume may be @@ -313,10 +329,20 @@ func (p *HostVolumePluginExternal) Delete(ctx context.Context, } log := p.log.With("volume_name", req.Name, "volume_id", req.ID) - _, _, err = p.runPlugin(ctx, log, "delete", envVars) + stdout, _, err := p.runPlugin(ctx, log, "delete", envVars) if err != nil { - return fmt.Errorf("error deleting volume %q with plugin %q: %w", req.ID, p.ID, err) + var pluginResp HostVolumePluginDeleteResponse + jsonErr := json.Unmarshal(stdout, &pluginResp) + if jsonErr != nil { + // if we got an error, we can't actually count on getting JSON, so + // optimistically look for it and return the original error + // otherwise + return fmt.Errorf("error reading plugin response when deleting volume %q with plugin %q: original error: %w", req.ID, p.ID, err) + } + return fmt.Errorf("error deleting volume %q with plugin %q: %w: %s", + req.ID, p.ID, err, pluginResp.Error) } + return nil } diff --git a/client/hostvolumemanager/host_volume_plugin_test.go b/client/hostvolumemanager/host_volume_plugin_test.go index 826f7b943..cebdb0498 100644 --- a/client/hostvolumemanager/host_volume_plugin_test.go +++ b/client/hostvolumemanager/host_volume_plugin_test.go @@ -188,7 +188,7 @@ func TestHostVolumePluginExternal(t *testing.T) { &cstructs.ClientHostVolumeCreateRequest{ ID: volID, }) - must.EqError(t, err, `error creating volume "test-vol-id" with plugin "test_plugin_sad.sh": exit status 1`) + must.EqError(t, err, `error creating volume "test-vol-id" with plugin "test_plugin_sad.sh": exit status 1: create: sad plugin is sad`) must.Nil(t, resp) logged = getLogs() must.StrContains(t, logged, "create: sad plugin is sad") @@ -201,7 +201,7 @@ func TestHostVolumePluginExternal(t *testing.T) { &cstructs.ClientHostVolumeDeleteRequest{ ID: volID, }) - must.EqError(t, err, `error deleting volume "test-vol-id" with plugin "test_plugin_sad.sh": exit status 1`) + must.EqError(t, err, `error deleting volume "test-vol-id" with plugin "test_plugin_sad.sh": exit status 1: delete: sad plugin is sad`) logged = getLogs() must.StrContains(t, logged, "delete: sad plugin is sad") must.StrContains(t, logged, "delete: it tells you all about it in stderr") diff --git a/client/hostvolumemanager/test_fixtures/test_plugin_sad.sh b/client/hostvolumemanager/test_fixtures/test_plugin_sad.sh index 6f883297a..9af277e01 100755 --- a/client/hostvolumemanager/test_fixtures/test_plugin_sad.sh +++ b/client/hostvolumemanager/test_fixtures/test_plugin_sad.sh @@ -2,6 +2,6 @@ # Copyright (c) HashiCorp, Inc. # SPDX-License-Identifier: BUSL-1.1 -echo "$1: sad plugin is sad" +printf '{"error": "%s: sad plugin is sad"}' $1 echo "$1: it tells you all about it in stderr" 1>&2 exit 1 diff --git a/command/agent/host_volume_endpoint.go b/command/agent/host_volume_endpoint.go index db12cca92..a3791dee6 100644 --- a/command/agent/host_volume_endpoint.go +++ b/command/agent/host_volume_endpoint.go @@ -139,5 +139,5 @@ func (s *HTTPServer) hostVolumeDelete(id string, resp http.ResponseWriter, req * setIndex(resp, out.Index) - return nil, nil + return out, nil } diff --git a/command/agent/host_volume_endpoint_test.go b/command/agent/host_volume_endpoint_test.go index ddff7a33f..abb51524e 100644 --- a/command/agent/host_volume_endpoint_test.go +++ b/command/agent/host_volume_endpoint_test.go @@ -87,8 +87,9 @@ func TestHostVolumeEndpoint_CRUD(t *testing.T) { req, err = http.NewRequest(http.MethodDelete, fmt.Sprintf("/v1/volume/host/%s", volID), nil) must.NoError(t, err) - _, err = s.Server.HostVolumeSpecificRequest(respW, req) + obj, err = s.Server.HostVolumeSpecificRequest(respW, req) must.NoError(t, err) + must.NotNil(t, obj) // Verify volume was deleted diff --git a/command/volume_delete.go b/command/volume_delete.go index e23e02f6f..427fd9c59 100644 --- a/command/volume_delete.go +++ b/command/volume_delete.go @@ -195,7 +195,7 @@ func (c *VolumeDeleteCommand) deleteHostVolume(client *api.Client, volID string) c.namespace = stub.Namespace } - _, err := client.HostVolumes().Delete(&api.HostVolumeDeleteRequest{ID: volID}, nil) + _, _, err := client.HostVolumes().Delete(&api.HostVolumeDeleteRequest{ID: volID}, nil) if err != nil { c.Ui.Error(fmt.Sprintf("Error deleting volume: %s", err)) return 1 diff --git a/website/content/docs/concepts/plugins/storage/host-volumes.mdx b/website/content/docs/concepts/plugins/storage/host-volumes.mdx index 709df40f5..719bbbdf8 100644 --- a/website/content/docs/concepts/plugins/storage/host-volumes.mdx +++ b/website/content/docs/concepts/plugins/storage/host-volumes.mdx @@ -291,6 +291,15 @@ DHV_PARAMETERS={stringified json of parameters from the volume spec} {"path": "/path/to/created/volume", "bytes": 50000000} ``` +**Expected stdout on error:** + +``` +{"error": "error message"} +``` + +Returning an error message is optional. Nomad returns the error message in any +error returned to the user. + **Requirements:** * Must complete within 60 seconds, or Nomad will kill it. @@ -333,6 +342,15 @@ DHV_PARAMETERS={stringified json of parameters from the volume spec} **Expected stdout:** none (stdout is discarded) +**Expected stdout on error:** + +``` +{"error": "error message"} +``` + +Returning an error message is optional. Nomad returns the error message in any +error returned to the user. + **Requirements:** * Must complete within 60 seconds, or Nomad will kill it.