-
Notifications
You must be signed in to change notification settings - Fork 52
[WIP] konflux: hermetic build #748
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,9 +27,17 @@ spec: | |
| value: quay.io/redhat-user-workloads/lightspeed-core-tenant/lightspeed-stack:on-pr-{{revision}} | ||
| - name: image-expires-after | ||
| value: 5d | ||
| # todo: add arm64. refer to https://konflux.pages.redhat.com/docs/users/getting-started/multi-platform-builds.html#arm64-2 | ||
| - name: build-platforms | ||
| value: | ||
| - linux/x86_64 | ||
| # todo: change on-push pipeline,too | ||
| - name: build-source-image | ||
| value: 'true' | ||
| - name: prefetch-input | ||
| value: '[{"type": "rpm", "path": "."}, {"type": "pip", "path": ".", "allow_binary": "true", "requirements_files": ["requirements.txt", "requirements.hermetic.txt"]}]' | ||
| - name: hermetic | ||
| value: 'true' | ||
|
Comment on lines
+35
to
+40
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainVerify requirements.hermetic.txt is included in Containerfile and properly used. The prefetch-input parameter references both "requirements.txt" and "requirements.hermetic.txt" in the pip requirements_files array (line 38), but the Containerfile hermetic build path (line 33) only installs from requirements.txt. If requirements.hermetic.txt contains additional hermetic-specific dependencies, they may not be installed. Run the following to check if requirements.hermetic.txt exists and is used: 🏁 Script executed: #!/bin/bash
# Check if requirements.hermetic.txt exists and its usage
echo "=== Check if requirements.hermetic.txt exists ==="
if [ -f requirements.hermetic.txt ]; then
echo "✓ requirements.hermetic.txt exists"
echo "First 10 lines:"
head -10 requirements.hermetic.txt
else
echo "✗ requirements.hermetic.txt not found"
fi
echo -e "\n=== Check Containerfile for requirements.hermetic.txt usage ==="
rg -n "requirements\.hermetic\.txt" ContainerfileLength of output: 806 🏁 Script executed: #!/bin/bash
# Examine Containerfile for pip/requirements usage and build logic
echo "=== Containerfile pip install and requirements handling ==="
rg -n -A3 -B3 "requirements\.txt|pip install" Containerfile
echo -e "\n=== Check Containerfile structure for hermetic build ==="
cat ContainerfileLength of output: 4420 Add requirements.hermetic.txt to Containerfile and use it in the hermetic build path. Verification confirms the issue: requirements.hermetic.txt exists with pinned dependencies (autogenerated by uv), but the Containerfile never references it. Line 25 only copies requirements.txt, and line 33 (hermetic build path) installs from requirements.txt, not requirements.hermetic.txt. This defeats the purpose of prefetching hermetic-specific dependencies in the Tekton task. Fix required:
🤖 Prompt for AI Agents |
||
| - name: dockerfile | ||
| value: Containerfile | ||
| pipelineSpec: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,10 +22,18 @@ RUN pip3.12 install "uv==0.8.15" | |
| # Add explicit files and directories | ||
| # (avoid accidental inclusion of local directories or env files or credentials) | ||
| COPY ${LSC_SOURCE_DIR}/src ./src | ||
| COPY ${LSC_SOURCE_DIR}/pyproject.toml ${LSC_SOURCE_DIR}/LICENSE ${LSC_SOURCE_DIR}/README.md ${LSC_SOURCE_DIR}/uv.lock ./ | ||
| COPY ${LSC_SOURCE_DIR}/pyproject.toml ${LSC_SOURCE_DIR}/LICENSE ${LSC_SOURCE_DIR}/README.md ${LSC_SOURCE_DIR}/uv.lock ${LSC_SOURCE_DIR}/requirements.txt ./ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainVerify requirements.txt is properly generated and kept in sync. The hermetic build path relies on requirements.txt (line 33), but a past review comment noted that the Makefile still uses an outdated Run the following to verify requirements.txt generation is using uv: 🏁 Script executed: #!/bin/bash
# Check how requirements.txt is generated
echo "=== Check Makefile requirements.txt target ==="
rg -n "requirements\.txt" Makefile -A 2 -B 2
echo -e "\n=== Check header of requirements.txt for generation method ==="
head -5 requirements.txtLength of output: 769 Update Makefile to use uv for requirements.txt generation. The Makefile still uses Update line 44 in the Makefile to: 🤖 Prompt for AI Agents |
||
|
|
||
| # Bundle additional dependencies for library mode. | ||
| RUN uv sync --locked --no-dev --group llslibdev | ||
| # Source cachi2 environment for hermetic builds if available, otherwise use normal installation | ||
| # cachi2.env has these env vars: | ||
| # PIP_FIND_LINKS=/cachi2/output/deps/pip | ||
| # PIP_NO_INDEX=true | ||
| RUN if [ -f /cachi2/cachi2.env ]; then \ | ||
| . /cachi2/cachi2.env && uv venv --seed --no-index --find-links ${PIP_FIND_LINKS} && . .venv/bin/activate && pip install --no-index --find-links ${PIP_FIND_LINKS} -r requirements.txt; \ | ||
| else \ | ||
| uv sync --locked --no-dev --group llslibdev; \ | ||
| fi | ||
|
|
||
| # Explicitly remove some packages to mitigate some CVEs | ||
| # - GHSA-wj6h-64fc-37mp: python-ecdsa package won't fix it upstream. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -87,8 +87,23 @@ Issues = "https://github.com/lightspeed-core/lightspeed-stack/issues" | |
| name = "pytorch-cpu" | ||
| url = "https://download.pytorch.org/whl/cpu" | ||
| explicit = true | ||
|
|
||
| [[tool.uv.index]] | ||
| name = "pypi-default" | ||
| url = "https://pypi.org/simple" | ||
| explicit = true | ||
| [tool.uv.sources] | ||
| torch = [{ index = "pytorch-cpu" }] | ||
| torch = [ | ||
| { index = "pytorch-cpu", group = "llslibdev" }, | ||
| { index = "pypi-default", group = "llslibdev-hermetic" } | ||
| ] | ||
| [tool.uv] | ||
| conflicts = [ | ||
| [ | ||
| { group = "llslibdev" }, | ||
| { group = "llslibdev-hermetic" }, | ||
| ], | ||
| ] | ||
|
|
||
| [dependency-groups] | ||
| dev = [ | ||
|
|
@@ -159,6 +174,50 @@ llslibdev = [ | |
| "blobfile>=3.0.0", | ||
| "psutil>=7.0.0", | ||
| ] | ||
| llslibdev-hermetic = [ | ||
| # the same as llslibdev, just using default index. | ||
| "matplotlib>=3.10.0", | ||
| "pillow>=11.1.0", | ||
| "pandas>=2.2.3", | ||
| "scikit-learn>=1.5.2", | ||
| "psycopg2-binary>=2.9.10", | ||
| # API eval: inline::meta-reference | ||
| "tree_sitter>=0.24.0", | ||
| "pythainlp>=3.0.10", | ||
| "langdetect>=1.0.9", | ||
| "emoji>=2.1.0", | ||
| "nltk>=3.8.1", | ||
| # API inference: remote::gemini | ||
| "litellm>=1.75.5.post1", | ||
| # API inference: inline::sentence-transformers | ||
| "sentence-transformers>=5.0.0", | ||
| # API vector_io: inline::faiss | ||
| "faiss-cpu>=1.11.0", | ||
| # API scoring: inline::basic | ||
| "requests>=2.32.4", | ||
| # API datasetio: inline::localfs | ||
| "aiosqlite>=0.21.0", | ||
| # API datasetio: remote::huggingface | ||
| "datasets>=3.6.0", | ||
| # API telemetry: inline::meta-reference | ||
| "opentelemetry-sdk>=1.34.1", | ||
| "opentelemetry-exporter-otlp>=1.34.1", | ||
| # API tool_runtime: inline::rag-runtime | ||
| "transformers>=4.34.0", | ||
| "numpy==2.2.6", | ||
| # API tool_runtime: remote::model-context-protocol | ||
| "mcp>=1.9.4", | ||
| # API post_training: inline::huggingface | ||
| "torch==2.7.1; sys_platform == 'linux'", | ||
| "trl>=0.18.2", | ||
| "peft>=0.15.2", | ||
| # Other | ||
| "autoevals>=0.0.129", | ||
| "fire>=0.7.0", | ||
| "opentelemetry-instrumentation>=0.55b0", | ||
| "blobfile>=3.0.0", | ||
| "psutil>=7.0.0", | ||
| ] | ||
|
Comment on lines
+177
to
+220
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Inconsistent torch platform marker—creates behavioral divergence. The
This contradicts the comment at line 178 stating the groups are "the same". If the Linux restriction is intentional for hermetic builds, document why. Otherwise, remove the platform marker for consistency. Apply this diff if the platform restriction is unintentional: - "torch==2.7.1; sys_platform == 'linux'",
+ "torch==2.7.1",🤖 Prompt for AI Agents |
||
|
|
||
| build = [ | ||
| "build>=1.2.2.post1", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| uv==0.8.15 | ||
| pip==24.2 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| # This file was autogenerated by uv via the following command: | ||
| # uv pip compile requirements.hermetic.in -o requirements.hermetic.txt --generate-hashes | ||
| pip==24.2 \ | ||
| --hash=sha256:2cd581cf58ab7fcfca4ce8efa6dcacd0de5bf8d0a3eb9ec927e07405f4d9e2a2 \ | ||
| --hash=sha256:5b5e490b5e9cb275c879595064adce9ebd31b854e3e803740b72f9ccf34a45b8 | ||
| # via -r requirements.hermetic.in | ||
| uv==0.8.15 \ | ||
| --hash=sha256:069eed78b79d1e88bced23e3d4303348edb0a0209e7cae0f20024c42430bf50f \ | ||
| --hash=sha256:07765f99fd5fd3b257d7e210e8d0844c0a8fd111612e31fcca66a85656cc728e \ | ||
| --hash=sha256:226360003e71084e0a73cbec72170e88634b045e95529654d067ea3741bba242 \ | ||
| --hash=sha256:3330bb4f206a6180679a75a8b2e77ff0f933fcb06c028b6f4da877b10a5e4f95 \ | ||
| --hash=sha256:333a93bb6af64f3b95ee99e82b4ea227e2af6362c45f91c89a24e2bfefb628f9 \ | ||
| --hash=sha256:3780d2f3951d83e55812fdeb7eee233787b70c774497dbfc55b0fdf6063aa345 \ | ||
| --hash=sha256:3ec78a54a8eb0bbb9a9c653982390af84673657c8a48a0be6cdcb81d7d3e95c3 \ | ||
| --hash=sha256:56f2451c9193ee1754ce1d8390ded68e9cb8dee0aaf7e2f38a9bd04d99be1be7 \ | ||
| --hash=sha256:6aa824ab933dfafe11efe32e6541c6bcd65ecaa927e8e834ea6b14d3821020f6 \ | ||
| --hash=sha256:7d5b19ac2bdda3d1456b5d6013af50b443ffb0e40c66d42874f71190a5364711 \ | ||
| --hash=sha256:89c7c10089e07d944c72d388fd88666c650dec2f8c79ca541e365f32843882c6 \ | ||
| --hash=sha256:8ea57b78be9f0911a2a50b6814d15aec7d1f8aa6517059dc8250b1414156f93a \ | ||
| --hash=sha256:9488260536b35b94a79962fea76837f279c0cd0ae5021c761e66b311f47ffa70 \ | ||
| --hash=sha256:a022a752d20da80d2a49fc0721522a81e3a32efe539152d756d84ebdba29dbc3 \ | ||
| --hash=sha256:a395fa1fc8948eacdd18e4592ed489fad13558b13fea6b3544cb16e5006c5b02 \ | ||
| --hash=sha256:b0461bb1ad616c8bcb59c9b39ae9363245ca33815ebb1d11130385236eca21b9 \ | ||
| --hash=sha256:c4868e6a4e1a8c777a5ba3cff452c405837318fb0b272ff203bfda0e1b8fc54d \ | ||
| --hash=sha256:de9896ad4fa724ab317a8048f4891b9b23df1403b3724e96606f3be2dbbbf009 \ | ||
| --hash=sha256:f02e6b8be08b840f86b8d5997b658b657acdda95bc216ecf62fce6c71414bdc7 | ||
| # via -r requirements.hermetic.in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incomplete TODOs block mergeability.
Lines 30 and 34 contain TODO comments that indicate incomplete work:
These must be resolved or tracked as separate issues before merge.
Create follow-up issues or PRs to address:
.tekton/lightspeed-stack-on-push.yamlAlso applies to: 34-34
🤖 Prompt for AI Agents