Skip to content
Closed
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
8 changes: 7 additions & 1 deletion .github/workflows/build-notebooks-TEMPLATE.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,13 @@ jobs:
id: prefetch
run: |
set -Eeuxo pipefail
COMPONENT_DIR=$(echo "${{ inputs.target }}" | sed 's|-|/|')
# Derive component dir from Makefile (single source of truth). Works for all
# image targets (codeserver, jupyter-*, runtime-*, rstudio-*, base-images-*).
COMPONENT_DIR=$(make -n "${{ inputs.target }}" 2>&1 | sed -n 's/.*#\*# Image build directory: <\([^>]*\)>.*/\1/p' | head -n1)
if [ -z "$COMPONENT_DIR" ]; then
echo "Could not derive COMPONENT_DIR from Makefile for target ${{ inputs.target }}" >&2
exit 1
fi
if [ -d "$COMPONENT_DIR/prefetch-input" ]; then
echo "Hermetic build detected — prefetching dependencies for $COMPONENT_DIR"
pip3 install --quiet --break-system-packages pyyaml
Expand Down
18 changes: 12 additions & 6 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ SHELL := bash
.DELETE_ON_ERROR:
MAKEFLAGS += --warn-undefined-variables
MAKEFLAGS += --no-builtin-rules
# Used where we need an empty expansion (avoids undefined variable warning).
empty :=

# todo: leave the default recipe prefix for now
ifeq ($(origin .RECIPEPREFIX), undefined)
Expand Down Expand Up @@ -85,14 +87,18 @@ define build_image
awk -F= '!/^#/ && NF {gsub(/^[ \t]+|[ \t]+$$/, "", $$1); gsub(/^[ \t]+|[ \t]+$$/, "", $$2); printf "--build-arg %s=%s ", $$1, $$2}' $(CONF_FILE); \
fi))

# Hermetic local build: when cachi2/output/ exists AND this target has a
# prefetch-input/ directory, mount pre-downloaded deps and set LOCAL_BUILD=true.
$(eval CACHI2_VOLUME := $(if $(and $(wildcard cachi2/output),$(wildcard $(BUILD_DIR)prefetch-input)),--volume $(ROOT_DIR)cachi2/output:/cachi2/output:Z --build-arg LOCAL_BUILD=true,))
# Make is only used for local and GHA builds (Konflux does not run make).
# Default to local build for hermetic-capable targets: always set LOCAL_BUILD=true
# when prefetch-input/ exists; mount cachi2/output only when it exists (after prefetch).
$(eval LOCAL_BUILD_ARG := $(if $(wildcard $(BUILD_DIR)prefetch-input),--build-arg LOCAL_BUILD=true,))
$(eval CACHI2_VOLUME := $(if $(and $(wildcard cachi2/output),$(wildcard $(BUILD_DIR)prefetch-input)),--volume $(ROOT_DIR)cachi2/output:/cachi2/output:Z,))
# Fail fast with a clear message if team runs make without prefetching first.
$(if $(wildcard $(BUILD_DIR)prefetch-input),$(if $(wildcard cachi2/output),,$(error Prefetch required for hermetic build. Run: scripts/lockfile-generators/prefetch-all.sh --component-dir $(patsubst %/,%,$(BUILD_DIR)) — see scripts/lockfile-generators/README.md)),)
Comment on lines +90 to +96
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

$(error) triggers during make -n dry-run, breaking COMPONENT_DIR derivation.

The pipeline failures show this error occurring at line 96. The issue is that the GHA workflow (line 285) runs make -n "${{ inputs.target }}" to derive COMPONENT_DIR before running prefetch-all.sh. However, the $(error ...) on line 96 is evaluated during Makefile parsing/expansion—even during a dry-run—causing the workflow to fail before it can determine whether to run prefetch.

The $(if ...) with $(error ...) is evaluated when the recipe is expanded, which happens during make -n to show what commands would run.

Consider either:

  1. Move the guard to shell code within the recipe so it only runs during actual execution, not dry-run
  2. Skip the guard during dry-run by checking a variable or using a different pattern
🐛 Proposed fix: Move guard to shell code in the recipe
 	# Fail fast with a clear message if team runs make without prefetching first.
-	$(if $(wildcard $(BUILD_DIR)prefetch-input),$(if $(wildcard cachi2/output),,$(error Prefetch required for hermetic build. Run: scripts/lockfile-generators/prefetch-all.sh --component-dir $(patsubst %/,%,$(BUILD_DIR)) — see scripts/lockfile-generators/README.md)),)
+	$(eval PREFETCH_GUARD := $(if $(wildcard $(BUILD_DIR)prefetch-input),$(if $(wildcard cachi2/output),,FAIL),))

 	$(info # Building $(IMAGE_NAME) using $(DOCKERFILE_NAME) with $(CONF_FILE) and $(BUILD_ARGS)...)

+	$(if $(PREFETCH_GUARD),`@echo` "Error: Prefetch required for hermetic build. Run: scripts/lockfile-generators/prefetch-all.sh --component-dir $(patsubst %/,%,$(BUILD_DIR)) — see scripts/lockfile-generators/README.md" >&2 && exit 1)
 	$(ROOT_DIR)/scripts/sandbox.py --dockerfile '$(2)' --platform '$(BUILD_ARCH)' -- \

Alternatively, use a shell check inside the recipe:

-	$(if $(wildcard $(BUILD_DIR)prefetch-input),$(if $(wildcard cachi2/output),,$(error Prefetch required for hermetic build. Run: scripts/lockfile-generators/prefetch-all.sh --component-dir $(patsubst %/,%,$(BUILD_DIR)) — see scripts/lockfile-generators/README.md)),)

 	$(info # Building $(IMAGE_NAME) using $(DOCKERFILE_NAME) with $(CONF_FILE) and $(BUILD_ARGS)...)

 	$(ROOT_DIR)/scripts/sandbox.py --dockerfile '$(2)' --platform '$(BUILD_ARCH)' -- \
-		$(CONTAINER_ENGINE) build $(CONTAINER_BUILD_CACHE_ARGS) $(LOCAL_BUILD_ARG) $(CACHI2_VOLUME) --platform=$(BUILD_ARCH) --label release=$(RELEASE) --tag $(IMAGE_NAME) --file '$(2)' $(BUILD_ARGS) {}\;
+		sh -c 'if [ -d "$(BUILD_DIR)prefetch-input" ] && [ ! -d "cachi2/output" ]; then echo "Error: Prefetch required for hermetic build. Run: scripts/lockfile-generators/prefetch-all.sh --component-dir $(patsubst %/,%,$(BUILD_DIR)) — see scripts/lockfile-generators/README.md" >&2; exit 1; fi' && \
+		$(CONTAINER_ENGINE) build $(CONTAINER_BUILD_CACHE_ARGS) $(LOCAL_BUILD_ARG) $(CACHI2_VOLUME) --platform=$(BUILD_ARCH) --label release=$(RELEASE) --tag $(IMAGE_NAME) --file '$(2)' $(BUILD_ARGS) {}\;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Makefile` around lines 90 - 96, The $(error ...) guard (triggered via the
conditional that checks $(wildcard $(BUILD_DIR)prefetch-input) and
cachi2/output) is being evaluated at parse/expansion time (even during make -n)
and must be deferred; replace the top-level $(if ... $(error ...)) with a
runtime check inside the recipe (or skip it when MAKEFLAGS includes -n).
Concretely, remove the $(error ...) usage from the global expansion and instead
perform the prefetch guard in shell code within the relevant target's recipe
(use the same tests on $(BUILD_DIR)prefetch-input and cachi2/output), or wrap
the existing check with a guard that detects dry-run via $(filter
-n,$(MAKEFLAGS))/$(MAKEFLAGS) so the error is suppressed during make -n; keep
references to LOCAL_BUILD_ARG, CACHI2_VOLUME and the same messages but ensure
the check runs at execution time not parse time.


$(info # Building $(IMAGE_NAME) using $(DOCKERFILE_NAME) with $(CONF_FILE) and $(BUILD_ARGS)...)

$(ROOT_DIR)/scripts/sandbox.py --dockerfile '$(2)' --platform '$(BUILD_ARCH)' -- \
$(CONTAINER_ENGINE) build $(CONTAINER_BUILD_CACHE_ARGS) $(CACHI2_VOLUME) --platform=$(BUILD_ARCH) --label release=$(RELEASE) --tag $(IMAGE_NAME) --file '$(2)' $(BUILD_ARGS) {}\;
$(CONTAINER_ENGINE) build $(CONTAINER_BUILD_CACHE_ARGS) $(LOCAL_BUILD_ARG) $(CACHI2_VOLUME) --platform=$(BUILD_ARCH) --label release=$(RELEASE) --tag $(IMAGE_NAME) --file '$(2)' $(BUILD_ARGS) {}\;
endef

# Push function for the notebook image:
Expand All @@ -111,8 +117,8 @@ endef
define image
$(eval BUILD_DIRECTORY := $(shell echo $(2) | sed 's/\/Dockerfile.*//'))
$(eval VARIANT := $(shell echo $(notdir $(2)) | cut -d. -f2))
$(eval DOCKERFILE := $(BUILD_DIRECTORY)/Dockerfile$(if $(KONFLUX:no=),.konflux,$()).$(VARIANT))
$(eval CONF_FILE := $(BUILD_DIRECTORY)/build-args/$(if $(KONFLUX:no=),konflux.,$())$(shell echo $(VARIANT)).conf)
$(eval DOCKERFILE := $(BUILD_DIRECTORY)/Dockerfile$(if $(KONFLUX:no=),.konflux,$(empty)).$(VARIANT))
$(eval CONF_FILE := $(BUILD_DIRECTORY)/build-args/$(if $(KONFLUX:no=),konflux.,$(empty))$(shell echo $(VARIANT)).conf)
$(info #*# Image build Dockerfile: <$(DOCKERFILE)> #(MACHINE-PARSED LINE)#*#...)
$(info #*# Image build directory: <$(BUILD_DIRECTORY)> #(MACHINE-PARSED LINE)#*#...)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,15 @@ artifacts:
- download_url: https://github.com/microsoft/ripgrep-prebuilt/releases/download/v13.0.0-13/ripgrep-v13.0.0-13-x86_64-unknown-linux-musl.tar.gz
checksum: sha256:324cd645481db1ceda8621409c9151fc58d182b90cc5c428db80edb21fb26df3
filename: ripgrep-v13.0.0-13-x86_64-unknown-linux-musl.tar.gz
- download_url: https://github.com/microsoft/ripgrep-prebuilt/releases/download/v13.0.0-13/ripgrep-v13.0.0-13-aarch64-unknown-linux-musl.tar.gz
checksum: sha256:0f308620a428f56fe871fcc5d7c668c461dfed3244f717b698f3e9e92aca037a
- download_url: https://github.com/microsoft/ripgrep-prebuilt/releases/download/v13.0.0-13/ripgrep-v13.0.0-13-aarch64-unknown-linux-gnu.tar.gz
checksum: sha256:1b0ca509f8707f2128f1b3ef245c3ea666d49a737431288536d49bd74652d143
filename: ripgrep-v13.0.0-13-aarch64-unknown-linux-musl.tar.gz
- download_url: https://github.com/microsoft/ripgrep-prebuilt/releases/download/v13.0.0-13/ripgrep-v13.0.0-13-powerpc64le-unknown-linux-gnu.tar.gz
checksum: sha256:a3fdb2c6ef9d4ff927ca1cb1e56f7aed7913d1be4dd4546aec400118c26452ab
filename: ripgrep-v13.0.0-13-powerpc64le-unknown-linux-gnu.tar.gz
filename: ripgrep-v13.0.0-13-powerpc64le-unknown-linux-musl.tar.gz
- download_url: https://github.com/microsoft/ripgrep-prebuilt/releases/download/v13.0.0-13/ripgrep-v13.0.0-13-s390x-unknown-linux-gnu.tar.gz
checksum: sha256:555983c74789d553b107c5a2de90b883727b3e43d680f0cd289a07407efb2755
filename: ripgrep-v13.0.0-13-s390x-unknown-linux-gnu.tar.gz
filename: ripgrep-v13.0.0-13-s390x-unknown-linux-musl.tar.gz
- download_url: https://github.com/microsoft/ripgrep-prebuilt/releases/download/v13.0.0-4/ripgrep-v13.0.0-4-powerpc64le-unknown-linux-gnu.tar.gz
checksum: sha256:3ddd7c0797c14cefd3ee61f13f15ac219bfecee8e6f6e27fd15c102ef229653a
filename: ripgrep-v13.0.0-4-powerpc64le-unknown-linux-gnu.tar.gz
Expand Down
2 changes: 1 addition & 1 deletion codeserver/ubi9-python-3.12/uv.lock.d/pylock.cpu.toml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

77 changes: 53 additions & 24 deletions scripts/lockfile-generators/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,18 +75,30 @@ gmake codeserver-ubi9-python-3.12 BUILD_ARCH=linux/arm64 PUSH_IMAGES=no
| `--component-dir DIR` | Component directory (required), e.g. `codeserver/ubi9-python-3.12` |
| `--rhds` | Use downstream (RHDS) lockfiles instead of upstream (ODH, the default) |
| `--flavor NAME` | Lock file flavor (default: `cpu`) |
| `--tekton-file FILE` | Tekton PipelineRun YAML for npm path discovery (auto-detected from `.tekton/` if omitted) |
| `--activation-key KEY` | Red Hat activation key for RHEL RPMs (optional) |
| `--org ORG` | Red Hat organization ID for RHEL RPMs (optional) |

### What it does

| Step | Condition | Script called |
|------|-----------|---------------|
| 1. Generic artifacts | `artifacts.in.yaml` exists | `create-artifact-lockfile.py` |
| 2. Pip wheels | `pyproject.toml` exists | `create-requirements-lockfile.sh --download` |
| 3. NPM packages | `package-lock.json` files found | `download-npm.sh` |
| 4. RPMs | `rpms.in.yaml` exists | `hermeto-fetch-rpm.sh` (if lockfile committed) or `create-rpm-lockfile.sh --download` |
| 1. Generic artifacts | `prefetch-input/<variant>/artifacts.in.yaml` exists | `create-artifact-lockfile.py` |
| 2. Pip wheels | `pyproject.toml` exists in component dir | `create-requirements-lockfile.sh --download` |
| 3. NPM packages | Tekton PipelineRun found for component (see below) | `download-npm.sh --tekton-file` |
| 4. RPMs | `prefetch-input/<variant>/rpms.in.yaml` exists | `hermeto-fetch-rpm.sh` (if lockfile committed) or `create-rpm-lockfile.sh --download` |

**Variant directory:** Lockfiles live under `prefetch-input/odh/` (upstream) or
`prefetch-input/rhds/` (downstream). If that directory is missing, steps 1 and 4
are skipped; steps 2 (pip) and 3 (npm) still run when their inputs exist
(`pyproject.toml`, or a Tekton file for the component).

**Step 3 (NPM):** The script finds the Tekton file automatically via
`find_tekton_yaml`: it looks for a `.tekton/*pull-request*.yaml` whose
`dockerfile` param matches this component — RHDS first
(`COMPONENT_DIR/Dockerfile.konflux.*`), then ODH (`COMPONENT_DIR/Dockerfile.*`).
If no Tekton file is found, npm is skipped. If the Tekton file has no
`npm`-type `prefetch-input` entries, `download-npm.sh` exits successfully
(nothing to download).

Steps are skipped if their input files don't exist. For RPMs, if
`rpms.lock.yaml` is already committed, it downloads directly (skipping
Expand All @@ -95,11 +107,13 @@ lockfile regeneration) — this avoids cross-platform issues on arm64 CI runners
### GitHub Actions integration

The GHA workflow template (`.github/workflows/build-notebooks-TEMPLATE.yaml`)
calls `prefetch-all.sh` automatically for codeserver targets before running
`make`. Non-codeserver targets skip the prefetch step entirely. After the
build, container tests run (e.g. `tests/containers` with pytest); image
metadata is read from both Docker `Config` and `ContainerConfig` so labels
work when the daemon is Podman (see
derives the component directory from the **Makefile** (dry-run of the build
target, parsing `#*# Image build directory: <...>`), so it works for all image
targets (codeserver, jupyter-*, runtime-*, rstudio-*, base-images-*). Prefetch
runs when `COMPONENT_DIR/prefetch-input` exists; otherwise the step is skipped.
After the build, container tests run (e.g. `tests/containers` with pytest);
image metadata is read from both Docker `Config` and `ContainerConfig` so
labels work when the daemon is Podman (see
[tests/containers/docs/github-vs-local-image-metadata.md](../../tests/containers/docs/github-vs-local-image-metadata.md)).

**uv version:** The repo root `uv.toml` specifies the `uv` version (e.g.
Expand Down Expand Up @@ -516,7 +530,8 @@ collisions. Files that already exist are skipped.
- `--lock-file <path>` — process a single `package-lock.json`.
- `--tekton-file <path>` — parse a Tekton PipelineRun YAML to discover all
`npm`-type `prefetch-input` paths, then process every `package-lock.json`
found under them.
found under them. If the file has **no** `npm`-type entries, the script
exits 0 (nothing to download) instead of erroring.

Both flags can be combined. URLs that are already local (`file:///cachi2/...`)
are automatically skipped.
Expand Down Expand Up @@ -712,34 +727,48 @@ python3 scripts/lockfile-generators/helpers/download-pip-packages.py \
After running `prefetch-all.sh`, the **recommended** way to build is via make:

```bash
# Makefile auto-detects cachi2/output/ and injects --volume + LOCAL_BUILD=true
# Make sets LOCAL_BUILD=true for hermetic targets; mounts cachi2/output when it exists
gmake codeserver-ubi9-python-3.12 BUILD_ARCH=linux/arm64 PUSH_IMAGES=no
```

The Makefile evaluates each target independently: `CACHI2_VOLUME` is only set
when both `cachi2/output/` exists AND the target directory has a
`prefetch-input/` subdirectory. Non-hermetic targets are completely unaffected.
The Makefile sets `LOCAL_BUILD=true` for any target that has `prefetch-input/`;
it adds the cachi2 volume only when `cachi2/output/` exists (after prefetch).
Non-hermetic targets are unaffected.

### Alternative: manual podman build

For developers who want to run `podman build` directly, the key flags are:
Running `podman build` directly differs from `gmake` in these ways:

- `-v $(realpath ./cachi2):/cachi2:z` bind-mount the prefetched dependencies
so the Dockerfile can install from them offline.
- `--build-arg LOCAL_BUILD=true` signals the Dockerfile that this is a local
build (configures dnf to use the local cachi2 RPM repo).
| Aspect | `gmake codeserver-ubi9-python-3.12 BUILD_ARCH=... PUSH_IMAGES=no` | Manual `podman build ...` |
|--------|-------------------------------------------------------------------|---------------------------|
| **Build context** | Minimal (via `scripts/sandbox.py`: only files needed by the Dockerfile) | Full repo (`.`). |
| **Volume** | `--volume $(ROOT_DIR)cachi2/output:/cachi2/output:Z` (mounts only `cachi2/output`) | Often `-v ./cachi2:/cachi2` (mounts whole dir); equivalent is `-v ./cachi2/output:/cachi2/output:z`. |
| **Build args** | From `build-args/cpu.conf`: `INDEX_URL`, `BASE_IMAGE`, `PYLOCK_FLAVOR` | You must pass these (and `LOCAL_BUILD=true`) explicitly. |
| **Tag** | `$(IMAGE_REGISTRY):codeserver-ubi9-python-3.12-$(RELEASE)_$(DATE)` | Whatever you pass with `-t`. |
| **Label** | `--label release=$(RELEASE)` | Omitted unless you add it. |
| **Cache** | Default `CONTAINER_BUILD_CACHE_ARGS ?= --no-cache` | Podman uses its default cache unless you pass `--no-cache`. |

To approximate the make build when running podman manually, use the same volume
path as make and pass all build-args from `build-args/cpu.conf`:

- Bind-mount **only** `cachi2/output` at `/cachi2/output` (same as make).
- Pass `LOCAL_BUILD=true` and the same `BASE_IMAGE`, `PYLOCK_FLAVOR`, and
`INDEX_URL` as in `codeserver/ubi9-python-3.12/build-args/cpu.conf`.

```bash
# Same volume path as Makefile; build-args from build-args/cpu.conf
podman build \
-f codeserver/ubi9-python-3.12/Dockerfile.cpu \
--platform linux/amd64 \
--platform linux/arm64 \
-t code-server-test \
--build-arg LOCAL_BUILD=true \
--build-arg BASE_IMAGE=quay.io/opendatahub/odh-base-image-cpu-py312-c9s:latest \
--build-arg PYLOCK_FLAVOR=cpu \
-v "$(realpath ./cachi2):/cachi2:z" \
--build-arg INDEX_URL=https://console.redhat.com/api/pypi/public-rhai/rhoai/3.4-EA1/cpu-ubi9/simple/ \
-v "$(realpath ./cachi2/output):/cachi2/output:z" \
.
```

To build for a different architecture, change `--platform` and `ARCH`
accordingly (e.g. `linux/arm64` / `aarch64`, `linux/ppc64le` / `ppc64le`).
To build for a different architecture, change `--platform` (e.g. `linux/amd64`,
`linux/arm64`, `linux/ppc64le`). The manual command uses the **full repo** as
context; make uses a **sandboxed** context for reproducibility.
3 changes: 2 additions & 1 deletion scripts/lockfile-generators/download-npm.sh
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,8 @@ if [[ -n "$TEKTON_FILE" ]]; then
npm_paths=$(extract_npm_paths_from_tekton "$TEKTON_FILE")

if [[ -z "$npm_paths" ]]; then
error_exit "No npm-type prefetch-input entries found in $TEKTON_FILE"
echo "No npm-type prefetch-input entries in $TEKTON_FILE — nothing to download"
exit 0
fi

while IFS= read -r npm_path; do
Expand Down
Loading
Loading