Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 16 additions & 15 deletions ci/cached-builds/gen_gha_matrix_jobs.py

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@coderabbitai evaluate in light of the AI transcript from session that pushed the PR

cursor_dockerfile_konflux_pairs.md

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧩 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 || true

Length 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.cpu

Length 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]
    Runs git 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 sorted Dockerfile.konflux.* files when present; otherwise returns sorted files matching Dockerfile*.

  • 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 with bin/buildinputs and 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 which should_build_target returns a truthy reason, and logs inclusion/exclusion decisions.

  • Lines 66–71 — get_build_directory and get_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
    Runs make (or gmake on macOS) in the specified directory, captures stdout, and raises after reporting Makefile or execution errors.

You are interacting with an AI system.

Original file line number Diff line number Diff line change
Expand Up @@ -217,28 +217,29 @@ class SelfTests(unittest.TestCase):
def test_select_changed_targets_dockerfile(self):
targets = extract_image_targets(makefile_dir=project_dir)

changed_files = ["jupyter/datascience/ubi9-python-3.11/Dockerfile.cpu"]
changed_files = ["jupyter/datascience/ubi9-python-3.12/Dockerfile.konflux.cpu"]

targets = gha_pr_changed_files.filter_out_unchanged(targets, changed_files)
assert set(targets) == {"jupyter-datascience-ubi9-python-3.11"}
assert set(targets) == {"jupyter-datascience-ubi9-python-3.12"}

def test_select_changed_targets_shared_file(self):
targets = extract_image_targets(makefile_dir=project_dir)

changed_files = ["cuda/ubi9-python-3.11/NGC-DL-CONTAINER-LICENSE"]
changed_files = ["jupyter/utils/install_pdf_deps.sh"]

# With the removal of chained builds - which now potentially has multiple Dockerfiles defined in a given
# directory, there is an inefficiency introduced to 'gha_pr_changed_files' as demonstrated by this unit test.
# Even though this test only changes a (shared) CUDA file - you will notice the 'cpu' and 'rocm' targets
# also being returned. Odds of this inefficiency noticably "hurting us" is low - so of the opinion we can
# simply treat this as technical debt.
# With multiple Dockerfile.konflux.* files per directory, should_build_target scans
# every Konflux Dockerfile in a build directory for buildinputs dependencies. A shared
# jupyter/utils change therefore fans out to CPU, CUDA, and ROCm targets (not just CUDA).
targets = gha_pr_changed_files.filter_out_unchanged(targets, changed_files)
assert set(targets) == {
"jupyter-minimal-ubi9-python-3.11",
"cuda-jupyter-minimal-ubi9-python-3.11",
"cuda-jupyter-pytorch-ubi9-python-3.11",
"runtime-cuda-pytorch-ubi9-python-3.11",
"cuda-jupyter-tensorflow-ubi9-python-3.11",
"rocm-jupyter-minimal-ubi9-python-3.11",
"runtime-cuda-tensorflow-ubi9-python-3.11",
"jupyter-minimal-ubi9-python-3.12",
"jupyter-datascience-ubi9-python-3.12",
"jupyter-trustyai-ubi9-python-3.12",
"cuda-jupyter-minimal-ubi9-python-3.12",
"cuda-jupyter-pytorch-ubi9-python-3.12",
"cuda-jupyter-pytorch-llmcompressor-ubi9-python-3.12",
"cuda-jupyter-tensorflow-ubi9-python-3.12",
"rocm-jupyter-minimal-ubi9-python-3.12",
"rocm-jupyter-pytorch-ubi9-python-3.12",
"rocm-jupyter-tensorflow-ubi9-python-3.12",
}
14 changes: 8 additions & 6 deletions ci/cached-builds/gha_pr_changed_files.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,14 @@ def get_build_dockerfile(make_target: str, env: dict[str, str] | None = None) ->


def find_dockerfiles(directory: str) -> list:
"""Finds and returns a list of files matching the pattern 'Dockerfile*' in the specified directory."""
matching_files = []
for filename in os.listdir(directory):
if fnmatch.fnmatch(filename, "Dockerfile*") and filename != "Dockerfile.konflux":
matching_files.append(filename)
return matching_files
"""Find Dockerfile(s) in a build directory for buildinputs dependency analysis."""
dir_path = pathlib.Path(directory)
if not dir_path.is_absolute():
dir_path = PROJECT_ROOT / dir_path
konflux = sorted(f for f in os.listdir(dir_path) if f.startswith("Dockerfile.konflux."))
if konflux:
return konflux
return sorted(f for f in os.listdir(dir_path) if fnmatch.fnmatch(f, "Dockerfile*"))


def _is_file_in_directory(changed_file: str, directory: str) -> bool:
Expand Down
2 changes: 1 addition & 1 deletion codeserver/ubi9-python-3.12/Dockerfile.konflux.cpu
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ set -Eeuxo pipefail
dnf install -y tar perl mesa-libGL skopeo
dnf clean all
rm -rf /var/cache/dnf
# Install the oc client (same approach as pre-hermetic Dockerfile.cpu)
# Install the oc client
curl -fsSL "https://mirror.openshift.com/pub/openshift-v4/$(uname -m)/clients/ocp/stable/openshift-client-linux.tar.gz" \
-o /tmp/openshift-client-linux.tar.gz
tar -xzf /tmp/openshift-client-linux.tar.gz -C /usr/local/bin oc
Expand Down
Loading
Loading