Collect and expose runtime's image storage usage via Kubelet's /stats/summary endpoint #23595

vishh · 2016-03-29T19:43:35Z

This information is useful to users since docker images are typically not stored on the root filesystem.

Kubelet will also consume this feature in the future to decide is evicting images will help with disk usage on the nodes.

cc @kubernetes/sig-node

k8s-bot · 2016-03-29T20:27:58Z

GCE e2e build/test passed for commit c8c7c82971899545d5a4302b53c62580f2a945b1.

vishh · 2016-03-31T21:10:03Z

@timstclair: Can you do an initial review?

k8s-bot · 2016-03-31T21:47:31Z

GCE e2e build/test passed for commit 008b5221cfcb0f0297b3ff83a5f3fb27714b9eb2.

timstclair · 2016-03-31T22:45:45Z

cluster/images/hyperkube/master.json

@@ -31,7 +31,7 @@
              "/hyperkube",
              "apiserver",
              "--service-cluster-ip-range=10.0.0.1/24",
-              "--insecure-bind-address=127.0.0.1",
+              "--insecure-bind-address=0.0.0.0",


This looks unrelated? Also, doesn't this expose the insecure outside the local machine? Intentional?

Nope. Reverting this..

timstclair · 2016-03-31T23:12:21Z

Finished review pass.

yujuhong · 2016-03-31T23:18:42Z

Kubelet will also consume this feature in the future to decide is evicting images will help with disk usage on the nodes.

Is the intention to look at the total size to see if it's even worth evicting some images? The size of all images won't help decide what image to evict.

yujuhong · 2016-03-31T23:20:14Z

/cc @Random-Liu, this will affect your PR #23506 to switch to using docker engine-api

Random-Liu · 2016-03-31T23:29:21Z

@yujuhong Thanks. This needs a little rebase after #23506 get merged. It shouldn't be hard (Hopefully :P)

vishh · 2016-03-31T23:30:50Z

Is the intention to look at the total size to see if it's even worth evicting some images? The size of all images won't help decide what image to evict.

Yes. In a separate PR, I plan on cleaning up the image GC logic as well.

vishh · 2016-03-31T23:32:57Z

@timstclair: Thanks for the review. Addressed all comments.

k8s-bot · 2016-04-01T00:04:52Z

GCE e2e build/test passed for commit 0c4b18e2a38198ee0604f56f6c212412ca5aac6b.

vishh · 2016-04-22T23:09:18Z

@timstclair Just did a rebase. PTAL when you get a chance.

k8s-bot · 2016-04-22T23:55:38Z

GCE e2e build/test failed for commit 7faccbb2ef1d8cce1c9cacb4eb23355e22f2bfd0.

Please reference the list of currently known flakes when examining this failure. If you request a re-test, you must reference the issue describing the flake.

timstclair · 2016-04-25T20:35:57Z

pkg/kubelet/dockertools/images.go

+			if !imageMap.Has(key) {
+				ret.TotalStorageBytes += uint64(layer.Size)
+			}
+			imageMap.Insert(key)


Doesn't this mean we'll only add the first layer for each image? Is that intentional?

Do the unit tests still pass with this? If it's not intentional and the tests do still pass, please update the tests to fail on this.

Hmm. history contains all the layers for an image. Let's look at an example:

$ docker history bef5c7a893fd IMAGE CREATED CREATED BY SIZE COMMENT bef5c7a893fd 4 days ago /bin/sh -c #(nop) ENTRYPOINT ["/usr/bin/cadvi 0 B 5714946567e8 4 days ago /bin/sh -c #(nop) EXPOSE 8080/tcp 0 B d5092c6dfc49 4 days ago /bin/sh -c #(nop) ADD file:5047bc1f57482bfdc9 23.75 MB fb1790e81294 4 days ago /bin/sh -c apk add --update ca-certificates d 19.99 MB 3d239f12c85f 4 weeks ago /bin/sh -c #(nop) ENV GLIBC_VERSION=2.23-r1 0 B 86fbae24586c 4 weeks ago /bin/sh -c #(nop) MAINTAINER dengnan@google.c 0 B 342c0650e86b 7 weeks ago /bin/sh -c #(nop) ADD file:cda4b589f22e7984e3 5.258 MB

For this example, history would be a list of layer elements where each line in the output above matches a layer.
The key for each layer will be mostly unique, expect for the <missing> case, which is handled by using the Created By field. That field is expected to contain the command that was run for that layer and hence it must be unique.

Am I misreading my code?

Ah, sorry, my mistake. I misread key as the image key, not the layer key. What you have makes sense.

timstclair · 2016-04-25T22:28:13Z

LGTM, needs rebase.

add image fs info to summary stats API. Adding node e2e test for image stats. Signed-off-by: Vishnu kannan <vishnuk@google.com>

vishh · 2016-04-25T23:01:25Z

Just did a rebase. I'm adding the lgtm label again. FYI @timstclair

k8s-bot · 2016-04-25T23:35:07Z

GCE e2e build/test passed for commit e566948.

k8s-github-robot · 2016-04-26T03:58:50Z

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

k8s-bot · 2016-04-26T04:33:14Z

GCE e2e build/test passed for commit e566948.

k8s-github-robot · 2016-04-26T04:34:30Z

Automatic merge from submit-queue

Random-Liu · 2016-04-30T01:16:51Z

Just FYI, the perfdash shows that this PR increases the runtime CPU usage by about 0.05 cores (from 0.3 cores to 0.35 cores).
It is not significant, but it would be good to track it here so that in the future if we want to do some performance optimization, we can revisit this. :)

yujuhong · 2016-05-02T04:47:35Z

Just FYI, the perfdash shows that this PR increases the runtime CPU usage by about 0.05 cores (from 0.3 cores to 0.35 cores).

What build was this? I couldn't find it.

Random-Liu · 2016-05-02T06:12:16Z

@yujuhong kubernetes-e2e-gce-serial 1155

yujuhong · 2016-05-02T17:44:18Z

IIUC, the PR lists the images and invoke ImageHistory call for each image. We should be able to cache the output, and only call ImageHistory for images we haven't seen before.

Random-Liu · 2016-05-02T22:51:43Z

@yujuhong Agree. We could file an issue for this, :)

googlebot added the cla: yes label Mar 29, 2016

dchen1107 self-assigned this Mar 29, 2016

k8s-github-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 29, 2016

vishh force-pushed the image-accounting branch from c8c7c82 to 008b522 Compare March 31, 2016 21:11

timstclair reviewed Mar 31, 2016
View reviewed changes

vishh force-pushed the image-accounting branch from 008b522 to 0c4b18e Compare March 31, 2016 23:32

k8s-github-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 31, 2016

k8s-github-robot added the release-note-label-needed label Mar 31, 2016

vishh assigned timstclair and unassigned dchen1107 Apr 8, 2016

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 14, 2016

vishh force-pushed the image-accounting branch from 712fbec to 7faccbb Compare April 22, 2016 23:08

k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 22, 2016

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 24, 2016

timstclair reviewed Apr 25, 2016
View reviewed changes

timstclair added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 25, 2016

k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 25, 2016

vishh changed the title ~~Collect and expose runtime's image storage usage~~ Collect and expose runtime's image storage usage vis Kubelet's /stats/summary endpoint Apr 25, 2016

vishh changed the title ~~Collect and expose runtime's image storage usage vis Kubelet's /stats/summary endpoint~~ Collect and expose runtime's image storage usage via Kubelet's /stats/summary endpoint Apr 25, 2016

vishh added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-label-needed labels Apr 25, 2016

Track image storage usage for docker containers

e566948

add image fs info to summary stats API. Adding node e2e test for image stats. Signed-off-by: Vishnu kannan <vishnuk@google.com>

vishh force-pushed the image-accounting branch from 7faccbb to e566948 Compare April 25, 2016 23:00

vishh added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 25, 2016

k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 25, 2016

k8s-github-robot merged commit cf38d68 into kubernetes:master Apr 26, 2016

yifan-gu mentioned this pull request Apr 26, 2016

rkt: Support ImageStats in the container runtime interface #24780

Closed

yujuhong mentioned this pull request May 2, 2016

kubelet should cache image history #25057

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Collect and expose runtime's image storage usage via Kubelet's /stats/summary endpoint #23595

Collect and expose runtime's image storage usage via Kubelet's /stats/summary endpoint #23595

vishh commented Mar 29, 2016

k8s-bot commented Mar 29, 2016

vishh commented Mar 31, 2016

k8s-bot commented Mar 31, 2016

timstclair Mar 31, 2016

vishh Mar 31, 2016

timstclair commented Mar 31, 2016

yujuhong commented Mar 31, 2016

yujuhong commented Mar 31, 2016

Random-Liu commented Mar 31, 2016

vishh commented Mar 31, 2016

vishh commented Mar 31, 2016

k8s-bot commented Apr 1, 2016

vishh commented Apr 22, 2016

k8s-bot commented Apr 22, 2016

timstclair Apr 25, 2016

timstclair Apr 25, 2016

vishh Apr 25, 2016

timstclair Apr 25, 2016

timstclair commented Apr 25, 2016

vishh commented Apr 25, 2016

k8s-bot commented Apr 25, 2016

k8s-github-robot commented Apr 26, 2016

k8s-bot commented Apr 26, 2016

k8s-github-robot commented Apr 26, 2016

Random-Liu commented Apr 30, 2016 •

edited

Loading

yujuhong commented May 2, 2016

Random-Liu commented May 2, 2016

yujuhong commented May 2, 2016

Random-Liu commented May 2, 2016

Collect and expose runtime's image storage usage via Kubelet's /stats/summary endpoint #23595

Collect and expose runtime's image storage usage via Kubelet's /stats/summary endpoint #23595

Conversation

vishh commented Mar 29, 2016

k8s-bot commented Mar 29, 2016

vishh commented Mar 31, 2016

k8s-bot commented Mar 31, 2016

timstclair Mar 31, 2016

Choose a reason for hiding this comment

vishh Mar 31, 2016

Choose a reason for hiding this comment

timstclair commented Mar 31, 2016

yujuhong commented Mar 31, 2016

yujuhong commented Mar 31, 2016

Random-Liu commented Mar 31, 2016

vishh commented Mar 31, 2016

vishh commented Mar 31, 2016

k8s-bot commented Apr 1, 2016

vishh commented Apr 22, 2016

k8s-bot commented Apr 22, 2016

timstclair Apr 25, 2016

Choose a reason for hiding this comment

timstclair Apr 25, 2016

Choose a reason for hiding this comment

vishh Apr 25, 2016

Choose a reason for hiding this comment

timstclair Apr 25, 2016

Choose a reason for hiding this comment

timstclair commented Apr 25, 2016

vishh commented Apr 25, 2016

k8s-bot commented Apr 25, 2016

k8s-github-robot commented Apr 26, 2016

k8s-bot commented Apr 26, 2016

k8s-github-robot commented Apr 26, 2016

Random-Liu commented Apr 30, 2016 • edited Loading

yujuhong commented May 2, 2016

Random-Liu commented May 2, 2016

yujuhong commented May 2, 2016

Random-Liu commented May 2, 2016

Random-Liu commented Apr 30, 2016 •

edited

Loading