RHAIENG-6546: align rhoai-2.25 GHA disk layout with main (no LVM, /mnt, ubuntu-26.04) - #2647
Conversation
…rlay (opendatahub-io#3967) * chore: fix podman registry auth by ensuring storage directory exists * chore: standardize TMPDIR across GHA workflows to /var/tmp * chore: simplify monitor_resources.py and fix hardcoded values - Replace subprocess `df` + fragile text parsing with shutil.disk_usage() - Fix monitored path /var/lib/containers → /mnt/containers/storage - Cache shutil.which('free') at module level instead of per-iteration - Parse free -h into structured dict for proper JSON logging - Use ${USER} instead of hardcoded runner:runner in build template * chore: remove custom TMPDIR in Podman workflows * chore: handle timeout in memory usage retrieval with subprocess timeout retry * chore(ci): update crio.conf comment to clarify runroot configuration * chore: add signal handling to monitor_resources.py for clean shutdown Co-authored-by: Junie <junie@jetbrains.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> (cherry picked from commit e4cf912)
scripts/monitor_resources.py already imports ci.logging_config on rhoai-2.25 but the module was missing, causing background ModuleNotFoundError in Build Notebooks. Drop the matching triage rows now that /mnt layout + logging are aligned with main. Co-authored-by: Cursor <cursoragent@cursor.com>
Align rhoai-2.25 with main's runner pin (f3c39c5): replace ubuntu-24.04 and ubuntu-latest with ubuntu-26.04 across workflows. Cherry-pick of that commit conflicted heavily with RHDS-only workflows, so apply the equivalent mechanical migration instead. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR pins GitHub Actions jobs to Ubuntu 26.04, updates Podman and disk management, adds structured logging, introduces disk cleanup validation, adjusts container AppArmor handling, and refactors ROCm PyTorch image build steps. ChangesCI platform updates
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
rhoai-2.25 sets reportAttributeAccessIssue=none, so the ignore comments copied from main trip reportUnnecessaryTypeIgnoreComment under pre-commit. Co-authored-by: Cursor <cursoragent@cursor.com>
CI status [antigravity]Run: Build Notebooks (pr) #30535307047 — 3/3 complete · 1 passed · 2 cancelled Failures so far
Likely root causes
Suggested next steps
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
ci/logging_config.py (1)
56-71: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReaches into a private structlog attribute.
self._default_column_formatteris an internal implementation detail ofstructlog.dev.ConsoleRenderer(populated by its private_configure_columns()method), not part of the documented public API. The documented, supported customization path is thecolumnsconstructor parameter. Relying on the underscore-prefixed attribute risks silent breakage on a future structlog upgrade (e.g. any code path that later sets one of the new 25.5.0 properties would trigger_configure_columns()and discard this override).Since this depends on structlog's internal implementation, please verify against the pinned structlog version and consider building an explicit
columnslist instead (defining a default column withkey="") for a stable, documented API surface.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ci/logging_config.py` around lines 56 - 71, The PrettyConsoleRenderer initializer relies on private _default_column_formatter, which can be overwritten by structlog. Replace this override with an explicit columns configuration using the documented columns parameter, including a default column with key="" and the existing key/value styles, pretty value representation, and "\n " prefix; verify the configuration against the pinned structlog version.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ci/logging_config.py`:
- Around line 45-53: Update _pretty_value_repr so its json.dumps call safely
handles non-JSON-serializable values by using a repr-based fallback for
unsupported objects, while preserving the existing indented JSON and
highlighting behavior for serializable dicts and lists.
---
Nitpick comments:
In `@ci/logging_config.py`:
- Around line 56-71: The PrettyConsoleRenderer initializer relies on private
_default_column_formatter, which can be overwritten by structlog. Replace this
override with an explicit columns configuration using the documented columns
parameter, including a default column with key="" and the existing key/value
styles, pretty value representation, and "\n " prefix; verify the configuration
against the pinned structlog version.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b0fd1480-dee0-4573-b957-65f15c090f8a
📒 Files selected for processing (37)
.github/actions/free-up-disk-space/action.yml.github/workflows/auto-add-issue-to-project.yml.github/workflows/build-notebooks-TEMPLATE.yaml.github/workflows/build-notebooks-pr-aipcc.yaml.github/workflows/build-notebooks-pr-rhel.yaml.github/workflows/build-notebooks-push.yaml.github/workflows/code-quality.yaml.github/workflows/codeql.yaml.github/workflows/create-release.yaml.github/workflows/docs.yaml.github/workflows/gitleaks.yaml.github/workflows/insta-merge.yaml.github/workflows/instant-merge.yaml.github/workflows/notebook-digest-updater.yaml.github/workflows/notebooks-digest-updater.yaml.github/workflows/notebooks-release.yaml.github/workflows/notify-team-to-review-pr.yml.github/workflows/params-env.yaml.github/workflows/piplock-renewal.yaml.github/workflows/pr-merge-image-delete.yml.github/workflows/purge-ghcr.yaml.github/workflows/rpms-lock-renewal.yaml.github/workflows/sec-scan.yml.github/workflows/security.yaml.github/workflows/semgrep.yaml.github/workflows/software-versions.yaml.github/workflows/sync-branches-through-pr.yml.github/workflows/test-capture-kernel-logs.yaml.github/workflows/test-containers.yaml.github/workflows/test-provision-k8s.yaml.github/workflows/test-trivy-scan-action.yaml.github/workflows/update-buildconfigs.yaml.github/workflows/update-commit-latest-env.yaml.github/workflows/update-tags.yamlci/cached-builds/gha_lvm_overlay.shci/logging_config.pydocs/ci-failure-triage.md
💤 Files with no reviewable changes (3)
- .github/actions/free-up-disk-space/action.yml
- ci/cached-builds/gha_lvm_overlay.sh
- docs/ci-failure-triage.md
Port main's Fix AppArmor pasta/podman signal conflict into test-containers.yaml. Without it, container stop fails with CNI permission denied on ubuntu-26.04 (Test Infrastructure Self-Test). Co-authored-by: Cursor <cursoragent@cursor.com>
CI status [antigravity]Run: Build Notebooks (push) #30535872380 — 2/2 complete · 2 skipped Workflow completed with skipped jobs. |
CI status [antigravity]Run: Build Notebooks (pr) #30535450798 — 3/3 complete · 1 passed · 2 cancelled Failures so far
Likely root causes
Suggested next steps
|
Address CodeRabbit: avoid TypeError in pretty log rendering when a dict/list value contains a non-JSON-serializable object. Co-authored-by: Cursor <cursoragent@cursor.com>
CI status [antigravity]Run: Build Notebooks (push) #30536210175 — 2/2 complete · 2 skipped Workflow completed with skipped jobs. |
CI status [antigravity]Run: Build Notebooks (pr) #30535877175 — 3/3 complete · 1 passed · 2 cancelled Failures so far
Likely root causes
Suggested next steps
|
Reclaim runner preinstalls only for the two RHDS AIPCC targets that ENOSPC on single-volume ubuntu-26.04, and add a path-triggered self-test. Co-authored-by: Cursor <cursoragent@cursor.com>
CI status [antigravity]Run: Build Notebooks (push) #30543457245 — 2/2 complete · 2 skipped Workflow completed with skipped jobs. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/test-free-up-disk-space.yaml:
- Around line 64-70: Update the fixed-path validation loop in the workflow to
include /usr/local/share/boost and /usr/local/lib/node_modules alongside the
existing paths, ensuring both action-managed deletions are verified by the
self-test.
- Around line 56-62: Update the cleanup self-test around min_gain_kb so it does
not require a fixed 5 GiB reclaimed-space threshold tied to the runner image
layout. Assert that cleanup reclaims a positive amount and verify the expected
cleanup paths instead, or replace the current threshold with an explicitly
maintained contract.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: be714957-24c9-44bb-9d39-3c37496f41b8
📒 Files selected for processing (3)
.github/actions/free-up-disk-space/action.yml.github/workflows/build-notebooks-TEMPLATE.yaml.github/workflows/test-free-up-disk-space.yaml
| # Expect at least ~5 GiB reclaimed on a stock ubuntu-26.04 runner | ||
| # (android/dotnet/CodeQL alone are usually well above this). | ||
| min_gain_kb=$((5 * 1024 * 1024)) | ||
| if (( gained_kb < min_gain_kb )); then | ||
| echo "::error::Expected at least 5 GiB freed, got $((gained_kb / 1024 / 1024)) GiB" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Avoid making this self-test depend on a fixed 5 GiB image layout.
GitHub currently lists ubuntu-26.04 as a public-preview image and warns that preview images are provided as-is, so installed contents can change. (docs.github.com) A correct cleanup can therefore fail this gate after runner-image updates. Prefer asserting positive reclaimed space plus the expected paths, or make the threshold an explicit maintained contract.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/test-free-up-disk-space.yaml around lines 56 - 62, Update
the cleanup self-test around min_gain_kb so it does not require a fixed 5 GiB
reclaimed-space threshold tied to the runner image layout. Assert that cleanup
reclaims a positive amount and verify the expected cleanup paths instead, or
replace the current threshold with an explicitly maintained contract.
| # Paths the action deletes; tolerate already-missing. | ||
| for path in \ | ||
| /usr/local/lib/android \ | ||
| /usr/share/dotnet \ | ||
| /opt/ghc \ | ||
| /usr/local/.ghcup \ | ||
| /opt/hostedtoolcache/CodeQL |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Validate every fixed path removed by the action.
The action also deletes /usr/local/share/boost and /usr/local/lib/node_modules, but this loop does not verify either path. Add both entries so cleanup regressions cannot pass the self-test unnoticed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/test-free-up-disk-space.yaml around lines 64 - 70, Update
the fixed-path validation loop in the workflow to include /usr/local/share/boost
and /usr/local/lib/node_modules alongside the existing paths, ensuring both
action-managed deletions are verified by the self-test.
CI status [antigravity]Run: Build Notebooks (push) #30547662601 — 2/2 complete · 2 skipped Workflow completed with skipped jobs. |
CI status [antigravity]Run: Build Notebooks (pr) #30543463730 — 3/3 complete · 1 passed · 2 cancelled Failures so far
Likely root causes
Suggested next steps
|
ide-developer
left a comment
ide-developer
left a comment
There was a problem hiding this comment.
Thorough backport — aligns rhoai-2.25 GHA infra with main. Reviewed the full 42-file diff. Key areas and findings below.
1. ubuntu-26.04 migration (~25 workflows)
Mechanical runs-on replacements. Consistent with the crun/podman fix (opendatahub-io#4211 / actions/runner-images#14473). Non-podman workflows (issue-to-project, insta-merge, digest-updater, etc.) don't strictly need it but consistency across the repo is reasonable.
2. LVM overlay removal + /mnt layout
Deleting gha_lvm_overlay.sh (79-line LVM+btrfs+swap setup) in favor of sudo mkdir -p /mnt/containers/storage is a major simplification. The old approach was fragile (LVM resize, btrfs mount opts, swap carved from VG). The new layout trusts GitHub's promise of ≥15GiB on / plus whatever /mnt provides on two-volume machines.
Risk is ENOSPC on single-volume runners. Mitigated by: (a) conditional free-up-disk-space for the two largest ROCm targets, (b) the Dockerfile layer merge below.
3. ROCm Dockerfile layer merge (the clever part)
Moving de-vendor-torch.sh into the same RUN layer as uv pip install means the intermediate state (vendored ROCm .so + installed copies) is never committed to the image store. This directly reduces peak disk during podman commit, which is where ENOSPC was hitting. The user context is correct: de-vendor-torch.sh runs as USER 1001 — same user that installed the packages, so it has write access to remove the vendored copies from site-packages.
The heredoc RUN /bin/bash <<'EOF' syntax requires BuildKit, which is standard for both Podman 5.7+ (ubuntu-26.04) and Konflux builds.
All four ROCm Dockerfiles (jupyter + runtime, .rocm + .konflux.rocm) are updated consistently.
4. free-up-disk-space action rework
docker system prune -afreplaces the old backgrounddocker image prune— more thorough and runs synchronously before builds.- Added
apt-get clean+rm -rf /tmp/*. - The new self-test workflow (
test-free-up-disk-space.yaml) is a nice addition — asserts ≥5GiB reclaimed and specific paths removed.
5. ci/logging_config.py backport
Fixes ModuleNotFoundError: ci.logging_config that monitor_resources.py was hitting. The _pretty_value_repr uses json.dumps(value, indent=2, default=str) — the default=str makes it safe for non-JSON-serializable values (datetime, Path, etc.).
6. software-versions.yaml simplification
Removed the TMPDIR env var and /var/tmp bind mount. On ubuntu-26.04 with proper storage.conf pointing to /mnt, this should work. Validated by the test plan.
7. Codeserver swap removal
The 16GB swap fallocate for codeserver npm builds is gone. This was added when codeserver builds were hitting OOM during parallel npm postinstall. If this resurfaces, it would manifest as npm segfaults or OOM kills during npm ci. Test plan covers this (codeserver build running).
8. Triage doc updates
Correctly removes the two fixed items (ModuleNotFoundError — backported; ROCm ENOSPC — layer merge).
LGTM. Pending the in-progress build results (codeserver + full ROCm matrix).
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: ide-developer The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
New changes are detected. LGTM label has been removed. |
CI status [antigravity]Run: Build Notebooks (push) #30550792399 — 2/2 complete · 2 skipped Workflow completed with skipped jobs. |
CI status [antigravity]Run: Build Notebooks (pr) #30547666857 — 5/5 complete · 3 passed · 2 cancelled Failures so far
Likely root causes
Suggested next steps
|
Fold de-vendor-torch.sh into the USER 1001 pip layer so the giant vendored ROCm libs never land in a committed layer, cutting GHA ENOSPC peak. Keep a thin root RUN for chmod/fix-permissions only. Co-authored-by: Cursor <cursoragent@cursor.com>
4bce483 to
e237975
Compare
CI status [antigravity]Run: Build Notebooks (pr) #30550798241 — 5/5 complete · 1 passed · 4 cancelled Failures so far
Likely root causes
Suggested next steps
|
v2.12.0 cannot parse BuildKit heredoc syntax (RUN <<'EOF' … EOF); the parser treats content after the closing EOF as still inside the RUN instruction (hadolint/hadolint#1137). Bump to v2.15.0 which handles this correctly when a blank line follows the delimiter. Suppress new rules that flag pre-existing patterns: DL3064 — false positive on rstudio SECRET_DIR (a path, not a secret) DL3066 — named USER directives (intentional in our images) SC3046 — source in bash RUN blocks (not POSIX sh) (cherry picked from commit 89ac408) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
e237975 to
d173498
Compare
CI status [antigravity]Run: Build Notebooks (push) #30552740500 — 2/2 complete · 2 skipped Workflow completed with skipped jobs. |
GHA result: same-RUN de-vendor fixed ROCm pytorch ENOSPC@coderabbitai — FYI follow-up data for the incremental port tracked here (baseline already on #2647). VerdictBoth ROCm pytorch GHA builds are green on
Both attempts ran the restored Why (root cause)Podman commits a layer from the container filesystem after the
So the ENOSPC was a layer-commit peak problem, not “runner too small after free-up” alone. Aligning de-vendor with the pip layer (as on ODH main) is what moved the needle; #2650 remains the place for fuller main ports ( |
CI status [antigravity]Run: Build Notebooks (pr) #30551249055 — 5/5 complete · 2 passed · 3 cancelled Failures so far
Likely root causes
Suggested next steps
|
|
Tip For best results, initiate chat on the files or code changes.
I’ll treat this as an invariant for these ROCm PyTorch Dockerfiles: do not separate 🐇 ✅ ✏️ Learnings added
You are interacting with an AI system. |
2 participants
Summary
e4cf912dfb): remove LVM overlay + orphaned free-up-disk-space action; finish/mntPodman storage layout; drop codeserver swap; fixsoftware-versionsto use/mntwithout custom TMPDIR bindci/logging_config.pysomonitor_resources.pystopsModuleNotFoundError; drop related rows fromdocs/ci-failure-triage.mdubuntu-24.04/ubuntu-latest→ubuntu-26.04(equivalent of mainf3c39c50c9; cherry-pick conflicted with RHDS-only workflows)Test plan
Done
uv runimport ofci.logging_config+scripts.monitor_resourcessucceeds (paths/,/mnt,/mnt/containers/storage)typeCheckingMode=off)ubuntu-26.04— https://github.com/red-hat-data-services/notebooks/actions/runs/30536212561jupyter-minimal) green with/mntprepare + no codeserver swap — https://github.com/red-hat-data-services/notebooks/actions/runs/30536212782ubuntu-26.04after AppArmor pasta/podman workaround — https://github.com/red-hat-data-services/notebooks/actions/runs/30536212497In progress / outstanding
workflow_dispatchon this branch; builds everything including ROCm / CUDA / multi-arch) — https://github.com/red-hat-data-services/notebooks/actions/runs/30538388851/mntlayout (no LVM, free-up removed to match main; restore free-up in a follow-up only if needed)monitor_resourcesno longer spamsModuleNotFoundError: ci.logging_configSummary by CodeRabbit
Summary by CodeRabbit
Related issues
de-vendor-torch.sh, free-up-disk-space, jupyter + runtime ROCm pytorch Dockerfiles)