HIVE-2862: Build containers without python deps#2865
HIVE-2862: Build containers without python deps#2865suhanime wants to merge 2 commits intoopenshift:masterfrom
Conversation
This change allows building the container images without the python dependencies that are getting harder to get by. As a part of this change, version2.py is faithfully converted to a shell script of the same name. Co-authored-by: Antoni Segura Puimedon <antoni@redhat.com> Co-authored-by: Mark Old <mold@redhat.com> Assisted-by: Claude Sonnet 4.5
openshift-ci-robot
added
the
jira/valid-reference
|
@suhanime: This pull request references HIVE-2862 which is a valid jira issue. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughReplaces the Python-based Git versioning utility with a Bash implementation, removes Python bootstrap/install from build steps, and updates Makefile to call the new shell script for computing SOURCE_GIT_TAG. Docker and build-deps adjustments skip Python installation but preserve existing build commands. Changes
Sequence Diagram(s)sequenceDiagram
participant Make as Makefile
participant Script as hack/version2.sh
participant Git as GitRepo
participant Build as Docker/BuildEnv
Make->>Script: invoke hack/version2.sh
Script->>Git: resolve COMMIT (HEAD or provided)
Script->>Git: discover/validate BRANCH (local/remotes)
Script->>Git: compute COMMIT_COUNT and short SHA
Script-->>Make: return version string (vX.Y.Z-shortsha)
Make->>Build: use SOURCE_GIT_TAG in build (Dockerfile / make)
Build-->Make: build artifacts
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@suhanime: This pull request references HIVE-2862 which is a valid jira issue. 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 openshift-eng/jira-lifecycle-plugin repository. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
hack/version2.sh (1)
29-44: Consider adding level prefix to log messages.The
logfunction accepts alevelparameter but doesn't include it in the output. This makes it harder to distinguish debug, info, warning, and fatal messages when reading logs.♻️ Optional: Add level prefix to log output
log() { local level="$1" shift local message="$*" if [[ "$MODE" == "standalone" ]]; then - echo "$message" >&2 + echo "[$level] $message" >&2 else - echo "$message" + echo "[$level] $message" fi if [[ "$level" == "fatal" ]]; then exit 1 fi }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hack/version2.sh` around lines 29 - 44, The log function currently accepts a level parameter but prints only the message; update the log() function to prefix printed output with the level (e.g., "[level] message") so callers can distinguish debug/info/warn/fatal; preserve existing MODE logic (when MODE=="standalone" write to stderr, otherwise stdout) and keep the exit behavior when level == "fatal". Ensure you modify the log() implementation (references: function name log, parameter level, variable MODE) to build and echo the prefixed string rather than echoing just "$message".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hack/version2.sh`:
- Around line 291-294: The version_string() function currently echoes
"v$(semver)" which duplicates the 'v' because the Makefile's SOURCE_GIT_TAG :=
v$(shell hack/version2.sh) also prepends a 'v'; fix by removing the leading 'v'
from version_string() so it echoes "$(semver)" (leave semver() behavior
unchanged), or alternatively remove the extra 'v' in the Makefile assignment to
avoid producing "vv..."—preferably update version_string() to return the raw
semver value to keep tag formatting centralized.
In `@Makefile`:
- Around line 83-85: SOURCE_GIT_TAG is being set to v$(shell hack/version2.sh)
which causes a double "v" when version_string() already emits a leading "v"; fix
by ensuring only one source of the "v" prefix: either remove the hardcoded "v"
in SOURCE_GIT_TAG or (preferred) change version_string() in hack/version2.sh to
return the version without a leading "v" so SOURCE_GIT_TAG remains v<version>.
Update the version_string() implementation accordingly and keep SOURCE_GIT_TAG
as-is to preserve the existing Makefile pattern.
---
Nitpick comments:
In `@hack/version2.sh`:
- Around line 29-44: The log function currently accepts a level parameter but
prints only the message; update the log() function to prefix printed output with
the level (e.g., "[level] message") so callers can distinguish
debug/info/warn/fatal; preserve existing MODE logic (when MODE=="standalone"
write to stderr, otherwise stdout) and keep the exit behavior when level ==
"fatal". Ensure you modify the log() implementation (references: function name
log, parameter level, variable MODE) to build and echo the prefixed string
rather than echoing just "$message".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d1df7309-73af-414c-9abb-ca8841a1c1a4
📒 Files selected for processing (5)
DockerfileMakefilehack/ubi-build-deps.shhack/version2.pyhack/version2.sh
💤 Files with no reviewable changes (2)
- Dockerfile
- hack/version2.py
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: suhanime 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 |
openshift-ci
bot
added
the
approved
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
@suhanime: This pull request references HIVE-2862 which is a valid jira issue. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
@suhanime: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. I understand the commands that are listed here. |
3 participants
This change allows building the container images without the python dependencies that are getting harder to get by.
As a part of this change, version2.py is faithfully converted to a shell script of the same name.
Co-authored-by: Antoni Segura Puimedon antoni@redhat.com
Co-authored-by: Mark Old mold@redhat.com
Assisted-by: Claude Sonnet 4.5
Summary by CodeRabbit