CI: mark 320-system-df *NOT* parallel safe #23916
Conversation
...because it requires 100% control and knowledge of the state of all images, containers, and volumes. Use safename anyway, just in case we ever have a leak from here. I'm finding safename sooooooo helpful when reading journal. Signed-off-by: Ed Santiago <[email protected]>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: edsantiago 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
bot
added
the
approved
|
LGTM |
|
/lgtm |
| run_podman pod rm -a -f | ||
| run_podman rm -f -a -t0 | ||
| run_podman volume rm -a | ||
| run_podman image rm -f -a |
There was a problem hiding this comment.
Doesn't this break the leak check for all prior tests basically? It seems we must check for leaks before this here.
There was a problem hiding this comment.
I'm sorry, I've tried a few times but still don't understand your concern. You mean the leak check in basic_teardown? I don't see how this could break that?
There was a problem hiding this comment.
Well basic_teardown doesn't do a leak_check by default since #22909.
The only leak check done is in teardown_suite, this means that if a test run before the systemd df tests leaked a container we will not catch it due this change. Granted there are other tests that actual test --all so they behave similar so maybe it is not that big of a deal. And we do use PODMAN_BATS_LEAK_CHECK in nightly runs to catch leaks hopefully anyways
There was a problem hiding this comment.
Oh I think I see. But: the whole reason for this change is that a pre-320 test did leak a container, and that caused this entire file to fail with lots of noisy and unhelpful errors. I don't remember if the final leak check also caught the leaks or not, because I only focused on the errors in order and probably never made it to the bottom of the log.
One ugly workaround could be to run leak_check here, before all the rms. It doesn't appeal to me, in the sense of separation of responsibilities (test code should not have to worry about infrastructure) and because a leak check failure would still be confusing to a reader, but probably less confusing than a bunch of system-df failures. Any ideas on how to do this better?
There was a problem hiding this comment.
Yeah I am afraid there is no good answer here... I can live with it but if there are leaks the nightly run should catch it already. So if you say you need this because of leaks then this seems to be a concern that we should address and then maybe remove the rm --all here?
stale-locking-app
bot
added
the
locked - please file new issue/PR
...because it requires 100% control and knowledge of the
state of all images, containers, and volumes.
Use safename anyway, just in case we ever have a leak from here.
I'm finding safename sooooooo helpful when reading journal.
Signed-off-by: Ed Santiago [email protected]