Fix image inspect exit code on image not found error #3792
Conversation
|
What is "dnf" here? |
austinvazquez
force-pushed
the
fix-inspect-err-dnf
branch
from
fe0f613 to
58fb773
Compare
austinvazquez
changed the title
|
@AkihiroSuda, oops sorry about that. I had shorthanded "did not find" not meaning to confuse dnf package manager. Lastest commit should have more clear wording. |
pkg/cmd/image/inspect.go
Outdated
| var entries []interface{} | ||
|
|
||
| snapshotter := containerdutil.SnapshotService(client, options.GOptions.Snapshotter) | ||
| // We have to query per provided identifier, as we need to post-process results for the case name + digest | ||
| for _, identifier := range identifiers { | ||
| candidateImageList, requestedName, requestedTag, err := inspectIdentifier(ctx, client, identifier) | ||
| if err != nil { | ||
| log.G(ctx).WithError(err).WithField("identifier", identifier).Error("failure calling inspect") | ||
| errs = append(errs, fmt.Sprintf("invalid reference format: %s", identifier)) |
There was a problem hiding this comment.
Is invalid reference format the only possible error here? Or we should return the (wrapped) original error? e.g., something like fmt.Errorf("invalid reference %s: %w", identifier, err)
There was a problem hiding this comment.
It looks like other errors from inspectIdentifier are errors returned from containerd image service. i.e. https://github.com/austinvazquez/nerdctl/blob/58fb7731a84ac3f260d971f8822aee8329bf1bbf/pkg/cmd/image/inspect.go#L62
So wrapping is useful in that case, but no in the case the error is invalid reference format. Perhaps it is best to revert back to log statement? I originally brought this in as error to align more with Docker behavior, but maybe it isn't worth losing the information here when error talking to containerd.
austin@nimbus:~/workspace/nerdctl$ mysudo nerdctl image inspect ∞∞∞∞∞∞∞∞∞∞ dne:latest
FATA[0000] 2 errors:
invalid reference format: ∞∞∞∞∞∞∞∞∞∞: invalid reference format
no such image: dne:latest
austin@nimbus:~/workspace/nerdctl$
There was a problem hiding this comment.
How about let's do fmt.Errorf("%w: %s", err, identifier) 🤔?
The output will be something like:
FATA[0000] 3 errors:
invalid reference format: ∞∞∞∞∞∞∞∞∞∞
no such image: dne:latest
containerd image service failed: containerd_err: dne:latest2
There was a problem hiding this comment.
Thanks for the suggestion btw; that solution was way simpler.
This change fixes image inspect command to error when image is not found. Signed-off-by: Austin Vazquez <macedonv@amazon.com>
austinvazquez
force-pushed
the
fix-inspect-err-dnf
branch
from
58fb773 to
aa5eb28
Compare
Issue
Fixes regression inadvertently introduced with #3017 for image inspect command exit code when image does not exist.
Description
This change updates the image inspect utility function to serialize an array of encountered errors. This aligns with the behavior of
nerdctl inspect.Testing
Adds simple test case for image inspect on images that do not exist.
Required updates to test cases which assumed exit code 0 for image inspects on images that do not exist.