container create: do not clear image name #8623
Conversation
When creating a container, do not clear the input-image name before looking up image names. Also add a regression test. Fixes: containers#8558 Signed-off-by: Valentin Rothberg <[email protected]>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vrothberg 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
|
This will need to be back ported to 2.2.1 LGTM |
|
/lgtm |
openshift-ci-robot
added
the
do-not-merge/hold
|
/hold cancel |
openshift-ci-robot
removed
the
do-not-merge/hold
| imageID="$output" | ||
|
|
||
| run_podman untag $IMAGE | ||
| run_podman run --rm $imageID ls |
There was a problem hiding this comment.
I find it helpful in regression tests to include the problem being fixed/tested. For example:
# check exit status only; prior to #8623 this would error out with:
# Error: both RootfsImageName and RootfsImageID must be set if either is set: invalid argumentFirst: should the error ever recur, this would help a maintainer understand how to fix it.
Second: it really isn't clear what this test is doing, which could lead to some future maintainer "helpfully" refactoring it or changing the invocation in some subtle way such that the test would no longer catch the original problem. A comment helps future maintainers understand that the code should not be touched.
Oh well, merged, too late now - but possibly for the backport, and for future such instances.
There was a problem hiding this comment.
Thanks for the feedback. I will take care to better document in the future.
github-actions
bot
added
the
locked - please file new issue/PR
When creating a container, do not clear the input-image name before
looking up image names. Also add a regression test.
Fixes: #8558
Signed-off-by: Valentin Rothberg [email protected]