Cache image reference #865
Conversation
Random-Liu
force-pushed
the
cache-image-reference
branch
2 times, most recently
from
c5741aa to
96ece38
Compare
| // Get returns the image with specified id. Returns store.ErrNotExist if the | ||
| // image doesn't exist. | ||
| func (s *Store) Get(id string) (Image, error) { | ||
| func (s *store) get(id string) (Image, error) { |
There was a problem hiding this comment.
Suggest updating the function descriptions. For example, here we should mention returns any error found from docker/distribution/digestSet.Lookup, such as digest.ErrDigestNotFound (returned as storeutil.ErrNotExist), digest.ErrDigestInvalidFormat, and digest.ErrDigestAmbiguous.
pkg/server/helpers.go
Outdated
| // nil without error if the reference doesn't exist. | ||
| func (c *criService) localResolve(ctx context.Context, refOrID string) (*imagestore.Image, error) { | ||
| // localResolve resolves image reference locally and returns corresponding image metadata. It | ||
| // store.ErrNotExist if the reference doesn't exist. |
There was a problem hiding this comment.
/s/./or docker/distribution/digestset.ErrDigestInvalidFormat, or ErrDigestAmbiguous./
There was a problem hiding this comment.
Add a todo, we should not be eating the err returned from util.NormalizeImageRef. We are currently assuming it must be a digest if it's not a valid image ref.. but it could just be a poorly formed image ref and not a digest. But right now if it's a poorly formed image ref we return the error from digest lookup. Let's keep "both" errors and combine them here, so the user can see it's not a valid digest or image reference.
| // localResolve resolves image reference locally and returns corresponding image metadata. It returns | ||
| // nil without error if the reference doesn't exist. | ||
| func (c *criService) localResolve(ctx context.Context, refOrID string) (*imagestore.Image, error) { | ||
| // localResolve resolves image reference locally and returns corresponding image metadata. It |
pkg/server/events.go
Outdated
| } | ||
|
|
||
| // start starts the event monitor which monitors and handles all container events. It returns | ||
| // start starts the event monitor which monitors and handles all important events. It returns |
| if err != nil { | ||
| return errors.Wrap(err, "failed to update container status for TaskOOM event") | ||
| } | ||
| case *eventtypes.ImageCreate: |
|
This approach looks good |
Signed-off-by: Lantao Liu <[email protected]>
Random-Liu
force-pushed
the
cache-image-reference
branch
from
96ece38 to
953d67d
Compare
|
@mikebrow Addressed comments. |
|
Something with travis. All test passing, but the bullet doesn't turn green. |
4 participants
Fixes #760.
Based on #864.
This PR:
a)
criplugin creates or removes an image reference. This makes sure image reference cache is up-to-date aftercripulling or deleting an image.b) Event monitor receives an image event (create/update/delete). This makes sure image references created/deleted outside of
crican trigger a cache update, e.g.ctr --namespace=k8s.io images pull.crifunctions, which only read data image information, will treat the cache as the source of truth for images, so thatcridoesn't need to frequently access containerd metadata/content store and easily index by image id.RepoTagsandRepoDigestsinto aReferencesfield in the image store. For containerd, there is only image reference, only CRI has theRepoTagandRepoDigestconcepts.Referencesfield reflects the references in containerd regardless of the reference format.crigenerates CRI image status, it goes through all references, and pick outRepoTagsandRepoDigests.cripulls images, we'll still create repodigest and repotag as the image references, because of kubelet can't see images loaded into containerd #760 (comment). However, when users usectr --namespace=k8s.io images pullto pull images, they can do whatever they want. They just need to understand that if their image doesn't have a reference inrepotagorrepodigestformat, the corresponding CRI fieldsRepoTagsandRepoDigestswill be empty.With this PR:
criwill be able to see images pulled/loaded withctr, and we can removectr cricommand once we havectr importready.repotagwhich has pointed to a new image, won't show up inRepoTagsfield of the old image. We have this bug before this PR./cc @crosbymichael @dmcgowan @stevvooe