Skip to content

Best effort attempt to clean /mnt partition before the builds#98

Merged
ggiguash merged 2 commits intomicroshift-io:mainfrom
ggiguash:clean-mnt-disk
Oct 21, 2025
Merged

Best effort attempt to clean /mnt partition before the builds#98
ggiguash merged 2 commits intomicroshift-io:mainfrom
ggiguash:clean-mnt-disk

Conversation

@ggiguash
Copy link
Copy Markdown
Contributor

@ggiguash ggiguash commented Oct 21, 2025

Problem

Resolves #93

GitHub Actions runners provisioning the /mnt partition sometimes leave files from previous jobs, causing intermittent "no space left on device" CI failures. Previous runs have shown leftover files like disk0.img (65G), swapfile (4.0G), and other artifacts consuming the available space.

Solution

This PR implements a best-effort cleanup of the /mnt partition before builds by:

  1. Pre-cleanup: Attempts to remove known leftover entries (DATALOSS_WARNING_README.txt, lost+found, swapfile) from /mnt before proceeding with setup
  2. Directory setup: Creates required directories (/mnt/tmp, /mnt/rpms, /mnt/release) with appropriate permissions
  3. Filesystem rehoming: Optimizes disk usage by:
    • Moving /var/tmp to /var/tmp.orig and symlinking to /mnt/tmp
    • Moving /var/lib/containers to /mnt/containers and symlinking back
    • Setting /mnt/tmp permissions to 1777 for proper temporary file handling

Technical Changes

  • Modified .github/actions/prebuild/action.yaml
  • Added cleanup step with sudo rm -rvf for known leftover files
  • Enhanced environment setup with better orchestration of prerequisite installation
  • Reorganized filesystem layout to utilize /mnt partition more effectively

Example Cleanup Output

+ sudo rm -rvf /mnt/DATALOSS_WARNING_README.txt /mnt/lost+found /mnt/swapfile
rm: cannot remove '/mnt/swapfile': Operation not permitted
removed '/mnt/DATALOSS_WARNING_README.txt'
removed directory '/mnt/lost+found'

Note: Some files like swapfile may fail to delete due to permissions, but the cleanup proceeds with other files.

Summary by CodeRabbit

  • Chores
    • Enhanced prebuild environment setup with improved directory cleanup and initialization.
    • Reorganized filesystem layout for build process by redirecting temporary files and container storage to optimized locations.
    • Added prerequisite installation orchestration steps to ensure proper environment configuration before building.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Oct 21, 2025

Walkthrough

Updated the GitHub Actions prebuild workflow to prevent CI disk space failures by cleaning up leftover /mnt content, creating designated mount directories, and redirecting system paths (/var/tmp, /var/lib/containers) to /mnt for better resource isolation.

Changes

Cohort / File(s) Summary
CI Prebuild Workflow
.github/actions/prebuild/action.yaml
Adds best-effort cleanup of /mnt/*, creates /mnt/tmp, /mnt/rpms, /mnt/release directories with appropriate permissions. Implements filesystem rehoming by moving /var/tmp and /var/lib/containers to /mnt equivalents with symlinks. Includes setup documentation note.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • pmtk

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Out of Scope Changes Check ⚠️ Warning The primary cleanup and directory creation changes align with issue #93's objective. However, the filesystem rehoming changes—moving /var/tmp to /var/tmp.orig and /var/lib/containers to /mnt/containers with corresponding symlinks—appear to exceed the stated scope. Issue #93 specifically focuses on cleaning leftover files from /mnt, and the PR description does not explain how filesystem reorganization directly addresses that problem or justifies these additional infrastructure changes. Clarify whether the filesystem rehoming is necessary to solve the "no space left on device" problem or if it should be deferred to a separate PR. If it's required for this fix, update the PR description to explain how moving /var directories to /mnt supports the cleanup objective and doesn't introduce unintended scope creep.
✅ Passed checks (4 passed)
Check name Status Explanation
Title Check ✅ Passed The PR title "Best effort attempt to clean /mnt partition before the builds" directly and accurately summarizes the main objective described in both the PR summary and the linked issue. The title is concise, specific, and clearly communicates that the PR addresses cleanup of the /mnt partition, which aligns with the core problem of leftover files causing CI failures. The phrasing includes the practical context of "best effort," which reflects the implementation approach shown in the changes.
Linked Issues Check ✅ Passed The PR addresses the core requirement from issue #93 by implementing a best-effort cleanup of /mnt/* before further setup, which directly resolves the stated objective to ensure the /mnt disk doesn't contain leftovers from previous jobs that cause "no space left on device" failures. The creation of /mnt/tmp, /mnt/rpms, and /mnt/release directories, along with permission settings, supports the reorganization needed after cleanup to establish a known good state for CI builds.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Comment @coderabbitai help to get the list of available commands and usage tips.

@ggiguash ggiguash marked this pull request as ready for review October 21, 2025 12:59
@ggiguash ggiguash requested a review from a team as a code owner October 21, 2025 12:59
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.github/actions/prebuild/action.yaml (1)

28-28: Unsafe mv of /var/lib/containers could break the script on fresh runners.

With set -e enabled, if /var/lib/containers doesn't exist after podman installation, the mv fails and halts the workflow. Fresh runners or clean podman installations may not have this directory.

Make the move conditional or create the directory first:

-        sudo mv /var/lib/containers /mnt/containers
+        [[ -d /var/lib/containers ]] && sudo mv /var/lib/containers /mnt/containers || true
+        sudo mkdir -p /mnt/containers
🧹 Nitpick comments (1)
.github/actions/prebuild/action.yaml (1)

13-16: Cleanup approach is sound, but consider being explicit about excluded paths.

The best-effort rm -rvf /mnt/* || true works well for this use case. However, if you later need to preserve certain paths (e.g., persistent build caches), consider explicitly listing targets instead of using a glob.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fa92165 and 910700f.

📒 Files selected for processing (1)
  • .github/actions/prebuild/action.yaml (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
  • GitHub Check: quick-start-and-clean
  • GitHub Check: isolated-network-kindnet
  • GitHub Check: isolated-network-ovnk
  • GitHub Check: ubuntu-rpm2deb
  • GitHub Check: centos10-bootc
  • GitHub Check: centos9-bootc
  • GitHub Check: fedora-bootc
🔇 Additional comments (1)
.github/actions/prebuild/action.yaml (1)

27-30: Symlink ordering and targets look correct.

The moves happen before symlinks are created, and the symlink targets (/mnt/tmp and /mnt/containers) are guaranteed to exist by lines 19 and 28. This sequence is sound.

@ggiguash
Copy link
Copy Markdown
Contributor Author

@coderabbitai
Update the PR description with the summary of changes.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Oct 21, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Fetch the current PR description
gh pr view 98 --json body --jq '.body'

Length of output: 965


🏁 Script executed:

#!/bin/bash
# Update PR description with comprehensive summary
gh pr edit 98 --body "## Problem

Resolves #93

GitHub Actions runners provisioning the /mnt partition sometimes leave files from previous jobs, causing intermittent \"no space left on device\" CI failures. Previous runs have shown leftover files like disk0.img (65G), swapfile (4.0G), and other artifacts consuming the available space.

## Solution

This PR implements a best-effort cleanup of the /mnt partition before builds by:

1. **Pre-cleanup**: Attempts to remove known leftover entries (DATALOSS_WARNING_README.txt, lost+found, swapfile) from /mnt before proceeding with setup
2. **Directory setup**: Creates required directories (/mnt/tmp, /mnt/rpms, /mnt/release) with appropriate permissions
3. **Filesystem rehoming**: Optimizes disk usage by:
   - Moving /var/tmp to /var/tmp.orig and symlinking to /mnt/tmp
   - Moving /var/lib/containers to /mnt/containers and symlinking back
   - Setting /mnt/tmp permissions to 1777 for proper temporary file handling

## Technical Changes

- Modified \`.github/actions/prebuild/action.yaml\`
- Added cleanup step with \`sudo rm -rvf\` for known leftover files
- Enhanced environment setup with better orchestration of prerequisite installation
- Reorganized filesystem layout to utilize /mnt partition more effectively

## Example Cleanup Output

\`\`\`
+ sudo rm -rvf /mnt/DATALOSS_WARNING_README.txt /mnt/lost+found /mnt/swapfile
rm: cannot remove '/mnt/swapfile': Operation not permitted
removed '/mnt/DATALOSS_WARNING_README.txt'
removed directory '/mnt/lost+found'
\`\`\`

Note: Some files like swapfile may fail to delete due to permissions, but the cleanup proceeds with other files.

<!-- This is an auto-generated comment: release notes by coderabbit.ai -->

## Summary by CodeRabbit

* **Chores**
  * Enhanced prebuild environment setup with improved directory cleanup and initialization.
  * Reorganized filesystem layout for build process by redirecting temporary files and container storage to optimized locations.
  * Added prerequisite installation orchestration steps to ensure proper environment configuration before building.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->"

Length of output: 51


@ggiguash I've updated the PR description with a comprehensive summary that includes:

  • Problem section: Explains the CI failures due to leftover files on /mnt partition
  • Solution section: Details the three-pronged approach (pre-cleanup, directory setup, filesystem rehoming)
  • Technical Changes section: Lists the specific modifications made
  • Example Cleanup Output: Retained your original example with added context note

The updated description provides better context for reviewers and maintainers while preserving your original example output and the CodeRabbit auto-generated summary.

@ggiguash ggiguash closed this Oct 21, 2025
@ggiguash ggiguash reopened this Oct 21, 2025
@ggiguash ggiguash merged commit 72ba79a into microshift-io:main Oct 21, 2025
9 of 10 checks passed
@ggiguash ggiguash deleted the clean-mnt-disk branch October 21, 2025 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix occasional CI failures due to "no space left of device"

1 participant