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 <aaron.lehmann@docker.com>

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
 }

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

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

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"

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
 }

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)
 	}

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
 		}

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
 		}

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) {

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
 		}

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
 	}

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
 	}

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})
 	}
 }
 

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)

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) {
+func (cli *Client) ServiceInspectWithRaw(ctx context.Context, serviceID string, opts types.ServiceInspectOptions) (swarm.Service, []byte, error) {
-	serverResp, err := cli.get(ctx, "/services/"+serviceID, nil, nil)
+	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}

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)
 	}

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) {

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
 		}

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
 				}

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))