-
Notifications
You must be signed in to change notification settings - Fork 18.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Sort output of docker ps --filter
with order by creation time
#25387
Conversation
de42f9d
to
e75d2fe
Compare
name4 := "789-123" | ||
out, err = runSleepingContainer(c, "--name", name4) | ||
c.Assert(err, checker.NotNil) | ||
c.Assert(strings.TrimSpace(out), checker.Not(checker.Equals), "") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you need the above code from
name3 := "789-abc"
...
<above_line>
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zhanghan177 the above is for the scenario where you have more containers overall than the filtered containers output (we are testing ps -f name=xyz
in the following)
c.Assert(strings.TrimSpace(out), checker.Not(checker.Equals), "") | ||
|
||
// Run multiple time should have the same result | ||
out, err = dockerCmd(c, "ps", "-q", "-f", "name=xyz") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: perhaps add --no-trunc
, and use the full ID's?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @thaJeztah! I forgot there is --no-trunc
for docker ps
.
Thanks @yongtang; left some comments/suggestions, but works good otherwise |
This fix tries to address the issue raised in 25374 where the output of `docker ps --filter` is in random order and not deterministic. This fix sorts the list of containers by creation time so that the output is deterministic. An integration test has been added. This fix fixes 25374. Signed-off-by: Yong Tang <[email protected]>
e75d2fe
to
3f97133
Compare
@thaJeztah The pull request has been updated. Please let me know if there are any issues. |
Awesome! LGTM ping @estesp @vdemeester PTAL |
LGTM 🐱 |
Added this to the 1.12.1 milestone as this is a regression introduced in 1.12 (through #23084) |
- What I did
This fix tries to address the issue raised in #25374 where the output of
docker ps --filter
is in random order and not deterministic.- How I did it
This fix sorts the list of containers by creation time so that the output is deterministic.
- How to verify it
An integration test has been added.
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)
This fix fixes #25374.
Signed-off-by: Yong Tang [email protected]