NewFromLocal can return multiple images #8178
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rhatdan The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
|
Fixes #8176 |
pkg/api/handlers/libpod/images.go
Outdated
| if err != nil { | ||
| utils.Error(w, "Something went wrong.", http.StatusNotFound, errors.Wrapf(err, "failed to find image %s", name)) | ||
| return | ||
| } | ||
| utils.WriteResponse(w, http.StatusNoContent, "") | ||
|
|
||
| utils.WriteResponse(w, http.StatusOK, report) |
There was a problem hiding this comment.
This looks like a breaking change to the API
There was a problem hiding this comment.
I don't know, I believe the previous version was broken. @jwhonce WDYT?
There was a problem hiding this comment.
Ok I went back to original API, although I still believe it is slightly broken, but usable.
pkg/domain/infra/abi/images.go
Outdated
| if err != nil && errors.Cause(err) != define.ErrNoSuchImage { | ||
| return nil, err | ||
| if err != nil { | ||
| if errors.Cause(err) != define.ErrMultipleImages { |
There was a problem hiding this comment.
Shouldn't this be == instead of !=
If you use additional stores and pull the same image into writable stores, you can end up with the situation where you have the same image twice. This causes image exists to return the wrong error. It should return true in this situation rather then an error. Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
|
@containers/podman-maintainers PTAL |
| @@ -44,11 +44,16 @@ func ImageExists(w http.ResponseWriter, r *http.Request) { | |||
| runtime := r.Context().Value("runtime").(*libpod.Runtime) | |||
| name := utils.GetName(r) | |||
|
|
|||
| _, err := runtime.ImageRuntime().NewFromLocal(name) | |||
| ir := abi.ImageEngine{Libpod: runtime} | |||
| report, err := ir.Exists(r.Context(), name) | |||
|
/lgtm |
PR containers#8178 fixed containers#8176 but didn't remove the Skip(). This reinstates the test. Signed-off-by: Ed Santiago <santiago@redhat.com>
github-actions
bot
added
the
locked - please file new issue/PR
If you use additional stores and pull the same image into
writable stores, you can end up with the situation where
you have the same image twice. This causes image exists
to return the wrong error. It should return true in this
situation rather then an error.
Signed-off-by: Daniel J Walsh dwalsh@redhat.com