Skip to content
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

Show uptime docker compose #6905

Closed
wants to merge 2 commits into from

Conversation

zelahi
Copy link

@zelahi zelahi commented Sep 20, 2019

Resolves #6867

Problem Statement:
In this PR, we are reporting the output when the user executes docker-compose ps after launching containers via docker-compose up

Expected Output
Screen Shot 2019-09-20 at 2 45 21 PM

Implementation Details
I currently used the dateutils library and retrieved the uptimes from the container properties. After doing so, parsed them and displayed it in a human readable format. I added one unit test, but had trouble writing an unit test with the code path using datetime.now(). I didn't see any mocks, but I am open to suggestions and/or okay with reimplementing this without using pyzar and/or dateutils

Signed-off-by: Zuhayr Elahi <elahi.zuhayr@gmail.com>
Signed-off-by: Zuhayr Elahi <elahi.zuhayr@gmail.com>
@@ -753,6 +754,7 @@ def ps(self, options):
command,
container.human_readable_state,
container.human_readable_ports,
container.human_readable_uptime,
Copy link
Member

Choose a reason for hiding this comment

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

Wondering why you didn't use the Status field that's returned by the API (which is the field that's used by the docker CLI;

curl --unix-socket /var/run/docker.sock "http://localhost/containers/json" | jq .
  {
    "Id": "82950c6535204a462c8a3c1f175408b456fbb91971d2d21a33ba04c8b91c74fd",
    "Names": [
      "/cranky_keldysh"
    ],
    "Image": "libnetworkbuild",
    "ImageID": "sha256:bc6bbc6a0032300d8818182eff0101ecb7af2fc1fb21da6f290943c286946a1e",
    "Command": "make unit-tests-local",
    "Created": 1573091888,
    "Ports": [],
    "Labels": {},
    "State": "running",
    "Status": "Up 5 minutes",
    "HostConfig": {
      "NetworkMode": "default"
    },
    "NetworkSettings": {
      "Networks": {
        "bridge": {
          "IPAMConfig": null,
          "Links": null,
          "Aliases": null,
          "NetworkID": "5a59d43d598f910579535ffb2cbbb4d0987807d7b5593c264c83337c4220ec1a",
          "EndpointID": "bf8b6b629b353988d047bcd0bbb897249d7a73a7811b2210b783338e6a975cd9",
          "Gateway": "172.17.0.1",
          "IPAddress": "172.17.0.5",
          "IPPrefixLen": 16,
          "IPv6Gateway": "",
          "GlobalIPv6Address": "",
          "GlobalIPv6PrefixLen": 0,
          "MacAddress": "02:42:ac:11:00:05",
          "DriverOpts": null
        }
      }
    },
    "Mounts": [
      {
        "Type": "bind",
        "Source": "/Users/sebastiaan/projects/libnetwork",
        "Destination": "/go/src/github.com/docker/libnetwork",
        "Mode": "",
        "RW": true,
        "Propagation": "rprivate"
      }
    ]
  }
]

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with this. Also, if it's not running, there is no Uptime.
Changing the State column to Status would do the job.

The only thing here is the people that could be parsing this output, but since it's not a guaranteed format, we should be fine.

Copy link
Author

Choose a reason for hiding this comment

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

Ahh I believe I just missed it. I didn't use the curl command and actually used the values I saw when I did a docker inspect <container_name>. I'll go ahead and fix it so we use Status instead. Thanks for the feedback!! =)

Copy link
Contributor

@ulyssessouza ulyssessouza left a comment

Choose a reason for hiding this comment

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

IMHO, replacing State with Status would be a better approach.

@@ -753,6 +754,7 @@ def ps(self, options):
command,
container.human_readable_state,
container.human_readable_ports,
container.human_readable_uptime,
Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with this. Also, if it's not running, there is no Uptime.
Changing the State column to Status would do the job.

The only thing here is the people that could be parsing this output, but since it's not a guaranteed format, we should be fine.

@glours
Copy link
Contributor

glours commented Jul 13, 2022

Thanks for taking the time to create this issue/pull request!

Unfortunately, Docker Compose V1 has reached end-of-life and we are not accepting any more changes (except for security issues). Please try and reproduce your issue with Compose V2 or rewrite your pull request to be based on the v2 branch and create a new issue or PR with the relevant Compose V2 information.

@glours glours closed this Jul 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

docker-compose ps should show uptime
5 participants