From 3a43b5b559ec327a06157c81b0fd1c5c81146f81 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 23 Oct 2025 15:38:09 +0200 Subject: [PATCH] client: refactor ServiceCreate, ServiceUpdate, SwarmUpdate Put the version and spec in the options-struct, as we did for other swarm-related methods. Signed-off-by: Sebastiaan van Stijn --- client/client_interfaces.go | 6 +- client/service_create.go | 28 +++++---- client/service_create_test.go | 33 +++++----- client/service_update.go | 27 ++++---- client/service_update_test.go | 14 +++-- client/swarm_update.go | 9 +-- client/swarm_update_test.go | 6 +- integration-cli/docker_api_swarm_test.go | 9 ++- integration/internal/swarm/service.go | 4 +- integration/service/create_test.go | 4 +- integration/service/inspect_test.go | 3 +- integration/service/jobs_test.go | 6 +- integration/service/list_test.go | 3 +- integration/service/update_test.go | 63 ++++++++++++++----- internal/testutil/daemon/service.go | 12 ++-- internal/testutil/daemon/swarm.go | 10 +-- .../moby/moby/client/client_interfaces.go | 6 +- .../moby/moby/client/service_create.go | 28 +++++---- .../moby/moby/client/service_update.go | 27 ++++---- .../moby/moby/client/swarm_update.go | 9 +-- 20 files changed, 188 insertions(+), 119 deletions(-) diff --git a/client/client_interfaces.go b/client/client_interfaces.go index 67f880f3b4..14d59b642e 100644 --- a/client/client_interfaces.go +++ b/client/client_interfaces.go @@ -158,11 +158,11 @@ type PluginAPIClient interface { // ServiceAPIClient defines API client methods for the services type ServiceAPIClient interface { - ServiceCreate(ctx context.Context, service swarm.ServiceSpec, options ServiceCreateOptions) (ServiceCreateResult, error) + ServiceCreate(ctx context.Context, options ServiceCreateOptions) (ServiceCreateResult, error) ServiceInspect(ctx context.Context, serviceID string, options ServiceInspectOptions) (ServiceInspectResult, error) ServiceList(ctx context.Context, options ServiceListOptions) (ServiceListResult, error) ServiceRemove(ctx context.Context, serviceID string, options ServiceRemoveOptions) (ServiceRemoveResult, error) - ServiceUpdate(ctx context.Context, serviceID string, version swarm.Version, service swarm.ServiceSpec, options ServiceUpdateOptions) (ServiceUpdateResult, error) + ServiceUpdate(ctx context.Context, serviceID string, options ServiceUpdateOptions) (ServiceUpdateResult, error) ServiceLogs(ctx context.Context, serviceID string, options ServiceLogsOptions) (ServiceLogsResult, error) TaskLogs(ctx context.Context, taskID string, options TaskLogsOptions) (TaskLogsResult, error) TaskInspect(ctx context.Context, taskID string, options TaskInspectOptions) (TaskInspectResult, error) @@ -177,7 +177,7 @@ type SwarmAPIClient interface { SwarmUnlock(ctx context.Context, options SwarmUnlockOptions) (SwarmUnlockResult, error) SwarmLeave(ctx context.Context, options SwarmLeaveOptions) (SwarmLeaveResult, error) SwarmInspect(ctx context.Context, options SwarmInspectOptions) (SwarmInspectResult, error) - SwarmUpdate(ctx context.Context, version swarm.Version, options SwarmUpdateOptions) (SwarmUpdateResult, error) + SwarmUpdate(ctx context.Context, options SwarmUpdateOptions) (SwarmUpdateResult, error) } // SystemAPIClient defines API client methods for the system diff --git a/client/service_create.go b/client/service_create.go index 9155d508a4..204d21816c 100644 --- a/client/service_create.go +++ b/client/service_create.go @@ -16,6 +16,8 @@ import ( // ServiceCreateOptions contains the options to use when creating a service. type ServiceCreateOptions struct { + Spec swarm.ServiceSpec + // EncodedRegistryAuth is the encoded registry authorization credentials to // use when updating the service. // @@ -39,33 +41,33 @@ type ServiceCreateResult struct { } // ServiceCreate creates a new service. -func (cli *Client) ServiceCreate(ctx context.Context, service swarm.ServiceSpec, options ServiceCreateOptions) (ServiceCreateResult, error) { +func (cli *Client) ServiceCreate(ctx context.Context, options ServiceCreateOptions) (ServiceCreateResult, error) { // Make sure containerSpec is not nil when no runtime is set or the runtime is set to container - if service.TaskTemplate.ContainerSpec == nil && (service.TaskTemplate.Runtime == "" || service.TaskTemplate.Runtime == swarm.RuntimeContainer) { - service.TaskTemplate.ContainerSpec = &swarm.ContainerSpec{} + if options.Spec.TaskTemplate.ContainerSpec == nil && (options.Spec.TaskTemplate.Runtime == "" || options.Spec.TaskTemplate.Runtime == swarm.RuntimeContainer) { + options.Spec.TaskTemplate.ContainerSpec = &swarm.ContainerSpec{} } - if err := validateServiceSpec(service); err != nil { + if err := validateServiceSpec(options.Spec); err != nil { return ServiceCreateResult{}, err } // ensure that the image is tagged var warnings []string switch { - case service.TaskTemplate.ContainerSpec != nil: - if taggedImg := imageWithTagString(service.TaskTemplate.ContainerSpec.Image); taggedImg != "" { - service.TaskTemplate.ContainerSpec.Image = taggedImg + case options.Spec.TaskTemplate.ContainerSpec != nil: + if taggedImg := imageWithTagString(options.Spec.TaskTemplate.ContainerSpec.Image); taggedImg != "" { + options.Spec.TaskTemplate.ContainerSpec.Image = taggedImg } if options.QueryRegistry { - resolveWarning := resolveContainerSpecImage(ctx, cli, &service.TaskTemplate, options.EncodedRegistryAuth) + resolveWarning := resolveContainerSpecImage(ctx, cli, &options.Spec.TaskTemplate, options.EncodedRegistryAuth) warnings = append(warnings, resolveWarning) } - case service.TaskTemplate.PluginSpec != nil: - if taggedImg := imageWithTagString(service.TaskTemplate.PluginSpec.Remote); taggedImg != "" { - service.TaskTemplate.PluginSpec.Remote = taggedImg + case options.Spec.TaskTemplate.PluginSpec != nil: + if taggedImg := imageWithTagString(options.Spec.TaskTemplate.PluginSpec.Remote); taggedImg != "" { + options.Spec.TaskTemplate.PluginSpec.Remote = taggedImg } if options.QueryRegistry { - resolveWarning := resolvePluginSpecRemote(ctx, cli, &service.TaskTemplate, options.EncodedRegistryAuth) + resolveWarning := resolvePluginSpecRemote(ctx, cli, &options.Spec.TaskTemplate, options.EncodedRegistryAuth) warnings = append(warnings, resolveWarning) } } @@ -74,7 +76,7 @@ func (cli *Client) ServiceCreate(ctx context.Context, service swarm.ServiceSpec, if options.EncodedRegistryAuth != "" { headers[registry.AuthHeader] = []string{options.EncodedRegistryAuth} } - resp, err := cli.post(ctx, "/services/create", nil, service, headers) + resp, err := cli.post(ctx, "/services/create", nil, options.Spec, headers) defer ensureReaderClosed(resp) if err != nil { return ServiceCreateResult{}, err diff --git a/client/service_create_test.go b/client/service_create_test.go index 47b06d9de3..83402a32e5 100644 --- a/client/service_create_test.go +++ b/client/service_create_test.go @@ -2,7 +2,6 @@ package client import ( "bytes" - "context" "encoding/json" "errors" "fmt" @@ -23,7 +22,7 @@ import ( func TestServiceCreateError(t *testing.T) { client, err := NewClientWithOpts(WithMockClient(errorMock(http.StatusInternalServerError, "Server error"))) assert.NilError(t, err) - _, err = client.ServiceCreate(context.Background(), swarm.ServiceSpec{}, ServiceCreateOptions{}) + _, err = client.ServiceCreate(t.Context(), ServiceCreateOptions{}) assert.Check(t, is.ErrorType(err, cerrdefs.IsInternal)) } @@ -35,7 +34,7 @@ func TestServiceCreateConnectionError(t *testing.T) { client, err := NewClientWithOpts(WithAPIVersionNegotiation(), WithHost("tcp://no-such-host.invalid")) assert.NilError(t, err) - _, err = client.ServiceCreate(context.Background(), swarm.ServiceSpec{}, ServiceCreateOptions{}) + _, err = client.ServiceCreate(t.Context(), ServiceCreateOptions{}) assert.Check(t, is.ErrorType(err, IsErrConnectionFailed)) } @@ -58,7 +57,7 @@ func TestServiceCreate(t *testing.T) { })) assert.NilError(t, err) - r, err := client.ServiceCreate(context.Background(), swarm.ServiceSpec{}, ServiceCreateOptions{}) + r, err := client.ServiceCreate(t.Context(), ServiceCreateOptions{}) assert.NilError(t, err) assert.Check(t, is.Equal(r.ID, "service_id")) } @@ -113,9 +112,14 @@ func TestServiceCreateCompatiblePlatforms(t *testing.T) { })) assert.NilError(t, err) - spec := swarm.ServiceSpec{TaskTemplate: swarm.TaskSpec{ContainerSpec: &swarm.ContainerSpec{Image: "foobar:1.0"}}} - - r, err := client.ServiceCreate(context.Background(), spec, ServiceCreateOptions{QueryRegistry: true}) + r, err := client.ServiceCreate(t.Context(), ServiceCreateOptions{ + Spec: swarm.ServiceSpec{ + TaskTemplate: swarm.TaskSpec{ + ContainerSpec: &swarm.ContainerSpec{Image: "foobar:1.0"}, + }, + }, + QueryRegistry: true, + }) assert.NilError(t, err) assert.Check(t, is.Equal("service_linux_amd64", r.ID)) } @@ -186,17 +190,18 @@ func TestServiceCreateDigestPinning(t *testing.T) { // run pin by digest tests for _, p := range pinByDigestTests { - r, err := client.ServiceCreate(context.Background(), swarm.ServiceSpec{ - TaskTemplate: swarm.TaskSpec{ - ContainerSpec: &swarm.ContainerSpec{ - Image: p.img, + r, err := client.ServiceCreate(t.Context(), ServiceCreateOptions{ + Spec: swarm.ServiceSpec{ + TaskTemplate: swarm.TaskSpec{ + ContainerSpec: &swarm.ContainerSpec{ + Image: p.img, + }, }, }, - }, ServiceCreateOptions{QueryRegistry: true}) + QueryRegistry: true, + }) assert.NilError(t, err) - assert.Check(t, is.Equal(r.ID, "service_id")) - assert.Check(t, is.Equal(p.expected, serviceCreateImage)) } } diff --git a/client/service_update.go b/client/service_update.go index 3910d2bc5f..36061c9d16 100644 --- a/client/service_update.go +++ b/client/service_update.go @@ -12,6 +12,9 @@ import ( // ServiceUpdateOptions contains the options to be used for updating services. type ServiceUpdateOptions struct { + Version swarm.Version + Spec swarm.ServiceSpec + // EncodedRegistryAuth is the encoded registry authorization credentials to // use when updating the service. // @@ -50,13 +53,13 @@ type ServiceUpdateResult struct { // conflicting writes. It must be the value as set *before* the update. // You can find this value in the [swarm.Service.Meta] field, which can // be found using [Client.ServiceInspectWithRaw]. -func (cli *Client) ServiceUpdate(ctx context.Context, serviceID string, version swarm.Version, service swarm.ServiceSpec, options ServiceUpdateOptions) (ServiceUpdateResult, error) { +func (cli *Client) ServiceUpdate(ctx context.Context, serviceID string, options ServiceUpdateOptions) (ServiceUpdateResult, error) { serviceID, err := trimID("service", serviceID) if err != nil { return ServiceUpdateResult{}, err } - if err := validateServiceSpec(service); err != nil { + if err := validateServiceSpec(options.Spec); err != nil { return ServiceUpdateResult{}, err } @@ -69,25 +72,25 @@ func (cli *Client) ServiceUpdate(ctx context.Context, serviceID string, version query.Set("rollback", options.Rollback) } - query.Set("version", version.String()) + query.Set("version", options.Version.String()) // ensure that the image is tagged var warnings []string switch { - case service.TaskTemplate.ContainerSpec != nil: - if taggedImg := imageWithTagString(service.TaskTemplate.ContainerSpec.Image); taggedImg != "" { - service.TaskTemplate.ContainerSpec.Image = taggedImg + case options.Spec.TaskTemplate.ContainerSpec != nil: + if taggedImg := imageWithTagString(options.Spec.TaskTemplate.ContainerSpec.Image); taggedImg != "" { + options.Spec.TaskTemplate.ContainerSpec.Image = taggedImg } if options.QueryRegistry { - resolveWarning := resolveContainerSpecImage(ctx, cli, &service.TaskTemplate, options.EncodedRegistryAuth) + resolveWarning := resolveContainerSpecImage(ctx, cli, &options.Spec.TaskTemplate, options.EncodedRegistryAuth) warnings = append(warnings, resolveWarning) } - case service.TaskTemplate.PluginSpec != nil: - if taggedImg := imageWithTagString(service.TaskTemplate.PluginSpec.Remote); taggedImg != "" { - service.TaskTemplate.PluginSpec.Remote = taggedImg + case options.Spec.TaskTemplate.PluginSpec != nil: + if taggedImg := imageWithTagString(options.Spec.TaskTemplate.PluginSpec.Remote); taggedImg != "" { + options.Spec.TaskTemplate.PluginSpec.Remote = taggedImg } if options.QueryRegistry { - resolveWarning := resolvePluginSpecRemote(ctx, cli, &service.TaskTemplate, options.EncodedRegistryAuth) + resolveWarning := resolvePluginSpecRemote(ctx, cli, &options.Spec.TaskTemplate, options.EncodedRegistryAuth) warnings = append(warnings, resolveWarning) } } @@ -96,7 +99,7 @@ func (cli *Client) ServiceUpdate(ctx context.Context, serviceID string, version if options.EncodedRegistryAuth != "" { headers.Set(registry.AuthHeader, options.EncodedRegistryAuth) } - resp, err := cli.post(ctx, "/services/"+serviceID+"/update", query, service, headers) + resp, err := cli.post(ctx, "/services/"+serviceID+"/update", query, options.Spec, headers) defer ensureReaderClosed(resp) if err != nil { return ServiceUpdateResult{}, err diff --git a/client/service_update_test.go b/client/service_update_test.go index eaef973995..4894c23da0 100644 --- a/client/service_update_test.go +++ b/client/service_update_test.go @@ -2,7 +2,6 @@ package client import ( "bytes" - "context" "fmt" "io" "net/http" @@ -18,14 +17,14 @@ func TestServiceUpdateError(t *testing.T) { client, err := NewClientWithOpts(WithMockClient(errorMock(http.StatusInternalServerError, "Server error"))) assert.NilError(t, err) - _, err = client.ServiceUpdate(context.Background(), "service_id", swarm.Version{}, swarm.ServiceSpec{}, ServiceUpdateOptions{}) + _, err = client.ServiceUpdate(t.Context(), "service_id", ServiceUpdateOptions{}) assert.Check(t, is.ErrorType(err, cerrdefs.IsInternal)) - _, err = client.ServiceUpdate(context.Background(), "", swarm.Version{}, swarm.ServiceSpec{}, ServiceUpdateOptions{}) + _, err = client.ServiceUpdate(t.Context(), "", ServiceUpdateOptions{}) assert.Check(t, is.ErrorType(err, cerrdefs.IsInvalidArgument)) assert.Check(t, is.ErrorContains(err, "value is empty")) - _, err = client.ServiceUpdate(context.Background(), " ", swarm.Version{}, swarm.ServiceSpec{}, ServiceUpdateOptions{}) + _, err = client.ServiceUpdate(t.Context(), " ", ServiceUpdateOptions{}) assert.Check(t, is.ErrorType(err, cerrdefs.IsInvalidArgument)) assert.Check(t, is.ErrorContains(err, "value is empty")) } @@ -38,7 +37,7 @@ func TestServiceUpdateConnectionError(t *testing.T) { client, err := NewClientWithOpts(WithAPIVersionNegotiation(), WithHost("tcp://no-such-host.invalid")) assert.NilError(t, err) - _, err = client.ServiceUpdate(context.Background(), "service_id", swarm.Version{}, swarm.ServiceSpec{}, ServiceUpdateOptions{}) + _, err = client.ServiceUpdate(t.Context(), "service_id", ServiceUpdateOptions{}) assert.Check(t, is.ErrorType(err, IsErrConnectionFailed)) } @@ -82,7 +81,10 @@ func TestServiceUpdate(t *testing.T) { })) assert.NilError(t, err) - _, err = client.ServiceUpdate(context.Background(), "service_id", updateCase.swarmVersion, swarm.ServiceSpec{}, ServiceUpdateOptions{}) + _, err = client.ServiceUpdate(t.Context(), "service_id", ServiceUpdateOptions{ + Version: updateCase.swarmVersion, + Spec: swarm.ServiceSpec{}, + }) assert.NilError(t, err) } } diff --git a/client/swarm_update.go b/client/swarm_update.go index 8f62876a56..81f62b2c02 100644 --- a/client/swarm_update.go +++ b/client/swarm_update.go @@ -10,7 +10,8 @@ import ( // SwarmUpdateOptions contains options for updating a swarm. type SwarmUpdateOptions struct { - Swarm swarm.Spec + Version swarm.Version + Spec swarm.Spec RotateWorkerToken bool RotateManagerToken bool RotateManagerUnlockKey bool @@ -20,13 +21,13 @@ type SwarmUpdateOptions struct { type SwarmUpdateResult struct{} // SwarmUpdate updates the swarm. -func (cli *Client) SwarmUpdate(ctx context.Context, version swarm.Version, options SwarmUpdateOptions) (SwarmUpdateResult, error) { +func (cli *Client) SwarmUpdate(ctx context.Context, options SwarmUpdateOptions) (SwarmUpdateResult, error) { query := url.Values{} - query.Set("version", version.String()) + query.Set("version", options.Version.String()) query.Set("rotateWorkerToken", strconv.FormatBool(options.RotateWorkerToken)) query.Set("rotateManagerToken", strconv.FormatBool(options.RotateManagerToken)) query.Set("rotateManagerUnlockKey", strconv.FormatBool(options.RotateManagerUnlockKey)) - resp, err := cli.post(ctx, "/swarm/update", query, options.Swarm, nil) + resp, err := cli.post(ctx, "/swarm/update", query, options.Spec, nil) defer ensureReaderClosed(resp) return SwarmUpdateResult{}, err } diff --git a/client/swarm_update_test.go b/client/swarm_update_test.go index 3fad797899..9933d125e3 100644 --- a/client/swarm_update_test.go +++ b/client/swarm_update_test.go @@ -2,13 +2,11 @@ package client import ( "bytes" - "context" "io" "net/http" "testing" cerrdefs "github.com/containerd/errdefs" - "github.com/moby/moby/api/types/swarm" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" ) @@ -17,7 +15,7 @@ func TestSwarmUpdateError(t *testing.T) { client, err := NewClientWithOpts(WithMockClient(errorMock(http.StatusInternalServerError, "Server error"))) assert.NilError(t, err) - _, err = client.SwarmUpdate(context.Background(), swarm.Version{}, SwarmUpdateOptions{}) + _, err = client.SwarmUpdate(t.Context(), SwarmUpdateOptions{}) assert.Check(t, is.ErrorType(err, cerrdefs.IsInternal)) } @@ -35,6 +33,6 @@ func TestSwarmUpdate(t *testing.T) { })) assert.NilError(t, err) - _, err = client.SwarmUpdate(context.Background(), swarm.Version{}, SwarmUpdateOptions{}) + _, err = client.SwarmUpdate(t.Context(), SwarmUpdateOptions{}) assert.NilError(t, err) } diff --git a/integration-cli/docker_api_swarm_test.go b/integration-cli/docker_api_swarm_test.go index bbc37d5647..8c8f15d871 100644 --- a/integration-cli/docker_api_swarm_test.go +++ b/integration-cli/docker_api_swarm_test.go @@ -413,7 +413,9 @@ func (s *DockerSwarmSuite) TestAPISwarmRaftQuorum(c *testing.T) { // d1 will eventually step down from leader because there is no longer an active quorum, wait for that to happen poll.WaitOn(c, pollCheck(c, func(t *testing.T) (any, string) { - _, err := cli.ServiceCreate(testutil.GetContext(t), service.Spec, client.ServiceCreateOptions{}) + _, err := cli.ServiceCreate(testutil.GetContext(t), client.ServiceCreateOptions{ + Spec: service.Spec, + }) return err.Error(), "" }, checker.Contains("Make sure more than half of the managers are online.")), poll.WithTimeout(defaultReconciliationTimeout*2)) @@ -889,7 +891,10 @@ func (s *DockerSwarmSuite) TestAPISwarmServicesUpdateWithName(c *testing.T) { setInstances(instances)(service) cli := d.NewClientT(c) defer cli.Close() - _, err := cli.ServiceUpdate(ctx, service.Spec.Name, service.Version, service.Spec, client.ServiceUpdateOptions{}) + _, err := cli.ServiceUpdate(ctx, service.Spec.Name, client.ServiceUpdateOptions{ + Version: service.Version, + Spec: service.Spec, + }) assert.NilError(c, err) poll.WaitOn(c, pollCheck(c, d.CheckActiveContainerCount(ctx), checker.Equals(instances)), poll.WithTimeout(defaultReconciliationTimeout)) } diff --git a/integration/internal/swarm/service.go b/integration/internal/swarm/service.go index c2b3cfb323..d86798929f 100644 --- a/integration/internal/swarm/service.go +++ b/integration/internal/swarm/service.go @@ -61,7 +61,9 @@ func CreateService(ctx context.Context, t *testing.T, d *daemon.Daemon, opts ... defer apiClient.Close() spec := CreateServiceSpec(t, opts...) - resp, err := apiClient.ServiceCreate(ctx, spec, client.ServiceCreateOptions{}) + resp, err := apiClient.ServiceCreate(ctx, client.ServiceCreateOptions{ + Spec: spec, + }) assert.NilError(t, err, "error creating service") return resp.ID } diff --git a/integration/service/create_test.go b/integration/service/create_test.go index a2766854ff..f2398afcf4 100644 --- a/integration/service/create_test.go +++ b/integration/service/create_test.go @@ -163,7 +163,9 @@ func TestCreateServiceConflict(t *testing.T) { swarm.CreateService(ctx, t, d, serviceSpec...) spec := swarm.CreateServiceSpec(t, serviceSpec...) - _, err := c.ServiceCreate(ctx, spec, client.ServiceCreateOptions{}) + _, err := c.ServiceCreate(ctx, client.ServiceCreateOptions{ + Spec: spec, + }) assert.Check(t, cerrdefs.IsConflict(err)) assert.ErrorContains(t, err, "service "+serviceName+" already exists") } diff --git a/integration/service/inspect_test.go b/integration/service/inspect_test.go index f186067366..ea3d4b8521 100644 --- a/integration/service/inspect_test.go +++ b/integration/service/inspect_test.go @@ -30,7 +30,8 @@ func TestInspect(t *testing.T) { var instances uint64 = 2 serviceSpec := fullSwarmServiceSpec("test-service-inspect"+t.Name(), instances) - resp, err := apiClient.ServiceCreate(ctx, serviceSpec, client.ServiceCreateOptions{ + resp, err := apiClient.ServiceCreate(ctx, client.ServiceCreateOptions{ + Spec: serviceSpec, QueryRegistry: false, }) assert.NilError(t, err) diff --git a/integration/service/jobs_test.go b/integration/service/jobs_test.go index 22b7e1559b..1e8af91ce7 100644 --- a/integration/service/jobs_test.go +++ b/integration/service/jobs_test.go @@ -119,8 +119,10 @@ func TestUpdateReplicatedJob(t *testing.T) { spec := result.Service.Spec spec.TaskTemplate.ForceUpdate++ - _, err = apiClient.ServiceUpdate( - ctx, id, result.Service.Version, spec, client.ServiceUpdateOptions{}, + _, err = apiClient.ServiceUpdate(ctx, id, client.ServiceUpdateOptions{ + Version: result.Service.Version, + Spec: spec, + }, ) assert.NilError(t, err) diff --git a/integration/service/list_test.go b/integration/service/list_test.go index f480b2d1c7..081b190974 100644 --- a/integration/service/list_test.go +++ b/integration/service/list_test.go @@ -43,7 +43,8 @@ func TestServiceListWithStatuses(t *testing.T) { // tasks to fail and exit. instead, we'll just pass no args, which // works. spec.TaskTemplate.ContainerSpec.Args = []string{} - resp, err := apiClient.ServiceCreate(ctx, spec, client.ServiceCreateOptions{ + resp, err := apiClient.ServiceCreate(ctx, client.ServiceCreateOptions{ + Spec: spec, QueryRegistry: false, }) assert.NilError(t, err) diff --git a/integration/service/update_test.go b/integration/service/update_test.go index b8be558f6d..8f1502bfa0 100644 --- a/integration/service/update_test.go +++ b/integration/service/update_test.go @@ -32,7 +32,10 @@ func TestServiceUpdateLabel(t *testing.T) { // add label to empty set service.Spec.Labels["foo"] = "bar" - _, err := apiClient.ServiceUpdate(ctx, serviceID, service.Version, service.Spec, client.ServiceUpdateOptions{}) + _, err := apiClient.ServiceUpdate(ctx, serviceID, client.ServiceUpdateOptions{ + Version: service.Version, + Spec: service.Spec, + }) assert.NilError(t, err) poll.WaitOn(t, serviceSpecIsUpdated(ctx, apiClient, serviceID, service.Version.Index), swarm.ServicePoll) service = getService(ctx, t, apiClient, serviceID) @@ -40,21 +43,30 @@ func TestServiceUpdateLabel(t *testing.T) { // add label to non-empty set service.Spec.Labels["foo2"] = "bar" - _, err = apiClient.ServiceUpdate(ctx, serviceID, service.Version, service.Spec, client.ServiceUpdateOptions{}) + _, err = apiClient.ServiceUpdate(ctx, serviceID, client.ServiceUpdateOptions{ + Version: service.Version, + Spec: service.Spec, + }) assert.NilError(t, err) poll.WaitOn(t, serviceSpecIsUpdated(ctx, apiClient, serviceID, service.Version.Index), swarm.ServicePoll) service = getService(ctx, t, apiClient, serviceID) assert.Check(t, is.DeepEqual(service.Spec.Labels, map[string]string{"foo": "bar", "foo2": "bar"})) delete(service.Spec.Labels, "foo2") - _, err = apiClient.ServiceUpdate(ctx, serviceID, service.Version, service.Spec, client.ServiceUpdateOptions{}) + _, err = apiClient.ServiceUpdate(ctx, serviceID, client.ServiceUpdateOptions{ + Version: service.Version, + Spec: service.Spec, + }) assert.NilError(t, err) poll.WaitOn(t, serviceSpecIsUpdated(ctx, apiClient, serviceID, service.Version.Index), swarm.ServicePoll) service = getService(ctx, t, apiClient, serviceID) assert.Check(t, is.DeepEqual(service.Spec.Labels, map[string]string{"foo": "bar"})) delete(service.Spec.Labels, "foo") - _, err = apiClient.ServiceUpdate(ctx, serviceID, service.Version, service.Spec, client.ServiceUpdateOptions{}) + _, err = apiClient.ServiceUpdate(ctx, serviceID, client.ServiceUpdateOptions{ + Version: service.Version, + Spec: service.Spec, + }) assert.NilError(t, err) poll.WaitOn(t, serviceSpecIsUpdated(ctx, apiClient, serviceID, service.Version.Index), swarm.ServicePoll) service = getService(ctx, t, apiClient, serviceID) @@ -62,7 +74,10 @@ func TestServiceUpdateLabel(t *testing.T) { // now make sure we can add again service.Spec.Labels["foo"] = "bar" - _, err = apiClient.ServiceUpdate(ctx, serviceID, service.Version, service.Spec, client.ServiceUpdateOptions{}) + _, err = apiClient.ServiceUpdate(ctx, serviceID, client.ServiceUpdateOptions{ + Version: service.Version, + Spec: service.Spec, + }) assert.NilError(t, err) poll.WaitOn(t, serviceSpecIsUpdated(ctx, apiClient, serviceID, service.Version.Index), swarm.ServicePoll) service = getService(ctx, t, apiClient, serviceID) @@ -111,7 +126,10 @@ func TestServiceUpdateSecrets(t *testing.T) { SecretName: secretName, }, ) - _, err = apiClient.ServiceUpdate(ctx, serviceID, service.Version, service.Spec, client.ServiceUpdateOptions{}) + _, err = apiClient.ServiceUpdate(ctx, serviceID, client.ServiceUpdateOptions{ + Version: service.Version, + Spec: service.Spec, + }) assert.NilError(t, err) poll.WaitOn(t, serviceIsUpdated(ctx, apiClient, serviceID), swarm.ServicePoll) @@ -126,7 +144,10 @@ func TestServiceUpdateSecrets(t *testing.T) { // remove service.Spec.TaskTemplate.ContainerSpec.Secrets = []*swarmtypes.SecretReference{} - _, err = apiClient.ServiceUpdate(ctx, serviceID, service.Version, service.Spec, client.ServiceUpdateOptions{}) + _, err = apiClient.ServiceUpdate(ctx, serviceID, client.ServiceUpdateOptions{ + Version: service.Version, + Spec: service.Spec, + }) assert.NilError(t, err) poll.WaitOn(t, serviceIsUpdated(ctx, apiClient, serviceID), swarm.ServicePoll) service = getService(ctx, t, apiClient, serviceID) @@ -175,7 +196,10 @@ func TestServiceUpdateConfigs(t *testing.T) { ConfigName: configName, }, ) - _, err = apiClient.ServiceUpdate(ctx, serviceID, service.Version, service.Spec, client.ServiceUpdateOptions{}) + _, err = apiClient.ServiceUpdate(ctx, serviceID, client.ServiceUpdateOptions{ + Version: service.Version, + Spec: service.Spec, + }) assert.NilError(t, err) poll.WaitOn(t, serviceIsUpdated(ctx, apiClient, serviceID), swarm.ServicePoll) @@ -190,7 +214,10 @@ func TestServiceUpdateConfigs(t *testing.T) { // remove service.Spec.TaskTemplate.ContainerSpec.Configs = []*swarmtypes.ConfigReference{} - _, err = apiClient.ServiceUpdate(ctx, serviceID, service.Version, service.Spec, client.ServiceUpdateOptions{}) + _, err = apiClient.ServiceUpdate(ctx, serviceID, client.ServiceUpdateOptions{ + Version: service.Version, + Spec: service.Spec, + }) assert.NilError(t, err) poll.WaitOn(t, serviceIsUpdated(ctx, apiClient, serviceID), swarm.ServicePoll) service = getService(ctx, t, apiClient, serviceID) @@ -233,7 +260,10 @@ func TestServiceUpdateNetwork(t *testing.T) { // Remove network from service service.Spec.TaskTemplate.Networks = []swarmtypes.NetworkAttachmentConfig{} - _, err = apiClient.ServiceUpdate(ctx, serviceID, service.Version, service.Spec, client.ServiceUpdateOptions{}) + _, err = apiClient.ServiceUpdate(ctx, serviceID, client.ServiceUpdateOptions{ + Version: service.Version, + Spec: service.Spec, + }) assert.NilError(t, err) poll.WaitOn(t, serviceIsUpdated(ctx, apiClient, serviceID), swarm.ServicePoll) @@ -301,22 +331,25 @@ func TestServiceUpdatePidsLimit(t *testing.T) { service.Spec.TaskTemplate.Resources.Limits = &swarmtypes.Limit{} } service.Spec.TaskTemplate.Resources.Limits.Pids = tc.pidsLimit - _, err := apiClient.ServiceUpdate(ctx, serviceID, service.Version, service.Spec, client.ServiceUpdateOptions{}) + _, err := apiClient.ServiceUpdate(ctx, serviceID, client.ServiceUpdateOptions{ + Version: service.Version, + Spec: service.Spec, + }) assert.NilError(t, err) poll.WaitOn(t, serviceIsUpdated(ctx, apiClient, serviceID), swarm.ServicePoll) } poll.WaitOn(t, swarm.RunningTasksCount(ctx, apiClient, serviceID, 1), swarm.ServicePoll) service = getService(ctx, t, apiClient, serviceID) - container := getServiceTaskContainer(ctx, t, apiClient, serviceID) + ctr := getServiceTaskContainer(ctx, t, apiClient, serviceID) assert.Equal(t, service.Spec.TaskTemplate.Resources.Limits.Pids, tc.expected) if tc.expected == 0 { - if container.HostConfig.Resources.PidsLimit != nil { + if ctr.HostConfig.Resources.PidsLimit != nil { t.Fatalf("Expected container.HostConfig.Resources.PidsLimit to be nil") } } else { - assert.Assert(t, container.HostConfig.Resources.PidsLimit != nil) - assert.Equal(t, *container.HostConfig.Resources.PidsLimit, tc.expected) + assert.Assert(t, ctr.HostConfig.Resources.PidsLimit != nil) + assert.Equal(t, *ctr.HostConfig.Resources.PidsLimit, tc.expected) } }) } diff --git a/internal/testutil/daemon/service.go b/internal/testutil/daemon/service.go index 105ec9d607..c3a6d7c188 100644 --- a/internal/testutil/daemon/service.go +++ b/internal/testutil/daemon/service.go @@ -21,6 +21,7 @@ func (d *Daemon) createServiceWithOptions(ctx context.Context, t testing.TB, opt for _, fn := range f { fn(&service) } + opts.Spec = service.Spec cli := d.NewClientT(t) defer cli.Close() @@ -28,7 +29,7 @@ func (d *Daemon) createServiceWithOptions(ctx context.Context, t testing.TB, opt ctx, cancel := context.WithTimeout(ctx, 30*time.Second) defer cancel() - res, err := cli.ServiceCreate(ctx, service.Spec, opts) + res, err := cli.ServiceCreate(ctx, opts) assert.NilError(t, err) return res.ID } @@ -78,14 +79,17 @@ func (d *Daemon) GetServiceTasksWithFilters(ctx context.Context, t testing.TB, s // UpdateService updates a swarm service with the specified service constructor func (d *Daemon) UpdateService(ctx context.Context, t testing.TB, service *swarm.Service, f ...ServiceConstructor) { t.Helper() - cli := d.NewClientT(t) - defer cli.Close() + apiClient := d.NewClientT(t) + defer apiClient.Close() for _, fn := range f { fn(service) } - _, err := cli.ServiceUpdate(ctx, service.ID, service.Version, service.Spec, client.ServiceUpdateOptions{}) + _, err := apiClient.ServiceUpdate(ctx, service.ID, client.ServiceUpdateOptions{ + Version: service.Version, + Spec: service.Spec, + }) assert.NilError(t, err) } diff --git a/internal/testutil/daemon/swarm.go b/internal/testutil/daemon/swarm.go index 557522be84..b848345e9f 100644 --- a/internal/testutil/daemon/swarm.go +++ b/internal/testutil/daemon/swarm.go @@ -205,8 +205,9 @@ func (d *Daemon) UpdateSwarm(t testing.TB, f ...SpecConstructor) { fn(&sw.Spec) } - _, err := cli.SwarmUpdate(context.Background(), sw.Version, client.SwarmUpdateOptions{ - Swarm: sw.Spec, + _, err := cli.SwarmUpdate(t.Context(), client.SwarmUpdateOptions{ + Version: sw.Version, + Spec: sw.Spec, }) assert.NilError(t, err) } @@ -220,8 +221,9 @@ func (d *Daemon) RotateTokens(t testing.TB) { result, err := cli.SwarmInspect(t.Context(), client.SwarmInspectOptions{}) assert.NilError(t, err) - _, err = cli.SwarmUpdate(t.Context(), result.Swarm.Version, client.SwarmUpdateOptions{ - Swarm: result.Swarm.Spec, + _, err = cli.SwarmUpdate(t.Context(), client.SwarmUpdateOptions{ + Version: result.Swarm.Version, + Spec: result.Swarm.Spec, RotateManagerToken: true, RotateWorkerToken: true, }) diff --git a/vendor/github.com/moby/moby/client/client_interfaces.go b/vendor/github.com/moby/moby/client/client_interfaces.go index 67f880f3b4..14d59b642e 100644 --- a/vendor/github.com/moby/moby/client/client_interfaces.go +++ b/vendor/github.com/moby/moby/client/client_interfaces.go @@ -158,11 +158,11 @@ type PluginAPIClient interface { // ServiceAPIClient defines API client methods for the services type ServiceAPIClient interface { - ServiceCreate(ctx context.Context, service swarm.ServiceSpec, options ServiceCreateOptions) (ServiceCreateResult, error) + ServiceCreate(ctx context.Context, options ServiceCreateOptions) (ServiceCreateResult, error) ServiceInspect(ctx context.Context, serviceID string, options ServiceInspectOptions) (ServiceInspectResult, error) ServiceList(ctx context.Context, options ServiceListOptions) (ServiceListResult, error) ServiceRemove(ctx context.Context, serviceID string, options ServiceRemoveOptions) (ServiceRemoveResult, error) - ServiceUpdate(ctx context.Context, serviceID string, version swarm.Version, service swarm.ServiceSpec, options ServiceUpdateOptions) (ServiceUpdateResult, error) + ServiceUpdate(ctx context.Context, serviceID string, options ServiceUpdateOptions) (ServiceUpdateResult, error) ServiceLogs(ctx context.Context, serviceID string, options ServiceLogsOptions) (ServiceLogsResult, error) TaskLogs(ctx context.Context, taskID string, options TaskLogsOptions) (TaskLogsResult, error) TaskInspect(ctx context.Context, taskID string, options TaskInspectOptions) (TaskInspectResult, error) @@ -177,7 +177,7 @@ type SwarmAPIClient interface { SwarmUnlock(ctx context.Context, options SwarmUnlockOptions) (SwarmUnlockResult, error) SwarmLeave(ctx context.Context, options SwarmLeaveOptions) (SwarmLeaveResult, error) SwarmInspect(ctx context.Context, options SwarmInspectOptions) (SwarmInspectResult, error) - SwarmUpdate(ctx context.Context, version swarm.Version, options SwarmUpdateOptions) (SwarmUpdateResult, error) + SwarmUpdate(ctx context.Context, options SwarmUpdateOptions) (SwarmUpdateResult, error) } // SystemAPIClient defines API client methods for the system diff --git a/vendor/github.com/moby/moby/client/service_create.go b/vendor/github.com/moby/moby/client/service_create.go index 9155d508a4..204d21816c 100644 --- a/vendor/github.com/moby/moby/client/service_create.go +++ b/vendor/github.com/moby/moby/client/service_create.go @@ -16,6 +16,8 @@ import ( // ServiceCreateOptions contains the options to use when creating a service. type ServiceCreateOptions struct { + Spec swarm.ServiceSpec + // EncodedRegistryAuth is the encoded registry authorization credentials to // use when updating the service. // @@ -39,33 +41,33 @@ type ServiceCreateResult struct { } // ServiceCreate creates a new service. -func (cli *Client) ServiceCreate(ctx context.Context, service swarm.ServiceSpec, options ServiceCreateOptions) (ServiceCreateResult, error) { +func (cli *Client) ServiceCreate(ctx context.Context, options ServiceCreateOptions) (ServiceCreateResult, error) { // Make sure containerSpec is not nil when no runtime is set or the runtime is set to container - if service.TaskTemplate.ContainerSpec == nil && (service.TaskTemplate.Runtime == "" || service.TaskTemplate.Runtime == swarm.RuntimeContainer) { - service.TaskTemplate.ContainerSpec = &swarm.ContainerSpec{} + if options.Spec.TaskTemplate.ContainerSpec == nil && (options.Spec.TaskTemplate.Runtime == "" || options.Spec.TaskTemplate.Runtime == swarm.RuntimeContainer) { + options.Spec.TaskTemplate.ContainerSpec = &swarm.ContainerSpec{} } - if err := validateServiceSpec(service); err != nil { + if err := validateServiceSpec(options.Spec); err != nil { return ServiceCreateResult{}, err } // ensure that the image is tagged var warnings []string switch { - case service.TaskTemplate.ContainerSpec != nil: - if taggedImg := imageWithTagString(service.TaskTemplate.ContainerSpec.Image); taggedImg != "" { - service.TaskTemplate.ContainerSpec.Image = taggedImg + case options.Spec.TaskTemplate.ContainerSpec != nil: + if taggedImg := imageWithTagString(options.Spec.TaskTemplate.ContainerSpec.Image); taggedImg != "" { + options.Spec.TaskTemplate.ContainerSpec.Image = taggedImg } if options.QueryRegistry { - resolveWarning := resolveContainerSpecImage(ctx, cli, &service.TaskTemplate, options.EncodedRegistryAuth) + resolveWarning := resolveContainerSpecImage(ctx, cli, &options.Spec.TaskTemplate, options.EncodedRegistryAuth) warnings = append(warnings, resolveWarning) } - case service.TaskTemplate.PluginSpec != nil: - if taggedImg := imageWithTagString(service.TaskTemplate.PluginSpec.Remote); taggedImg != "" { - service.TaskTemplate.PluginSpec.Remote = taggedImg + case options.Spec.TaskTemplate.PluginSpec != nil: + if taggedImg := imageWithTagString(options.Spec.TaskTemplate.PluginSpec.Remote); taggedImg != "" { + options.Spec.TaskTemplate.PluginSpec.Remote = taggedImg } if options.QueryRegistry { - resolveWarning := resolvePluginSpecRemote(ctx, cli, &service.TaskTemplate, options.EncodedRegistryAuth) + resolveWarning := resolvePluginSpecRemote(ctx, cli, &options.Spec.TaskTemplate, options.EncodedRegistryAuth) warnings = append(warnings, resolveWarning) } } @@ -74,7 +76,7 @@ func (cli *Client) ServiceCreate(ctx context.Context, service swarm.ServiceSpec, if options.EncodedRegistryAuth != "" { headers[registry.AuthHeader] = []string{options.EncodedRegistryAuth} } - resp, err := cli.post(ctx, "/services/create", nil, service, headers) + resp, err := cli.post(ctx, "/services/create", nil, options.Spec, headers) defer ensureReaderClosed(resp) if err != nil { return ServiceCreateResult{}, err diff --git a/vendor/github.com/moby/moby/client/service_update.go b/vendor/github.com/moby/moby/client/service_update.go index 3910d2bc5f..36061c9d16 100644 --- a/vendor/github.com/moby/moby/client/service_update.go +++ b/vendor/github.com/moby/moby/client/service_update.go @@ -12,6 +12,9 @@ import ( // ServiceUpdateOptions contains the options to be used for updating services. type ServiceUpdateOptions struct { + Version swarm.Version + Spec swarm.ServiceSpec + // EncodedRegistryAuth is the encoded registry authorization credentials to // use when updating the service. // @@ -50,13 +53,13 @@ type ServiceUpdateResult struct { // conflicting writes. It must be the value as set *before* the update. // You can find this value in the [swarm.Service.Meta] field, which can // be found using [Client.ServiceInspectWithRaw]. -func (cli *Client) ServiceUpdate(ctx context.Context, serviceID string, version swarm.Version, service swarm.ServiceSpec, options ServiceUpdateOptions) (ServiceUpdateResult, error) { +func (cli *Client) ServiceUpdate(ctx context.Context, serviceID string, options ServiceUpdateOptions) (ServiceUpdateResult, error) { serviceID, err := trimID("service", serviceID) if err != nil { return ServiceUpdateResult{}, err } - if err := validateServiceSpec(service); err != nil { + if err := validateServiceSpec(options.Spec); err != nil { return ServiceUpdateResult{}, err } @@ -69,25 +72,25 @@ func (cli *Client) ServiceUpdate(ctx context.Context, serviceID string, version query.Set("rollback", options.Rollback) } - query.Set("version", version.String()) + query.Set("version", options.Version.String()) // ensure that the image is tagged var warnings []string switch { - case service.TaskTemplate.ContainerSpec != nil: - if taggedImg := imageWithTagString(service.TaskTemplate.ContainerSpec.Image); taggedImg != "" { - service.TaskTemplate.ContainerSpec.Image = taggedImg + case options.Spec.TaskTemplate.ContainerSpec != nil: + if taggedImg := imageWithTagString(options.Spec.TaskTemplate.ContainerSpec.Image); taggedImg != "" { + options.Spec.TaskTemplate.ContainerSpec.Image = taggedImg } if options.QueryRegistry { - resolveWarning := resolveContainerSpecImage(ctx, cli, &service.TaskTemplate, options.EncodedRegistryAuth) + resolveWarning := resolveContainerSpecImage(ctx, cli, &options.Spec.TaskTemplate, options.EncodedRegistryAuth) warnings = append(warnings, resolveWarning) } - case service.TaskTemplate.PluginSpec != nil: - if taggedImg := imageWithTagString(service.TaskTemplate.PluginSpec.Remote); taggedImg != "" { - service.TaskTemplate.PluginSpec.Remote = taggedImg + case options.Spec.TaskTemplate.PluginSpec != nil: + if taggedImg := imageWithTagString(options.Spec.TaskTemplate.PluginSpec.Remote); taggedImg != "" { + options.Spec.TaskTemplate.PluginSpec.Remote = taggedImg } if options.QueryRegistry { - resolveWarning := resolvePluginSpecRemote(ctx, cli, &service.TaskTemplate, options.EncodedRegistryAuth) + resolveWarning := resolvePluginSpecRemote(ctx, cli, &options.Spec.TaskTemplate, options.EncodedRegistryAuth) warnings = append(warnings, resolveWarning) } } @@ -96,7 +99,7 @@ func (cli *Client) ServiceUpdate(ctx context.Context, serviceID string, version if options.EncodedRegistryAuth != "" { headers.Set(registry.AuthHeader, options.EncodedRegistryAuth) } - resp, err := cli.post(ctx, "/services/"+serviceID+"/update", query, service, headers) + resp, err := cli.post(ctx, "/services/"+serviceID+"/update", query, options.Spec, headers) defer ensureReaderClosed(resp) if err != nil { return ServiceUpdateResult{}, err diff --git a/vendor/github.com/moby/moby/client/swarm_update.go b/vendor/github.com/moby/moby/client/swarm_update.go index 8f62876a56..81f62b2c02 100644 --- a/vendor/github.com/moby/moby/client/swarm_update.go +++ b/vendor/github.com/moby/moby/client/swarm_update.go @@ -10,7 +10,8 @@ import ( // SwarmUpdateOptions contains options for updating a swarm. type SwarmUpdateOptions struct { - Swarm swarm.Spec + Version swarm.Version + Spec swarm.Spec RotateWorkerToken bool RotateManagerToken bool RotateManagerUnlockKey bool @@ -20,13 +21,13 @@ type SwarmUpdateOptions struct { type SwarmUpdateResult struct{} // SwarmUpdate updates the swarm. -func (cli *Client) SwarmUpdate(ctx context.Context, version swarm.Version, options SwarmUpdateOptions) (SwarmUpdateResult, error) { +func (cli *Client) SwarmUpdate(ctx context.Context, options SwarmUpdateOptions) (SwarmUpdateResult, error) { query := url.Values{} - query.Set("version", version.String()) + query.Set("version", options.Version.String()) query.Set("rotateWorkerToken", strconv.FormatBool(options.RotateWorkerToken)) query.Set("rotateManagerToken", strconv.FormatBool(options.RotateManagerToken)) query.Set("rotateManagerUnlockKey", strconv.FormatBool(options.RotateManagerUnlockKey)) - resp, err := cli.post(ctx, "/swarm/update", query, options.Swarm, nil) + resp, err := cli.post(ctx, "/swarm/update", query, options.Spec, nil) defer ensureReaderClosed(resp) return SwarmUpdateResult{}, err }