From 1d274e9acfe96b98be3ec956636ff4e5c70e98af Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Thu, 30 Mar 2017 17:15:54 -0700 Subject: [PATCH] Change "service inspect" to show defaults in place of empty fields This adds a new parameter insertDefaults to /services/{id}. When this is set, an empty field (such as UpdateConfig) will be populated with default values in the API response. Make "service inspect" use this, so that empty fields do not result in missing information when inspecting a service. Signed-off-by: Aaron Lehmann --- api/server/router/swarm/backend.go | 6 +++--- api/server/router/swarm/cluster_routes.go | 12 +++++++++++- api/server/router/swarm/helpers.go | 2 +- api/swagger.yaml | 5 +++++ api/types/client.go | 10 ++++++++-- cli/command/idresolver/client_test.go | 3 ++- cli/command/idresolver/idresolver.go | 3 ++- cli/command/service/inspect.go | 4 +++- cli/command/service/logs.go | 2 +- cli/command/service/progress/progress.go | 2 +- cli/command/service/scale.go | 2 +- cli/command/service/update.go | 2 +- cli/command/system/inspect.go | 4 +++- client/interface.go | 2 +- client/service_inspect.go | 9 +++++++-- client/service_inspect_test.go | 7 ++++--- daemon/cluster/helpers.go | 14 +++++++++++--- daemon/cluster/services.go | 10 +++++----- daemon/cluster/tasks.go | 2 +- integration-cli/docker_api_swarm_service_test.go | 10 ++++++++++ 20 files changed, 81 insertions(+), 30 deletions(-) diff --git a/api/server/router/swarm/backend.go b/api/server/router/swarm/backend.go index 28b9a98018..3a5da97d2c 100644 --- a/api/server/router/swarm/backend.go +++ b/api/server/router/swarm/backend.go @@ -17,7 +17,7 @@ type Backend interface { GetUnlockKey() (string, error) UnlockSwarm(req types.UnlockRequest) error GetServices(basictypes.ServiceListOptions) ([]types.Service, error) - GetService(string) (types.Service, error) + GetService(idOrName string, insertDefaults bool) (types.Service, error) CreateService(types.ServiceSpec, string) (*basictypes.ServiceCreateResponse, error) UpdateService(string, uint64, types.ServiceSpec, basictypes.ServiceUpdateOptions) (*basictypes.ServiceUpdateResponse, error) RemoveService(string) error @@ -30,7 +30,7 @@ type Backend interface { GetTask(string) (types.Task, error) GetSecrets(opts basictypes.SecretListOptions) ([]types.Secret, error) CreateSecret(s types.SecretSpec) (string, error) - RemoveSecret(id string) error + RemoveSecret(idOrName string) error GetSecret(id string) (types.Secret, error) - UpdateSecret(id string, version uint64, spec types.SecretSpec) error + UpdateSecret(idOrName string, version uint64, spec types.SecretSpec) error } diff --git a/api/server/router/swarm/cluster_routes.go b/api/server/router/swarm/cluster_routes.go index dfae13f1dd..4c60b6b6ee 100644 --- a/api/server/router/swarm/cluster_routes.go +++ b/api/server/router/swarm/cluster_routes.go @@ -151,7 +151,17 @@ func (sr *swarmRouter) getServices(ctx context.Context, w http.ResponseWriter, r } func (sr *swarmRouter) getService(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { - service, err := sr.backend.GetService(vars["id"]) + var insertDefaults bool + if value := r.URL.Query().Get("insertDefaults"); value != "" { + var err error + insertDefaults, err = strconv.ParseBool(value) + if err != nil { + err := fmt.Errorf("invalid value for insertDefaults: %s", value) + return errors.NewBadRequestError(err) + } + } + + service, err := sr.backend.GetService(vars["id"], insertDefaults) if err != nil { logrus.Errorf("Error getting service %s: %v", vars["id"], err) return err diff --git a/api/server/router/swarm/helpers.go b/api/server/router/swarm/helpers.go index af745b84c3..ea692ea368 100644 --- a/api/server/router/swarm/helpers.go +++ b/api/server/router/swarm/helpers.go @@ -39,7 +39,7 @@ func (sr *swarmRouter) swarmLogs(ctx context.Context, w http.ResponseWriter, r * // checking for whether logs are TTY involves iterating over every service // and task. idk if there is a better way for _, service := range selector.Services { - s, err := sr.backend.GetService(service) + s, err := sr.backend.GetService(service, false) if err != nil { // maybe should return some context with this error? return err diff --git a/api/swagger.yaml b/api/swagger.yaml index 2154e0cff3..0580e19493 100644 --- a/api/swagger.yaml +++ b/api/swagger.yaml @@ -7584,6 +7584,11 @@ paths: description: "ID or name of service." required: true type: "string" + - name: "insertDefaults" + in: "query" + description: "Fill empty fields with default values." + type: "boolean" + default: false tags: ["Service"] delete: summary: "Delete a service" diff --git a/api/types/client.go b/api/types/client.go index 56ec211293..3d916bb262 100644 --- a/api/types/client.go +++ b/api/types/client.go @@ -315,12 +315,18 @@ type ServiceUpdateOptions struct { Rollback string } -// ServiceListOptions holds parameters to list services with. +// ServiceListOptions holds parameters to list services with. type ServiceListOptions struct { Filters filters.Args } -// TaskListOptions holds parameters to list tasks with. +// ServiceInspectOptions holds parameters related to the "service inspect" +// operation. +type ServiceInspectOptions struct { + InsertDefaults bool +} + +// TaskListOptions holds parameters to list tasks with. type TaskListOptions struct { Filters filters.Args } diff --git a/cli/command/idresolver/client_test.go b/cli/command/idresolver/client_test.go index 8c02d7ebcf..f84683b907 100644 --- a/cli/command/idresolver/client_test.go +++ b/cli/command/idresolver/client_test.go @@ -1,6 +1,7 @@ package idresolver import ( + "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/swarm" "github.com/docker/docker/client" "golang.org/x/net/context" @@ -19,7 +20,7 @@ func (cli *fakeClient) NodeInspectWithRaw(ctx context.Context, nodeID string) (s return swarm.Node{}, []byte{}, nil } -func (cli *fakeClient) ServiceInspectWithRaw(ctx context.Context, serviceID string) (swarm.Service, []byte, error) { +func (cli *fakeClient) ServiceInspectWithRaw(ctx context.Context, serviceID string, options types.ServiceInspectOptions) (swarm.Service, []byte, error) { if cli.serviceInspectFunc != nil { return cli.serviceInspectFunc(serviceID) } diff --git a/cli/command/idresolver/idresolver.go b/cli/command/idresolver/idresolver.go index 25c51a27eb..6088b64b59 100644 --- a/cli/command/idresolver/idresolver.go +++ b/cli/command/idresolver/idresolver.go @@ -3,6 +3,7 @@ package idresolver import ( "golang.org/x/net/context" + "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/swarm" "github.com/docker/docker/client" "github.com/pkg/errors" @@ -39,7 +40,7 @@ func (r *IDResolver) get(ctx context.Context, t interface{}, id string) (string, } return id, nil case swarm.Service: - service, _, err := r.client.ServiceInspectWithRaw(ctx, id) + service, _, err := r.client.ServiceInspectWithRaw(ctx, id, types.ServiceInspectOptions{}) if err != nil { return id, nil } diff --git a/cli/command/service/inspect.go b/cli/command/service/inspect.go index 8a8b51cd0e..fae24eeaf1 100644 --- a/cli/command/service/inspect.go +++ b/cli/command/service/inspect.go @@ -5,6 +5,7 @@ import ( "golang.org/x/net/context" + "github.com/docker/docker/api/types" "github.com/docker/docker/cli" "github.com/docker/docker/cli/command" "github.com/docker/docker/cli/command/formatter" @@ -51,7 +52,8 @@ func runInspect(dockerCli *command.DockerCli, opts inspectOptions) error { } getRef := func(ref string) (interface{}, []byte, error) { - service, _, err := client.ServiceInspectWithRaw(ctx, ref) + // Service inspect shows defaults values in empty fields. + service, _, err := client.ServiceInspectWithRaw(ctx, ref, types.ServiceInspectOptions{InsertDefaults: true}) if err == nil || !apiclient.IsErrServiceNotFound(err) { return service, nil, err } diff --git a/cli/command/service/logs.go b/cli/command/service/logs.go index cfcb7ed105..6dcaa118cf 100644 --- a/cli/command/service/logs.go +++ b/cli/command/service/logs.go @@ -84,7 +84,7 @@ func runLogs(dockerCli *command.DockerCli, opts *logsOptions) error { tty bool ) - service, _, err := cli.ServiceInspectWithRaw(ctx, opts.target) + service, _, err := cli.ServiceInspectWithRaw(ctx, opts.target, types.ServiceInspectOptions{}) if err != nil { // if it's any error other than service not found, it's Real if !client.IsErrServiceNotFound(err) { diff --git a/cli/command/service/progress/progress.go b/cli/command/service/progress/progress.go index ccc7e60cfc..bfeaa314a4 100644 --- a/cli/command/service/progress/progress.go +++ b/cli/command/service/progress/progress.go @@ -85,7 +85,7 @@ func ServiceProgress(ctx context.Context, client client.APIClient, serviceID str ) for { - service, _, err := client.ServiceInspectWithRaw(ctx, serviceID) + service, _, err := client.ServiceInspectWithRaw(ctx, serviceID, types.ServiceInspectOptions{}) if err != nil { return err } diff --git a/cli/command/service/scale.go b/cli/command/service/scale.go index ed76c862fe..98163c87c9 100644 --- a/cli/command/service/scale.go +++ b/cli/command/service/scale.go @@ -71,7 +71,7 @@ func runServiceScale(dockerCli *command.DockerCli, serviceID string, scale uint6 client := dockerCli.Client() ctx := context.Background() - service, _, err := client.ServiceInspectWithRaw(ctx, serviceID) + service, _, err := client.ServiceInspectWithRaw(ctx, serviceID, types.ServiceInspectOptions{}) if err != nil { return err } diff --git a/cli/command/service/update.go b/cli/command/service/update.go index b59f163829..3a6bc58d0e 100644 --- a/cli/command/service/update.go +++ b/cli/command/service/update.go @@ -101,7 +101,7 @@ func runUpdate(dockerCli *command.DockerCli, flags *pflag.FlagSet, opts *service apiClient := dockerCli.Client() ctx := context.Background() - service, _, err := apiClient.ServiceInspectWithRaw(ctx, serviceID) + service, _, err := apiClient.ServiceInspectWithRaw(ctx, serviceID, types.ServiceInspectOptions{}) if err != nil { return err } diff --git a/cli/command/system/inspect.go b/cli/command/system/inspect.go index 361902a9e7..ad23d35a09 100644 --- a/cli/command/system/inspect.go +++ b/cli/command/system/inspect.go @@ -4,6 +4,7 @@ import ( "fmt" "strings" + "github.com/docker/docker/api/types" "github.com/docker/docker/cli" "github.com/docker/docker/cli/command" "github.com/docker/docker/cli/command/inspect" @@ -79,7 +80,8 @@ func inspectNode(ctx context.Context, dockerCli *command.DockerCli) inspect.GetR func inspectService(ctx context.Context, dockerCli *command.DockerCli) inspect.GetRefFunc { return func(ref string) (interface{}, []byte, error) { - return dockerCli.Client().ServiceInspectWithRaw(ctx, ref) + // Service inspect shows defaults values in empty fields. + return dockerCli.Client().ServiceInspectWithRaw(ctx, ref, types.ServiceInspectOptions{InsertDefaults: true}) } } diff --git a/client/interface.go b/client/interface.go index 6f8c094b31..8dbe4300dc 100644 --- a/client/interface.go +++ b/client/interface.go @@ -123,7 +123,7 @@ type PluginAPIClient interface { // ServiceAPIClient defines API client methods for the services type ServiceAPIClient interface { ServiceCreate(ctx context.Context, service swarm.ServiceSpec, options types.ServiceCreateOptions) (types.ServiceCreateResponse, error) - ServiceInspectWithRaw(ctx context.Context, serviceID string) (swarm.Service, []byte, error) + ServiceInspectWithRaw(ctx context.Context, serviceID string, options types.ServiceInspectOptions) (swarm.Service, []byte, error) ServiceList(ctx context.Context, options types.ServiceListOptions) ([]swarm.Service, error) ServiceRemove(ctx context.Context, serviceID string) error ServiceUpdate(ctx context.Context, serviceID string, version swarm.Version, service swarm.ServiceSpec, options types.ServiceUpdateOptions) (types.ServiceUpdateResponse, error) diff --git a/client/service_inspect.go b/client/service_inspect.go index ca71cbde1a..d7e051e3a4 100644 --- a/client/service_inspect.go +++ b/client/service_inspect.go @@ -3,16 +3,21 @@ package client import ( "bytes" "encoding/json" + "fmt" "io/ioutil" "net/http" + "net/url" + "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/swarm" "golang.org/x/net/context" ) // ServiceInspectWithRaw returns the service information and the raw data. -func (cli *Client) ServiceInspectWithRaw(ctx context.Context, serviceID string) (swarm.Service, []byte, error) { - serverResp, err := cli.get(ctx, "/services/"+serviceID, nil, nil) +func (cli *Client) ServiceInspectWithRaw(ctx context.Context, serviceID string, opts types.ServiceInspectOptions) (swarm.Service, []byte, error) { + query := url.Values{} + query.Set("insertDefaults", fmt.Sprintf("%v", opts.InsertDefaults)) + serverResp, err := cli.get(ctx, "/services/"+serviceID, query, nil) if err != nil { if serverResp.statusCode == http.StatusNotFound { return swarm.Service{}, nil, serviceNotFoundError{serviceID} diff --git a/client/service_inspect_test.go b/client/service_inspect_test.go index 0346847317..d53f583e90 100644 --- a/client/service_inspect_test.go +++ b/client/service_inspect_test.go @@ -9,6 +9,7 @@ import ( "strings" "testing" + "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/swarm" "golang.org/x/net/context" ) @@ -18,7 +19,7 @@ func TestServiceInspectError(t *testing.T) { client: newMockClient(errorMock(http.StatusInternalServerError, "Server error")), } - _, _, err := client.ServiceInspectWithRaw(context.Background(), "nothing") + _, _, err := client.ServiceInspectWithRaw(context.Background(), "nothing", types.ServiceInspectOptions{}) if err == nil || err.Error() != "Error response from daemon: Server error" { t.Fatalf("expected a Server Error, got %v", err) } @@ -29,7 +30,7 @@ func TestServiceInspectServiceNotFound(t *testing.T) { client: newMockClient(errorMock(http.StatusNotFound, "Server error")), } - _, _, err := client.ServiceInspectWithRaw(context.Background(), "unknown") + _, _, err := client.ServiceInspectWithRaw(context.Background(), "unknown", types.ServiceInspectOptions{}) if err == nil || !IsErrServiceNotFound(err) { t.Fatalf("expected a serviceNotFoundError error, got %v", err) } @@ -55,7 +56,7 @@ func TestServiceInspect(t *testing.T) { }), } - serviceInspect, _, err := client.ServiceInspectWithRaw(context.Background(), "service_id") + serviceInspect, _, err := client.ServiceInspectWithRaw(context.Background(), "service_id", types.ServiceInspectOptions{}) if err != nil { t.Fatal(err) } diff --git a/daemon/cluster/helpers.go b/daemon/cluster/helpers.go index 6523a80e1c..98c7cc5472 100644 --- a/daemon/cluster/helpers.go +++ b/daemon/cluster/helpers.go @@ -58,9 +58,9 @@ func getNode(ctx context.Context, c swarmapi.ControlClient, input string) (*swar return rl.Nodes[0], nil } -func getService(ctx context.Context, c swarmapi.ControlClient, input string) (*swarmapi.Service, error) { +func getService(ctx context.Context, c swarmapi.ControlClient, input string, insertDefaults bool) (*swarmapi.Service, error) { // GetService to match via full ID. - if rg, err := c.GetService(ctx, &swarmapi.GetServiceRequest{ServiceID: input}); err == nil { + if rg, err := c.GetService(ctx, &swarmapi.GetServiceRequest{ServiceID: input, InsertDefaults: insertDefaults}); err == nil { return rg.Service, nil } @@ -91,7 +91,15 @@ func getService(ctx context.Context, c swarmapi.ControlClient, input string) (*s return nil, fmt.Errorf("service %s is ambiguous (%d matches found)", input, l) } - return rl.Services[0], nil + if !insertDefaults { + return rl.Services[0], nil + } + + rg, err := c.GetService(ctx, &swarmapi.GetServiceRequest{ServiceID: rl.Services[0].ID, InsertDefaults: true}) + if err == nil { + return rg.Service, nil + } + return nil, err } func getTask(ctx context.Context, c swarmapi.ControlClient, input string) (*swarmapi.Task, error) { diff --git a/daemon/cluster/services.go b/daemon/cluster/services.go index 8fd730eee7..8d5d4a5edd 100644 --- a/daemon/cluster/services.go +++ b/daemon/cluster/services.go @@ -87,10 +87,10 @@ func (c *Cluster) GetServices(options apitypes.ServiceListOptions) ([]types.Serv } // GetService returns a service based on an ID or name. -func (c *Cluster) GetService(input string) (types.Service, error) { +func (c *Cluster) GetService(input string, insertDefaults bool) (types.Service, error) { var service *swarmapi.Service if err := c.lockedManagerAction(func(ctx context.Context, state nodeState) error { - s, err := getService(ctx, state.controlClient, input) + s, err := getService(ctx, state.controlClient, input, insertDefaults) if err != nil { return err } @@ -187,7 +187,7 @@ func (c *Cluster) UpdateService(serviceIDOrName string, version uint64, spec typ return apierrors.NewBadRequestError(err) } - currentService, err := getService(ctx, state.controlClient, serviceIDOrName) + currentService, err := getService(ctx, state.controlClient, serviceIDOrName, false) if err != nil { return err } @@ -289,7 +289,7 @@ func (c *Cluster) UpdateService(serviceIDOrName string, version uint64, spec typ // RemoveService removes a service from a managed swarm cluster. func (c *Cluster) RemoveService(input string) error { return c.lockedManagerAction(func(ctx context.Context, state nodeState) error { - service, err := getService(ctx, state.controlClient, input) + service, err := getService(ctx, state.controlClient, input, false) if err != nil { return err } @@ -442,7 +442,7 @@ func convertSelector(ctx context.Context, cc swarmapi.ControlClient, selector *b // don't rely on swarmkit to resolve IDs, do it ourselves swarmSelector := &swarmapi.LogSelector{} for _, s := range selector.Services { - service, err := getService(ctx, cc, s) + service, err := getService(ctx, cc, s, false) if err != nil { return nil, err } diff --git a/daemon/cluster/tasks.go b/daemon/cluster/tasks.go index 001a345a68..6a6c59ffe5 100644 --- a/daemon/cluster/tasks.go +++ b/daemon/cluster/tasks.go @@ -23,7 +23,7 @@ func (c *Cluster) GetTasks(options apitypes.TaskListOptions) ([]types.Task, erro if filter.Include("service") { serviceFilters := filter.Get("service") for _, serviceFilter := range serviceFilters { - service, err := c.GetService(serviceFilter) + service, err := c.GetService(serviceFilter, false) if err != nil { return err } diff --git a/integration-cli/docker_api_swarm_service_test.go b/integration-cli/docker_api_swarm_service_test.go index a96f684965..6a3c9f170b 100644 --- a/integration-cli/docker_api_swarm_service_test.go +++ b/integration-cli/docker_api_swarm_service_test.go @@ -60,6 +60,16 @@ func (s *DockerSwarmSuite) TestAPISwarmServicesCreate(c *check.C) { id := d.CreateService(c, simpleTestService, setInstances(instances)) waitAndAssert(c, defaultReconciliationTimeout, d.CheckActiveContainerCount, checker.Equals, instances) + // insertDefaults inserts UpdateConfig when service is fetched by ID + _, out, err := d.SockRequest("GET", "/services/"+id+"?insertDefaults=true", nil) + c.Assert(err, checker.IsNil, check.Commentf("%s", out)) + c.Assert(string(out), checker.Contains, "UpdateConfig") + + // insertDefaults inserts UpdateConfig when service is fetched by ID + _, out, err = d.SockRequest("GET", "/services/top?insertDefaults=true", nil) + c.Assert(err, checker.IsNil, check.Commentf("%s", out)) + c.Assert(string(out), checker.Contains, "UpdateConfig") + service := d.GetService(c, id) instances = 5 d.UpdateService(c, service, setInstances(instances))