Fix issue of filter in docker ps where health=starting returns nothing#35940
Merged
vdemeester merged 2 commits intomoby:masterfrom Jan 10, 2018
Merged
Fix issue of filter in docker ps where health=starting returns nothing#35940vdemeester merged 2 commits intomoby:masterfrom
docker ps where health=starting returns nothing#35940vdemeester merged 2 commits intomoby:masterfrom
Conversation
…thing This fix tries to address the issue raised in 35920 where the filter of `docker ps` with `health=starting` always returns nothing. The issue was that in container view, the human readable string (`HealthString()` => `Health.String()`) of health status was used. In case of starting it is `"health: starting"`. However, the filter still uses `starting` so no match returned. This fix fixes the issue by using `container.Health.Status()` instead so that it matches the string (`starting`) passed by filter. This fix fixes 35920. Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
yongtang
force-pushed
the
35920-filter-health-starting
branch
from
d422a1b to
0bb2b6f
Compare
dnephin
reviewed
Jan 5, 2018
container/view.go
Outdated
Member
There was a problem hiding this comment.
I believe this was the only place that (s *State) HealthString() was called, so please remove that method.
dnephin
reviewed
Jan 5, 2018
integration/container/health_test.go
Outdated
Member
There was a problem hiding this comment.
This change seems like it could be easily tested with a unit test of transform() instead of an integration test.
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
yongtang
force-pushed
the
35920-filter-health-starting
branch
from
0bb2b6f to
f509a54
Compare
Member
Author
|
Thanks @dnephin for the review. The PR has been updated with unit test. Please take a look. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
- What I did
This fix tries to address the issue raised in 35920 where the filter of
docker pswithhealth=startingalways returns nothing.- How I did it
The issue was that in container view, the human readable string (
HealthString()=>Health.String()) of health status was used. In case of starting it is"health: starting". However, the filter still usesstartingso no match returned.This fix fixes the issue by using
container.Health.Status()instead so that it matches the string (starting) passed by filter.- 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 #35920.
Signed-off-by: Yong Tang yong.tang.github@outlook.com