Add docker stats --no-stream show cpu usage #13320
Conversation
coolljt0725
force-pushed
the
add_stats_no_stream_show_cpu_usage
branch
2 times, most recently
from
7518062 to
bb6e67f
Compare
| @@ -29,6 +29,37 @@ type containerStats struct { | |||
| } | |||
|
|
|||
| func (s *containerStats) Collect(cli *DockerCli, streamStats bool) { | |||
| first_v := url.Values{} | |||
There was a problem hiding this comment.
I wonder if this should be on daemon side.
coolljt0725
force-pushed
the
add_stats_no_stream_show_cpu_usage
branch
from
bb6e67f to
7077d91
Compare
|
@LK4D4 Update, move get first stats to daemon side. WDYT? |
|
This could use a test |
|
Confirmed this fixes the issue. |
|
Code okay for me. Test would be nice. |
|
I'll add test later |
Signed-off-by: Lei Jitang <leijitang@huawei.com>
coolljt0725
force-pushed
the
add_stats_no_stream_show_cpu_usage
branch
from
7077d91 to
8b58860
Compare
| ) | ||
|
|
||
| func (s *DockerSuite) TestCliStatsNoStreamGetCpu(c *check.C) { | ||
| dockerFile := fmt.Sprintf("FROM busybox\nRUN echo \"#/bin/sh\" > /bin/test.sh && echo \"while true;do\" >> /bin/test.sh && echo \"echo \"Hello world\"\" >> /bin/test.sh && echo \"done\" >> /bin/test.sh\nRUN chmod +x /bin/test.sh\nCMD /bin/test.sh") |
There was a problem hiding this comment.
this will generate a Dockerfile like this
FROM busybox
RUN echo "#/bin/sh" > /bin/test.sh && echo "while true;do" >> /bin/test.sh && echo "echo "Hello world"" >> /bin/test.sh && echo "done" >> /bin/test.sh
RUN chmod +x /bin/test.sh
CMD /bin/test.sh
and the test.sh is like this
#/bin/sh
while true;do
echo Hello world
done
There was a problem hiding this comment.
Thanks for the test, but some notes here:
I would prefer to not build an image since it is unnecessary to perform the test, and adds a significant amount of time to the actual test.
out, _ := dockerCmd(c, "run", "-d", "busybox", "/bin/sh", "-c", "while true; do echo 'hello'; sleep 0.01; done")
That ought to be sufficient to create the required test and also not completely saturate the cpu.
There was a problem hiding this comment.
This is much more simple, thanks, I'll update
coolljt0725
force-pushed
the
add_stats_no_stream_show_cpu_usage
branch
from
8b58860 to
96123a1
Compare
|
code LGTM. I just wonder if the test can be written without streaming the stats, it might be more easy to understand, but definitely not blocking. |
|
LGTM |
…pu_usage Add docker stats --no-stream show cpu usage
|
ping @jfrazelle for cherry-pick since this is broken as is in 1.7 |
|
cherry-picked |
Signed-off-by: Lei Jitang leijitang@huawei.com
The
CPU %ofdocker stats --no-streamis always0.0even though the real time cpu usage is not zero of the container.
It's quite weird or misleading when user what use
docker stats --no-streamto get the cpu usage of the container. This PR add docker stats --no-stream to
show the cpu usage.
Before:
After:
ping @cpuguy83