Skip to content

Conversation

@edsantiago
Copy link
Member

BATS teardown logs are unreadable, making it almost impossible
to see tiny "Leaked this-or-that" messages.

Solution: new _run_podman_quiet() helper, replaces run_podman
in a small number of cases within teardown. Clunky, and
duplicative, sorry.

New helper for leak_check, basically spits out warnings (and
bumps error count) if it sees any output whatsoever from
individual "podman XXX ls" commands.

Signed-off-by: Ed Santiago santiago@redhat.com

None

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 30, 2024

[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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 30, 2024
@edsantiago
Copy link
Member Author

Sample output, from an instrumented leaky test:

   # [teardown]
   vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
   *** Leaked volume: v-t2-hjiynoxe local
   vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
   *** Leaked network: ab9b8a85da70  n-t2-hjiynoxe                bridge
   vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
   *** Leaked pod: ea9ebc98aa49 p-t2-hjiynoxe status=Running (2 containers)
   vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
   *** Leaked container: 8977caec43ce localhost/podman-pause:5.2.0-dev-1722336465 ea9ebc98aa49-infra  Up 2 seconds
   *** Leaked container: 9b87b635ba56 docker.io/library/alpine:latest p-t2-hjiynoxe-c-t2-hjiynoxe  Up 2 seconds
   vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
   *** Leaked storage container: 6d18a153164b quay.io/libpod/testimage:20240123 b-t2-hjiynoxe  Storage
   vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
   *** Leaked image: 961769676411 docker.io/library/alpine:latest

Sample output from a run of my parallelized system tests:

# vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
# *** Leaked container: 801e865f2df4 quay.io/libpod/testimage:20240123 c-socat-t160-0iorymqk  Up 2 minutes
# *** Leaked container: 6c5a24aef9e7 quay.io/libpod/testimage:20240123 c-socat-t177-7j1egwth  Up 2 minutes
# *** Leaked container: a429efe7a7d1 quay.io/libpod/testimage:20240123 c-socat-t187-8jcmyued  Up 2 minutes
# vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
# *** Leaked storage container: a6dd24b5bba8 quay.io/libpod/testimage:20240123 p-t207-m7nu6d87-c-t207-m7nu6d87  Storage

This is a stepping-stone PR. I think more fine-tuning is warranted here, and will look into it over the course of my parallelization work. I also think the whole clean_setup() model needs to go away. That will take careful work.

@rhatdan
Copy link
Member

rhatdan commented Jul 30, 2024

LGTM

@edsantiago
Copy link
Member Author

Hahahaha, it works! As in, I forgot that #23431 has not merged yet, and since it hasn't, the external-containers test blew up. Lots of shiny pretty red. Here's what the new leak failure looks like. Some day I will figure out how to clean up the icky multi-line commands above.

With that PR merged, in #23275, all green. (Confirmation that this PR works in the no-problems condition).

Copy link
Member

@Luap99 Luap99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

BATS teardown logs are unreadable, making it almost impossible
to see tiny "Leaked this-or-that" messages.

Solution: new _run_podman_quiet() helper, replaces run_podman
in a small number of cases within teardown. Clunky, and
duplicative, sorry.

New helper for leak_check, basically spits out warnings (and
bumps error count) if it sees any output whatsoever from
individual "podman XXX ls" commands.

Signed-off-by: Ed Santiago <santiago@redhat.com>
Copy link
Member

@Luap99 Luap99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 1, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit bb07993 into containers:main Aug 1, 2024
@edsantiago edsantiago deleted the leak-test-cleanup branch August 1, 2024 16:28
@stale-locking-app stale-locking-app bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Oct 31, 2024
@stale-locking-app stale-locking-app bot locked as resolved and limited conversation to collaborators Oct 31, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. release-note-none

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants