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

docker: Support Stats on Windows #5355

Merged
merged 1 commit into from
Feb 26, 2019
Merged

Conversation

endocrimes
Copy link
Contributor

@endocrimes endocrimes commented Feb 22, 2019

This PR adds support for fetching container stats on Windows.

This is based on the implementation in the Docker integration tests that
are referenced inline.

image

fixes #2897

@endocrimes endocrimes force-pushed the dani/windows-dockerstats branch from 1591702 to 6624d36 Compare February 22, 2019 13:20
@notnoop
Copy link
Contributor

notnoop commented Feb 22, 2019

Should this be conditioning over the container OSType instead of the nomad binary OS? How do Linux Containers behave on Windows, do they report "normal" Linux stats similar to how Docker for Mac works?

@endocrimes
Copy link
Contributor Author

@notnoop Potentially - But we don't currently officially support Linux Containers on Windows (they didn't work in many cases on <=0.8 anyway). It's easy enough to switch this out based on that if we want to support them though.

Copy link
Contributor

@notnoop notnoop left a comment

Choose a reason for hiding this comment

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

lgtm.

Given that the code isn't actually platform-dependent, you may be able to avoid compiler flags and run tests. stats_test.go doesn't actually get any data from Docker so conditioning on test host OS isn't so great.

@endocrimes
Copy link
Contributor Author

@notnoop gonna merge this and follow up with removing build flags later

@endocrimes endocrimes merged commit 2657bf0 into master Feb 26, 2019
@endocrimes endocrimes deleted the dani/windows-dockerstats branch February 26, 2019 15:39
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support runtime resource usage metrics in Docker for Windows
2 participants