Merge pull request moby#51320 from thaJeztah/refactor_client_system_step1

client: refactor Events, Info, RegistryLogin

Author: thaJeztah

client/client_interfaces.go

@@ -7,9 +7,7 @@ import (
 
 	"github.com/moby/moby/api/types"
 	"github.com/moby/moby/api/types/container"
-	"github.com/moby/moby/api/types/events"
 	"github.com/moby/moby/api/types/network"
-	"github.com/moby/moby/api/types/registry"
 	"github.com/moby/moby/api/types/system"
 )
 
@@ -176,9 +174,9 @@ type SwarmAPIClient interface {
 
 // SystemAPIClient defines API client methods for the system
 type SystemAPIClient interface {
-	Events(ctx context.Context, options EventsListOptions) (<-chan events.Message, <-chan error)
-	Info(ctx context.Context) (system.Info, error)
-	RegistryLogin(ctx context.Context, auth registry.AuthConfig) (registry.AuthenticateOKBody, error)
+	Events(ctx context.Context, options EventsListOptions) EventsResult
+	Info(ctx context.Context, options InfoOptions) (SystemInfoResult, error)
+	RegistryLogin(ctx context.Context, auth RegistryLoginOptions) (RegistryLoginResult, error)
 	DiskUsage(ctx context.Context, options DiskUsageOptions) (system.DiskUsage, error)
 	Ping(ctx context.Context, options PingOptions) (PingResult, error)
 }

client/client_test.go

@@ -408,13 +408,13 @@ func TestNegotiateAPIVersionAutomatic(t *testing.T) {
 	// First request should trigger negotiation
 	pingVersion = "1.50"
 	expected = "1.50"
-	_, _ = client.Info(ctx)
+	_, _ = client.Info(ctx, InfoOptions{})
 	assert.Check(t, is.Equal(client.ClientVersion(), expected))
 
 	// Once successfully negotiated, subsequent requests should not re-negotiate
 	pingVersion = "1.49"
 	expected = "1.50"
-	_, _ = client.Info(ctx)
+	_, _ = client.Info(ctx, InfoOptions{})
 	assert.Check(t, is.Equal(client.ClientVersion(), expected))
 }
 

client/login.go

@@ -8,17 +8,38 @@ import (
 	"github.com/moby/moby/api/types/registry"
 )
 
+type RegistryLoginOptions struct {
+	Username      string
+	Password      string
+	ServerAddress string
+	IdentityToken string
+	RegistryToken string
+}
+
+// RegistryLoginResult holds the result of a RegistryLogin query.
+type RegistryLoginResult struct {
+	Auth registry.AuthenticateOKBody
+}
+
 // RegistryLogin authenticates the docker server with a given docker registry.
 // It returns unauthorizedError when the authentication fails.
-func (cli *Client) RegistryLogin(ctx context.Context, auth registry.AuthConfig) (registry.AuthenticateOKBody, error) {
+func (cli *Client) RegistryLogin(ctx context.Context, options RegistryLoginOptions) (RegistryLoginResult, error) {
+	auth := registry.AuthConfig{
+		Username:      options.Username,
+		Password:      options.Password,
+		ServerAddress: options.ServerAddress,
+		IdentityToken: options.IdentityToken,
+		RegistryToken: options.RegistryToken,
+	}
+
 	resp, err := cli.post(ctx, "/auth", url.Values{}, auth, nil)
 	defer ensureReaderClosed(resp)
 
 	if err != nil {
-		return registry.AuthenticateOKBody{}, err
+		return RegistryLoginResult{}, err
 	}
 
 	var response registry.AuthenticateOKBody
 	err = json.NewDecoder(resp.Body).Decode(&response)
-	return response, err
+	return RegistryLoginResult{Auth: response}, err
 }

client/system_events.go

@@ -19,11 +19,17 @@ type EventsListOptions struct {
 	Filters Filters
 }
 
+// EventsResult holds the result of an Events query.
+type EventsResult struct {
+	Messages <-chan events.Message
+	Err      <-chan error
+}
+
 // Events returns a stream of events in the daemon. It's up to the caller to close the stream
 // by cancelling the context. Once the stream has been completely read an [io.EOF] error is
 // sent over the error channel. If an error is sent, all processing is stopped. It's up
 // to the caller to reopen the stream in the event of an error by reinvoking this method.
-func (cli *Client) Events(ctx context.Context, options EventsListOptions) (<-chan events.Message, <-chan error) {
+func (cli *Client) Events(ctx context.Context, options EventsListOptions) EventsResult {
 	messages := make(chan events.Message)
 	errs := make(chan error, 1)
 
@@ -76,7 +82,10 @@ func (cli *Client) Events(ctx context.Context, options EventsListOptions) (<-cha
 	}()
 	<-started
 
-	return messages, errs
+	return EventsResult{
+		Messages: messages,
+		Err:      errs,
+	}
 }
 
 func buildEventsQueryParams(options EventsListOptions) (url.Values, error) {

client/system_events_test.go

@@ -37,17 +37,17 @@ func TestEventsErrorInOptions(t *testing.T) {
 	for _, tc := range errorCases {
 		client, err := NewClientWithOpts(WithMockClient(errorMock(http.StatusInternalServerError, "Server error")))
 		assert.NilError(t, err)
-		_, errs := client.Events(context.Background(), tc.options)
-		err = <-errs
+		events := client.Events(context.Background(), tc.options)
+		err = <-events.Err
 		assert.Check(t, is.ErrorContains(err, tc.expectedError))
 	}
 }
 
 func TestEventsErrorFromServer(t *testing.T) {
 	client, err := NewClientWithOpts(WithMockClient(errorMock(http.StatusInternalServerError, "Server error")))
 	assert.NilError(t, err)
-	_, errs := client.Events(context.Background(), EventsListOptions{})
-	err = <-errs
+	events := client.Events(context.Background(), EventsListOptions{})
+	err = <-events.Err
 	assert.Check(t, is.ErrorType(err, cerrdefs.IsInternal))
 }
 
@@ -133,18 +133,18 @@ func TestEvents(t *testing.T) {
 		}))
 		assert.NilError(t, err)
 
-		messages, errs := client.Events(context.Background(), eventsCase.options)
+		events := client.Events(context.Background(), eventsCase.options)
 
 	loop:
 		for {
 			select {
-			case err := <-errs:
+			case err := <-events.Err:
 				if err != nil && !errors.Is(err, io.EOF) {
 					t.Fatal(err)
 				}
 
 				break loop
-			case e := <-messages:
+			case e := <-events.Messages:
 				_, ok := eventsCase.expectedEvents[e.Actor.ID]
 				assert.Check(t, ok, "event received not expected with action %s & id %s", e.Action, e.Actor.ID)
 			}

client/system_info.go

@@ -9,18 +9,26 @@ import (
 	"github.com/moby/moby/api/types/system"
 )
 
+type InfoOptions struct {
+	// No options currently; placeholder for future use
+}
+
+type SystemInfoResult struct {
+	Info system.Info
+}
+
 // Info returns information about the docker server.
-func (cli *Client) Info(ctx context.Context) (system.Info, error) {
-	var info system.Info
+func (cli *Client) Info(ctx context.Context, options InfoOptions) (SystemInfoResult, error) {
 	resp, err := cli.get(ctx, "/info", url.Values{}, nil)
 	defer ensureReaderClosed(resp)
 	if err != nil {
-		return info, err
+		return SystemInfoResult{}, err
 	}
 
+	var info system.Info
 	if err := json.NewDecoder(resp.Body).Decode(&info); err != nil {
-		return info, fmt.Errorf("Error reading remote info: %v", err)
+		return SystemInfoResult{}, fmt.Errorf("Error reading remote info: %v", err)
 	}
 
-	return info, nil
+	return SystemInfoResult{Info: info}, nil
 }

client/system_info_test.go

@@ -14,14 +14,14 @@ import (
 func TestInfoServerError(t *testing.T) {
 	client, err := NewClientWithOpts(WithMockClient(errorMock(http.StatusInternalServerError, "Server error")))
 	assert.NilError(t, err)
-	_, err = client.Info(context.Background())
+	_, err = client.Info(context.Background(), InfoOptions{})
 	assert.Check(t, is.ErrorType(err, cerrdefs.IsInternal))
 }
 
 func TestInfoInvalidResponseJSONError(t *testing.T) {
 	client, err := NewClientWithOpts(WithMockClient(mockResponse(http.StatusOK, nil, "invalid json")))
 	assert.NilError(t, err)
-	_, err = client.Info(context.Background())
+	_, err = client.Info(context.Background(), InfoOptions{})
 	assert.Check(t, is.ErrorContains(err, "invalid character"))
 }
 
@@ -38,8 +38,9 @@ func TestInfo(t *testing.T) {
 	}))
 	assert.NilError(t, err)
 
-	info, err := client.Info(context.Background())
+	result, err := client.Info(context.Background(), InfoOptions{})
 	assert.NilError(t, err)
+	info := result.Info
 
 	assert.Check(t, is.Equal(info.ID, "daemonID"))
 	assert.Check(t, is.Equal(info.Containers, 3))
@@ -68,8 +69,9 @@ func TestInfoWithDiscoveredDevices(t *testing.T) {
 	}))
 	assert.NilError(t, err)
 
-	info, err := client.Info(context.Background())
+	result, err := client.Info(context.Background(), InfoOptions{})
 	assert.NilError(t, err)
+	info := result.Info
 
 	assert.Check(t, is.Equal(info.ID, "daemonID"))
 	assert.Check(t, is.Equal(info.Containers, 3))

integration-cli/docker_api_swarm_test.go

@@ -756,9 +756,9 @@ func checkClusterHealth(t *testing.T, cl []*daemon.Daemon, managerCount, workerC
 
 		// check info in a poll.WaitOn(), because if the cluster doesn't have a leader, `info` will return an error
 		checkInfo := func(t *testing.T) (any, string) {
-			client := d.NewClientT(t)
-			daemonInfo, err := client.Info(ctx)
-			info = daemonInfo.Swarm
+			apiClient := d.NewClientT(t)
+			result, err := apiClient.Info(ctx, client.InfoOptions{})
+			info = result.Info.Swarm
 			return err, "cluster not ready in time"
 		}
 		poll.WaitOn(t, pollCheck(t, checkInfo, checker.IsNil()), poll.WithTimeout(defaultReconciliationTimeout))

integration-cli/docker_cli_info_unix_test.go

@@ -21,8 +21,9 @@ func (s *DockerCLIInfoSuite) TestInfoSecurityOptions(c *testing.T) {
 	apiClient, err := client.NewClientWithOpts(client.FromEnv)
 	assert.NilError(c, err)
 	defer apiClient.Close()
-	info, err := apiClient.Info(testutil.GetContext(c))
+	result, err := apiClient.Info(testutil.GetContext(c), client.InfoOptions{})
 	assert.NilError(c, err)
+	info := result.Info
 
 	if Apparmor() {
 		assert.Check(c, is.Contains(info.SecurityOptions, "name=apparmor"))

integration-cli/docker_cli_plugins_logdriver_test.go

@@ -54,8 +54,9 @@ func (s *DockerCLIPluginLogDriverSuite) TestPluginLogDriverInfoList(c *testing.T
 	assert.NilError(c, err)
 	defer apiClient.Close()
 
-	info, err := apiClient.Info(testutil.GetContext(c))
+	result, err := apiClient.Info(testutil.GetContext(c), client.InfoOptions{})
 	assert.NilError(c, err)
+	info := result.Info
 
 	drivers := strings.Join(info.Plugins.Log, " ")
 	assert.Assert(c, is.Contains(drivers, "json-file"))