From f9bd8ec8b268581f93095c5a80679f0a8ff498bf Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Thu, 16 Feb 2017 09:27:01 -0800 Subject: [PATCH] Implement server-side rollback, for daemon versions that support this Server-side rollback can take advantage of the rollback-specific update parameters, instead of being treated as a normal update that happens to go back to a previous version of the spec. Signed-off-by: Aaron Lehmann --- api/server/router/swarm/backend.go | 2 +- api/server/router/swarm/cluster_routes.go | 10 +++-- api/swagger.yaml | 6 +++ api/types/client.go | 6 +++ cli/command/service/update.go | 43 ++++++++++++++++--- client/service_update.go | 4 ++ daemon/cluster/convert/service.go | 2 +- daemon/cluster/services.go | 16 ++++++- .../docker_api_swarm_service_test.go | 11 ++--- integration-cli/docker_api_swarm_test.go | 5 +++ 10 files changed, 85 insertions(+), 20 deletions(-) diff --git a/api/server/router/swarm/backend.go b/api/server/router/swarm/backend.go index 1a2b876b23..14c4c1cad5 100644 --- a/api/server/router/swarm/backend.go +++ b/api/server/router/swarm/backend.go @@ -19,7 +19,7 @@ type Backend interface { GetServices(basictypes.ServiceListOptions) ([]types.Service, error) GetService(string) (types.Service, error) CreateService(types.ServiceSpec, string) (*basictypes.ServiceCreateResponse, error) - UpdateService(string, uint64, types.ServiceSpec, string, string) (*basictypes.ServiceUpdateResponse, error) + UpdateService(string, uint64, types.ServiceSpec, basictypes.ServiceUpdateOptions) (*basictypes.ServiceUpdateResponse, error) RemoveService(string) error ServiceLogs(context.Context, string, *backend.ContainerLogsConfig, chan struct{}) error GetNodes(basictypes.NodeListOptions) ([]types.Node, error) diff --git a/api/server/router/swarm/cluster_routes.go b/api/server/router/swarm/cluster_routes.go index 076d4a4452..31a230e9af 100644 --- a/api/server/router/swarm/cluster_routes.go +++ b/api/server/router/swarm/cluster_routes.go @@ -192,12 +192,14 @@ func (sr *swarmRouter) updateService(ctx context.Context, w http.ResponseWriter, return errors.NewBadRequestError(err) } + var flags basictypes.ServiceUpdateOptions + // Get returns "" if the header does not exist - encodedAuth := r.Header.Get("X-Registry-Auth") + flags.EncodedRegistryAuth = r.Header.Get("X-Registry-Auth") + flags.RegistryAuthFrom = r.URL.Query().Get("registryAuthFrom") + flags.Rollback = r.URL.Query().Get("rollback") - registryAuthFrom := r.URL.Query().Get("registryAuthFrom") - - resp, err := sr.backend.UpdateService(vars["id"], version, service, encodedAuth, registryAuthFrom) + resp, err := sr.backend.UpdateService(vars["id"], version, service, flags) if err != nil { logrus.Errorf("Error updating service %s: %v", vars["id"], err) return err diff --git a/api/swagger.yaml b/api/swagger.yaml index 51d7f24068..568379f29d 100644 --- a/api/swagger.yaml +++ b/api/swagger.yaml @@ -7631,6 +7631,12 @@ paths: parameter indicates where to find registry authorization credentials. The valid values are `spec` and `previous-spec`." default: "spec" + - name: "rollback" + in: "query" + type: "string" + description: "Set to this parameter to `previous` to cause a + server-side rollback to the previous service spec. The supplied spec will be + ignored in this case." - name: "X-Registry-Auth" in: "header" description: "A base64-encoded auth configuration for pulling from private registries. [See the authentication section for details.](#section/Authentication)" diff --git a/api/types/client.go b/api/types/client.go index 0fccea081d..ff0e46b6a4 100644 --- a/api/types/client.go +++ b/api/types/client.go @@ -320,6 +320,12 @@ type ServiceUpdateOptions struct { // credentials if they are not given in EncodedRegistryAuth. Valid // values are "spec" and "previous-spec". RegistryAuthFrom string + + // Rollback indicates whether a server-side rollback should be + // performed. When this is set, the provided spec will be ignored. + // The valid values are "previous" and "none". An empty value is the + // same as "none". + Rollback string } // ServiceListOptions holds parameters to list services with. diff --git a/cli/command/service/update.go b/cli/command/service/update.go index b529331500..ab8391e038 100644 --- a/cli/command/service/update.go +++ b/cli/command/service/update.go @@ -1,6 +1,7 @@ package service import ( + "errors" "fmt" "sort" "strings" @@ -10,6 +11,7 @@ import ( "github.com/docker/docker/api/types/container" mounttypes "github.com/docker/docker/api/types/mount" "github.com/docker/docker/api/types/swarm" + "github.com/docker/docker/api/types/versions" "github.com/docker/docker/cli" "github.com/docker/docker/cli/command" "github.com/docker/docker/client" @@ -95,7 +97,6 @@ func newListOptsVar() *opts.ListOpts { func runUpdate(dockerCli *command.DockerCli, flags *pflag.FlagSet, serviceID string) error { apiClient := dockerCli.Client() ctx := context.Background() - updateOpts := types.ServiceUpdateOptions{} service, _, err := apiClient.ServiceInspectWithRaw(ctx, serviceID) if err != nil { @@ -107,12 +108,44 @@ func runUpdate(dockerCli *command.DockerCli, flags *pflag.FlagSet, serviceID str return err } + // There are two ways to do user-requested rollback. The old way is + // client-side, but with a sufficiently recent daemon we prefer + // server-side, because it will honor the rollback parameters. + var ( + clientSideRollback bool + serverSideRollback bool + ) + spec := &service.Spec if rollback { - spec = service.PreviousSpec - if spec == nil { - return fmt.Errorf("service does not have a previous specification to roll back to") + // Rollback can't be combined with other flags. + otherFlagsPassed := false + flags.VisitAll(func(f *pflag.Flag) { + if f.Name == "rollback" { + return + } + if flags.Changed(f.Name) { + otherFlagsPassed = true + } + }) + if otherFlagsPassed { + return errors.New("other flags may not be combined with --rollback") } + + if versions.LessThan(dockerCli.Client().ClientVersion(), "1.27") { + clientSideRollback = true + spec = service.PreviousSpec + if spec == nil { + return fmt.Errorf("service does not have a previous specification to roll back to") + } + } else { + serverSideRollback = true + } + } + + updateOpts := types.ServiceUpdateOptions{} + if serverSideRollback { + updateOpts.Rollback = "previous" } err = updateService(flags, spec) @@ -147,7 +180,7 @@ func runUpdate(dockerCli *command.DockerCli, flags *pflag.FlagSet, serviceID str return err } updateOpts.EncodedRegistryAuth = encodedAuth - } else if rollback { + } else if clientSideRollback { updateOpts.RegistryAuthFrom = types.RegistryAuthFromPreviousSpec } else { updateOpts.RegistryAuthFrom = types.RegistryAuthFromSpec diff --git a/client/service_update.go b/client/service_update.go index afa94d47e2..873a1e0556 100644 --- a/client/service_update.go +++ b/client/service_update.go @@ -27,6 +27,10 @@ func (cli *Client) ServiceUpdate(ctx context.Context, serviceID string, version query.Set("registryAuthFrom", options.RegistryAuthFrom) } + if options.Rollback != "" { + query.Set("rollback", options.Rollback) + } + query.Set("version", strconv.FormatUint(version.Index, 10)) var response types.ServiceUpdateResponse diff --git a/daemon/cluster/convert/service.go b/daemon/cluster/convert/service.go index 779f102084..0d1fad6a59 100644 --- a/daemon/cluster/convert/service.go +++ b/daemon/cluster/convert/service.go @@ -176,7 +176,7 @@ func ServiceSpecToGRPC(s types.ServiceSpec) (swarmapi.ServiceSpec, error) { } spec.Rollback, err = updateConfigToGRPC(s.RollbackConfig) if err != nil { - return swarmapi.Servicepec{}, err + return swarmapi.ServiceSpec{}, err } if s.EndpointSpec != nil { diff --git a/daemon/cluster/services.go b/daemon/cluster/services.go index e8bb9234c0..ec5a58c02d 100644 --- a/daemon/cluster/services.go +++ b/daemon/cluster/services.go @@ -132,7 +132,7 @@ func (c *Cluster) CreateService(s types.ServiceSpec, encodedAuth string) (*apity } // UpdateService updates existing service to match new properties. -func (c *Cluster) UpdateService(serviceIDOrName string, version uint64, spec types.ServiceSpec, encodedAuth string, registryAuthFrom string) (*apitypes.ServiceUpdateResponse, error) { +func (c *Cluster) UpdateService(serviceIDOrName string, version uint64, spec types.ServiceSpec, flags apitypes.ServiceUpdateOptions) (*apitypes.ServiceUpdateResponse, error) { var resp *apitypes.ServiceUpdateResponse err := c.lockedManagerAction(func(ctx context.Context, state nodeState) error { @@ -157,13 +157,14 @@ func (c *Cluster) UpdateService(serviceIDOrName string, version uint64, spec typ return errors.New("service does not use container tasks") } + encodedAuth := flags.EncodedRegistryAuth if encodedAuth != "" { newCtnr.PullOptions = &swarmapi.ContainerSpec_PullOptions{RegistryAuth: encodedAuth} } else { // this is needed because if the encodedAuth isn't being updated then we // shouldn't lose it, and continue to use the one that was already present var ctnr *swarmapi.ContainerSpec - switch registryAuthFrom { + switch flags.RegistryAuthFrom { case apitypes.RegistryAuthFromSpec, "": ctnr = currentService.Spec.Task.GetContainer() case apitypes.RegistryAuthFromPreviousSpec: @@ -208,6 +209,16 @@ func (c *Cluster) UpdateService(serviceIDOrName string, version uint64, spec typ } } + var rollback swarmapi.UpdateServiceRequest_Rollback + switch flags.Rollback { + case "", "none": + rollback = swarmapi.UpdateServiceRequest_NONE + case "previous": + rollback = swarmapi.UpdateServiceRequest_PREVIOUS + default: + return fmt.Errorf("unrecognized rollback option %s", flags.Rollback) + } + _, err = state.controlClient.UpdateService( ctx, &swarmapi.UpdateServiceRequest{ @@ -216,6 +227,7 @@ func (c *Cluster) UpdateService(serviceIDOrName string, version uint64, spec typ ServiceVersion: &swarmapi.Version{ Index: version, }, + Rollback: rollback, }, ) return err diff --git a/integration-cli/docker_api_swarm_service_test.go b/integration-cli/docker_api_swarm_service_test.go index 1ef1a84cde..9213eae75b 100644 --- a/integration-cli/docker_api_swarm_service_test.go +++ b/integration-cli/docker_api_swarm_service_test.go @@ -138,6 +138,7 @@ func (s *DockerSwarmSuite) TestAPISwarmServicesUpdate(c *check.C) { // create service instances := 5 parallelism := 2 + rollbackParallelism := 3 id := daemons[0].CreateService(c, serviceForUpdate, setInstances(instances)) // wait for tasks ready @@ -161,19 +162,15 @@ func (s *DockerSwarmSuite) TestAPISwarmServicesUpdate(c *check.C) { map[string]int{image2: instances}) // Roll back to the previous version. This uses the CLI because - // rollback is a client-side operation. + // rollback used to be a client-side operation. out, err := daemons[0].Cmd("service", "update", "--rollback", id) c.Assert(err, checker.IsNil, check.Commentf(out)) // first batch waitAndAssert(c, defaultReconciliationTimeout, daemons[0].CheckRunningTaskImages, checker.DeepEquals, - map[string]int{image2: instances - parallelism, image1: parallelism}) + map[string]int{image2: instances - rollbackParallelism, image1: rollbackParallelism}) // 2nd batch - waitAndAssert(c, defaultReconciliationTimeout, daemons[0].CheckRunningTaskImages, checker.DeepEquals, - map[string]int{image2: instances - 2*parallelism, image1: 2 * parallelism}) - - // 3nd batch waitAndAssert(c, defaultReconciliationTimeout, daemons[0].CheckRunningTaskImages, checker.DeepEquals, map[string]int{image1: instances}) } @@ -210,7 +207,7 @@ func (s *DockerSwarmSuite) TestAPISwarmServicesFailedUpdate(c *check.C) { c.Assert(v, checker.Equals, instances-2) // Roll back to the previous version. This uses the CLI because - // rollback is a client-side operation. + // rollback used to be a client-side operation. out, err := daemons[0].Cmd("service", "update", "--rollback", id) c.Assert(err, checker.IsNil, check.Commentf(out)) diff --git a/integration-cli/docker_api_swarm_test.go b/integration-cli/docker_api_swarm_test.go index f96326970d..0c25b1b7d2 100644 --- a/integration-cli/docker_api_swarm_test.go +++ b/integration-cli/docker_api_swarm_test.go @@ -556,6 +556,11 @@ func serviceForUpdate(s *swarm.Service) { Delay: 4 * time.Second, FailureAction: swarm.UpdateFailureActionContinue, }, + RollbackConfig: &swarm.UpdateConfig{ + Parallelism: 3, + Delay: 4 * time.Second, + FailureAction: swarm.UpdateFailureActionContinue, + }, } s.Spec.Name = "updatetest" }