improve memory and cleanup

[pengshanyu]

Changes:

• Added function ffi_qm_nested_ready
• Code alignment

Resolve: #970

Summary by Sourcery

Improve ffi test reliability around nested podman execution and memory test handling.

New Features:

• Add helper to wait for nested ffi-qm podman to be ready before executing commands.

Bug Fixes:

• Guard disk cleanup from running nested podman removal when the qm container is not running.
• Capture and check the exit status of the memory stress test command without being affected by shell -e behavior.

Enhancements:

• Align shell script formatting in common FFI test helpers for consistency.

Tests:

• Strengthen ffi memory test scripts to wait for nested podman readiness and robustly assert SIGKILL behavior.

[sourcery-ai]

Reviewer's Guide

Adds a readiness helper for nested podman exec in the ffi-qm container, improves robustness of the memory test by explicitly capturing the nested command’s exit code, and guards cleanup to only run nested podman rm when the qm container is actually running, along with minor shell formatting alignment.

File-Level Changes

Change	Details	Files
Add ffi_qm_nested_ready helper to ensure nested podman in ffi-qm is ready before tests run.	Introduce ffi_qm_nested_ready(wait_sec) function that polls podman exec qm /usr/bin/podman exec ffi-qm true with a timeout and logs success or failure. Use timeout with a default of 60 seconds and exit the script with an error if the nested podman exec cannot be established in time. Document the reason for the helper in a comment about systemd reporting Active before nested podman accepts exec.	tests/ffi/common/prepare.sh
Harden cleanup logic to avoid nested podman rm when qm is not running.	Wrap podman exec qm /usr/bin/podman rm -af in a check that qm is running via podman inspect qm --format '{{.State.Running}}'. If qm is not running, skip the nested podman rm and emit an informational SKIP message instead of failing.	tests/ffi/common/prepare.sh
Improve ffi memory test robustness by waiting for nested readiness and reliably capturing the exit status of the memory-eating command.	Invoke ffi_qm_nested_ready after running_container_in_qm to ensure nested podman in ffi-qm can accept exec before the test workload is started. Temporarily disable set -e around the memory_eat command so that its exit code can be captured into a variable even when non-zero. Check the stored exit code for 137 to confirm ffi-qm was killed by SIGKILL, and log a message when this condition is met while keeping the script overall using set -e.	tests/ffi/memory/test.sh
Align shell script indentation and minor formatting for consistency.	Adjust indentation of is_ostree, disk_cleanup, reload_config, and running_container_in_qm functions to use consistent two-space indentation. Align info_message output formatting to match surrounding style and improve readability.	tests/ffi/common/prepare.sh

Assessment against linked issues

Issue	Objective	Addressed	Explanation
#970	Prevent /tests/ffi/memory from failing during the disk_cleanup step when the qm service or nested podman is no longer running (i.e., avoid hard failure of podman exec qm /usr/bin/podman rm -af).	✅
#970	Increase robustness of /tests/ffi/memory by ensuring the nested ffi-qm container is fully ready for podman exec before running the memory stress test.	✅

Possibly linked issues

• improve /tests/ffi/memory #970: The PR fixes /tests/ffi/memory by guarding qm cleanup and improving nested podman exec readiness, matching the reported failure.

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've left some high level feedback:

• Consider reusing or consolidating running_container_in_qm and ffi_qm_nested_ready, as they are both readiness checks for ffi-qm and could share a single, clearer readiness helper.
• In ffi_qm_nested_ready, you might want to short-circuit when the qm container is not running instead of waiting for the full timeout, reusing the podman inspect qm --format '{{.State.Running}}' logic from disk_cleanup.
• The indentation in prepare.sh is inconsistent (e.g., 3 vs 4 spaces in functions like is_ostree and disk_cleanup); aligning these to a single convention will keep the script more readable.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- Consider reusing or consolidating `running_container_in_qm` and `ffi_qm_nested_ready`, as they are both readiness checks for `ffi-qm` and could share a single, clearer readiness helper.
- In `ffi_qm_nested_ready`, you might want to short-circuit when the `qm` container is not running instead of waiting for the full timeout, reusing the `podman inspect qm --format '{{.State.Running}}'` logic from `disk_cleanup`.
- The indentation in `prepare.sh` is inconsistent (e.g., 3 vs 4 spaces in functions like `is_ostree` and `disk_cleanup`); aligning these to a single convention will keep the script more readable.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[pengshanyu]

Hi @Yarboa , @dougsland , could you help to review this PR, thanks.

[gemini-code-assist]

Code Review

This pull request improves the robustness of the FFI test scripts by adding container status checks and implementing timeouts for waiting operations. Key changes include a check in disk_cleanup to verify if the qm container is running before execution, the addition of a ffi_qm_nested_ready function to wait for nested podman readiness, and updates to memory tests to safely capture exit codes. Feedback was provided to replace an infinite loop in running_container_in_qm with a timeout-based check to prevent potential hangs in CI environments.

[Yarboa]

Consider using timeout here also,
It could wait forever

[pengshanyu]

Hi @Yarboa , it seems this continuous waiting is intentionally designed, because if ffi-qm does not start, the subsequent test is meaningless, and once the duration (defined in .fmf files) expires, it will terminate this infinite loop.