images: support checking status of image content by stevvooe · Pull Request #1567 · containerd/containerd

stevvooe · 2017-09-28T00:06:19Z

The Check function returns information about an image's content components
over a content provider. From this information, one can tell which content is
required, present or missing to run an image.

The utility can be demonstrated with the check command:

$ ctr images check
REF                               TYPE                                                      DIGEST                                                                  STATUS            SIZE
docker.io/library/alpine:latest   application/vnd.docker.distribution.manifest.list.v2+json sha256:f006ecbb824d87947d0b51ab8488634bf69fe4094959d935c0c103f4820a417d incomplete (1/2)  1.5 KiB/1.9 MiB
docker.io/library/postgres:latest application/vnd.docker.distribution.manifest.v2+json      sha256:2f8080b9910a8b4f38ff5a55a82e77cb43d88bdbb16d723c71d18493590832e9 complete (13/13)  99.3 MiB/99.3 MiB
docker.io/library/redis:alpine    application/vnd.docker.distribution.manifest.v2+json      sha256:e633cded055a94202e4ccccb8125b7f383cd6ee56527ab890db643383a2647dd incomplete (6/7)  8.1 MiB/10.0 MiB
docker.io/library/ubuntu:latest   application/vnd.docker.distribution.manifest.list.v2+json sha256:60f835698ea19e8d9d3a59e68fb96fb35bc43e745941cb2ea9eaf4ba3029ed8a unavailable (0/?) 0.0 B/?
docker.io/trollin/busybox:latest  application/vnd.docker.distribution.manifest.list.v2+json sha256:54a6424f7a2d5f4f27b3d69e5f9f2bc25fe9087f0449d3cb4215db349f77feae complete (2/2)    699.9 KiB/699.9 KiB

The above shows us that we have two incomplete images and one that is
unavailable. The incomplete images are those that we know the complete
size of all content but some are missing. "Unavailable" means that the
check could not get enough information about the image to get its full
size.

Signed-off-by: Stephen J Day [email protected]

stevvooe · 2017-09-28T00:10:23Z

Closes #1514

codecov-io · 2017-09-28T01:13:00Z

Codecov Report

Merging #1567 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1567   +/-   ##
=======================================
  Coverage   42.44%   42.44%           
=======================================
  Files          24       24           
  Lines        3362     3362           
=======================================
  Hits         1427     1427           
  Misses       1608     1608           
  Partials      327      327

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7c9b0ea...c555df5. Read the comment docs.

qingyunha · 2017-09-28T01:42:18Z

maybe a typo

mlaventure · 2017-09-28T16:59:09Z

Microsoft? 😅

You may want to use a different variable name ;-)

Na, this is the standard abbreviation for manifest. I don't think microsoft is a part of this context, so its not confusing.

@jhowardmsft "John Howard Manifest" Any objections to abbreviating Manifest to msft?

how is msft an abbreviation for manifest? The order of the letters do not match at all. wouldn't it be mfst ?

LMAO. Yeah, mfst ;)

Random-Liu · 2017-09-28T18:53:22Z

@stevvooe So for us, during the start, we need to recover all ready images, to check whether a image is ready or not we need to:

Call this Check helper function to see whether all contents are ready;
Check top snapshot readiness with ChainID:
a. If yes, image is ready;
b. If not, unpack it.

Then we could be sure that the image is ready for use?

stevvooe · 2017-09-29T18:59:01Z

@Random-Liu Yes, this provides the check for the metadata content.

I think we should move the snapshotter preparation to the container rootfs creation stage, rather than using it to decide if an image is "Ready". If you move snapshot preparation to the image readiness size, it requires that the "image" is parameterized on which snapshot is use, which should really be a property of a given container.

Basically, Unpack should handle partials and successful return from Check should be enough to declare readiness.

Random-Liu · 2017-09-30T00:14:55Z

I think we should move the snapshotter preparation to the container rootfs creation stage, rather than using it to decide if an image is "Ready".

Hm. Maybe I misunderstood something. Now when create container rootfs, we usually just use WithNewSnapshotter, which creates a writable layer on top of the parent (top) snapshot, right? It should assume that parent snapshot exists already.

If we don't check top snapshot for image readiness, does it mean that whenever we create container rootfs we should try to unpack the image?

stevvooe · 2017-09-30T00:22:13Z

@Random-Liu Well, that is the current behavior, but I am wondering if we should have WithNewSnapshotter go further to completely prepare the image. If that means doing the initial unpack, it might take longer the first time, but it would ensure that the system is always in the right state to run the container. Classically, this is done at pull time, but I think creation time has better behavioral properties, especially when operating in mixed-snapshotter scenarios.

Random-Liu · 2017-09-30T00:54:17Z

Well, that is the current behavior, but I am wondering if we should have WithNewSnapshotter go further to completely prepare the image. If that means doing the initial unpack, it might take longer the first time, but it would ensure that the system is always in the right state to run the container. Classically, this is done at pull time, but I think creation time has better behavioral properties, especially when operating in mixed-snapshotter scenarios.

We could always add options to tell WithNewSnapshotter to do unpack, I'm totally fine with it. :) However, we may not be able to use it. If I understand correctly, unpack may take quite a long time:

In Kubernetes, we add timeout to different CRI operations to avoid operation stuck forever. The container creation timeout is much shorter than image pulling.
In Kubernetes, we schedule pod based on data locality including whether the image presents or not. This makes container creation time unpredictable even when Kubernetes see the image on the node.
I remember you said that content store may be remote or P2P based in the future, if I understand correctly Unpack may take even longer time in that case?

Anyway, containerd provided the API to check existence of top snapshot, so it's enough for us. :)

stevvooe · 2017-10-02T19:03:21Z

@Random-Liu I agree that those are good approaches where unpack is amortized into pull. However, in cases where the snapshotter isn't known until container start time, we move the unpack to container preparation/creation. For CRI-containerd, this is easy to just model as it is done in docker today, with unpack amortized into pull.

Does this concern stop this PR or would you like me to add a version of this that handles the snapshotter situation, as well?

Random-Liu · 2017-10-02T23:59:06Z

I agree that those are good approaches where unpack is amortized into pull. However, in cases where the snapshotter isn't known until container start time, we move the unpack to container preparation/creation. For CRI-containerd, this is easy to just model as it is done in docker today, with unpack amortized into pull.

As long as it's still possible to unpack during Pull (keep current WithUnpack option), it's fine to us. :)

Does this concern stop this PR or would you like me to add a version of this that handles the snapshotter situation, as well?

If it's simply check top snapshot existence by chain ID, I think it's fine to do it ourselves in cri-containerd.

stevvooe · 2017-10-03T00:39:13Z

@Random-Liu The problem is that the snapshot may not exist. What does cri-containerd do in that situation?

Random-Liu · 2017-10-03T07:20:27Z

@stevvooe If cri-containerd always does unpack during image pulling, non-exist snapshot should be rare case. I think when we find it during list/inspect/restart, we may just remove the reference and let next garbage collection handle it.

The `Check` function returns information about an image's content components over a content provider. From this information, one can tell which content is required, present or missing to run an image. The utility can be demonstrated with the `check` command: ```console $ ctr images check REF TYPE DIGEST STATUS SIZE docker.io/library/alpine:latest application/vnd.docker.distribution.manifest.list.v2+json sha256:f006ecbb824d87947d0b51ab8488634bf69fe4094959d935c0c103f4820a417d incomplete (1/2) 1.5 KiB/1.9 MiB docker.io/library/postgres:latest application/vnd.docker.distribution.manifest.v2+json sha256:2f8080b9910a8b4f38ff5a55a82e77cb43d88bdbb16d723c71d18493590832e9 complete (13/13) 99.3 MiB/99.3 MiB docker.io/library/redis:alpine application/vnd.docker.distribution.manifest.v2+json sha256:e633cded055a94202e4ccccb8125b7f383cd6ee56527ab890db643383a2647dd incomplete (6/7) 8.1 MiB/10.0 MiB docker.io/library/ubuntu:latest application/vnd.docker.distribution.manifest.list.v2+json sha256:60f835698ea19e8d9d3a59e68fb96fb35bc43e745941cb2ea9eaf4ba3029ed8a unavailable (0/?) 0.0 B/? docker.io/trollin/busybox:latest application/vnd.docker.distribution.manifest.list.v2+json sha256:54a6424f7a2d5f4f27b3d69e5f9f2bc25fe9087f0449d3cb4215db349f77feae complete (2/2) 699.9 KiB/699.9 KiB ``` The above shows us that we have two incomplete images and one that is unavailable. The incomplete images are those that we know the complete size of all content but some are missing. "Unavailable" means that the check could not get enough information about the image to get its full size. Signed-off-by: Stephen J Day <[email protected]>

dmcgowan · 2017-10-03T23:18:00Z

LGTM

crosbymichael · 2017-10-04T15:40:37Z

LGTM

stevvooe force-pushed the image-check-function branch from d060cbc to 484b735 Compare September 28, 2017 01:05

qingyunha reviewed Sep 28, 2017

View reviewed changes

Comment thread cmd/ctr/images.go Outdated

Copy link
Copy Markdown

Contributor

qingyunha Sep 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe a typo

stevvooe reacted with thumbs up emoji

mlaventure mentioned this pull request Sep 28, 2017

client: Add an helper to check if image is Unpacked #1572

Closed

mlaventure reviewed Sep 28, 2017

View reviewed changes

stevvooe force-pushed the image-check-function branch from 484b735 to 16a1c10 Compare September 28, 2017 17:34

Random-Liu mentioned this pull request Sep 28, 2017

Use containerd image readiness check function to properly recover image during restart. containerd/cri#303

Closed

stevvooe force-pushed the image-check-function branch from 16a1c10 to c7062ef Compare September 30, 2017 00:27

stevvooe force-pushed the image-check-function branch from c7062ef to b7e91c4 Compare October 2, 2017 22:49

stevvooe force-pushed the image-check-function branch from b7e91c4 to c555df5 Compare October 3, 2017 22:19

crosbymichael merged commit 0ae9d8f into containerd:master Oct 4, 2017

stevvooe deleted the image-check-function branch October 4, 2017 18:09

crosbymichael mentioned this pull request Oct 25, 2017

Verify image components are "ready"/available for use #1514

Closed

Random-Liu mentioned this pull request Oct 25, 2017

Also unpack image during creation. containerd/cri#358

Merged

Conversation

stevvooe commented Sep 28, 2017

Uh oh!

stevvooe commented Sep 28, 2017

Uh oh!

codecov-io commented Sep 28, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

qingyunha Sep 28, 2017

Choose a reason for hiding this comment

Uh oh!

mlaventure Sep 28, 2017

Choose a reason for hiding this comment

Uh oh!

stevvooe Sep 28, 2017

Choose a reason for hiding this comment

Uh oh!

crosbymichael Sep 28, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lowenna Sep 28, 2017

Choose a reason for hiding this comment

Uh oh!

Random-Liu commented Sep 28, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stevvooe commented Sep 29, 2017

Uh oh!

Random-Liu commented Sep 30, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stevvooe commented Sep 30, 2017

Uh oh!

Random-Liu commented Sep 30, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stevvooe commented Oct 2, 2017

Uh oh!

Random-Liu commented Oct 2, 2017

Uh oh!

stevvooe commented Oct 3, 2017

Uh oh!

Random-Liu commented Oct 3, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dmcgowan commented Oct 3, 2017

Uh oh!

crosbymichael commented Oct 4, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

codecov-io commented Sep 28, 2017 •

edited

Loading

crosbymichael Sep 28, 2017 •

edited

Loading

Random-Liu commented Sep 28, 2017 •

edited

Loading

Random-Liu commented Sep 30, 2017 •

edited

Loading

Random-Liu commented Sep 30, 2017 •

edited

Loading

Random-Liu commented Oct 3, 2017 •

edited

Loading