[rhoai-2.25] chore: remove orphan Dockerfiles and PDF install scripts - #2694
Conversation
The Konflux minimal CPU Dockerfile had a pdf-builder stage that was never referenced by COPY --from; PDF export already uses install_pdf_deps.sh. Delete the unused stage and uninstall_texlive/install_pandoc helpers. Co-authored-by: Cursor <cursoragent@cursor.com>
Delete Dockerfile.cpu/cuda/rocm where Dockerfile.konflux.* is the build source of truth for Makefile, Tekton, and GHA. RStudio Dockerfile.cpu/cuda remain as Konflux symlink targets. Co-authored-by: Cursor <cursoragent@cursor.com>
Update manifest accelerator detection, PR changed-file Dockerfile discovery, and docs/tests to reference Dockerfile.konflux.* after removing legacy Dockerfile.cpu/cuda/rocm from jupyter and runtimes. Co-authored-by: Cursor <cursoragent@cursor.com>
Replace stale cuda/ubi9-python-3.11 license path and 3.11 target names with jupyter/utils/install_pdf_deps.sh and current 3.12 Makefile targets. Co-authored-by: Cursor <cursoragent@cursor.com>
📝 WalkthroughWalkthroughThe PR updates Python 3.12 changed-target detection for Konflux Dockerfiles, removes legacy CPU, CUDA, and ROCm image builds, removes Pandoc and TeX Live installer scripts, and updates accelerator filename references. ChangesPython 3.12 Konflux migration
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Skipping CI for Draft Pull Request. |
CI status [antigravity]Run: Build Notebooks (push) #30813437428 — 2/2 complete · 2 skipped Workflow completed with skipped jobs. |
There was a problem hiding this comment.
@coderabbitai evaluate in light of the AI transcript from session that pushed the PR
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
attachment_url='https://github.com/user-attachments/files/30661078/cursor_dockerfile_konflux_pairs.md'
tmpfile="$(mktemp)"
trap 'rm -f "$tmpfile"' EXIT
curl --fail --location --silent --show-error "$attachment_url" -o "$tmpfile"
printf '%s\n' '--- AI session transcript ---'
sed -n '1,260p' "$tmpfile"
printf '%s\n' '--- Related module candidates ---'
fd -t f 'gha_pr_changed_files\.py$' .
printf '%s\n' '--- Outline of related module ---'
module="$(fd -t f 'gha_pr_changed_files\.py$' . | head -n 1)"
if [ -n "$module" ]; then
ast-grep outline "$module" --items all
printf '%s\n' '--- Target filtering implementation ---'
rg -n -C 8 'def (filter_out_unchanged|should_build_target)\b|Dockerfile\.konflux|buildinputs' "$module"
fi
printf '%s\n' '--- Referenced build inputs ---'
rg -n -C 3 'install_pdf_deps\.sh|Dockerfile\.konflux\.(cpu|cuda|rocm)' jupyter 2>/dev/null || trueLength of output: 26924
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- All Konflux Dockerfiles that reference install_pdf_deps.sh ---'
rg -l -g 'Dockerfile.konflux.*' 'install_pdf_deps\.sh' . | sort
printf '%s\n' '--- 3.12 image-target mappings for the expected build directories ---'
rg -n -C 2 \
'jupyter/(minimal|datascience|trustyai|pytorch(\+llmcompressor)?|tensorflow|rocm/(pytorch|tensorflow))/ubi9-python-3\.12' \
Makefile makefiles . 2>/dev/null || true
printf '%s\n' '--- Direct Dockerfile test fixture exists and is a selected Konflux Dockerfile ---'
ls -l jupyter/datascience/ubi9-python-3.12/Dockerfile.konflux.cpuLength of output: 26582
@jiridanek, the transcript and the repository evidence support this change.
test_select_changed_targets_dockerfile now uses jupyter/datascience/ubi9-python-3.12/Dockerfile.konflux.cpu. This is correct because find_dockerfiles() selects Dockerfile.konflux.* files when they exist.
The shared-file test is also correct. The repository contains exactly ten Konflux Dockerfiles that reference jupyter/utils/install_pdf_deps.sh. The asserted target set has one target for each of those Dockerfiles:
- CPU: minimal, datascience, trustyai
- CUDA: minimal, pytorch, pytorch-llmcompressor, tensorflow
- ROCm: minimal, pytorch, tensorflow
should_build_target() scans all selected Konflux Dockerfiles in a build directory. Therefore, this fan-out is the expected current behavior.
The transcript also identifies the same 3.12 Konflux Dockerfile structure. The repository scan independently confirms the test fixture and the full ten-target expectation.
No issue found in this diff.
🧬 Code Graph Analysis Results
ci/cached-builds/gha_pr_changed_files.py
-
Lines 22–34 —
list_changed_files(from_ref, to_ref) -> list[str]
Runsgit diff --name-only <from_ref>...<to_ref> --and returns the PR’s changed file paths. Propagates subprocess failures. -
Lines 74–84 —
find_dockerfiles(directory) -> list
Resolves the build directory and returns sortedDockerfile.konflux.*files when present; otherwise returns sorted files matchingDockerfile*. -
Lines 92–121 —
should_build_target(changed_files, target_directory) -> str
Returns the first changed file inside the target directory, or scans every Dockerfile in that directory withbin/buildinputsand returns the first changed dependency. Returns""when no relevant change exists; propagates subprocess and JSON errors. -
Lines 124–136 —
filter_out_unchanged(targets, changed_files) -> list[str]
Determines each target’s build directory using its Python version, retains targets for whichshould_build_targetreturns a truthy reason, and logs inclusion/exclusion decisions. -
Lines 66–71 —
get_build_directoryandget_build_dockerfile
Query Makefile metadata for a target’s build directory or Dockerfile via_query_build; raises if the Make query does not produce exactly one result.
ci/cached-builds/makefile_helper.py
-
Lines 42–52 —
dry_run_makefile(target, makefile_dir, env) -> str
Executes the Makefile in dry-run/database mode with supplied environment variables and returns stdout. -
Lines 14–39 —
exec_makefile(target, makefile_dir, options) -> str
Runsmake(orgmakeon macOS) in the specified directory, captures stdout, and raises after reporting Makefile or execution errors.
You are interacting with an AI system.
Fix pre-commit ruff-format failure on pytest-tests workflow. Co-authored-by: Cursor <cursoragent@cursor.com>
CI status [antigravity]Run: Build Notebooks (push) #30813629618 — 2/2 complete · 2 skipped Workflow completed with skipped jobs. |
CI status [antigravity]Run: Build Notebooks (pr) #30813449775 — 28/28 complete · 1 passed · 27 cancelled Failures so far
(+ 19 other failed matrix jobs) Likely root causes
Suggested next steps
|
jiridanek
changed the title
|
/build-konflux |
![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
🤖 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 `@scripts/lockfile-generators/README.md`:
- Line 881: Align the custom flavor Dockerfile naming contract across the
README, the --flavor option description, and create-requirements-lockfile.sh
validation; choose either Dockerfile.konflux.<flavor> or Dockerfile.<flavor>,
then update all documented examples and checks to use that same convention.
🪄 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: Pro Plus
Run ID: ff86f637-0e30-4d79-8539-3e6b01140697
📒 Files selected for processing (26)
ci/cached-builds/gen_gha_matrix_jobs.pyci/cached-builds/gha_pr_changed_files.pycodeserver/ubi9-python-3.12/Dockerfile.konflux.cpujupyter/datascience/ubi9-python-3.12/Dockerfile.cpujupyter/minimal/ubi9-python-3.12/Dockerfile.cpujupyter/minimal/ubi9-python-3.12/Dockerfile.cudajupyter/minimal/ubi9-python-3.12/Dockerfile.konflux.cpujupyter/minimal/ubi9-python-3.12/Dockerfile.rocmjupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.cudajupyter/pytorch/ubi9-python-3.12/Dockerfile.cudajupyter/rocm/pytorch/ubi9-python-3.12/Dockerfile.rocmjupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.rocmjupyter/tensorflow/ubi9-python-3.12/Dockerfile.cudajupyter/trustyai/ubi9-python-3.12/Dockerfile.cpujupyter/utils/install_pandoc.shjupyter/utils/install_texlive.shruntimes/datascience/ubi9-python-3.12/Dockerfile.cpuruntimes/minimal/ubi9-python-3.12/Dockerfile.cpuruntimes/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.cudaruntimes/pytorch/ubi9-python-3.12/Dockerfile.cudaruntimes/rocm-pytorch/ubi9-python-3.12/Dockerfile.rocmruntimes/rocm-tensorflow/ubi9-python-3.12/Dockerfile.rocmruntimes/tensorflow/ubi9-python-3.12/Dockerfile.cudascripts/buildinputs/buildinputs_test.goscripts/lockfile-generators/README.mdtests/manifests.py
💤 Files with no reviewable changes (20)
- runtimes/rocm-pytorch/ubi9-python-3.12/Dockerfile.rocm
- jupyter/rocm/pytorch/ubi9-python-3.12/Dockerfile.rocm
- jupyter/pytorch/ubi9-python-3.12/Dockerfile.cuda
- runtimes/tensorflow/ubi9-python-3.12/Dockerfile.cuda
- jupyter/utils/install_texlive.sh
- runtimes/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.cuda
- runtimes/minimal/ubi9-python-3.12/Dockerfile.cpu
- jupyter/tensorflow/ubi9-python-3.12/Dockerfile.cuda
- jupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.cuda
- jupyter/minimal/ubi9-python-3.12/Dockerfile.konflux.cpu
- jupyter/utils/install_pandoc.sh
- jupyter/minimal/ubi9-python-3.12/Dockerfile.cuda
- jupyter/trustyai/ubi9-python-3.12/Dockerfile.cpu
- jupyter/datascience/ubi9-python-3.12/Dockerfile.cpu
- runtimes/rocm-tensorflow/ubi9-python-3.12/Dockerfile.rocm
- jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.rocm
- runtimes/pytorch/ubi9-python-3.12/Dockerfile.cuda
- jupyter/minimal/ubi9-python-3.12/Dockerfile.rocm
- runtimes/datascience/ubi9-python-3.12/Dockerfile.cpu
- jupyter/minimal/ubi9-python-3.12/Dockerfile.cpu
| --pyproject-toml codeserver/ubi9-python-3.12/pyproject.toml | ||
|
|
||
| # Custom flavor (e.g. cuda — requires Dockerfile.cuda and build-args/cuda.conf) | ||
| # Custom flavor (e.g. cuda — requires Dockerfile.konflux.cuda and build-args/cuda.conf) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- README and script filename rules ---'
rg -n -C 4 'Dockerfile(\.konflux)?|--flavor|PYLOCK_FLAVOR' \
scripts/lockfile-generators/README.md \
scripts/lockfile-generators/create-requirements-lockfile.sh
printf '%s\n' '--- Codeserver Dockerfiles ---'
fd -t f '^Dockerfile' codeserver/ubi9-python-3.12 | sortRepository: red-hat-data-services/notebooks
Length of output: 20674
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- Generator option parsing and Dockerfile path usage ---'
sed -n '1,240p' scripts/pylocks_generator.py | nl -ba
printf '%s\n' '--- Deterministic path construction probe from generator source ---'
python3 - <<'PY'
from pathlib import Path
import ast, sys
path = Path("scripts/pylocks_generator.py")
tree = ast.parse(path.read_text())
for node in tree.body:
if isinstance(node, ast.Expr) and isinstance(node.value, ast.Constant) and isinstance(node.value.value, str):
print("TOPLEVEL:", node.lineno, repr(node.value.value[:200]))
for node in ast.walk(tree):
if isinstance(node, ast.Assign):
src = ast.get_source_segment(path.read_text(), node)
if any(s in src for s in ["flavor", "dockerfile", "docker_file", "Dockerfile", "PYLOCK_FLAVOR"]):
print(node.lineno, src.strip().replace("\n", " "))
PYRepository: red-hat-data-services/notebooks
Length of output: 268
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- Generator option parsing and Dockerfile path usage ---'
awk '{printf "%5d %s\n", NR, $0}' scripts/pylocks_generator.py | sed -n '1,240p'
printf '%s\n' '--- Read-only source probe for flavor-related assignments/expressions ---'
python3 - <<'PY'
from pathlib import Path
tree = None
try:
import ast
tree = ast.parse(Path("scripts/pylocks_generator.py").read_text())
except Exception as e:
print("NO_AST:", repr(e))
text = Path("scripts/pylocks_generator.py").read_text().splitlines()
for i, line in enumerate(text, 1):
if any(term in line for term in ["flavor", "flavour", "dockerfile", "docker_file", "Dockerfile", "PYLOCK_FLAVOR"]):
print(f"{i}: {line}")
PYRepository: red-hat-data-services/notebooks
Length of output: 13746
Align the documented Dockerfile filename contract.
--flavor NAME uses Dockerfile.konflux.<flavor> in rh-index mode, but scripts/lockfile-generators/create-requirements-lockfile.sh documents Dockerfile.<flavor>. Update the option description and script validation, or change the example/README to use Dockerfile.<flavor> consistently.
🤖 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 `@scripts/lockfile-generators/README.md` at line 881, Align the custom flavor
Dockerfile naming contract across the README, the --flavor option description,
and create-requirements-lockfile.sh validation; choose either
Dockerfile.konflux.<flavor> or Dockerfile.<flavor>, then update all documented
examples and checks to use that same convention.
There was a problem hiding this comment.
@coderabbitai create github issue for me to do subsequently, I don't want to fix this in the same PR because that would retrigger all builds if I push any more changes
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
repo="red-hat-data-services/notebooks"
body=$(cat <<'EOF'
## Summary
Align the Dockerfile filename contract for custom `--flavor` lockfile generation.
The documentation currently refers to `Dockerfile.konflux.<flavor>` in one location. Other `--flavor` documentation and validation refer to `Dockerfile.<flavor>`.
## Required changes
Choose one Dockerfile naming convention:
- `Dockerfile.konflux.<flavor>`, or
- `Dockerfile.<flavor>`
Update all related documentation, examples, and validation to use the selected convention consistently.
## Affected areas
- `scripts/lockfile-generators/README.md`
- Custom flavor examples.
- `--flavor` option description.
- `scripts/lockfile-generators/create-requirements-lockfile.sh`
- Dockerfile validation for a custom flavor.
## Rationale
A single filename contract prevents incorrect custom-flavor instructions and validation failures.
## Acceptance criteria
- The README uses one Dockerfile filename convention for custom flavors.
- The `--flavor` option description uses that convention.
- `create-requirements-lockfile.sh` validates the same convention.
- Relevant examples use the same convention.
## Backlinks
- Deferred from PR `#2694`: https://github.com/red-hat-data-services/notebooks/pull/2694
- Review comment: https://github.com/red-hat-data-services/notebooks/pull/2694#discussion_r3705223077
- Requested by: `@jiridanek`
EOF
)
gh issue create \
--repo "$repo" \
--title "Align custom flavor Dockerfile naming for lockfile generation" \
--assignee jiridanek \
--body "$body"Length of output: 1796
@jiridanek, I created and assigned #2700: Align custom flavor Dockerfile naming for lockfile generation.
You are interacting with an AI system.
CI status [antigravity]Run: Build Notebooks (pr) #30813631319 — 28/28 complete · 27 passed · 1 failed Failures so far
Likely root causes
Suggested next steps
|
|
/build-runtime-pytorch-rocm |
|
/build-minimal-rocm |
|
/build-pytorch-rocm |
|
/build-tensorflow-cuda |
|
/build-tensorflow-rocm |
|
/build-trustyai |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: atheo89 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/build-runtime-pytorch-rocm |
|
/build-minimal-rocm |
|
/build-pytorch-rocm |
|
/build-tensorflow-rocm |
|
/test odh-workbench-jupyter-tensorflow-rocm-py312-on-pull-request |
|
@jiridanek: No presubmit jobs available for red-hat-data-services/notebooks@rhoai-2.25 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
3 participants
Description
Konflux is the sole build path for jupyter and runtime workbench images on
rhoai-2.25(Makefile, Tekton, and GHA all useDockerfile.konflux.*). This PR removes dead artifacts left from the dual-Dockerfile era:pdf-builderstage fromjupyter/minimalKonflux CPU Dockerfile and remove orphanedinstall_texlive.sh/install_pandoc.sh(PDF export usesinstall_pdf_deps.sh)Dockerfile.cpu/.cuda/.rocmfiles underjupyter/andruntimes/where Konflux pairs existKept:
rstudio/rhel9-python-3.11/Dockerfile.cpuandDockerfile.cuda(Konflux symlink targets), and allbase-images/**Dockerfiles (no Konflux pairs).AI Transcript
cursor_dockerfile_konflux_pairs.md
How Has This Been Tested?
python3 -m unittest gen_gha_matrix_jobs.SelfTests gha_pr_changed_files.SelfTests(fromci/cached-builds/)Self checklist (all need to be checked):
make test(gmakeon macOS) before asking for reviewDockerfile.konfluxfiles should be done inodh/notebooksand automatically synced torhds/notebooks. For Konflux-specific changes, modifyDockerfile.konfluxfiles directly inrhds/notebooksas these require special attention in the downstream repository and flow to the upcoming RHOAI release.Merge criteria:
Made with Cursor
Summary by CodeRabbit
Build and Image Updates
Documentation