Merge pull request moby#51278 from vvoland/client-container-opts2

client_(attach,commit,create,diff): Wrap result and options

Author: thaJeztah

client/client_interfaces.go

@@ -12,7 +12,6 @@ import (
 	"github.com/moby/moby/api/types/registry"
 	"github.com/moby/moby/api/types/swarm"
 	"github.com/moby/moby/api/types/system"
-	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 )
 
 // APIClient is an interface that clients that talk with a docker server must implement.
@@ -58,10 +57,10 @@ type HijackDialer interface {
 
 // ContainerAPIClient defines API client methods for the containers
 type ContainerAPIClient interface {
-	ContainerAttach(ctx context.Context, container string, options ContainerAttachOptions) (HijackedResponse, error)
-	ContainerCommit(ctx context.Context, container string, options ContainerCommitOptions) (container.CommitResponse, error)
-	ContainerCreate(ctx context.Context, config *container.Config, hostConfig *container.HostConfig, networkingConfig *network.NetworkingConfig, platform *ocispec.Platform, containerName string) (container.CreateResponse, error)
-	ContainerDiff(ctx context.Context, container string) ([]container.FilesystemChange, error)
+	ContainerAttach(ctx context.Context, container string, options ContainerAttachOptions) (ContainerAttachResult, error)
+	ContainerCommit(ctx context.Context, container string, options ContainerCommitOptions) (ContainerCommitResult, error)
+	ContainerCreate(ctx context.Context, options ContainerCreateOptions) (ContainerCreateResult, error)
+	ContainerDiff(ctx context.Context, container string, options ContainerDiffOptions) (ContainerDiffResult, error)
 	ExecAPIClient
 	ContainerExport(ctx context.Context, container string) (io.ReadCloser, error)
 	ContainerInspect(ctx context.Context, container string) (container.InspectResponse, error)

client/container_attach.go

@@ -16,6 +16,11 @@ type ContainerAttachOptions struct {
 	Logs       bool
 }
 
+// ContainerAttachResult is the result from attaching to a container.
+type ContainerAttachResult struct {
+	HijackedResponse
+}
+
 // ContainerAttach attaches a connection to a container in the server.
 // It returns a [HijackedResponse] with the hijacked connection
 // and a reader to get output. It's up to the called to close
@@ -44,10 +49,10 @@ type ContainerAttachOptions struct {
 // [stdcopy.StdType]: https://pkg.go.dev/github.com/moby/moby/api/pkg/stdcopy#StdType
 // [Stdout]: https://pkg.go.dev/github.com/moby/moby/api/pkg/stdcopy#Stdout
 // [Stderr]: https://pkg.go.dev/github.com/moby/moby/api/pkg/stdcopy#Stderr
-func (cli *Client) ContainerAttach(ctx context.Context, containerID string, options ContainerAttachOptions) (HijackedResponse, error) {
+func (cli *Client) ContainerAttach(ctx context.Context, containerID string, options ContainerAttachOptions) (ContainerAttachResult, error) {
 	containerID, err := trimID("container", containerID)
 	if err != nil {
-		return HijackedResponse{}, err
+		return ContainerAttachResult{}, err
 	}
 
 	query := url.Values{}
@@ -70,7 +75,12 @@ func (cli *Client) ContainerAttach(ctx context.Context, containerID string, opti
 		query.Set("logs", "1")
 	}
 
-	return cli.postHijacked(ctx, "/containers/"+containerID+"/attach", query, nil, http.Header{
+	hijacked, err := cli.postHijacked(ctx, "/containers/"+containerID+"/attach", query, nil, http.Header{
 		"Content-Type": {"text/plain"},
 	})
+	if err != nil {
+		return ContainerAttachResult{}, err
+	}
+
+	return ContainerAttachResult{HijackedResponse: hijacked}, nil
 }

client/container_commit.go

@@ -20,22 +20,27 @@ type ContainerCommitOptions struct {
 	Config    *container.Config
 }
 
+// ContainerCommitResult is the result from committing a container.
+type ContainerCommitResult struct {
+	ID string
+}
+
 // ContainerCommit applies changes to a container and creates a new tagged image.
-func (cli *Client) ContainerCommit(ctx context.Context, containerID string, options ContainerCommitOptions) (container.CommitResponse, error) {
+func (cli *Client) ContainerCommit(ctx context.Context, containerID string, options ContainerCommitOptions) (ContainerCommitResult, error) {
 	containerID, err := trimID("container", containerID)
 	if err != nil {
-		return container.CommitResponse{}, err
+		return ContainerCommitResult{}, err
 	}
 
 	var repository, tag string
 	if options.Reference != "" {
 		ref, err := reference.ParseNormalizedNamed(options.Reference)
 		if err != nil {
-			return container.CommitResponse{}, err
+			return ContainerCommitResult{}, err
 		}
 
 		if _, ok := ref.(reference.Digested); ok {
-			return container.CommitResponse{}, errors.New("refusing to create a tag with a digest reference")
+			return ContainerCommitResult{}, errors.New("refusing to create a tag with a digest reference")
 		}
 		ref = reference.TagNameOnly(ref)
 
@@ -62,9 +67,9 @@ func (cli *Client) ContainerCommit(ctx context.Context, containerID string, opti
 	resp, err := cli.post(ctx, "/commit", query, options.Config, nil)
 	defer ensureReaderClosed(resp)
 	if err != nil {
-		return response, err
+		return ContainerCommitResult{}, err
 	}
 
 	err = json.NewDecoder(resp.Body).Decode(&response)
-	return response, err
+	return ContainerCommitResult{ID: response.ID}, err
 }

client/container_create.go

@@ -10,49 +10,63 @@ import (
 
 	cerrdefs "github.com/containerd/errdefs"
 	"github.com/moby/moby/api/types/container"
-	"github.com/moby/moby/api/types/network"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 )
 
 // ContainerCreate creates a new container based on the given configuration.
 // It can be associated with a name, but it's not mandatory.
-func (cli *Client) ContainerCreate(ctx context.Context, config *container.Config, hostConfig *container.HostConfig, networkingConfig *network.NetworkingConfig, platform *ocispec.Platform, containerName string) (container.CreateResponse, error) {
-	if config == nil {
-		return container.CreateResponse{}, cerrdefs.ErrInvalidArgument.WithMessage("config is nil")
+func (cli *Client) ContainerCreate(ctx context.Context, options ContainerCreateOptions) (ContainerCreateResult, error) {
+	cfg := options.Config
+
+	if cfg == nil {
+		cfg = &container.Config{}
+	}
+
+	if options.Image != "" {
+		if cfg.Image != "" {
+			return ContainerCreateResult{}, cerrdefs.ErrInvalidArgument.WithMessage("either Image or config.Image should be set")
+		}
+		newCfg := *cfg
+		newCfg.Image = options.Image
+		cfg = &newCfg
+	}
+
+	if cfg.Image == "" {
+		return ContainerCreateResult{}, cerrdefs.ErrInvalidArgument.WithMessage("config.Image or Image is required")
 	}
 
 	var response container.CreateResponse
 
-	if hostConfig != nil {
-		hostConfig.CapAdd = normalizeCapabilities(hostConfig.CapAdd)
-		hostConfig.CapDrop = normalizeCapabilities(hostConfig.CapDrop)
+	if options.HostConfig != nil {
+		options.HostConfig.CapAdd = normalizeCapabilities(options.HostConfig.CapAdd)
+		options.HostConfig.CapDrop = normalizeCapabilities(options.HostConfig.CapDrop)
 	}
 
 	query := url.Values{}
-	if platform != nil {
-		if p := formatPlatform(*platform); p != "unknown" {
+	if options.Platform != nil {
+		if p := formatPlatform(*options.Platform); p != "unknown" {
 			query.Set("platform", p)
 		}
 	}
 
-	if containerName != "" {
-		query.Set("name", containerName)
+	if options.Name != "" {
+		query.Set("name", options.Name)
 	}
 
 	body := container.CreateRequest{
-		Config:           config,
-		HostConfig:       hostConfig,
-		NetworkingConfig: networkingConfig,
+		Config:           cfg,
+		HostConfig:       options.HostConfig,
+		NetworkingConfig: options.NetworkingConfig,
 	}
 
 	resp, err := cli.post(ctx, "/containers/create", query, body, nil)
 	defer ensureReaderClosed(resp)
 	if err != nil {
-		return response, err
+		return ContainerCreateResult{}, err
 	}
 
 	err = json.NewDecoder(resp.Body).Decode(&response)
-	return response, err
+	return ContainerCreateResult{ID: response.ID, Warnings: response.Warnings}, err
 }
 
 // formatPlatform returns a formatted string representing platform (e.g., "linux/arm/v7").

client/container_create_opts.go

@@ -0,0 +1,25 @@
+package client
+
+import (
+	"github.com/moby/moby/api/types/container"
+	"github.com/moby/moby/api/types/network"
+	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
+)
+
+// ContainerCreateOptions holds parameters to create a container.
+type ContainerCreateOptions struct {
+	Config           *container.Config
+	HostConfig       *container.HostConfig
+	NetworkingConfig *network.NetworkingConfig
+	Platform         *ocispec.Platform
+	Name             string
+
+	// Image is a shortcut for Config.Image - only one of Image or Config.Image should be set.
+	Image string
+}
+
+// ContainerCreateResult is the result from creating a container.
+type ContainerCreateResult struct {
+	ID       string
+	Warnings []string
+}

client/container_create_test.go

@@ -20,21 +20,12 @@ func TestContainerCreateError(t *testing.T) {
 	)
 	assert.NilError(t, err)
 
-	_, err = client.ContainerCreate(context.Background(), nil, nil, nil, nil, "nothing")
-	assert.Error(t, err, "config is nil")
+	_, err = client.ContainerCreate(context.Background(), ContainerCreateOptions{Config: nil, Name: "nothing"})
+	assert.Error(t, err, "config.Image or Image is required")
 	assert.Check(t, is.ErrorType(err, cerrdefs.IsInvalidArgument))
 
-	_, err = client.ContainerCreate(context.Background(), &container.Config{}, nil, nil, nil, "nothing")
-	assert.Check(t, is.ErrorType(err, cerrdefs.IsInternal))
-
-	// 404 doesn't automatically means an unknown image
-	client, err = NewClientWithOpts(
-		WithMockClient(errorMock(http.StatusNotFound, "Server error")),
-	)
-	assert.NilError(t, err)
-
-	_, err = client.ContainerCreate(context.Background(), &container.Config{}, nil, nil, nil, "nothing")
-	assert.Check(t, is.ErrorType(err, cerrdefs.IsNotFound))
+	_, err = client.ContainerCreate(context.Background(), ContainerCreateOptions{Config: &container.Config{}, Name: "nothing"})
+	assert.Check(t, is.ErrorType(err, cerrdefs.IsInvalidArgument))
 }
 
 func TestContainerCreateImageNotFound(t *testing.T) {
@@ -43,7 +34,7 @@ func TestContainerCreateImageNotFound(t *testing.T) {
 	)
 	assert.NilError(t, err)
 
-	_, err = client.ContainerCreate(context.Background(), &container.Config{Image: "unknown_image"}, nil, nil, nil, "unknown")
+	_, err = client.ContainerCreate(context.Background(), ContainerCreateOptions{Config: &container.Config{Image: "unknown_image"}, Name: "unknown"})
 	assert.Check(t, is.ErrorType(err, cerrdefs.IsNotFound))
 }
 
@@ -65,7 +56,7 @@ func TestContainerCreateWithName(t *testing.T) {
 	)
 	assert.NilError(t, err)
 
-	r, err := client.ContainerCreate(context.Background(), &container.Config{}, nil, nil, nil, "container_name")
+	r, err := client.ContainerCreate(context.Background(), ContainerCreateOptions{Config: &container.Config{Image: "test"}, Name: "container_name"})
 	assert.NilError(t, err)
 	assert.Check(t, is.Equal(r.ID, "container_id"))
 }
@@ -87,7 +78,7 @@ func TestContainerCreateAutoRemove(t *testing.T) {
 	)
 	assert.NilError(t, err)
 
-	resp, err := client.ContainerCreate(context.Background(), &container.Config{}, &container.HostConfig{AutoRemove: true}, nil, nil, "")
+	resp, err := client.ContainerCreate(context.Background(), ContainerCreateOptions{Config: &container.Config{Image: "test"}, HostConfig: &container.HostConfig{AutoRemove: true}})
 	assert.NilError(t, err)
 	assert.Check(t, is.Equal(resp.ID, "container_id"))
 }
@@ -100,7 +91,7 @@ func TestContainerCreateConnectionError(t *testing.T) {
 	client, err := NewClientWithOpts(WithAPIVersionNegotiation(), WithHost("tcp://no-such-host.invalid"))
 	assert.NilError(t, err)
 
-	_, err = client.ContainerCreate(context.Background(), &container.Config{}, nil, nil, nil, "")
+	_, err = client.ContainerCreate(context.Background(), ContainerCreateOptions{Config: &container.Config{Image: "test"}})
 	assert.Check(t, is.ErrorType(err, IsErrConnectionFailed))
 }
 
@@ -142,6 +133,6 @@ func TestContainerCreateCapabilities(t *testing.T) {
 	)
 	assert.NilError(t, err)
 
-	_, err = client.ContainerCreate(context.Background(), &container.Config{}, &container.HostConfig{CapAdd: inputCaps, CapDrop: inputCaps}, nil, nil, "")
+	_, err = client.ContainerCreate(context.Background(), ContainerCreateOptions{Config: &container.Config{Image: "test"}, HostConfig: &container.HostConfig{CapAdd: inputCaps, CapDrop: inputCaps}})
 	assert.NilError(t, err)
 }

client/container_diff.go

@@ -9,22 +9,22 @@ import (
 )
 
 // ContainerDiff shows differences in a container filesystem since it was started.
-func (cli *Client) ContainerDiff(ctx context.Context, containerID string) ([]container.FilesystemChange, error) {
+func (cli *Client) ContainerDiff(ctx context.Context, containerID string, options ContainerDiffOptions) (ContainerDiffResult, error) {
 	containerID, err := trimID("container", containerID)
 	if err != nil {
-		return nil, err
+		return ContainerDiffResult{}, err
 	}
 
 	resp, err := cli.get(ctx, "/containers/"+containerID+"/changes", url.Values{}, nil)
 	defer ensureReaderClosed(resp)
 	if err != nil {
-		return nil, err
+		return ContainerDiffResult{}, err
 	}
 
 	var changes []container.FilesystemChange
 	err = json.NewDecoder(resp.Body).Decode(&changes)
 	if err != nil {
-		return nil, err
+		return ContainerDiffResult{}, err
 	}
-	return changes, err
+	return ContainerDiffResult{Changes: changes}, err
 }

client/container_diff_opts.go

@@ -0,0 +1,13 @@
+package client
+
+import "github.com/moby/moby/api/types/container"
+
+// ContainerDiffOptions holds parameters to show differences in a container filesystem.
+type ContainerDiffOptions struct {
+	// Currently no options, but this allows for future extensibility
+}
+
+// ContainerDiffResult is the result from showing differences in a container filesystem.
+type ContainerDiffResult struct {
+	Changes []container.FilesystemChange
+}

client/container_diff_test.go

@@ -17,14 +17,14 @@ func TestContainerDiffError(t *testing.T) {
 	)
 	assert.NilError(t, err)
 
-	_, err = client.ContainerDiff(context.Background(), "nothing")
+	_, err = client.ContainerDiff(context.Background(), "nothing", ContainerDiffOptions{})
 	assert.Check(t, is.ErrorType(err, cerrdefs.IsInternal))
 
-	_, err = client.ContainerDiff(context.Background(), "")
+	_, err = client.ContainerDiff(context.Background(), "", ContainerDiffOptions{})
 	assert.Check(t, is.ErrorType(err, cerrdefs.IsInvalidArgument))
 	assert.Check(t, is.ErrorContains(err, "value is empty"))
 
-	_, err = client.ContainerDiff(context.Background(), "    ")
+	_, err = client.ContainerDiff(context.Background(), "    ", ContainerDiffOptions{})
 	assert.Check(t, is.ErrorType(err, cerrdefs.IsInvalidArgument))
 	assert.Check(t, is.ErrorContains(err, "value is empty"))
 }
@@ -57,7 +57,7 @@ func TestContainerDiff(t *testing.T) {
 	)
 	assert.NilError(t, err)
 
-	changes, err := client.ContainerDiff(context.Background(), "container_id")
+	result, err := client.ContainerDiff(context.Background(), "container_id", ContainerDiffOptions{})
 	assert.NilError(t, err)
-	assert.Check(t, is.DeepEqual(changes, expected))
+	assert.Check(t, is.DeepEqual(result.Changes, expected))
 }