Register all possible repo tags and repo digests. #80
Conversation
Random-Liu
force-pushed
the
fix-image-pull
branch
from
f0bafd0 to
55b6bd2
Compare
Random-Liu
force-pushed
the
fix-image-pull
branch
2 times, most recently
from
34209d1 to
f9edb9e
Compare
pkg/server/image_pull.go
Outdated
| if r == "" { | ||
| continue | ||
| } | ||
| if err = c.imageStoreService.Put(ctx, r, desc); err != nil { |
There was a problem hiding this comment.
Will do. vetshadow check is problematic, which reports warning when we re-declare err here.
Will disable that check.
There was a problem hiding this comment.
| ref, desc, err) | ||
| repoDigest, repoTag := getRepoDigestAndTag(namedRef, desc.Digest) | ||
| if ref != repoTag && ref != repoDigest { | ||
| return "", "", "", fmt.Errorf("unexpected repo tag %q and repo digest %q for %q", repoTag, repoDigest, ref) |
There was a problem hiding this comment.
Is the sanity check necessary? You pass namedRef to the function, and ref is just namedRef.String()...
If you must have this check, I think it should be in the getRepoDigestAndTag function.
There was a problem hiding this comment.
Because in code below, we use ref to get image metadata from the containerd image store, which is making the assumption that ref is either repoTag or repoDigest.
Initially, I thought I should add a comment here, but then I remember once I read in the toilet that sanity check is better than comment. So...
There was a problem hiding this comment.
@yujuhong I added the sanity check back, because it did help me catch a bug for schema 1 image.
Please take a look at the latest commit.
Random-Liu
force-pushed
the
fix-image-pull
branch
from
73c9ccd to
4786224
Compare
|
lgtm |
|
Will apply label and merge after squash. @yujuhong Thanks for reviewing! |
Signed-off-by: Lantao Liu <[email protected]>
Random-Liu
force-pushed
the
fix-image-pull
branch
from
4786224 to
1bf0908
Compare
Register all possible repo tags and repo digests.
Update containerd vendoring to 1.4 release
4 participants
Register both repo tag and repo digest when pulling image.
Currently, Kubelet assumes that once image is pulled with repo tag, the repo digest also needs to be stored as existing reference.
Signed-off-by: Lantao Liu [email protected]