Skip to content

[23.0 backport] docker ps: always use --quiet, also combined with --format - #4201

Closed
thaJeztah wants to merge 2 commits into
docker:23.0from
thaJeztah:23.0_backport_ps_always_accept_quiet
Closed

[23.0 backport] docker ps: always use --quiet, also combined with --format#4201
thaJeztah wants to merge 2 commits into
docker:23.0from
thaJeztah:23.0_backport_ps_always_accept_quiet

Conversation

@thaJeztah

Copy link
Copy Markdown
Member

Previously, the formatter would ignore the quiet option if a custom format
was passed; this situation was handled in runPs(), where custom formats
would only be applied if the quiet option was not set, but only if the
format was set in the CLI's config.

This patch updates NewContainerFormat() to do the same, even if a --format
was passed on the command-line.

This is a change in behavior, so may need some discussion; possible alternatives;

  • produce an error if both --format and --quiet are passed
  • print a warning if both are passed (but use the logic from this patch)

Before this patch:

docker ps --format '{{.Image}}'ubuntu:22.04alpinedocker ps --format '{{.Image}}' --quietubuntu:22.04alpinemkdir -p ~/.docker/echo '{"psFormat": "{{.Image}}" > ~/.docker/config.jsondocker psubuntu:22.04alpinedocker ps --quietubuntu:22.04alpine

With this patch applied:

docker ps --format '{{.Image}}'ubuntu:22.04alpinedocker ps --format '{{.Image}}' --quiet40111f61d5c5482efdf39facmkdir -p ~/.docker/echo '{"psFormat": "{{.Image}}" > ~/.docker/config.jsondocker psubuntu:22.04alpinedocker ps --quiet40111f61d5c5482efdf39fac

docker ps: print warning if both --format and --quiet are set

Of both "--quiet" and "--format" are set, --quiet takes precedence. This
patch adds a warning to inform the user that their custom format is not
used:

docker ps --format='{{.Image}}'
ubuntu:22.04
alpine
docker ps --format='{{.Image}}' --quiet
WARNING: Ignoring custom format, because both --format and --quiet are set.
40111f61d5c5
482efdf39fac

The warning is printed on STDERR, so can be redirected:

docker ps --format='{{.Image}}' --quiet 2> /dev/null
40111f61d5c5
482efdf39fac

The warning is only shown if the format is set using the "--format" option.
No warning is shown if a custom format is set through the CLI configuration
file:

mkdir -p ~/.docker/
echo '{"psFormat": "{{.Image}}"}' > ~/.docker/config.json
docker ps
ubuntu:22.04
alpine
docker ps --quiet
40111f61d5c5
482efdf39fac

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

Previously, the formatter would ignore the quiet option if a custom format
was passed; this situation was handled in runPs(), where custom formats
would only be applied if the quiet option was not set, but only if the
format was set in the CLI's config.
This patch updates NewContainerFormat() to do the same, even if a `--format`
was passed on the command-line.
This is a change in behavior, so may need some discussion; possible alternatives;
- produce an error if both `--format` and `--quiet` are passed
- print a warning if both are passed (but use the logic from this patch)
Before this patch:
```console
docker ps --format '{{.Image}}'
ubuntu:22.04
alpine
docker ps --format '{{.Image}}' --quiet
ubuntu:22.04
alpine
mkdir -p ~/.docker/
echo '{"psFormat": "{{.Image}}"}' > ~/.docker/config.json
docker ps
ubuntu:22.04
alpine
docker ps --quiet
ubuntu:22.04
alpine
```
With this patch applied:
```console
docker ps --format '{{.Image}}'
ubuntu:22.04
alpine
docker ps --format '{{.Image}}' --quiet
40111f61d5c5
482efdf39fac
mkdir -p ~/.docker/
echo '{"psFormat": "{{.Image}}"}' > ~/.docker/config.json
docker ps
ubuntu:22.04
alpine
docker ps --quiet
40111f61d5c5
482efdf39fac
```
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit f522905)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Of both "--quiet" and "--format" are set, --quiet takes precedence. This
patch adds a warning to inform the user that their custom format is not
used:
docker ps --format='{{.Image}}'
ubuntu:22.04
alpine
docker ps --format='{{.Image}}' --quiet
WARNING: Ignoring custom format, because both --format and --quiet are set.
40111f61d5c5
482efdf39fac
The warning is printed on STDERR, so can be redirected:
docker ps --format='{{.Image}}' --quiet 2> /dev/null
40111f61d5c5
482efdf39fac
The warning is only shown if the format is set using the "--format" option.
No warning is shown if a custom format is set through the CLI configuration
file:
mkdir -p ~/.docker/
echo '{"psFormat": "{{.Image}}"}' > ~/.docker/config.json
docker ps
ubuntu:22.04
alpine
docker ps --quiet
40111f61d5c5
482efdf39fac
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 37e02ff)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@codecov-commenter

codecov-commenter commented Apr 13, 2023

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 58.84%. Comparing base (0b3f540) to head (de7e265).
Report is 177 commits behind head on 23.0.

Additional details and impacted files
@@ Coverage Diff @@## 23.0 #4201 +/- ##
=======================================
Coverage 58.83% 58.84% =======================================
Files 572 572 Lines 49610 49620 +10 =======================================
+ Hits 29188 29198 +10 
Misses 18648 18648 Partials 1774 1774 

@thaJeztah

Copy link
Copy Markdown
MemberAuthor

Discussing with @neersighted whether or not to backport this to v23.0;

👍 fixes docker ps --quiet with a custom format in ~/.docker/config.json
👎 it's a change in behaviour (notably for docker ps --format {''some format"}} --quiet)

/cc @tianon@vvoland@cpuguy83@AkihiroSuda WDYT?

@tianon

Copy link
Copy Markdown
Contributor

No strong feelings - I don't think it needs a backport, but I think the new behavior is much more correct (and matches the user expectation better), so it's totally reasonable to backport if 23.x is going to continue to live for a while.

@thaJeztah

Copy link
Copy Markdown
MemberAuthor

FWIW; I discussed with @neersighted and he was okay with this change, but wanted to have a quick peek from others

@AkihiroSuda

Copy link
Copy Markdown
Collaborator

For v23, it might be safer to just retain the old behavior but print a warning message

@thaJeztah

Copy link
Copy Markdown
MemberAuthor

Yeah, I guess the "complicated" one is the case where both as passed through --flags. The case where a default is configured in the ~/.docker/config.json (and --quiet not working) is easier.

I could consider adding a commit that only changes the behaviour for ~/.docker/config.json for 23.0, and we can decide on the other ones later (or keep it)

@thaJeztah
thaJeztah marked this pull request as draft April 14, 2023 10:24
@thaJeztahthaJeztah modified the milestones: 23.0.4, 23.0.5Apr 14, 2023
@thaJeztahthaJeztah modified the milestones: 23.0.5, 23.0.6Apr 26, 2023
@thaJeztahthaJeztah modified the milestones: 23.0.6, 23.0.7May 5, 2023
@thaJeztahthaJeztah modified the milestones: 23.0.7, 23.0.8Jul 17, 2023
@thaJeztahthaJeztah modified the milestones: 23.0.8, 23.0.7Sep 14, 2023
@thaJeztahthaJeztah modified the milestones: 23.0.7, 23.0.8Sep 28, 2023
@thaJeztahthaJeztah modified the milestones: 23.0.8, 23.0.9Dec 6, 2023
@thaJeztahthaJeztah modified the milestones: 23.0.9, 23.0.10Jan 31, 2024
@thaJeztahthaJeztah removed this from the 23.0.10 milestone Oct 19, 2024
@thaJeztah
thaJeztah deleted the 23.0_backport_ps_always_accept_quiet branch October 19, 2024 13:11
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@thaJeztah@codecov-commenter@tianon@AkihiroSuda