On clusters where CI is run, there will be a lot of images. Every field in the API around images will take up significant cluster bandwidth.

You only see data in RepoDigests if you have pulled an image by digest. Otherwise, this PR is a no-op.

In other words, this would only affect the CI clusters if the tests are pulling by images by digest.

@yifan-gu how do image references work in rkt - are there both tags and immutable identifiers, or ... ?

And note - if you are pulling by digest, the current code is broken
(doesn't cache the digest, doesn't benefit scheduling at all). So this
completes the feature that only worked in the one case, and makes the cost
we are already paying more palatable.

On Tue, May 3, 2016 at 1:50 PM, Andy Goldstein notifications@github.com
wrote:

In other words, this would only affect the CI clusters if the tests are
pulling by images by digest.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#25088 (comment)

The changes lgtm, but would want sign off from the GC or node side.

I think @vishh still has concerns that the image list could go unbounded (#23355). This is not a problem specific to the digests, but could get worse with the digests.
Maybe we can handle that first?

@yifan-gu how do image references work in rkt - are there both tags and immutable identifiers, or ... ?

@ncdc When the image is already on disk, we can reference it by the sha512, which is the image id here https://github.com/kubernetes/kubernetes/blob/v1.3.0-alpha.3/pkg/kubelet/rkt/image.go#L93

That's the hash of the image, so equivalent to the docker digest here, though we can't pull by that today... cc @philip @jonboulle

I'd like to look at this PR before it gets merged.

On Tue, May 3, 2016 at 4:42 PM, Yifan Gu notifications@github.com wrote:

@yifan-gu https://github.com/yifan-gu how do image references work in
rkt - are there both tags and immutable identifiers, or ... ?
@ncdc https://github.com/ncdc When the image is already on disk, we can
reference it by the sha512, which is the image id here
https://github.com/kubernetes/kubernetes/blob/v1.3.0-alpha.3/pkg/kubelet/rkt/image.go#L93

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#25088 (comment)

I'd like to look at this PR before it gets merged.

Assigned to you so it's easier to keep track.

I can split this into 2 PRs if that would make it easier - 1 for image GC, and 1 for node status.

@smarterclayton It is not theoretically unbounded. In reality though, the number of entries could be couple of hundreds on clusters than run CI workloads. The main use case for exposing images in the API is to lower pod startup latency. If we can assume that image pulling latency is mainly correlated with image size, then we can place a cap on the number of images that we will expose via NodeStatus and expose the largest set of images.

@vishh can we do that cap in a separate PR?

Sure, that makes sense to me.

@ncdc Yeah a separate PR makes sense.

LGTM

@vishh rebased, please take another look - thanks!

GCE e2e build/test passed for commit f091ea5.

• Build Log
• Test Artifacts
• Internal Jenkins Results

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

GCE e2e build/test passed for commit f091ea5.

• Build Log
• Test Artifacts
• Internal Jenkins Results

Automatic merge from submit-queue