Skip to content

add check quiet option and update usage #5395

Merged
crosbymichael merged 2 commits intocontainerd:masterfrom
mikebrow:cmd-check-improve-error-checking
Apr 20, 2021
Merged

add check quiet option and update usage #5395
crosbymichael merged 2 commits intocontainerd:masterfrom
mikebrow:cmd-check-improve-error-checking

Conversation

@mikebrow
Copy link
Copy Markdown
Member

@mikebrow mikebrow commented Apr 19, 2021

resolving issue: #5352

check command returns errors for some cases when check fails, but not for the case when the image being checked via filter does not exist..

   check       check that an image has all content available locally

we should fix or.. document and maybe add another sub command for the desired check

...

After discussion with Derek decision is to document existing usage a little better, and add a -q quiet option to output only the images that are ready. Idea being that script writers can then use wc -l and check for non-zero ... etc.

example outputs after changes:

mike@mike-VirtualBox:~/go/src/github.com/containerd/containerd$ sudo bin/ctr i check --help
NAME:
   ctr images check - check existing images to ensure all content is available locally

USAGE:
   ctr images check [command options] [flags] [<filter>, ...]

DESCRIPTION:
   check existing images to ensure all content is available locally

OPTIONS:
   --quiet, -q          print only the ready image refs (fully downloaded and unpacked)
   --snapshotter value  snapshotter name. Empty value stands for the default value. [$CONTAINERD_SNAPSHOTTER]

mike@mike-VirtualBox:~/go/src/github.com/containerd/containerd$ sudo bin/ctr i check -q 
docker.io/library/busybox:latest
docker.io/library/nats-streaming:0.11.2
docker.io/library/redis:alpine
mike@mike-VirtualBox:~/go/src/github.com/containerd/containerd$ sudo bin/ctr i check name==docker.io/library/busybox:latest -q | wc -l
1
mike@mike-VirtualBox:~/go/src/github.com/containerd/containerd$ sudo bin/ctr i check name==docker.io/library/busybox:ss -q | wc -l
0

Signed-off-by: Mike Brown [email protected]

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Apr 19, 2021

Build succeeded.

@mikebrow mikebrow force-pushed the cmd-check-improve-error-checking branch from dd15782 to a9d2705 Compare April 19, 2021 20:25
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Apr 19, 2021

Build succeeded.

Comment thread cmd/ctr/commands/images/images.go Outdated
@mikebrow mikebrow force-pushed the cmd-check-improve-error-checking branch from a9d2705 to 00f8d32 Compare April 20, 2021 00:37
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Apr 20, 2021

Build succeeded.

Comment on lines +224 to +225
log.G(ctx).Debugf("no images found")
return exitErr
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

exitErr seems always nil here. Is this expected?

Copy link
Copy Markdown
Member Author

@mikebrow mikebrow Apr 20, 2021

Choose a reason for hiding this comment

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

yes.... Derek didn't think it good to return an error on no images, due I believe to the case where check is used to determine if any of the images have an issue. I used exitErr by convention in the surrounding code in case someone ever sets it non nil at some future date.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the explanation. SGTM.

Signed-off-by: Mike Brown <[email protected]>
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Apr 20, 2021

Build succeeded.

@mikebrow
Copy link
Copy Markdown
Member Author

@dmcgowan ready for another review

@mikebrow mikebrow changed the title add not found error check for check cmd add check quiet option and update usage Apr 20, 2021
Copy link
Copy Markdown
Member

@dmcgowan dmcgowan left a comment

Choose a reason for hiding this comment

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

LGTM

@crosbymichael crosbymichael merged commit 079fe6b into containerd:master Apr 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants