From 3f2bc09231b0833fecba6bcc3f4b78c51388d7c8 Mon Sep 17 00:00:00 2001 From: Nalin Dahyabhai Date: Wed, 12 Jul 2017 12:41:38 -0400 Subject: [PATCH] Return image references in ImageStatus() The image's canonical reference is a name with a digest of the image's manifest, so compute and return that value as the image's reference in ImageStatus() and in ContainerStatus(). We don't auto-store a name based on the image digest when we pull one by tag, but then CRI doesn't need us to do that. Signed-off-by: Nalin Dahyabhai --- cmd/crioctl/container.go | 5 +--- pkg/storage/image.go | 22 ++++++++++---- server/container_status.go | 8 ++++++ server/image_status.go | 8 +++--- test/helpers.bash | 46 +++++++++++------------------ test/image.bats | 59 ++++++++++++++++++++++++++++++-------- 6 files changed, 94 insertions(+), 54 deletions(-) diff --git a/cmd/crioctl/container.go b/cmd/crioctl/container.go index 7be5a7d6..e420e6c9 100644 --- a/cmd/crioctl/container.go +++ b/cmd/crioctl/container.go @@ -472,10 +472,7 @@ func ContainerStatus(client pb.RuntimeServiceClient, ID string) error { if r.Status.Image != nil { fmt.Printf("Image: %v\n", r.Status.Image.Image) } - // - // TODO: https://github.com/kubernetes-incubator/cri-o/issues/531 - // - //fmt.Printf("ImageRef: %v\n", r.Status.ImageRef) + fmt.Printf("ImageRef: %v\n", r.Status.ImageRef) return nil } diff --git a/pkg/storage/image.go b/pkg/storage/image.go index 5aca3e8f..a5832e0f 100644 --- a/pkg/storage/image.go +++ b/pkg/storage/image.go @@ -22,9 +22,11 @@ import ( // ImageResult wraps a subset of information about an image: its ID, its names, // and the size, if known, or nil if it isn't. type ImageResult struct { - ID string - Names []string - Size *uint64 + ID string + Names []string + Digests []string + Size *uint64 + ImageRef string } type indexInfo struct { @@ -149,11 +151,21 @@ func (svc *imageService) ImageStatus(systemContext *types.SystemContext, nameOrI size := imageSize(img) img.Close() - return &ImageResult{ + result := ImageResult{ ID: image.ID, Names: image.Names, Size: size, - }, nil + } + if len(image.Names) > 0 { + result.ImageRef = image.Names[0] + if ref2, err2 := istorage.Transport.ParseStoreReference(svc.store, image.Names[0]); err2 == nil { + if dref := ref2.DockerReference(); dref != nil { + result.ImageRef = reference.FamiliarString(dref) + } + } + } + + return &result, nil } func imageSize(img types.Image) *uint64 { diff --git a/server/container_status.go b/server/container_status.go index b4684c9c..b1512e0c 100644 --- a/server/container_status.go +++ b/server/container_status.go @@ -46,6 +46,14 @@ func (s *Server) ContainerStatus(ctx context.Context, req *pb.ContainerStatusReq cState := s.Runtime().ContainerStatus(c) rStatus := pb.ContainerState_CONTAINER_UNKNOWN + imageName := c.Image() + status, err := s.StorageImageServer().ImageStatus(s.ImageContext(), imageName) + if err != nil { + return nil, err + } + + resp.Status.ImageRef = status.ImageRef + // If we defaulted to exit code -1 earlier then we attempt to // get the exit code from the exit file again. if cState.ExitCode == -1 { diff --git a/server/image_status.go b/server/image_status.go index 1e362a43..5571c302 100644 --- a/server/image_status.go +++ b/server/image_status.go @@ -42,10 +42,10 @@ func (s *Server) ImageStatus(ctx context.Context, req *pb.ImageStatusRequest) (* } resp := &pb.ImageStatusResponse{ Image: &pb.Image{ - Id: status.ID, - RepoTags: status.Names, - Size_: *status.Size, - // TODO: https://github.com/kubernetes-incubator/cri-o/issues/531 + Id: status.ID, + RepoTags: status.Names, + RepoDigests: status.Digests, + Size_: *status.Size, }, } logrus.Debugf("ImageStatusResponse: %+v", resp) diff --git a/test/helpers.bash b/test/helpers.bash index 5989e4ab..a8d61f52 100644 --- a/test/helpers.bash +++ b/test/helpers.bash @@ -105,7 +105,7 @@ cp "$CONMON_BINARY" "$TESTDIR/conmon" PATH=$PATH:$TESTDIR -# Make sure we have a copy of the redis:latest image. +# Make sure we have a copy of the redis:alpine image. if ! [ -d "$ARTIFACTS_PATH"/redis-image ]; then mkdir -p "$ARTIFACTS_PATH"/redis-image if ! "$COPYIMG_BINARY" --import-from=docker://redis:alpine --export-to=dir:"$ARTIFACTS_PATH"/redis-image --signature-policy="$INTEGRATION_ROOT"/policy.json ; then @@ -115,10 +115,10 @@ if ! [ -d "$ARTIFACTS_PATH"/redis-image ]; then fi fi -# TODO: remove the code below for redis digested image id when -# https://github.com/kubernetes-incubator/cri-o/issues/531 is complete -# as the digested reference will be auto-stored when pulling the tag -# above +# TODO: remove the code below for pulling redis:alpine using a canonical reference once +# https://github.com/kubernetes-incubator/cri-o/issues/531 is complete and we can +# pull the image using a tagged reference and then subsequently find the image without +# having to explicitly record the canonical reference as one of the image's names if ! [ -d "$ARTIFACTS_PATH"/redis-image-digest ]; then mkdir -p "$ARTIFACTS_PATH"/redis-image-digest if ! "$COPYIMG_BINARY" --import-from=docker://redis@sha256:03789f402b2ecfb98184bf128d180f398f81c63364948ff1454583b02442f73b --export-to=dir:"$ARTIFACTS_PATH"/redis-image-digest --signature-policy="$INTEGRATION_ROOT"/policy.json ; then @@ -235,11 +235,11 @@ function start_crio() { "$BIN2IMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTIONS --runroot "$TESTDIR/crio-run" --source-binary "$PAUSE_BINARY" fi "$COPYIMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTIONS --runroot "$TESTDIR/crio-run" --image-name=redis:alpine --import-from=dir:"$ARTIFACTS_PATH"/redis-image --add-name=docker.io/library/redis:alpine --signature-policy="$INTEGRATION_ROOT"/policy.json -# TODO: remove the code below for redis:alpine digested image id when -# https://github.com/kubernetes-incubator/cri-o/issues/531 is complete -# as the digested reference will be auto-stored when pulling the tag -# above - "$COPYIMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTIONS --runroot "$TESTDIR/crio-run" --image-name=redis@sha256:03789f402b2ecfb98184bf128d180f398f81c63364948ff1454583b02442f73b --import-from=dir:"$ARTIFACTS_PATH"/redis-image-digest --add-name=docker.io/library/redis@sha256:03789f402b2ecfb98184bf128d180f398f81c63364948ff1454583b02442f73b --signature-policy="$INTEGRATION_ROOT"/policy.json +# TODO: remove the code below for copying redis:alpine in using a canonical reference once +# https://github.com/kubernetes-incubator/cri-o/issues/531 is complete and we can +# copy the image using a tagged reference and then subsequently find the image without +# having to explicitly record the canonical reference as one of the image's names + "$COPYIMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTIONS --runroot "$TESTDIR/crio-run" --image-name=redis@sha256:03789f402b2ecfb98184bf128d180f398f81c63364948ff1454583b02442f73b --import-from=dir:"$ARTIFACTS_PATH"/redis-image-digest --signature-policy="$INTEGRATION_ROOT"/policy.json "$COPYIMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTIONS --runroot "$TESTDIR/crio-run" --image-name=mrunalp/oom --import-from=dir:"$ARTIFACTS_PATH"/oom-image --add-name=docker.io/library/mrunalp/oom --signature-policy="$INTEGRATION_ROOT"/policy.json "$COPYIMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTIONS --runroot "$TESTDIR/crio-run" --image-name=mrunalp/image-volume-test --import-from=dir:"$ARTIFACTS_PATH"/image-volume-test-image --add-name=docker.io/library/mrunalp/image-volume-test --signature-policy="$INTEGRATION_ROOT"/policy.json "$COPYIMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTIONS --runroot "$TESTDIR/crio-run" --image-name=busybox:latest --import-from=dir:"$ARTIFACTS_PATH"/busybox-image --add-name=docker.io/library/busybox:latest --signature-policy="$INTEGRATION_ROOT"/policy.json @@ -262,29 +262,17 @@ function start_crio() { crictl pull redis:alpine fi REDIS_IMAGEID=$(crictl inspecti redis:alpine | head -1 | sed -e "s/ID: //g") + run crictl inspecti redis@sha256:03789f402b2ecfb98184bf128d180f398f81c63364948ff1454583b02442f73b + if [ "$status" -ne 0 ] ; then + crictl pull redis@sha256:03789f402b2ecfb98184bf128d180f398f81c63364948ff1454583b02442f73b + fi + REDIS_IMAGEID_DIGESTED=$(crictl inspecti redis@sha256:03789f402b2ecfb98184bf128d180f398f81c63364948ff1454583b02442f73b | head -1 | sed -e "s/ID: //g") run crictl inspecti mrunalp/oom if [ "$status" -ne 0 ] ; then crictl pull mrunalp/oom fi - # - # - # - # TODO: remove the code below for redis digested image id when - # https://github.com/kubernetes-incubator/cri-o/issues/531 is complete - # as the digested reference will be auto-stored when pulling the tag - # above - # - # - # - REDIS_IMAGEID_DIGESTED="redis@sha256:03789f402b2ecfb98184bf128d180f398f81c63364948ff1454583b02442f73b" - run crictl inspecti $REDIS_IMAGEID_DIGESTED - if [ "$status" -ne 0 ]; then - crictl pull $REDIS_IMAGEID_DIGESTED - fi - # - # - # - run crictl inspecti runcom/stderr-test + OOM_IMAGEID=$(crictl inspecti mrunalp/oom | head -1 | sed -e "s/ID: //g") + run crioctl image status --id=runcom/stderr-test if [ "$status" -ne 0 ] ; then crictl pull runcom/stderr-test:latest fi diff --git a/test/image.bats b/test/image.bats index e62674a7..5458fe13 100644 --- a/test/image.bats +++ b/test/image.bats @@ -50,9 +50,7 @@ function teardown() { stop_crio } -@test "container status return image@digest if created by image ID and digest available" { - skip "depends on https://github.com/kubernetes-incubator/cri-o/issues/531" - +@test "container status return image@digest if created by image ID" { start_crio run crioctl pod run --config "$TESTDATA"/sandbox_config.json @@ -77,11 +75,27 @@ function teardown() { stop_crio } -@test "image pull" { +@test "image pull and list" { start_crio "" "" --no-pause-image run crioctl image pull "$IMAGE" echo "$output" [ "$status" -eq 0 ] + + run crioctl image list --quiet "$IMAGE" + [ "$status" -eq 0 ] + echo "$output" + [ "$output" != "" ] + imageid="$output" + + run crioctl image list --quiet @"$imageid" + [ "$status" -eq 0 ] + echo "$output" + [ "$output" != "" ] + + run crioctl image list --quiet "$imageid" + [ "$status" -eq 0 ] + echo "$output" + [ "$output" != "" ] cleanup_images stop_crio } @@ -104,7 +118,32 @@ function teardown() { stop_crio } -@test "image pull and list by digest" { +@test "image pull and list by tag and ID" { + start_crio "" "" --no-pause-image + run crioctl image pull "$IMAGE:go" + echo "$output" + [ "$status" -eq 0 ] + + run crioctl image list --quiet "$IMAGE:go" + [ "$status" -eq 0 ] + echo "$output" + [ "$output" != "" ] + imageid="$output" + + run crioctl image list --quiet @"$imageid" + [ "$status" -eq 0 ] + echo "$output" + [ "$output" != "" ] + + run crioctl image list --quiet "$imageid" + [ "$status" -eq 0 ] + echo "$output" + [ "$output" != "" ] + cleanup_images + stop_crio +} + +@test "image pull and list by digest and ID" { start_crio "" "" --no-pause-image run crioctl image pull nginx@sha256:33eb1ed1e802d4f71e52421f56af028cdf12bb3bfff5affeaf5bf0e328ffa1bc echo "$output" @@ -114,18 +153,14 @@ function teardown() { [ "$status" -eq 0 ] echo "$output" [ "$output" != "" ] + imageid="$output" - run crioctl image list --quiet nginx@33eb1ed1e802d4f71e52421f56af028cdf12bb3bfff5affeaf5bf0e328ffa1bc + run crioctl image list --quiet @"$imageid" [ "$status" -eq 0 ] echo "$output" [ "$output" != "" ] - run crioctl image list --quiet @33eb1ed1e802d4f71e52421f56af028cdf12bb3bfff5affeaf5bf0e328ffa1bc - [ "$status" -eq 0 ] - echo "$output" - [ "$output" != "" ] - - run crioctl image list --quiet 33eb1ed1e802d4f71e52421f56af028cdf12bb3bfff5affeaf5bf0e328ffa1bc + run crioctl image list --quiet "$imageid" [ "$status" -eq 0 ] echo "$output" [ "$output" != "" ]