Skip to content

ps show health (resolves #5525) - #5643

Closed
bosorawis wants to merge 12 commits into
docker:masterfrom
bosorawis:5525-ps-show-health
Closed

bosorawis wants to merge 12 commits into
docker:masterfrom
bosorawis:5525-ps-show-health

Conversation

@bosorawis

@bosorawis bosorawis commented Feb 3, 2018 •

Copy link
Copy Markdown

Add health string to docker-compose ps output. The command behaves like Docker CLI For starting Up (health: starting)
For no health check Up
For healthy or unhealthy Up (healthy/unhealthy)

Sorawis Nilparuk added 4 commits February 3, 2018 13:14
Signed-off-by: Sorawis Nilparuk <turbo2547@hotmail.com>
Signed-off-by: Sorawis Nilparuk <turbo2547@hotmail.com>
Container running in attached mode returns a JSON string for its exit code rather than just 0. Updated acceptance test to match this behavior

Signed-off-by: Sorawis Nilparuk <turbo2547@hotmail.com>
May fix the changes in another PR

Signed-off-by: Sorawis Nilparuk <turbo2547@hotmail.com>
I fat fingered a line in my first commit, this commit changes that to match current master.

Signed-off-by: Sorawis Nilparuk <turbo2547@hotmail.com>
@bosorawis bosorawis changed the title 5525 ps show health ps show health (resolves #5525) Feb 4, 2018
@jrd

jrd commented Feb 7, 2018

Copy link
Copy Markdown

Tested today, works very good with or without containers that have a healthcheck status

@shin- shin- left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Just mentioned a couple small improvements, otherwise this is good. Can you squash your commits as well?

Comment thread compose/container.py Outdated
return not log_type or log_type in ('json-file', 'journald')

@property
def create_health_status_string(self):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a method is marked with an @property decorator, it should have a property name instead of an action name. To be consistent with other similar properties, maybe rename this to human_readable_health_status?

Comment thread compose/container.py Outdated
"""
status_string = 'Up'
if 'Health' in self.get('State'):
container_status = self.get('State.Health.Status')

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because get will return None if the key is undefined, you can simplify this part of the code by writing

container_status = self.get('State.Health.Status')
if container_status == 'starting':
  ...
elif container_status is not None:
  ...

@shin- shin- added this to the 1.20.0 milestone Feb 7, 2018
Sorawis Nilparuk added 3 commits February 10, 2018 14:37
Signed-off-by: Sorawis Nilparuk <turbo2547@hotmail.com>
Signed-off-by: Sorawis Nilparuk <turbo2547@hotmail.com>
@GordonTheTurtle

Copy link
Copy Markdown

Please sign your commits following these rules:
https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "5525-ps-show-health" git@github.com:dihmuzikien/compose.git somewhere
$ cd somewhere
$ git rebase -i HEAD~842354332872
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

Sorawis Nilparuk added 2 commits February 10, 2018 15:40
Signed-off-by: Sorawis Nilparuk <turbo2547@hotmail.com>
…compose into 5525-ps-show-health

Signed-off-by: Sorawis Nilparuk <turbo2547@hotmail.com>
@bosorawis

bosorawis commented Feb 11, 2018 •

Copy link
Copy Markdown
Author

@shin- I made changes as requested. But I haven't able to figure out how to squash my commits (sorry, still a newbie).

Sorawis Nilparuk added 2 commits February 10, 2018 16:19
Signed-off-by: Sorawis Nilparuk <turbo2547@hotmail.com>

Add health string generator. Works like DockerCLI

Signed-off-by: Sorawis Nilparuk <turbo2547@hotmail.com>

Add unittests for healthy, starting, none case

Signed-off-by: Sorawis Nilparuk <turbo2547@hotmail.com>

Update test to use JSON status

Container running in attached mode returns a JSON string for its exit code rather than just 0. Updated acceptance test to match this behavior

Signed-off-by: Sorawis Nilparuk <turbo2547@hotmail.com>

Rollback previous changes.

May fix the changes in another PR

Signed-off-by: Sorawis Nilparuk <turbo2547@hotmail.com>

Fixed accidental changes causing fail tests

I fat fingered a line in my first commit, this commit changes that to match current master.

Signed-off-by: Sorawis Nilparuk <turbo2547@hotmail.com>

Rename health string property and made changes as requested

Signed-off-by: Sorawis Nilparuk <turbo2547@hotmail.com>
@shin-

shin- commented Feb 12, 2018

Copy link
Copy Markdown

@dihmuzikien Thank you! I'll take care of rebasing / squashing things together.

@shin-

shin- commented Feb 12, 2018

Copy link
Copy Markdown

Merged in bb16d9e

Thank you!

Sign up for free to 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.

5 participants