-
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
Windows: Add support for docker stats #25737
Conversation
Why is it priv working set and not just memory? |
97b25ed
to
baf0e9a
Compare
baf0e9a
to
51800fb
Compare
a376279
to
bfe49c1
Compare
9a3c15d
to
c852667
Compare
c852667
to
d49f14b
Compare
Rebased and updated now engine-api is back in docker/docker. |
@vdemeester as you reviewed what was the engine-api PR, but no longer needed. 😸 |
d49f14b
to
c83e64d
Compare
LGTM 👼 |
@@ -141,7 +163,7 @@ func (s *DockerSuite) TestApiStatsNetworkStats(c *check.C) { | |||
|
|||
func (s *DockerSuite) TestApiStatsNetworkStatsVersioning(c *check.C) { | |||
testRequires(c, SameHostDaemon) | |||
testRequires(c, DaemonIsLinux) | |||
//testRequires(c, DaemonIsLinux) |
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.
Oh, looks like this can be removed
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.
❤️
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.
Doh. Yes, removed and pushed.
Given that the output is slightly different on Windows, perhaps it's worth adding an example in the docs https://docs.docker.com/engine/reference/commandline/stats/#/examples |
07164c5
to
25cba8b
Compare
@thaJeztah Sure, will add an example. |
25cba8b
to
58b8a55
Compare
@thaJeztah Done 😄 |
Can you squash before merge? |
Sure. @thaJeztah Docs ok? If so will squash ready for merge. |
docs LGTM 🐸 |
OK, squashing... |
Signed-off-by: John Howard <jhoward@microsoft.com>
58b8a55
to
340e523
Compare
Squashed. Just need CI to be 💚 now 😄 |
Yes, docs LGTM, thanks! And a pretty cool feature, really nice to see this coming to Windows (sorry for the delay 😄) |
Windows: Add support for docker stats
Signed-off-by: John Howard jhoward@microsoft.com
This PR adds container stats for Windows. It supports both Windows Server and Hyper-V containers.
Stats exposed from the platform are fundamentally quite different to those exposed by Linux. Where possible (and sensible), I have re-utilised Linux fields in the REST structure returned from the daemon to the client. However, in other cases, noticeably storage, the platforms are significantly different enough to warrant a different structure.
The REST structure has been enhanced, the same as for docker build and other calls, so that the client is aware of the platform of the daemon. In that way, the client can display appropriate information (eg PIDs are not appropriate on Windows). As the client only knows the daemon platform once there is at least one
ContainerStat
returned from the daemon, rather than just printing the headers immediately (which could be for the wrong platform), it will indicate that it is waiting for statistics, and only print the right header once at least oneContainerStat
has been retrieved.Memory management is sufficiently different between Windows and Linux, and indeed different enough between Windows Server containers and Hyper-V containers, that the only field which makes sense for client display purposes is the private working set. This is the same as task manager in Windows using the default settings.
CPU usage calculation is also sufficiently different, but generally the same fields can be utilised. The main difference is that for each interval to calculate percentage, the previous read time and number of processors are required from the daemon. In that way it can calculate the available number of 100ns ticks on the host between time samples, and from that calculate percentage utilisation of each container.
The final note worth mentioning on this PR is that all stats in the platform are returned via HCS. This is
by design
of the Windows API surface. HCS will indirectly get the networking stats from HNS is that same call. Hence there is no call into libnetwork to obtain networking stats as is done on Linux.The net result of the change is that the client does "the right thing" regardless of whether it's Linux to Linux, Windows to Linux, Linux to Windows or any other platform combination you choose.
Here's an example of running Windows to Windows docker stats where there are a variety of Windows Server and Hyper-V containers running.
And a similar example on Linux to Windows. First before any stats are available (just running
docker stats
when there are no running containers)And with a bunch of containers running on the target Windows machine:
And the corresponding examples using the updated client pointing to a Linux daemon to demonstrate that the familiar column set for Linux daemons is still displayed and hasn't regressed
Finally an example of the full REST output to query the stats of a single (Windows Server) container against a Windows daemon.
~~
This PR is in multiple commits to break down the changes into reasonable sized chunks to ease code review and make the logical parts easy to understand. Otherwise this would be a difficult PR to understand.
~~
Squashed 9/16/2016 for merge