kill test: clean up warnings; document better #5334
Conversation
9f69c4e (part of the f31 pr, containers#3091) semi-broke the kill test, there's now an ugly warning: setup(): removing stray images quay.io/libpod/fedora-minimal:latest 7bb5a60e8a78 The comments also didn't actually explain the problem being addressed, and included a misleading reference to busybox. Here we switch to using fedora-minimal only with podman-remote, clean it up (rmi) when finished, and include an explanation in the comments about why this is needed; making it clear that this workaround can be removed once we get rid of podman-remote. We also reformat back to 80 columns. Signed-off-by: Ed Santiago <santiago@redhat.com>
|
LGTM |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: edsantiago, mheon 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 |
|
|
||
| if [[ $_image != $IMAGE ]]; then | ||
| run_podman rmi $_image | ||
| fi |
There was a problem hiding this comment.
I'm not quite sure why you need to conditionalize this, couldn't you just always do the rmi?
There was a problem hiding this comment.
No, that would be un-nice. The entire rationale behind this test suite is to minimize pulling. In order to do so, all tests are guaranteed to have one and exactly one $IMAGE loaded at start-time; so it is courteous for each test to (absent any failures) leave state as they found it.
|
One question, LGTM once the tests are happy. |
|
Test failures: flake, I think, but am logging for posterity: I've seen this before in system tests, basically UPDATE (few minutes later): had to restart this once again (two restarts total). Passed after that. |
|
The other flake is the one we've been discussing in IRC, "timed out waiting for file". |
Thanks @edsantiago I appreciate the help with all these "new" flakes. |
| local _image=$IMAGE | ||
| local _sh_cmd="sh" | ||
| if is_remote; then | ||
| _image=quay.io/libpod/fedora-minimal:latest |
There was a problem hiding this comment.
FWIW and/or maybe a helpful addition to this comment. This image is built automatically from ./contrib/fedora-minimal/ (since the fedoraproject registry is so often flaky).
There was a problem hiding this comment.
Thanks - I think the quay.io is enough to clue someone in that this is a special image; but in this test my goal is to minimize all pulls, even from quay. Every pull is a high-risk chance for a flake.
There was a problem hiding this comment.
Yes I agree with the methodology.
Actually, something that occurred to me: When the ginkgo tests run, we cache image tarballs in /tmp with some very stable/well-known filenames. If those files exist, pulling could be completely avoided...just import them directly.
|
/lgtm |
9f69c4e (part of the f31 pr, #3091) semi-broke the kill test,
there's now an ugly warning:
The comments also didn't actually explain the problem
being addressed, and included a misleading reference
to busybox.
Here we switch to using fedora-minimal only with podman-remote,
clean it up (rmi) when finished, and include an explanation in
the comments about why this is needed; making it clear that
this workaround can be removed once we get rid of podman-remote.
We also reformat back to 80 columns.
Signed-off-by: Ed Santiago santiago@redhat.com