-
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
Add host and container info to DockerInfo API #1091 #2607
Conversation
YB | ||
) | ||
|
||
func (b ByteSize) String() string { |
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.
We already have a HumanSize function. Can you either use it or remove it? (I like your implementation)
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.
ping @ecnahc515
I like the idea, but here is some feedback, based on our experience when designing and operating the dotCloud PAAS (which is also based on LXC containers):
I wrote a blog post about container metrics if you want more info about that topic; and for a more "end-user oriented" view, there is also a post on the dotCloud blog. IMHO, the Docker API should expose things that are unequivocal, I mean non ambiguous, and available on all platforms; i.e.:
"Memory and CPU quotas" here means whatever was given to Docker with In addition to that, it would make total sense to expose additional metrics, but in a kind of free format. What do others think? /cc @shykes @vieux @crosbymichael |
@jpetazzo I felt this way while working on a lot of this, definitely was having trouble deciding what 'used' memory consists of when reading the cgroup info. I'll be glad to change this to return only non-ambiguous data, and then add additional more detailed (possibly ambiguous) data in another area. |
@ecnahc515 could you please make sure public types and methods are documented with a comment on top? Thanks. |
It'd be great if you could use http://golang.org/pkg/expvar/ for the key value pairs, but I can follow up after this goes in to make that happen. |
@pnasrat it's in :) |
@ecnahc515 could you please rebase |
Sorry its finals week, been a bit busy. I can rebase it, but I still haven't gotten to fixing the things mentioned. This could be closed until I get the chance to make the requested changes if you prefer, and I could reopen a new one. |
If you don't have time I can base off you're initial pull request and make Paul On 6 December 2013 01:12, Chance Zibolski notifications@github.com wrote:
|
That would be fine. I'll have to check where this is later as I'd like to work more on making metric data for docker available through the API. |
@ecnahc515 could you please rebase this ? |
Shows number of running/total containers, system info (CPU, RAM, ect), and container memory usage.
@ecnahc515 Can you please rebase and address the comments? Let me know if you need help. |
I'll do this but I'd like to get pull #3310 in first as I'll move the cgroup specific stuff under there. |
I confirm I will do this, the last week has been very busy at the day job
On 13 January 2014 13:34, Guillaume J. Charmes notifications@github.comwrote:
|
ping @pnasrat still on this one ? |
Yeah there was some flux but I've made a start at porting this, I've some ideas to make it cleaner and faster though. If I don't get it done in the next week I'll have to come back to after my vacation. |
OK, thanks for the update
|
@pnasrat how was your trip ? :) |
I'm going to close this for now because we havn't had any progress in a while. It can be reopened when ready. |
This PR addresses issue #1091 and adds the following information to the DockerInfo API endpoint and the
docker info
command:It's probably rough around the edges, so I would really appreciate any feedback.