From 8c39b5b7fd4210a69d07885835f7ff826aaa1cd8 Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Sun, 11 Dec 2022 10:59:01 +0100 Subject: [PATCH] align `--format` flag and UX with docker cli Signed-off-by: Nicolas De Loof --- cmd/compose/images.go | 8 +++++--- cmd/compose/list.go | 2 +- cmd/compose/ps.go | 2 +- cmd/compose/ps_test.go | 13 +++++++------ cmd/formatter/consts.go | 5 ++++- cmd/formatter/formatter.go | 2 +- docs/reference/compose_images.md | 1 + docs/reference/compose_ls.md | 2 +- docs/reference/compose_ps.md | 2 +- docs/reference/docker_compose_images.yaml | 10 ++++++++++ docs/reference/docker_compose_ls.yaml | 4 ++-- docs/reference/docker_compose_ps.yaml | 4 ++-- pkg/compose/ps_test.go | 12 ++++++++---- pkg/e2e/compose_test.go | 6 ++---- pkg/e2e/cp_test.go | 2 +- pkg/e2e/framework.go | 22 ++++++++++++++++++++++ pkg/e2e/ps_test.go | 2 +- pkg/e2e/restart_test.go | 17 +++++++++-------- pkg/e2e/start_stop_test.go | 14 ++++---------- 19 files changed, 83 insertions(+), 47 deletions(-) diff --git a/cmd/compose/images.go b/cmd/compose/images.go index 469d0b103..186d47659 100644 --- a/cmd/compose/images.go +++ b/cmd/compose/images.go @@ -35,7 +35,8 @@ import ( type imageOptions struct { *projectOptions - Quiet bool + Quiet bool + Format string } func imagesCommand(p *projectOptions, backend api.Service) *cobra.Command { @@ -50,6 +51,7 @@ func imagesCommand(p *projectOptions, backend api.Service) *cobra.Command { }), ValidArgsFunction: completeServiceNames(p), } + imgCmd.Flags().StringVar(&opts.Format, "format", "table", "Format the output. Values: [table | json].") imgCmd.Flags().BoolVarP(&opts.Quiet, "quiet", "q", false, "Only display IDs") return imgCmd } @@ -88,7 +90,7 @@ func runImages(ctx context.Context, backend api.Service, opts imageOptions, serv return images[i].ContainerName < images[j].ContainerName }) - return formatter.Print(images, formatter.PRETTY, os.Stdout, + return formatter.Print(images, opts.Format, os.Stdout, func(w io.Writer) { for _, img := range images { id := stringid.TruncateID(img.ID) @@ -104,5 +106,5 @@ func runImages(ctx context.Context, backend api.Service, opts imageOptions, serv _, _ = fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\n", img.ContainerName, repo, tag, id, size) } }, - "Container", "Repository", "Tag", "Image Id", "Size") + "CONTAINER", "REPOSITORY", "TAG", "IMAGE ID", "SIZE") } diff --git a/cmd/compose/list.go b/cmd/compose/list.go index b2cbe221c..890757a58 100644 --- a/cmd/compose/list.go +++ b/cmd/compose/list.go @@ -49,7 +49,7 @@ func listCommand(backend api.Service) *cobra.Command { Args: cobra.NoArgs, ValidArgsFunction: noCompletion(), } - lsCmd.Flags().StringVar(&lsOpts.Format, "format", "pretty", "Format the output. Values: [pretty | json].") + lsCmd.Flags().StringVar(&lsOpts.Format, "format", "table", "Format the output. Values: [table | json].") lsCmd.Flags().BoolVarP(&lsOpts.Quiet, "quiet", "q", false, "Only display IDs.") lsCmd.Flags().Var(&lsOpts.Filter, "filter", "Filter output based on conditions provided.") lsCmd.Flags().BoolVarP(&lsOpts.All, "all", "a", false, "Show all stopped Compose projects") diff --git a/cmd/compose/ps.go b/cmd/compose/ps.go index cfaf46c96..43adb3403 100644 --- a/cmd/compose/ps.go +++ b/cmd/compose/ps.go @@ -83,7 +83,7 @@ func psCommand(p *projectOptions, backend api.Service) *cobra.Command { ValidArgsFunction: completeServiceNames(p), } flags := psCmd.Flags() - flags.StringVar(&opts.Format, "format", "pretty", "Format the output. Values: [pretty | json]") + flags.StringVar(&opts.Format, "format", "table", "Format the output. Values: [table | json]") flags.StringVar(&opts.Filter, "filter", "", "Filter services by a property (supported filters: status).") flags.StringArrayVar(&opts.Status, "status", []string{}, "Filter services by status. Values: [paused | restarting | removing | running | dead | created | exited]") flags.BoolVarP(&opts.Quiet, "quiet", "q", false, "Only display IDs") diff --git a/cmd/compose/ps_test.go b/cmd/compose/ps_test.go index 2a6d6bb6a..b08f0dc54 100644 --- a/cmd/compose/ps_test.go +++ b/cmd/compose/ps_test.go @@ -28,14 +28,15 @@ import ( "github.com/stretchr/testify/assert" ) -func TestPsPretty(t *testing.T) { +func TestPsTable(t *testing.T) { ctx := context.Background() origStdout := os.Stdout t.Cleanup(func() { os.Stdout = origStdout }) dir := t.TempDir() - f, err := os.Create(filepath.Join(dir, "output.txt")) + out := filepath.Join(dir, "output.txt") + f, err := os.Create(out) if err != nil { t.Fatal("could not create output file") } @@ -51,8 +52,9 @@ func TestPsPretty(t *testing.T) { DoAndReturn(func(ctx context.Context, projectName string, options api.PsOptions) ([]api.ContainerSummary, error) { return []api.ContainerSummary{ { - ID: "abc123", - Name: "ABC", + ID: "abc123", + Name: "ABC", + Image: "foo/bar", Publishers: api.PortPublishers{ { TargetPort: 8080, @@ -76,8 +78,7 @@ func TestPsPretty(t *testing.T) { _, err = f.Seek(0, 0) assert.NoError(t, err) - output := make([]byte, 256) - _, err = f.Read(output) + output, err := os.ReadFile(out) assert.NoError(t, err) assert.Contains(t, string(output), "8080/tcp, 8443/tcp") diff --git a/cmd/formatter/consts.go b/cmd/formatter/consts.go index 0bb06bf7c..c60338ad9 100644 --- a/cmd/formatter/consts.go +++ b/cmd/formatter/consts.go @@ -17,10 +17,13 @@ package formatter const ( - // JSON is the constant for Json formats on list commands + // JSON Print in JSON format JSON = "json" // TemplateLegacyJSON the legacy json formatting value using go template TemplateLegacyJSON = "{{json.}}" // PRETTY is the constant for default formats on list commands + // Deprecated: use TABLE PRETTY = "pretty" + // TABLE Print output in table format with column headers (default) + TABLE = "table" ) diff --git a/cmd/formatter/formatter.go b/cmd/formatter/formatter.go index cf26a7885..0f54dfcac 100644 --- a/cmd/formatter/formatter.go +++ b/cmd/formatter/formatter.go @@ -30,7 +30,7 @@ import ( // Print prints formatted lists in different formats func Print(toJSON interface{}, format string, outWriter io.Writer, writerFn func(w io.Writer), headers ...string) error { switch strings.ToLower(format) { - case PRETTY, "": + case TABLE, PRETTY, "": return PrintPrettySection(outWriter, writerFn, headers...) case TemplateLegacyJSON: switch reflect.TypeOf(toJSON).Kind() { diff --git a/docs/reference/compose_images.md b/docs/reference/compose_images.md index cfb0ad2cd..4b103bd7f 100644 --- a/docs/reference/compose_images.md +++ b/docs/reference/compose_images.md @@ -7,6 +7,7 @@ List images used by the created containers | Name | Type | Default | Description | | --- | --- | --- | --- | +| `--format` | `string` | `table` | Format the output. Values: [table \| json]. | | `-q`, `--quiet` | | | Only display IDs | diff --git a/docs/reference/compose_ls.md b/docs/reference/compose_ls.md index b556e493f..d99eaa0fc 100644 --- a/docs/reference/compose_ls.md +++ b/docs/reference/compose_ls.md @@ -9,7 +9,7 @@ List running compose projects | --- | --- | --- | --- | | `-a`, `--all` | | | Show all stopped Compose projects | | `--filter` | `filter` | | Filter output based on conditions provided. | -| `--format` | `string` | `pretty` | Format the output. Values: [pretty \| json]. | +| `--format` | `string` | `table` | Format the output. Values: [table \| json]. | | `-q`, `--quiet` | | | Only display IDs. | diff --git a/docs/reference/compose_ps.md b/docs/reference/compose_ps.md index 35c254024..5a4ff5626 100644 --- a/docs/reference/compose_ps.md +++ b/docs/reference/compose_ps.md @@ -9,7 +9,7 @@ List containers | --- | --- | --- | --- | | `-a`, `--all` | | | Show all stopped containers (including those created by the run command) | | [`--filter`](#filter) | `string` | | Filter services by a property (supported filters: status). | -| [`--format`](#format) | `string` | `pretty` | Format the output. Values: [pretty \| json] | +| [`--format`](#format) | `string` | `table` | Format the output. Values: [table \| json] | | `-q`, `--quiet` | | | Only display IDs | | `--services` | | | Display services | | [`--status`](#status) | `stringArray` | | Filter services by status. Values: [paused \| restarting \| removing \| running \| dead \| created \| exited] | diff --git a/docs/reference/docker_compose_images.yaml b/docs/reference/docker_compose_images.yaml index 8e8845180..bbb457695 100644 --- a/docs/reference/docker_compose_images.yaml +++ b/docs/reference/docker_compose_images.yaml @@ -5,6 +5,16 @@ usage: docker compose images [OPTIONS] [SERVICE...] pname: docker compose plink: docker_compose.yaml options: + - option: format + value_type: string + default_value: table + description: 'Format the output. Values: [table | json].' + deprecated: false + hidden: false + experimental: false + experimentalcli: false + kubernetes: false + swarm: false - option: quiet shorthand: q value_type: bool diff --git a/docs/reference/docker_compose_ls.yaml b/docs/reference/docker_compose_ls.yaml index 0b251f7ec..b3ff6bae5 100644 --- a/docs/reference/docker_compose_ls.yaml +++ b/docs/reference/docker_compose_ls.yaml @@ -27,8 +27,8 @@ options: swarm: false - option: format value_type: string - default_value: pretty - description: 'Format the output. Values: [pretty | json].' + default_value: table + description: 'Format the output. Values: [table | json].' deprecated: false hidden: false experimental: false diff --git a/docs/reference/docker_compose_ps.yaml b/docs/reference/docker_compose_ps.yaml index 19beb02c8..6325f285a 100644 --- a/docs/reference/docker_compose_ps.yaml +++ b/docs/reference/docker_compose_ps.yaml @@ -38,8 +38,8 @@ options: swarm: false - option: format value_type: string - default_value: pretty - description: 'Format the output. Values: [pretty | json]' + default_value: table + description: 'Format the output. Values: [table | json]' details_url: '#format' deprecated: false hidden: false diff --git a/pkg/compose/ps_test.go b/pkg/compose/ps_test.go index 6372a8ae1..9a03a88ec 100644 --- a/pkg/compose/ps_test.go +++ b/pkg/compose/ps_test.go @@ -61,10 +61,13 @@ func TestPs(t *testing.T) { containers, err := tested.Ps(ctx, strings.ToLower(testProject), compose.PsOptions{}) expected := []compose.ContainerSummary{ - {ID: "123", Name: "123", Project: strings.ToLower(testProject), Service: "service1", State: "running", Health: "healthy", Publishers: nil}, - {ID: "456", Name: "456", Project: strings.ToLower(testProject), Service: "service1", State: "running", Health: "", Publishers: []compose.PortPublisher{{URL: "localhost", TargetPort: 90, - PublishedPort: 80}}}, - {ID: "789", Name: "789", Project: strings.ToLower(testProject), Service: "service2", State: "exited", Health: "", ExitCode: 130, Publishers: nil}, + {ID: "123", Name: "123", Image: "foo", Project: strings.ToLower(testProject), Service: "service1", + State: "running", Health: "healthy", Publishers: nil}, + {ID: "456", Name: "456", Image: "foo", Project: strings.ToLower(testProject), Service: "service1", + State: "running", Health: "", + Publishers: []compose.PortPublisher{{URL: "localhost", TargetPort: 90, PublishedPort: 80}}}, + {ID: "789", Name: "789", Image: "foo", Project: strings.ToLower(testProject), Service: "service2", + State: "exited", Health: "", ExitCode: 130, Publishers: nil}, } assert.NilError(t, err) assert.DeepEqual(t, containers, expected) @@ -74,6 +77,7 @@ func containerDetails(service string, id string, status string, health string, e container := moby.Container{ ID: id, Names: []string{"/" + id}, + Image: "foo", Labels: containerLabels(service, false), State: status, } diff --git a/pkg/e2e/compose_test.go b/pkg/e2e/compose_test.go index 0dd92a241..5961e961c 100644 --- a/pkg/e2e/compose_test.go +++ b/pkg/e2e/compose_test.go @@ -88,13 +88,11 @@ func TestLocalComposeUp(t *testing.T) { t.Run("check healthcheck output", func(t *testing.T) { c.WaitForCmdResult(t, c.NewDockerComposeCmd(t, "-p", projectName, "ps", "--format", "json"), - StdoutContains(`"Name":"compose-e2e-demo-web-1","Command":"/dispatcher","Project":"compose-e2e-demo","Service":"web","State":"running","Health":"healthy"`), + IsHealthy(projectName+"-web-1"), 5*time.Second, 1*time.Second) res := c.RunDockerComposeCmd(t, "-p", projectName, "ps") - res.Assert(t, icmd.Expected{Out: `NAME COMMAND SERVICE STATUS PORTS`}) - res.Assert(t, icmd.Expected{Out: `compose-e2e-demo-web-1 "/dispatcher" web running (healthy) 0.0.0.0:90->80/tcp`}) - res.Assert(t, icmd.Expected{Out: `compose-e2e-demo-db-1 "docker-entrypoint.s…" db running 5432/tcp`}) + assertServiceStatus(t, projectName, "web", "(healthy)", res.Stdout()) }) t.Run("images", func(t *testing.T) { diff --git a/pkg/e2e/cp_test.go b/pkg/e2e/cp_test.go index ab187e7e0..4b58c8b6d 100644 --- a/pkg/e2e/cp_test.go +++ b/pkg/e2e/cp_test.go @@ -45,7 +45,7 @@ func TestCopy(t *testing.T) { t.Run("make sure service is running", func(t *testing.T) { res := c.RunDockerComposeCmd(t, "-p", projectName, "ps") - res.Assert(t, icmd.Expected{Out: `nginx running`}) + assertServiceStatus(t, projectName, "nginx", "Up", res.Stdout()) }) t.Run("copy to container copies the file to the all containers by default", func(t *testing.T) { diff --git a/pkg/e2e/framework.go b/pkg/e2e/framework.go index aeef8db5c..5cc846f52 100644 --- a/pkg/e2e/framework.go +++ b/pkg/e2e/framework.go @@ -17,6 +17,7 @@ package e2e import ( + "encoding/json" "fmt" "io" "net/http" @@ -314,6 +315,27 @@ func StdoutContains(expected string) func(*icmd.Result) bool { } } +func IsHealthy(service string) func(res *icmd.Result) bool { + return func(res *icmd.Result) bool { + type state struct { + Name string `json:"name"` + Health string `json:"health"` + } + + ps := []state{} + err := json.Unmarshal([]byte(res.Stdout()), &ps) + if err != nil { + return false + } + for _, state := range ps { + if state.Name == service && state.Health == "healthy" { + return true + } + } + return false + } +} + // WaitForCmdResult try to execute a cmd until resulting output matches given predicate func (c *CLI) WaitForCmdResult( t testing.TB, diff --git a/pkg/e2e/ps_test.go b/pkg/e2e/ps_test.go index 042892114..246648413 100644 --- a/pkg/e2e/ps_test.go +++ b/pkg/e2e/ps_test.go @@ -40,7 +40,7 @@ func TestPs(t *testing.T) { assert.Contains(t, res.Combined(), "Container e2e-ps-busybox-1 Started", res.Combined()) - t.Run("pretty", func(t *testing.T) { + t.Run("table", func(t *testing.T) { res = c.RunDockerComposeCmd(t, "-f", "./fixtures/ps-test/compose.yaml", "--project-name", projectName, "ps") lines := strings.Split(res.Stdout(), "\n") assert.Equal(t, 4, len(lines)) diff --git a/pkg/e2e/restart_test.go b/pkg/e2e/restart_test.go index 6ce62bdd7..7937785ba 100644 --- a/pkg/e2e/restart_test.go +++ b/pkg/e2e/restart_test.go @@ -26,16 +26,17 @@ import ( "gotest.tools/v3/assert" ) +func assertServiceStatus(t *testing.T, projectName, service, status string, ps string) { + // match output with random spaces like: + // e2e-start-stop-db-1 alpine:latest "echo hello" db 1 minutes ago Exited (0) 1 minutes ago + regx := fmt.Sprintf("%s-%s-1.+%s\\s+.+%s.+", projectName, service, service, status) + testify.Regexp(t, regx, ps) +} + func TestRestart(t *testing.T) { c := NewParallelCLI(t) const projectName = "e2e-restart" - getServiceRegx := func(service string, status string) string { - // match output with random spaces like: - // e2e-start-stop-db-1 "echo hello" db running - return fmt.Sprintf("%s-%s-1.+%s\\s+%s", projectName, service, service, status) - } - t.Run("Up a project", func(t *testing.T) { // This is just to ensure the containers do NOT exist c.RunDockerComposeCmd(t, "--project-name", projectName, "down") @@ -48,7 +49,7 @@ func TestRestart(t *testing.T) { StdoutContains(`"State":"exited"`), 10*time.Second, 1*time.Second) res = c.RunDockerComposeCmd(t, "--project-name", projectName, "ps", "-a") - testify.Regexp(t, getServiceRegx("restart", "exited"), res.Stdout()) + assertServiceStatus(t, projectName, "restart", "Exited", res.Stdout()) c.RunDockerComposeCmd(t, "-f", "./fixtures/restart-test/compose.yaml", "--project-name", projectName, "restart") @@ -56,7 +57,7 @@ func TestRestart(t *testing.T) { time.Sleep(time.Second) res = c.RunDockerComposeCmd(t, "--project-name", projectName, "ps") - testify.Regexp(t, getServiceRegx("restart", "running"), res.Stdout()) + assertServiceStatus(t, projectName, "restart", "Up", res.Stdout()) // Clean up c.RunDockerComposeCmd(t, "--project-name", projectName, "down") diff --git a/pkg/e2e/start_stop_test.go b/pkg/e2e/start_stop_test.go index 9c19c48db..0255e05fa 100644 --- a/pkg/e2e/start_stop_test.go +++ b/pkg/e2e/start_stop_test.go @@ -36,12 +36,6 @@ func TestStartStop(t *testing.T) { return fmt.Sprintf("%s\\s+%s\\(%d\\)", projectName, status, 2) } - getServiceRegx := func(service string, status string) string { - // match output with random spaces like: - // e2e-start-stop-db-1 "echo hello" db running - return fmt.Sprintf("%s-%s-1.+%s\\s+%s", projectName, service, service, status) - } - t.Run("Up a project", func(t *testing.T) { res := c.RunDockerComposeCmd(t, "-f", "./fixtures/start-stop/compose.yaml", "--project-name", projectName, "up", "-d") @@ -51,8 +45,8 @@ func TestStartStop(t *testing.T) { testify.Regexp(t, getProjectRegx("running"), res.Stdout()) res = c.RunDockerComposeCmd(t, "--project-name", projectName, "ps") - testify.Regexp(t, getServiceRegx("simple", "running"), res.Stdout()) - testify.Regexp(t, getServiceRegx("another", "running"), res.Stdout()) + assertServiceStatus(t, projectName, "simple", "Up", res.Stdout()) + assertServiceStatus(t, projectName, "another", "Up", res.Stdout()) }) t.Run("stop project", func(t *testing.T) { @@ -68,8 +62,8 @@ func TestStartStop(t *testing.T) { assert.Assert(t, !strings.Contains(res.Combined(), "e2e-start-stop-no-dependencies-words-1"), res.Combined()) res = c.RunDockerComposeCmd(t, "--project-name", projectName, "ps", "--all") - testify.Regexp(t, getServiceRegx("simple", "exited"), res.Stdout()) - testify.Regexp(t, getServiceRegx("another", "exited"), res.Stdout()) + assertServiceStatus(t, projectName, "simple", "Exited", res.Stdout()) + assertServiceStatus(t, projectName, "another", "Exited", res.Stdout()) }) t.Run("start project", func(t *testing.T) {