Merge pull request #51210 from austinvazquez/refactor-client-volume

client/volume: refactor volume options and responses

client/client_interfaces.go

@@ -196,10 +196,10 @@ type SystemAPIClient interface {
 // VolumeAPIClient defines API client methods for the volumes
 type VolumeAPIClient interface {
 	VolumeCreate(ctx context.Context, options volume.CreateOptions) (volume.Volume, error)
-	VolumeInspect(ctx context.Context, volumeID string) (volume.Volume, error)
-	VolumeInspectWithRaw(ctx context.Context, volumeID string) (volume.Volume, []byte, error)
-	VolumeList(ctx context.Context, options VolumeListOptions) (volume.ListResponse, error)
-	VolumeRemove(ctx context.Context, volumeID string, force bool) error
+	VolumeInspect(ctx context.Context, volumeID string) (VolumeInspectResult, error)
+	VolumeInspectWithRaw(ctx context.Context, volumeID string) (VolumeInspectResult, []byte, error)
+	VolumeList(ctx context.Context, options VolumeListOptions) (VolumeListResult, error)
+	VolumeRemove(ctx context.Context, volumeID string, options VolumeRemoveOptions) error
 	VolumesPrune(ctx context.Context, opts VolumePruneOptions) (VolumePruneResult, error)
 	VolumeUpdate(ctx context.Context, volumeID string, version swarm.Version, options VolumeUpdateOptions) error
 }

client/volume_inspect.go

@@ -9,32 +9,37 @@ import (
 	"github.com/moby/moby/api/types/volume"
 )
 
+// VolumeInspectResult holds the result from the [Client.VolumeInspect] method.
+type VolumeInspectResult struct {
+	Volume volume.Volume
+}
+
 // VolumeInspect returns the information about a specific volume in the docker host.
-func (cli *Client) VolumeInspect(ctx context.Context, volumeID string) (volume.Volume, error) {
+func (cli *Client) VolumeInspect(ctx context.Context, volumeID string) (VolumeInspectResult, error) {
 	vol, _, err := cli.VolumeInspectWithRaw(ctx, volumeID)
 	return vol, err
 }
 
 // VolumeInspectWithRaw returns the information about a specific volume in the docker host and its raw representation
-func (cli *Client) VolumeInspectWithRaw(ctx context.Context, volumeID string) (volume.Volume, []byte, error) {
+func (cli *Client) VolumeInspectWithRaw(ctx context.Context, volumeID string) (VolumeInspectResult, []byte, error) {
 	volumeID, err := trimID("volume", volumeID)
 	if err != nil {
-		return volume.Volume{}, nil, err
+		return VolumeInspectResult{}, nil, err
 	}
 
 	resp, err := cli.get(ctx, "/volumes/"+volumeID, nil, nil)
 	defer ensureReaderClosed(resp)
 	if err != nil {
-		return volume.Volume{}, nil, err
+		return VolumeInspectResult{}, nil, err
 	}
 
 	body, err := io.ReadAll(resp.Body)
 	if err != nil {
-		return volume.Volume{}, nil, err
+		return VolumeInspectResult{}, nil, err
 	}
 
 	var vol volume.Volume
 	rdr := bytes.NewReader(body)
 	err = json.NewDecoder(rdr).Decode(&vol)
-	return vol, body, err
+	return VolumeInspectResult{Volume: vol}, body, err
 }

client/volume_inspect_test.go

@@ -68,7 +68,7 @@ func TestVolumeInspect(t *testing.T) {
 	}))
 	assert.NilError(t, err)
 
-	vol, err := client.VolumeInspect(context.Background(), "volume_id")
+	result, err := client.VolumeInspect(context.Background(), "volume_id")
 	assert.NilError(t, err)
-	assert.Check(t, is.DeepEqual(expected, vol))
+	assert.Check(t, is.DeepEqual(expected, result.Volume))
 }

client/volume_list.go

@@ -8,18 +8,28 @@ import (
 	"github.com/moby/moby/api/types/volume"
 )
 
+// VolumeListOptions holds parameters to list volumes.
+type VolumeListOptions struct {
+	Filters Filters
+}
+
+// VolumeListResult holds the result from the [Client.VolumeList] method.
+type VolumeListResult struct {
+	List volume.ListResponse
+}
+
 // VolumeList returns the volumes configured in the docker host.
-func (cli *Client) VolumeList(ctx context.Context, options VolumeListOptions) (volume.ListResponse, error) {
+func (cli *Client) VolumeList(ctx context.Context, options VolumeListOptions) (VolumeListResult, error) {
 	query := url.Values{}
 
 	options.Filters.updateURLValues(query)
 	resp, err := cli.get(ctx, "/volumes", query, nil)
 	defer ensureReaderClosed(resp)
 	if err != nil {
-		return volume.ListResponse{}, err
+		return VolumeListResult{}, err
 	}
 
 	var volumes volume.ListResponse
 	err = json.NewDecoder(resp.Body).Decode(&volumes)
-	return volumes, err
+	return VolumeListResult{List: volumes}, err
 }

client/volume_list_opts.go

@@ -1,6 +0,0 @@
-package client
-
-// VolumeListOptions holds parameters to list volumes.
-type VolumeListOptions struct {
-	Filters Filters
-}

client/volume_list_test.go

@@ -72,8 +72,8 @@ func TestVolumeList(t *testing.T) {
 		}))
 		assert.NilError(t, err)
 
-		volumeResponse, err := client.VolumeList(context.Background(), VolumeListOptions{Filters: listCase.filters})
+		result, err := client.VolumeList(context.Background(), VolumeListOptions{Filters: listCase.filters})
 		assert.NilError(t, err)
-		assert.Check(t, is.Len(volumeResponse.Volumes, 1))
+		assert.Check(t, is.Len(result.List.Volumes, 1))
 	}
 }

client/volume_remove.go

@@ -5,15 +5,21 @@ import (
 	"net/url"
 )
 
+// VolumeRemoveOptions holds optional parameters for volume removal.
+type VolumeRemoveOptions struct {
+	// Force the removal of the volume
+	Force bool
+}
+
 // VolumeRemove removes a volume from the docker host.
-func (cli *Client) VolumeRemove(ctx context.Context, volumeID string, force bool) error {
+func (cli *Client) VolumeRemove(ctx context.Context, volumeID string, options VolumeRemoveOptions) error {
 	volumeID, err := trimID("volume", volumeID)
 	if err != nil {
 		return err
 	}
 
 	query := url.Values{}
-	if force {
+	if options.Force {
 		query.Set("force", "1")
 	}
 	resp, err := cli.delete(ctx, "/volumes/"+volumeID, query, nil)

client/volume_remove_test.go

@@ -17,14 +17,14 @@ func TestVolumeRemoveError(t *testing.T) {
 	client, err := NewClientWithOpts(WithMockClient(errorMock(http.StatusInternalServerError, "Server error")))
 	assert.NilError(t, err)
 
-	err = client.VolumeRemove(context.Background(), "volume_id", false)
+	err = client.VolumeRemove(context.Background(), "volume_id", VolumeRemoveOptions{})
 	assert.Check(t, is.ErrorType(err, cerrdefs.IsInternal))
 
-	err = client.VolumeRemove(context.Background(), "", false)
+	err = client.VolumeRemove(context.Background(), "", VolumeRemoveOptions{})
 	assert.Check(t, is.ErrorType(err, cerrdefs.IsInvalidArgument))
 	assert.Check(t, is.ErrorContains(err, "value is empty"))
 
-	err = client.VolumeRemove(context.Background(), "    ", false)
+	err = client.VolumeRemove(context.Background(), "    ", VolumeRemoveOptions{})
 	assert.Check(t, is.ErrorType(err, cerrdefs.IsInvalidArgument))
 	assert.Check(t, is.ErrorContains(err, "value is empty"))
 }
@@ -37,7 +37,7 @@ func TestVolumeRemoveConnectionError(t *testing.T) {
 	client, err := NewClientWithOpts(WithAPIVersionNegotiation(), WithHost("tcp://no-such-host.invalid"))
 	assert.NilError(t, err)
 
-	err = client.VolumeRemove(context.Background(), "volume_id", false)
+	err = client.VolumeRemove(context.Background(), "volume_id", VolumeRemoveOptions{})
 	assert.Check(t, is.ErrorType(err, IsErrConnectionFailed))
 }
 
@@ -59,6 +59,6 @@ func TestVolumeRemove(t *testing.T) {
 	}))
 	assert.NilError(t, err)
 
-	err = client.VolumeRemove(context.Background(), "volume_id", true)
+	err = client.VolumeRemove(context.Background(), "volume_id", VolumeRemoveOptions{Force: true})
 	assert.NilError(t, err)
 }

integration/container/mounts_linux_test.go

@@ -415,8 +415,8 @@ func TestContainerVolumeAnonymous(t *testing.T) {
 
 		// see [daemon.AnonymousLabel]; we don't want to import the daemon package here.
 		const expectedAnonymousLabel = "com.docker.volume.anonymous"
-		assert.Check(t, is.Contains(volInspect.Labels, expectedAnonymousLabel))
-		assert.Check(t, is.Equal(volInspect.Driver, volume.DefaultDriverName))
+		assert.Check(t, is.Contains(volInspect.Volume.Labels, expectedAnonymousLabel))
+		assert.Check(t, is.Equal(volInspect.Volume.Driver, volume.DefaultDriverName))
 	})
 
 	// Verify that specifying a custom driver is still taken into account.

integration/daemon/daemon_test.go

@@ -567,7 +567,7 @@ func testLiveRestoreVolumeReferences(t *testing.T) {
 			d.Restart(t, "--live-restore", "--iptables=false", "--ip6tables=false")
 
 			// Try to remove the volume
-			err = c.VolumeRemove(ctx, volName, false)
+			err = c.VolumeRemove(ctx, volName, client.VolumeRemoveOptions{})
 			assert.ErrorContains(t, err, "volume is in use")
 
 			_, err = c.VolumeInspect(ctx, volName)
@@ -626,7 +626,7 @@ func testLiveRestoreVolumeReferences(t *testing.T) {
 
 		// Try to remove the volume
 		// This should fail since its used by a container
-		err = c.VolumeRemove(ctx, v.Name, false)
+		err = c.VolumeRemove(ctx, v.Name, client.VolumeRemoveOptions{})
 		assert.ErrorContains(t, err, "volume is in use")
 
 		t.Run("volume still mounted", func(t *testing.T) {
@@ -660,7 +660,7 @@ func testLiveRestoreVolumeReferences(t *testing.T) {
 		assert.NilError(t, err)
 
 		// Now we should be able to remove the volume
-		err = c.VolumeRemove(ctx, v.Name, false)
+		err = c.VolumeRemove(ctx, v.Name, client.VolumeRemoveOptions{})
 		assert.NilError(t, err)
 	})
 

integration/plugin/authz/authz_plugin_v2_test.go

@@ -102,7 +102,7 @@ func TestAuthZPluginV2RejectVolumeRequests(t *testing.T) {
 	assert.ErrorContains(t, err, fmt.Sprintf("Error response from daemon: plugin %s failed with error:", authzPluginNameWithTag))
 
 	// The plugin will block the command before it can determine the volume does not exist
-	err = c.VolumeRemove(ctx, "test", false)
+	err = c.VolumeRemove(ctx, "test", client.VolumeRemoveOptions{})
 	assert.Assert(t, err != nil)
 	assert.ErrorContains(t, err, fmt.Sprintf("Error response from daemon: plugin %s failed with error:", authzPluginNameWithTag))
 

integration/volume/mount_test.go

@@ -238,7 +238,7 @@ func setupTestVolume(t *testing.T, apiClient client.APIClient) string {
 
 	volumeName := t.Name() + "-volume"
 
-	err := apiClient.VolumeRemove(ctx, volumeName, true)
+	err := apiClient.VolumeRemove(ctx, volumeName, client.VolumeRemoveOptions{Force: true})
 	assert.NilError(t, err, "failed to clean volume")
 
 	_, err = apiClient.VolumeCreate(ctx, volume.CreateOptions{

integration/volume/volume_test.go

@@ -47,8 +47,9 @@ func TestVolumesCreateAndList(t *testing.T) {
 	}
 	assert.Check(t, is.DeepEqual(vol, expected, cmpopts.EquateEmpty()))
 
-	volList, err := apiClient.VolumeList(ctx, client.VolumeListOptions{})
+	res, err := apiClient.VolumeList(ctx, client.VolumeListOptions{})
 	assert.NilError(t, err)
+	volList := res.List
 	assert.Assert(t, len(volList.Volumes) > 0)
 
 	volumes := volList.Volumes[:0]
@@ -76,7 +77,7 @@ func TestVolumesRemove(t *testing.T) {
 	vname := c.Mounts[0].Name
 
 	t.Run("volume in use", func(t *testing.T) {
-		err = apiClient.VolumeRemove(ctx, vname, false)
+		err = apiClient.VolumeRemove(ctx, vname, client.VolumeRemoveOptions{})
 		assert.Check(t, is.ErrorType(err, cerrdefs.IsConflict))
 		assert.Check(t, is.ErrorContains(err, "volume is in use"))
 	})
@@ -87,17 +88,17 @@ func TestVolumesRemove(t *testing.T) {
 		})
 		assert.NilError(t, err)
 
-		err = apiClient.VolumeRemove(ctx, vname, false)
+		err = apiClient.VolumeRemove(ctx, vname, client.VolumeRemoveOptions{})
 		assert.NilError(t, err)
 	})
 
 	t.Run("non-existing volume", func(t *testing.T) {
-		err = apiClient.VolumeRemove(ctx, "no_such_volume", false)
+		err = apiClient.VolumeRemove(ctx, "no_such_volume", client.VolumeRemoveOptions{})
 		assert.Check(t, is.ErrorType(err, cerrdefs.IsNotFound))
 	})
 
 	t.Run("non-existing volume force", func(t *testing.T) {
-		err = apiClient.VolumeRemove(ctx, "no_such_volume", true)
+		err = apiClient.VolumeRemove(ctx, "no_such_volume", client.VolumeRemoveOptions{Force: true})
 		assert.NilError(t, err)
 	})
 }
@@ -128,7 +129,7 @@ func TestVolumesRemoveSwarmEnabled(t *testing.T) {
 	vname := c.Mounts[0].Name
 
 	t.Run("volume in use", func(t *testing.T) {
-		err = apiClient.VolumeRemove(ctx, vname, false)
+		err = apiClient.VolumeRemove(ctx, vname, client.VolumeRemoveOptions{})
 		assert.Check(t, is.ErrorType(err, cerrdefs.IsConflict))
 		assert.Check(t, is.ErrorContains(err, "volume is in use"))
 	})
@@ -139,17 +140,17 @@ func TestVolumesRemoveSwarmEnabled(t *testing.T) {
 		})
 		assert.NilError(t, err)
 
-		err = apiClient.VolumeRemove(ctx, vname, false)
+		err = apiClient.VolumeRemove(ctx, vname, client.VolumeRemoveOptions{})
 		assert.NilError(t, err)
 	})
 
 	t.Run("non-existing volume", func(t *testing.T) {
-		err = apiClient.VolumeRemove(ctx, "no_such_volume", false)
+		err = apiClient.VolumeRemove(ctx, "no_such_volume", client.VolumeRemoveOptions{})
 		assert.Check(t, is.ErrorType(err, cerrdefs.IsNotFound))
 	})
 
 	t.Run("non-existing volume force", func(t *testing.T) {
-		err = apiClient.VolumeRemove(ctx, "no_such_volume", true)
+		err = apiClient.VolumeRemove(ctx, "no_such_volume", client.VolumeRemoveOptions{Force: true})
 		assert.NilError(t, err)
 	})
 }
@@ -165,22 +166,22 @@ func TestVolumesInspect(t *testing.T) {
 	inspected, err := apiClient.VolumeInspect(ctx, vol.Name)
 	assert.NilError(t, err)
 
-	assert.Check(t, is.DeepEqual(inspected, vol, cmpopts.EquateEmpty()))
+	assert.Check(t, is.DeepEqual(inspected.Volume, vol, cmpopts.EquateEmpty()))
 
 	// comparing CreatedAt field time for the new volume to now. Truncate to 1 minute precision to avoid false positive
-	createdAt, err := time.Parse(time.RFC3339, strings.TrimSpace(inspected.CreatedAt))
+	createdAt, err := time.Parse(time.RFC3339, strings.TrimSpace(inspected.Volume.CreatedAt))
 	assert.NilError(t, err)
 	assert.Check(t, createdAt.Unix()-now.Unix() < 60, "CreatedAt (%s) exceeds creation time (%s) 60s", createdAt, now)
 
 	// update atime and mtime for the "_data" directory (which would happen during volume initialization)
 	modifiedAt := time.Now().Local().Add(5 * time.Hour)
-	err = os.Chtimes(inspected.Mountpoint, modifiedAt, modifiedAt)
+	err = os.Chtimes(inspected.Volume.Mountpoint, modifiedAt, modifiedAt)
 	assert.NilError(t, err)
 
 	inspected, err = apiClient.VolumeInspect(ctx, vol.Name)
 	assert.NilError(t, err)
 
-	createdAt2, err := time.Parse(time.RFC3339, strings.TrimSpace(inspected.CreatedAt))
+	createdAt2, err := time.Parse(time.RFC3339, strings.TrimSpace(inspected.Volume.CreatedAt))
 	assert.NilError(t, err)
 
 	// Check that CreatedAt didn't change after updating atime and mtime of the "_data" directory

internal/testutil/environment/clean.go

@@ -127,14 +127,17 @@ func removeImage(ctx context.Context, t testing.TB, apiclient client.ImageAPICli
 
 func deleteAllVolumes(ctx context.Context, t testing.TB, c client.VolumeAPIClient, protectedVolumes map[string]struct{}) {
 	t.Helper()
-	volumes, err := c.VolumeList(ctx, client.VolumeListOptions{})
+	res, err := c.VolumeList(ctx, client.VolumeListOptions{})
 	assert.Check(t, err, "failed to list volumes")
+	volumeList := res.List
 
-	for _, v := range volumes.Volumes {
+	for _, v := range volumeList.Volumes {
 		if _, ok := protectedVolumes[v.Name]; ok {
 			continue
 		}
-		err := c.VolumeRemove(ctx, v.Name, true)
+		err := c.VolumeRemove(ctx, v.Name, client.VolumeRemoveOptions{
+			Force: true,
+		})
 		assert.Check(t, err, "failed to remove volume %s", v.Name)
 	}
 }

internal/testutil/environment/protect.go

@@ -227,8 +227,9 @@ func ProtectVolumes(ctx context.Context, t testing.TB, testEnv *Execution) {
 func getExistingVolumes(ctx context.Context, t testing.TB, testEnv *Execution) []string {
 	t.Helper()
 	apiClient := testEnv.APIClient()
-	volumeList, err := apiClient.VolumeList(ctx, client.VolumeListOptions{})
+	res, err := apiClient.VolumeList(ctx, client.VolumeListOptions{})
 	assert.NilError(t, err, "failed to list volumes")
+	volumeList := res.List
 
 	var volumes []string
 	for _, vol := range volumeList.Volumes {

vendor/github.com/moby/moby/client/client_interfaces.go

@@ -196,10 +196,10 @@ type SystemAPIClient interface {
 // VolumeAPIClient defines API client methods for the volumes
 type VolumeAPIClient interface {
 	VolumeCreate(ctx context.Context, options volume.CreateOptions) (volume.Volume, error)
-	VolumeInspect(ctx context.Context, volumeID string) (volume.Volume, error)
-	VolumeInspectWithRaw(ctx context.Context, volumeID string) (volume.Volume, []byte, error)
-	VolumeList(ctx context.Context, options VolumeListOptions) (volume.ListResponse, error)
-	VolumeRemove(ctx context.Context, volumeID string, force bool) error
+	VolumeInspect(ctx context.Context, volumeID string) (VolumeInspectResult, error)
+	VolumeInspectWithRaw(ctx context.Context, volumeID string) (VolumeInspectResult, []byte, error)
+	VolumeList(ctx context.Context, options VolumeListOptions) (VolumeListResult, error)
+	VolumeRemove(ctx context.Context, volumeID string, options VolumeRemoveOptions) error
 	VolumesPrune(ctx context.Context, opts VolumePruneOptions) (VolumePruneResult, error)
 	VolumeUpdate(ctx context.Context, volumeID string, version swarm.Version, options VolumeUpdateOptions) error
 }

vendor/github.com/moby/moby/client/volume_inspect.go

@@ -9,32 +9,37 @@ import (
 	"github.com/moby/moby/api/types/volume"
 )
 
+// VolumeInspectResult holds the result from the [Client.VolumeInspect] method.
+type VolumeInspectResult struct {
+	Volume volume.Volume
+}
+
 // VolumeInspect returns the information about a specific volume in the docker host.
-func (cli *Client) VolumeInspect(ctx context.Context, volumeID string) (volume.Volume, error) {
+func (cli *Client) VolumeInspect(ctx context.Context, volumeID string) (VolumeInspectResult, error) {
 	vol, _, err := cli.VolumeInspectWithRaw(ctx, volumeID)
 	return vol, err
 }
 
 // VolumeInspectWithRaw returns the information about a specific volume in the docker host and its raw representation
-func (cli *Client) VolumeInspectWithRaw(ctx context.Context, volumeID string) (volume.Volume, []byte, error) {
+func (cli *Client) VolumeInspectWithRaw(ctx context.Context, volumeID string) (VolumeInspectResult, []byte, error) {
 	volumeID, err := trimID("volume", volumeID)
 	if err != nil {
-		return volume.Volume{}, nil, err
+		return VolumeInspectResult{}, nil, err
 	}
 
 	resp, err := cli.get(ctx, "/volumes/"+volumeID, nil, nil)
 	defer ensureReaderClosed(resp)
 	if err != nil {
-		return volume.Volume{}, nil, err
+		return VolumeInspectResult{}, nil, err
 	}
 
 	body, err := io.ReadAll(resp.Body)
 	if err != nil {
-		return volume.Volume{}, nil, err
+		return VolumeInspectResult{}, nil, err
 	}
 
 	var vol volume.Volume
 	rdr := bytes.NewReader(body)
 	err = json.NewDecoder(rdr).Decode(&vol)
-	return vol, body, err
+	return VolumeInspectResult{Volume: vol}, body, err
 }

vendor/github.com/moby/moby/client/volume_list.go

@@ -8,18 +8,28 @@ import (
 	"github.com/moby/moby/api/types/volume"
 )
 
+// VolumeListOptions holds parameters to list volumes.
+type VolumeListOptions struct {
+	Filters Filters
+}
+
+// VolumeListResult holds the result from the [Client.VolumeList] method.
+type VolumeListResult struct {
+	List volume.ListResponse
+}
+
 // VolumeList returns the volumes configured in the docker host.
-func (cli *Client) VolumeList(ctx context.Context, options VolumeListOptions) (volume.ListResponse, error) {
+func (cli *Client) VolumeList(ctx context.Context, options VolumeListOptions) (VolumeListResult, error) {
 	query := url.Values{}
 
 	options.Filters.updateURLValues(query)
 	resp, err := cli.get(ctx, "/volumes", query, nil)
 	defer ensureReaderClosed(resp)
 	if err != nil {
-		return volume.ListResponse{}, err
+		return VolumeListResult{}, err
 	}
 
 	var volumes volume.ListResponse
 	err = json.NewDecoder(resp.Body).Decode(&volumes)
-	return volumes, err
+	return VolumeListResult{List: volumes}, err
 }

vendor/github.com/moby/moby/client/volume_list_opts.go

@@ -1,6 +0,0 @@
-package client
-
-// VolumeListOptions holds parameters to list volumes.
-type VolumeListOptions struct {
-	Filters Filters
-}

vendor/github.com/moby/moby/client/volume_remove.go

@@ -5,15 +5,21 @@ import (
 	"net/url"
 )
 
+// VolumeRemoveOptions holds optional parameters for volume removal.
+type VolumeRemoveOptions struct {
+	// Force the removal of the volume
+	Force bool
+}
+
 // VolumeRemove removes a volume from the docker host.
-func (cli *Client) VolumeRemove(ctx context.Context, volumeID string, force bool) error {
+func (cli *Client) VolumeRemove(ctx context.Context, volumeID string, options VolumeRemoveOptions) error {
 	volumeID, err := trimID("volume", volumeID)
 	if err != nil {
 		return err
 	}
 
 	query := url.Values{}
-	if force {
+	if options.Force {
 		query.Set("force", "1")
 	}
 	resp, err := cli.delete(ctx, "/volumes/"+volumeID, query, nil)