Add dist-gen - #367
Conversation
|
Warning Review limit reached
Next review available in: 53 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (42)
📝 WalkthroughWalkthroughAdds a multispec generation system for core and base images across Fedora, RHEL, and CentOS Stream. It introduces templated Dockerfiles, generated assets, updated package and metadata configurations, and smoke/cgroup validation tests. ChangesImage generation and validation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Multispec
participant Manifest
participant DockerfileTemplate
participant ContainerTests
Multispec->>Manifest: provide distro and variant matrix
Manifest->>DockerfileTemplate: generate image Dockerfiles and assets
DockerfileTemplate->>ContainerTests: provide generated image and test layout
ContainerTests->>ContainerTests: run smoke and cgroup validations
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
Pull Request validationFailed🔴 Failed or pending statuses:
🔴 Review - Missing review from a member (1 required) Triggered by Workflow Run |
|
[test-all] |
Testing Farm results
|
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@common`:
- Line 1: Update the common submodule gitlink to reference an existing,
reachable commit from the tracked sclorg/container-common-scripts remote instead
of 300f499e1f4f0fdc5d9b4b470977cc39000810f3. Verify clean checkouts can
initialize common successfully before generated assets are built.
In `@specs/multispec.yml`:
- Around line 117-125: Update the relevant spec definitions in the multispec
configuration so every image-name template’s spec.org and spec.short references
resolve: declare those fields with the generator-supported values, or replace
the references with the existing declared distroinfo/version fields. Apply this
consistently to common_image_name, c9s_image_name, and rhel_image_name in the
affected base specification.
In `@src/README.md`:
- Around line 84-110: Correct the CentOS Stream documentation by mapping c10s to
CentOS Stream 10, describing quay.io as the image registry rather than
DockerHub, and assigning CentOS Stream 9 to Dockerfile.c9s while retaining
Dockerfile.c10s for Stream 10. Update the pull command or source references as
needed so each release points to the correct image and Dockerfile.
In `@src/test/run`:
- Around line 23-25: Remove eval usage from all docker run checks in the test
script, including the paths around the memory and cgroup-limit validations.
Capture the candidate image’s stdout as data, then compare it against the exact
expected output lines and retain the existing failure behavior; do not execute
any output from the image in the host shell.
In `@src/test/run-pytest`:
- Around line 13-18: Update the Python interpreter selection in the run-pytest
script to verify that python3.12 is available before choosing it when python3 is
older than 3.13. Fall back to the available python3 interpreter when python3.12
is missing, while preserving the existing Python 3.13-or-newer selection
behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fedf2453-7c70-439b-94d1-4f05114fc85f
📒 Files selected for processing (42)
base/Dockerfile.c10sbase/Dockerfile.c8sbase/Dockerfile.c9sbase/Dockerfile.f38base/Dockerfile.f39base/Dockerfile.f40base/Dockerfile.f41base/Dockerfile.f42base/Dockerfile.f43base/Dockerfile.f44base/Dockerfile.rhel10base/Dockerfile.rhel8base/Dockerfile.rhel9base/rootbase/test/conftest.pybase/test/runcommoncore/.exclude-c8score/Dockerfile.c10score/Dockerfile.c8score/Dockerfile.c9score/Dockerfile.f38core/Dockerfile.f39core/Dockerfile.f40core/Dockerfile.f41core/Dockerfile.f42core/Dockerfile.f43core/Dockerfile.f44core/Dockerfile.rhel10core/Dockerfile.rhel8core/Dockerfile.rhel9core/test/conftest.pycore/test/runmanifest.ymlspecs/multispec.ymlsrc/Dockerfilesrc/README.mdsrc/test/__init__.pysrc/test/conftest.pysrc/test/runsrc/test/run-pytestsrc/test/test_container_basics.py
💤 Files with no reviewable changes (4)
- base/Dockerfile.c8s
- core/Dockerfile.c8s
- core/test/conftest.py
- core/test/run
| @@ -1 +1 @@ | |||
| Subproject commit 0bd53a383270457da180fdb05c8c2bc734bbe6e3 | |||
| Subproject commit 300f499e1f4f0fdc5d9b4b470977cc39000810f3 | |||
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
new_commit="$(git rev-parse HEAD:common)"
git -C common cat-file -e "${new_commit}^{commit}"
old_commit="$(git rev-parse HEAD^:common)"
git -C common diff --stat "$old_commit" "$new_commit"
git -C common log --oneline "$old_commit..$new_commit"Repository: sclorg/s2i-base-container
Length of output: 339
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== current common pointer =="
git ls-tree HEAD common || true
git submodule status common || true
echo "== submodule tracked remote refs on HEAD =="
git submodule absorbgitdirs 2>/dev/null || true
cat .gitmodules | sed -n '/\[submodule "common"\]/,/^\[/p'
git config --file .gitmodules --submodule common.url
echo "== submodule gitdir/ref availability =="
if [ -L .git/modules/common ]; then
gitdir="$(readlink .git/modules/common)"
elif [ -e .git/modules/common ]; then
gitdir=".git/modules/common"
fi
echo "gitdir=$gitdir"
gitdir="$(eval echo "$gitdir")"
git -C "$gitdir" rev-parse --is-inside-git-dir || true
git -C "$gitdir" rev-parse HEAD:common 2>&1 || true
git -C "$gitdir" cat-file -e HEAD:common 2>&1 || true
echo "== list refs mentioning hash/name =="
git -C "$gitdir" branch -a --contains 300f499e1f4f0fdc5d9b4b470977cc39000810f3 2>&1 || true
git -C "$gitdir" tag -l --contains 300f499e1f4f0fdc5d9b4b470977cc39000810f3 2>&1 || trueRepository: sclorg/s2i-base-container
Length of output: 3563
🌐 Web query:
sclorg container-common-scripts 300f499e1f4f0fdc5d9b4b470977cc39000810f3
💡 Result:
The character string 300f499e1f4f0fdc5d9b4b470977cc39000810f3 does not correspond to a known git commit hash or entity within the sclorg/container-common-scripts GitHub repository [1][2][3][4]. The sclorg/container-common-scripts repository contains common build helper scripts for containers and is maintained by the SCLorg project [1][2]. Searches for this specific string in the context of this repository yielded no results [5][6][7][8][9][10]. Similar numerical sequences found in online databases relate to unrelated blockchain transactions, epochs, or identifiers rather than software source code [11][12][13][14][15][16][17][18][19].
Citations:
- 1: https://github.com/sclorg/container-common-scripts
- 2: https://github.com/sclorg/container-common-scripts/tree/f994db126f12c383490fee5913bbf87266011896
- 3: https://github.com/sclorg/container-common-scripts/tree/bc49c74ec984e34321694f00a6b13021e547d827
- 4: https://github.com/sclorg/container-common-scripts/tree/1d0d26106141ea319833145c1674558035c46a2f
- 5: Use --iidfile to capture Docker image ID reliably container-common-scripts#428
- 6: sclorg/container-common-scripts@1f141d8
- 7: sclorg/container-common-scripts@498153e
- 8: sclorg/container-common-scripts@0bd53a3
- 9: https://github.com/sclorg/container-common-scripts/blob/0bd53a383270457da180fdb05c8c2bc734bbe6e3/.github/workflows/auto-merge-on-demand.yml
- 10: https://github.com/sclorg/container-common-scripts/blob/0bd53a383270457da180fdb05c8c2bc734bbe6e3/.github/workflows/sanity-tests.yaml
- 11: https://ordinals.com/tx/300f3003ff3a29abf4ab8d4bfc97fa0ecc0c15764891a4a6012f6b61b3e95a89
- 12: https://www.walletexplorer.com/txid/300f5256de3ab3d26dc344f535538adef2fa16d3b729a5e4f12da9e59bcd2651
- 13: https://www.walletexplorer.com/txid/300f09529f48a659625d854488428e3648e9bd2cce9b0666aa0d4f0972bfc571
- 14: https://www.walletexplorer.com/txid/f300ad55aaef8d76c53afb681fe9f5c35698f300443de99ffd3c57901ae4960e
- 15: https://www.walletexplorer.com/txid/300ba2ae1abde2d9809aa0d8b0b31e270bc5b72ce87184f3a89f77ea8a664551
- 16: https://ordinals.com/tx/30010d782451ebfbead0696d0238ba29fdcc4e9326fd2f22dc42006284a59ffa
- 17: https://shakeshift.com/name/300499
- 18: https://beaconscan.com/epoch/300499
- 19: https://web3.bio/300500.farcaster
🌐 Web query:
github.com/sclorg/container-common-scripts/commit/300f499e1f4f0fdc5d9b4b470977cc39000810f3
💡 Result:
The commit hash 300f499e1f4f0fdc5d9b4b470977cc39000810f3 does not appear to exist or be associated with the sclorg/container-common-scripts repository in the provided search results [1][2][3][4][5][6]. The available information indicates that the repository contains various commits related to build helpers for SCLorg containers, but this specific identifier does not match any record in the indexed data [1][2][6]. Results containing similar numbers refer to unrelated blockchain transactions, cryptocurrency-related identifiers, or medical terminology, and are not connected to the requested GitHub repository [7][8][9][10][11][12][13].
Citations:
- 1: https://github.com/sclorg/container-common-scripts
- 2: sclorg/container-common-scripts@1f141d8
- 3: https://github.com/sclorg/container-common-scripts/tree/bc49c74ec984e34321694f00a6b13021e547d827
- 4: https://github.com/sclorg/container-common-scripts/tree/1d0d26106141ea319833145c1674558035c46a2f
- 5: https://github.com/sclorg/container-common-scripts/tree/f994db126f12c383490fee5913bbf87266011896
- 6: https://github.com/sclorg/container-common-scripts/tree/93c2c51fbe77b247fc4d3ff0cf703bf22a8f2a66
- 7: https://ordinals.com/tx/300f3003ff3a29abf4ab8d4bfc97fa0ecc0c15764891a4a6012f6b61b3e95a89
- 8: https://ordinals.com/tx/30010d782451ebfbead0696d0238ba29fdcc4e9326fd2f22dc42006284a59ffa
- 9: https://shakeshift.com/name/300499
- 10: https://beaconscan.com/epoch/300499
- 11: https://web3.bio/300500.farcaster
- 12: https://bscscan.com/tx/0xd01f4739eb1770cf5c9cbdcb387df29b4ffbb63d7afe39f70d607dd052b73fc2
- 13: https://loinc.org/LP102300-3
Fix the invalid common submodule pointer.
common now points to 300f499e1f4f0fdc5d9b4b470977cc39000810f3, but that commit is not reachable from the tracked submodule remote. Clean checkouts can fail or leave common unavailable before generated assets are built. Update this gitlink to an existing commit in sclorg/container-common-scripts; the desired submodule diff cannot be shown until a valid target commit is chosen.
🤖 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 `@common` at line 1, Update the common submodule gitlink to reference an
existing, reachable commit from the tracked sclorg/container-common-scripts
remote instead of 300f499e1f4f0fdc5d9b4b470977cc39000810f3. Verify clean
checkouts can initialize common successfully before generated assets are built.
| common_image_name: "{{ spec.org }}/postgresql-{{ spec.short }}-{{ spec.prod }}" | ||
| rhel_image_name: "rhel8/postgresql-{{ spec.short }}" | ||
| latest_fedora: "f44" | ||
|
|
||
| "base": | ||
| name: "base" | ||
| common_image_name: "{{ spec.org }}/postgresql-{{ spec.short }}-{{ spec.prod }}" | ||
| c9s_image_name: "sclorg/postgresql-{{ spec.short }}-c9s" | ||
| rhel_image_name: "rhel9/postgresql-{{ spec.short }}" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | sed -n '1,200p'
echo "== specs/multispec.yml relevant sections =="
if [ -f specs/multispec.yml ]; then
nl -ba specs/multispec.yml | sed -n '1,220p'
fi
echo "== references to distgen/org/short/image names =="
rg -n "distgen|spec\.org|spec\.short|common_image_name|rhel_image_name|multispec|mulsispec|image_name" -S . --glob '!*.png' --glob '!*.jpg' --glob '!*.gif' | sed -n '1,240p'
echo "== git status/stat =="
git status --short
git diff --stat || true
git diff -- specs/multispec.yml | sed -n '1,220p' || trueRepository: sclorg/s2i-base-container
Length of output: 1578
🏁 Script executed:
#!/bin/bash
set -u
echo "== specs/multispec.yml relevant sections =="
if [ -f specs/multispec.yml ]; then
awk '{printf "%5d %s\n", NR, $0}' specs/multispec.yml | sed -n '1,220p'
fi
echo "== references to distgen/org/short/image names =="
rg -n "distgen|spec\.org|spec\.short|common_image_name|rhel_image_name|multispec|mulsispec|image_name" -S . --glob '!*.png' --glob '!*.jpg' --glob '!*.gif' | sed -n '1,240p'
echo "== git status/stat =="
git status --short || true
git diff --stat || true
git diff -- specs/multispec.yml | sed -n '1,220p' || true
echo "== python available and yaml parse =="
python3 - <<'PY'
import sys, os
try:
import yaml
print("pyyaml", getattr(yaml, "__version__", None))
except Exception as e:
print("pyyaml unavailable:", type(e).__name__, e)
yaml=None
if yaml and os.path.exists("specs/multispec.yml"):
data=yaml.safe_load(open("specs/multispec.yml"))
print("top keys:", list(data.keys()) if isinstance(data, dict) else type(data))
if isinstance(data, dict):
for k in data:
print(f"\n--- {k} ---")
print(yaml.safe_dump(data[k], sort_keys=False, indent=2, default_flow_style=False)[:2000])
PYRepository: sclorg/s2i-base-container
Length of output: 11897
🌐 Web query:
distgen sclorg s2i-base-container DISTGEN_MULTI_RULES manifests multispec.yml
💡 Result:
Distgen is a distribution-oriented templating system used by the Software Collections organization (sclorg) to automate the generation of container source files, such as Dockerfiles, across multiple operating system distributions and versions [1][2]. In sclorg container repositories (like the s2i-base-container or various language-specific S2I images), configuration files such as manifest.yml define the rules for this generation process [3][4]. Key components include: - DISTGEN_MULTI_RULES: A directive used within manifest.yml files to define how to generate multiple distribution-specific files from a single template [3][4][5]. It typically maps a source template (e.g., src/Dockerfile) to a destination path (e.g., Dockerfile.rhel9), allowing the tool to create variants for RHEL, CentOS Stream, or other supported distributions [4][5]. - manifests/multispec.yml: This file serves as the central "declarative" specification (the multispec) for the repository [3][6]. It contains the distribution and version matrix, defining the specific values (like software versions or distro-specific configuration) that the distgen tool injects into the templates during generation [3][1]. To update a Dockerfile in a repository that uses this system, developers typically modify the templates (in the src/ directory) or the values in specs/multispec.yml, and then run a command such as make generate to invoke distgen and apply the new configuration [7][8].
Citations:
- 1: https://distgen.readthedocs.io/en/latest/intro.html
- 2: https://github.com/devexp-db/distgen/
- 3: Dist gen generation s2i-nodejs-container#541
- 4: https://github.com/sclorg/postgresql-container/blob/master/manifest.yml
- 5: https://github.com/sclorg/s2i-python-container/blob/master/manifest-minimal.yml
- 6: Dist gen generation s2i-nodejs-container#541
- 7: https://github.com/lholmquist/postgresql-container
- 8: https://github.com/bdunne/postgresql-container
🌐 Web query:
distgen documentation {{ spec }} image-name common_image_name specs/multispec.yml
💡 Result:
The command format you are inquiring about relates to the use of distgen (dg), a templating and distribution database tool used to render container files (such as Dockerfiles) from templates and YAML specifications [1][2][3]. The syntax you provided—distgen {{ spec }} image-name common_image_name specs/multispec.yml—does not represent a standard, single command-line invocation for the dg tool. Based on documentation and common usage patterns in container image repositories, here is the breakdown of how these components are actually used: 1. The dg Tool: The primary command-line tool for distgen is dg [1][4]. 2. Specs and Multispecs: The --spec option is used to specify a YAML file containing configuration values [4]. When using a multispec file (like specs/multispec.yml), the --multispec option is paired with a --multispec-selector to identify specific configuration groups [1]. 3. Template Rendering: The tool typically requires a --template argument to specify the source template file (e.g., Dockerfile.template) [1][4]. 4. Automation via Makefiles: In many projects (e.g., SCLorg container repositories), this process is abstracted away inside a Makefile [5][6]. Developers typically run commands like make generate or make generate-all, which internally execute the dg tool with the necessary flags, including the appropriate multispec files and selectors [5][6][7]. If you are looking to generate a file using these components, the standard approach involves running: dg --template <path_to_template> --spec <path_to_spec> --multispec <path_to_multispec> --multispec-selector The {{ spec }} syntax you cited is a Jinja2 template variable often found inside the content of the template files themselves (e.g., {{ spec.some_value }}), which is then populated by the data provided in your YAML spec files during the generation process [1].
Citations:
- 1: https://distgen.readthedocs.io/en/latest/intro.html
- 2: https://github.com/devexp-db/distgen/
- 3: https://pypi.org/project/distgen/2.4/
- 4: https://www.mankier.com/1/dg
- 5: https://github.com/sclorg/s2i-python-container/blob/master/README.md
- 6: https://github.com/sclorg/postgresql-container/blob/master/README.md
- 7: https://github.com/kubesphere/s2i-python-container
🌐 Web query:
github devexp-db/distgen specs-maintainer spec org short common_image_name
💡 Result:
The devexp-db/distgen project is a distribution-oriented templating system designed to mitigate the complexity of maintaining portable scripts (like Dockerfiles) across different Linux distributions [1][2]. In the context of distgen, a spec file is a YAML file that provides declarative data to satisfy the requirements of a template (typically a Jinja2 file) [1][2]. The terms org and common_image_name are not hard-coded keywords in the distgen engine itself, but rather common variable names used within the YAML spec files and multispec configurations of projects that utilize distgen (such as container image repositories) to define metadata for image generation [3]. For example, in many projects using distgen: - org: Often used to specify the organization or namespace for the container image (e.g., in a registry). - common_image_name: Typically used to define the base name of the container image being generated [3]. These variables are processed by the distgen (dg) tool when generating files from templates [2]. To work with these specifications, users typically modify a multispec.yml or a specific YAML spec file and execute a command like make generate to trigger the template rendering process [3].
Citations:
- 1: https://github.com/devexp-db/distgen/
- 2: https://distgen.readthedocs.io/en/latest/intro.html
- 3: https://explore.market.dev/ecosystems/postgresql/projects/postgresql-container
🌐 Web query:
"specs-maintainer" "spec.org" "spec.short" distgen
💡 Result:
The terms you inquired about—specs-maintainer, spec.org, spec.short, and distgen—do not appear to form a single cohesive technical entity. Instead, they belong to two distinct, unrelated contexts: 1. Distgen (Templating Tool): Distgen is a tool for generating configuration files (such as Dockerfiles or scripts) for different Linux distributions [1][2]. It uses Jinja2 templates and declarative YAML spec files to handle OS-specific differences [1][3]. The term maintainer in this context typically refers to the metadata field within a distgen YAML spec file (e.g., maintainer: John Doe jdoe@example.com) [1][3] or the human maintainers of the distgen software package itself [2][3]. 2. SPEC (Standard Performance Evaluation Corporation): SPEC.org is the official website for the Standard Performance Evaluation Corporation, a non-profit organization that develops standardized benchmarks for evaluating the performance and energy efficiency of computer systems [4][5][6]. There is no standard technical component named spec.short or specs-maintainer associated with the SPEC organization [4][7][6]. It is highly likely that your query involves a conflation of these two separate topics. If you encountered these terms in a specific configuration file or software project, please double-check if they are part of a custom schema or a local naming convention, as they are not standard global identifiers for either project [8][9][4].
Citations:
- 1: https://github.com/devexp-db/distgen/
- 2: https://packages.fedoraproject.org/pkgs/distgen/distgen/
- 3: https://pypi.org/project/distgen/
- 4: https://spec.org/spec/
- 5: https://spec.org./
- 6: https://www.spec.org/spec/faq/01spec.general.10whatis/
- 7: https://spec.org/products/
- 8: https://distgen.readthedocs.io/en/latest/intro.html
- 9: https://www.mankier.com/1/dg
Define spec.org and spec.short for the image-name templates.
common_image_name, c9s_image_name, and rhel_image_name interpolate spec.org and/or spec.short, but these fields are not declared anywhere in specs/multispec.yml. These generated image names will not resolve unless the generator supplies those fields or the templates use declared distroinfo/version fields.
🤖 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 `@specs/multispec.yml` around lines 117 - 125, Update the relevant spec
definitions in the multispec configuration so every image-name template’s
spec.org and spec.short references resolve: declare those fields with the
generator-supported values, or replace the references with the existing declared
distroinfo/version fields. Apply this consistently to common_image_name,
c9s_image_name, and rhel_image_name in the affected base specification.
| if ! ( eval $(docker run --rm --memory=512M ${IMAGE_NAME} /usr/bin/cgroup-limits) | ||
| echo "MEMORY_LIMIT_IN_BYTES=$MEMORY_LIMIT_IN_BYTES" | ||
| [ "$MEMORY_LIMIT_IN_BYTES" -eq 536870912 ] ); then |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Do not eval output from the candidate image.
Each docker run result is controlled by code inside the image and is executed by the host shell. A malicious image can run arbitrary commands on the test runner; several paths run as root. Capture stdout and compare exact expected lines instead.
Proposed pattern
- if ! ( eval $(docker run --rm --memory=512M ${IMAGE_NAME} /usr/bin/cgroup-limits)
- echo "MEMORY_LIMIT_IN_BYTES=$MEMORY_LIMIT_IN_BYTES"
- [ "$MEMORY_LIMIT_IN_BYTES" -eq 536870912 ] ); then
+ output=$(docker run --rm --memory=512M "${IMAGE_NAME}" /usr/bin/cgroup-limits) || return 1
+ if ! grep -qx 'MEMORY_LIMIT_IN_BYTES=536870912' <<<"${output}"; thenAlso applies to: 32-34, 41-43, 54-56, 63-66
🤖 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 `@src/test/run` around lines 23 - 25, Remove eval usage from all docker run
checks in the test script, including the paths around the memory and
cgroup-limit validations. Capture the candidate image’s stdout as data, then
compare it against the exact expected output lines and retain the existing
failure behavior; do not execute any output from the image in the host shell.
| if python3 -c 'import sys; sys.exit(0 if sys.version_info < (3,13) else 1)'; then | ||
| PYTHON_VERSION="3.12" | ||
| else | ||
| PYTHON_VERSION="3" | ||
| fi | ||
| cd "${THISDIR}" && "python${PYTHON_VERSION}" -m pytest -s -rA --showlocals -vv test_container_*.py |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 --version
if python3 -c 'import sys; sys.exit(0 if sys.version_info < (3, 13) else 1)'; then
command -v python3.12
python3.12 --version
else
command -v python3
fiRepository: sclorg/s2i-base-container
Length of output: 177
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file =="
cat -n src/test/run-pytest
echo "== repository scripts mentioning run-pytest or pytest execution =="
rg -n "run-pytest|python3\.12|python3" . \
--glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' \
| head -200Repository: sclorg/s2i-base-container
Length of output: 2563
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import pathlib, re
def resolve_files():
# Treat files that are git-tracked, so untracked local results are ignored.
out = bytearray()
import subprocess
proc = subprocess.run(["git", "ls-files"], stdout=subprocess.PIPE, check=True)
files = proc.stdout.decode().splitlines()
candidates = [p for p in files if re.search(r'(Dockerfile|dist|dev|build|release|rpm|deb|apk|scl|rpm|centos|rhel|fedora|debian|ubuntu|cent)'] or '.py$' in p or 'pytest' in p])
for p in candidates:
out.append(p + "\n")
return out.decode()
# Lightweight scan: show installation/package references for python3/py
for p in pathlib.Path('.').glob('**/*'):
if not p.is_file():
continue
if '.git' in p.parts:
continue
try:
text = p.read_text(error='ignore')
except Exception:
continue
if 'python3' in text or 'pytest' in text or 'python' in text:
hit = []
for i,line in enumerate(text.splitlines(),1):
if re.search(r'python[-._]?3|pytest|python[3][.-]?', line, re.I):
hit.append(f"{p}:{i}:{line.strip()}")
if hit:
print("HITS", p)
for h in hit[:30]:
print(h)
print("---")
PYRepository: sclorg/s2i-base-container
Length of output: 590
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant Dockerfile/python/pytest snippets =="
python3 - <<'PY'
import pathlib, re
files = [
pathlib.Path('src/test/run-pytest'),
pathlib.Path('src/Dockerfile'),
pathlib.Path('manifest.yml'),
pathlib.Path('base/test/run-pytest'),
pathlib.Path('core/test/run-pytest'),
]
for p in files:
if not p.exists():
continue
text = p.read_text()
print(f"--- {p} ({len(text.splitlines())} lines) ---")
hits = []
for i, line in enumerate(text.splitlines(), 1):
if re.search(r'python[-._]?3|pytest|uv|pdm|pip|devtoolset|python[3][.-]?', line, re.I):
hits.append((i, line.strip()))
# show nearby lines for all hits
lines = text.splitlines()
shown = set()
for i, line in hits:
start=max(1, i-3); end=min(len(lines), i+4)
key=(start,end)
if key in shown: continue
shown.add(key)
for n in range(start,end+1):
print(f"{n:4}: {lines[n-1]}")
print("---")
PY
echo "== tracked python-related filenames =="
git ls-files | rg -n '(^|/)(Dockerfile|.*pytest|.*python|.*requirements|.*pyproject|.*setup|manifest\.yml|.*test.*run.*)$' || trueRepository: sclorg/s2i-base-container
Length of output: 2608
🌐 Web query:
GitHub sclorg s2i-base-container base test run pytest python3.12
💡 Result:
The sclorg/s2i-base-container repository provides the underlying base images for language-specific containers, such as the python-based images [1][2][3]. The testing framework specifically for Python applications, including those using Python 3.12, is maintained in the separate sclorg/s2i-python-container repository [4][5]. To run tests for a Python 3.12 image in the s2i-python-container project, use the Makefile provided in the repository [4]. 1. Clone the repository and navigate to the directory: git clone https://github.com/sclorg/s2i-python-container.git cd s2i-python-container 2. Run the test command using the target platform (e.g., c9s for CentOS Stream 9) and specify the Python version [4]: make test TARGET=c9s VERSIONS=3.12 In this command: - TARGET: Specifies the supported platform (e.g., c9s, c10s, rhel8, rhel9) [4][6]. - VERSIONS: Limits the testing to the specified Python version (3.12) [4]. Omitting this parameter will run tests for all supported Python versions [4]. The testing framework leverages Source-to-Image (S2I) to build and verify Python applications, including the execution of pytest if configured within the test applications found in the test/ directory [4]. For general development, you can also inspect the existing GitHub Actions workflows in.github/workflows/container-tests.yml to understand how these tests are automated in the CI environment [7][8].
Citations:
- 1: https://github.com/sclorg/s2i-base-container
- 2: https://github.com/sclorg/s2i-base-container/tree/master/base
- 3: https://sclorg.github.io/containers-doc/
- 4: https://github.com/sclorg/s2i-python-container
- 5: https://github.com/sclorg/s2i-python-container/blob/master/3.12/README.md
- 6: https://github.com/sclorg/s2i-python-container/tree/master/3.12
- 7: https://github.com/sclorg/s2i-python-container/blob/master/.github/workflows/container-tests.yml
- 8: https://github.com/sclorg/s2i-python-container/blob/fd1ef2c1ee6c9637d5bd961d849c672db9eccb08/.github/workflows/container-tests.yml
Ensure python3.12 exists before selecting it.
On environments where python3 < 3.13, this script unconditionally runs python3.12; it can fail while python3 is available. Add a preflight check or fallback when python3.12 is missing.
🤖 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 `@src/test/run-pytest` around lines 13 - 18, Update the Python interpreter
selection in the run-pytest script to verify that python3.12 is available before
choosing it when python3 is older than 3.13. Fall back to the available python3
interpreter when python3.12 is missing, while preserving the existing Python
3.13-or-newer selection behavior.
phracek
left a comment
There was a problem hiding this comment.
Thanks for updating base image.
Please address my comments.
| NODEJS_VER=22 \ | ||
| NAME=s2i-base | ||
| NAME=s2i-base \ | ||
| VERSION=10 |
There was a problem hiding this comment.
As far as I know, this is not needed at all. We do not use it. We were trying to remove it.
| zlib-devel" && \ | ||
| dnf install -y --setopt=tsflags=nodocs $INSTALL_PKGS && \ | ||
| rpm -V $INSTALL_PKGS && \ | ||
| #rpm -V $INSTALL_PKGS && \ |
There was a problem hiding this comment.
I would prefer to remove it at all.
| wget \ | ||
| which \ | ||
| zlib-ng-compat-devel" && \ | ||
| zlib-devel" && \ |
There was a problem hiding this comment.
From my point of view zlib-devel is bigger then zlib-ng-compat-devel. Am I right?
There was a problem hiding this comment.
At least in rhel10/c10s they're linked to the same package
| # consume. | ||
| RUN yum -y module enable nodejs:$NODEJS_VER && \ | ||
| INSTALL_PKGS="nodejs autoconf \ | ||
| # Also setup the 'openshift' user that is used for the build execution and for the |
There was a problem hiding this comment.
I don't follow this comment. As far as I know this is not needed.
| images layered on top of it with all the tools needed to use source-to-image \ | ||
| functionality. Additionally, s2i-base also contains various libraries needed for \ | ||
| it to serve as a base for other builder images, like s2i-python or s2i-ruby." \ | ||
| NODEJS_VER=18 \ |
There was a problem hiding this comment.
Dockerfile.f38 reach already EOL. I don't think, that modification is needed. I would prefer to remove it.
| images layered on top of it with all the tools needed to use source-to-image \ | ||
| functionality. Additionally, s2i-base also contains various libraries needed for \ | ||
| it to serve as a base for other builder images, like s2i-python or s2i-ruby." \ | ||
| NODEJS_VER=20 \ |
There was a problem hiding this comment.
Like for F39. It already reached EOL.
There was a problem hiding this comment.
But from what I know the fedora images for pg15 use an older fedora so I didn't want to break any existing images..
| com.redhat.component="s2i-base-container" \ | ||
| name="ubi8/s2i-base" \ | ||
| version="1" \ | ||
| version=8 \ |
There was a problem hiding this comment.
version is set by Konflux pipelines. So remove it.
| {% endif %} | ||
| # This is the list of basic dependencies that all language container image can | ||
| # consume. | ||
| # Also setup the 'openshift' user that is used for the build execution and for the |
There was a problem hiding this comment.
This is not needed, I do not know who wrote it.
Depends on devexp-db/distgen#159 and sclorg/container-common-scripts#430 being merged first.
Summary by CodeRabbit
New Features
Documentation
Tests
Refactor