diff --git a/components/cli/cli/command/service/ps.go b/components/cli/cli/command/service/ps.go index 87b29d2043..741f6b589f 100644 --- a/components/cli/cli/command/service/ps.go +++ b/components/cli/cli/command/service/ps.go @@ -12,6 +12,7 @@ import ( "github.com/docker/cli/opts" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/filters" + "github.com/docker/docker/client" "github.com/pkg/errors" "github.com/spf13/cobra" "golang.org/x/net/context" @@ -52,57 +53,10 @@ func runPS(dockerCli command.Cli, options psOptions) error { client := dockerCli.Client() ctx := context.Background() - filter := options.filter.Value() - - serviceIDFilter := filters.NewArgs() - serviceNameFilter := filters.NewArgs() - for _, service := range options.services { - serviceIDFilter.Add("id", service) - serviceNameFilter.Add("name", service) - } - serviceByIDList, err := client.ServiceList(ctx, types.ServiceListOptions{Filters: serviceIDFilter}) + filter, notfound, err := createFilter(ctx, client, options) if err != nil { return err } - serviceByNameList, err := client.ServiceList(ctx, types.ServiceListOptions{Filters: serviceNameFilter}) - if err != nil { - return err - } - - for _, service := range options.services { - serviceCount := 0 - // Lookup by ID/Prefix - for _, serviceEntry := range serviceByIDList { - if strings.HasPrefix(serviceEntry.ID, service) { - filter.Add("service", serviceEntry.ID) - serviceCount++ - } - } - - // Lookup by Name/Prefix - for _, serviceEntry := range serviceByNameList { - if strings.HasPrefix(serviceEntry.Spec.Annotations.Name, service) { - filter.Add("service", serviceEntry.ID) - serviceCount++ - } - } - // If nothing has been found, return immediately. - if serviceCount == 0 { - return errors.Errorf("no such services: %s", service) - } - } - - if filter.Include("node") { - nodeFilters := filter.Get("node") - for _, nodeFilter := range nodeFilters { - nodeReference, err := node.Reference(ctx, client, nodeFilter) - if err != nil { - return err - } - filter.Del("node", nodeFilter) - filter.Add("node", nodeReference) - } - } tasks, err := client.TaskList(ctx, types.TaskListOptions{Filters: filter}) if err != nil { @@ -117,6 +71,80 @@ func runPS(dockerCli command.Cli, options psOptions) error { format = formatter.TableFormatKey } } - - return task.Print(ctx, dockerCli, tasks, idresolver.New(client, options.noResolve), !options.noTrunc, options.quiet, format) + if err := task.Print(ctx, dockerCli, tasks, idresolver.New(client, options.noResolve), !options.noTrunc, options.quiet, format); err != nil { + return err + } + if len(notfound) != 0 { + return errors.New(strings.Join(notfound, "\n")) + } + return nil +} + +func createFilter(ctx context.Context, client client.APIClient, options psOptions) (filters.Args, []string, error) { + filter := options.filter.Value() + + serviceIDFilter := filters.NewArgs() + serviceNameFilter := filters.NewArgs() + for _, service := range options.services { + serviceIDFilter.Add("id", service) + serviceNameFilter.Add("name", service) + } + serviceByIDList, err := client.ServiceList(ctx, types.ServiceListOptions{Filters: serviceIDFilter}) + if err != nil { + return filter, nil, err + } + serviceByNameList, err := client.ServiceList(ctx, types.ServiceListOptions{Filters: serviceNameFilter}) + if err != nil { + return filter, nil, err + } + + var notfound []string + serviceCount := 0 +loop: + // Match services by 1. Full ID, 2. Full name, 3. ID prefix. An error is returned if the ID-prefix match is ambiguous + for _, service := range options.services { + for _, s := range serviceByIDList { + if s.ID == service { + filter.Add("service", s.ID) + serviceCount++ + continue loop + } + } + for _, s := range serviceByNameList { + if s.Spec.Annotations.Name == service { + filter.Add("service", s.ID) + serviceCount++ + continue loop + } + } + found := false + for _, s := range serviceByIDList { + if strings.HasPrefix(s.ID, service) { + if found { + return filter, nil, errors.New("multiple services found with provided prefix: " + service) + } + filter.Add("service", s.ID) + serviceCount++ + found = true + } + } + if !found { + notfound = append(notfound, "no such service: "+service) + } + } + if serviceCount == 0 { + return filter, nil, errors.New(strings.Join(notfound, "\n")) + } + if filter.Include("node") { + nodeFilters := filter.Get("node") + for _, nodeFilter := range nodeFilters { + nodeReference, err := node.Reference(ctx, client, nodeFilter) + if err != nil { + return filter, nil, err + } + filter.Del("node", nodeFilter) + filter.Add("node", nodeReference) + } + } + return filter, notfound, err } diff --git a/components/cli/cli/command/service/ps_test.go b/components/cli/cli/command/service/ps_test.go new file mode 100644 index 0000000000..a53748d6d3 --- /dev/null +++ b/components/cli/cli/command/service/ps_test.go @@ -0,0 +1,118 @@ +package service + +import ( + "testing" + + "bytes" + + "github.com/docker/cli/cli/internal/test" + "github.com/docker/cli/opts" + "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/filters" + "github.com/docker/docker/api/types/swarm" + "github.com/docker/docker/client" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/net/context" +) + +type fakeClient struct { + client.Client + serviceListFunc func(context.Context, types.ServiceListOptions) ([]swarm.Service, error) +} + +func (f *fakeClient) ServiceList(ctx context.Context, options types.ServiceListOptions) ([]swarm.Service, error) { + if f.serviceListFunc != nil { + return f.serviceListFunc(ctx, options) + } + return nil, nil +} + +func (f *fakeClient) TaskList(ctx context.Context, options types.TaskListOptions) ([]swarm.Task, error) { + return nil, nil +} + +func newService(id string, name string) swarm.Service { + return swarm.Service{ + ID: id, + Spec: swarm.ServiceSpec{Annotations: swarm.Annotations{Name: name}}, + } +} + +func TestCreateFilter(t *testing.T) { + client := &fakeClient{ + serviceListFunc: func(ctx context.Context, options types.ServiceListOptions) ([]swarm.Service, error) { + return []swarm.Service{ + {ID: "idmatch"}, + {ID: "idprefixmatch"}, + newService("cccccccc", "namematch"), + newService("01010101", "notfoundprefix"), + }, nil + }, + } + + filter := opts.NewFilterOpt() + require.NoError(t, filter.Set("node=somenode")) + options := psOptions{ + services: []string{"idmatch", "idprefix", "namematch", "notfound"}, + filter: filter, + } + + actual, notfound, err := createFilter(context.Background(), client, options) + require.NoError(t, err) + assert.Equal(t, notfound, []string{"no such service: notfound"}) + + expected := filters.NewArgs() + expected.Add("service", "idmatch") + expected.Add("service", "idprefixmatch") + expected.Add("service", "cccccccc") + expected.Add("node", "somenode") + assert.Equal(t, expected, actual) +} + +func TestCreateFilterWithAmbiguousIDPrefixError(t *testing.T) { + client := &fakeClient{ + serviceListFunc: func(ctx context.Context, options types.ServiceListOptions) ([]swarm.Service, error) { + return []swarm.Service{ + {ID: "aaaone"}, + {ID: "aaatwo"}, + }, nil + }, + } + options := psOptions{ + services: []string{"aaa"}, + filter: opts.NewFilterOpt(), + } + _, _, err := createFilter(context.Background(), client, options) + assert.EqualError(t, err, "multiple services found with provided prefix: aaa") +} + +func TestCreateFilterNoneFound(t *testing.T) { + client := &fakeClient{} + options := psOptions{ + services: []string{"foo", "notfound"}, + filter: opts.NewFilterOpt(), + } + _, _, err := createFilter(context.Background(), client, options) + assert.EqualError(t, err, "no such service: foo\nno such service: notfound") +} + +func TestRunPSWarnsOnNotFound(t *testing.T) { + client := &fakeClient{ + serviceListFunc: func(ctx context.Context, options types.ServiceListOptions) ([]swarm.Service, error) { + return []swarm.Service{ + {ID: "foo"}, + }, nil + }, + } + + out := new(bytes.Buffer) + cli := test.NewFakeCli(client, out) + options := psOptions{ + services: []string{"foo", "bar"}, + filter: opts.NewFilterOpt(), + format: "{{.ID}}", + } + err := runPS(cli, options) + assert.EqualError(t, err, "no such service: bar") +}