NO-JIRA: chore: make cross build consistent across plugins#355
Conversation
Signed-off-by: Gabriel Bernal <gbernal@redhat.com>
|
@jgbernalp: This pull request explicitly references no 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. |
openshift-ci-robot
added
the
jira/valid-reference
WalkthroughThe changes update the Go builder base image to a newer OpenShift image and modify the Makefile's cross-build target and variables. The Dockerfile switches from a Red Hat registry image to an OpenShift CI registry image with updated Go and OpenShift versions. The Makefile introduces a new cross-build implementation using Podman manifests and adds a FEATURES variable. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
/cherry-pick release-6.1 |
|
@jgbernalp: once the present PR merges, I will cherry-pick it on top of 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 kubernetes-sigs/prow repository. |
openshift-ci
bot
added
the
approved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Makefile (1)
81-83: Optional: align with checkmakeminphonyexpectations.Static analysis reports missing phony
all,clean, andtest. If checkmake is gating, add lightweight aliases to avoid future lint friction.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 81 - 83, Add lightweight phony aliases required by checkmake by updating the Makefile to declare .PHONY for the common targets and adding minimal implementations for them; specifically add .PHONY: all clean test deploy (or a minphony target if your lint config expects it) and implement simple no-op bodies such as "all: ;" "clean: ;" and "test: ;" (leave existing deploy as-is) so static analysis no longer flags missing phony targets.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Dockerfile`:
- Line 15: The project has a Go version mismatch: the Dockerfile uses Go 1.25
(FROM ... golang-1.25 ...) while go.mod declares "go 1.24.0"; update the go
directive in go.mod to "1.25" to match the Dockerfile (or alternatively change
the Dockerfile FROM back to golang-1.24 if you must remain on 1.24), and ensure
Dockerfile.ci’s Go version is aligned as well so all build descriptors (go.mod
'go' directive, Dockerfile FROM tag, and Dockerfile.ci) reference the same Go
minor version.
---
Nitpick comments:
In `@Makefile`:
- Around line 81-83: Add lightweight phony aliases required by checkmake by
updating the Makefile to declare .PHONY for the common targets and adding
minimal implementations for them; specifically add .PHONY: all clean test deploy
(or a minphony target if your lint config expects it) and implement simple no-op
bodies such as "all: ;" "clean: ;" and "test: ;" (leave existing deploy as-is)
so static analysis no longer flags missing phony targets.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cf758217-a327-46cb-91d8-3bd2989019c0
📒 Files selected for processing (2)
DockerfileMakefile
| RUN make build-frontend | ||
|
|
||
| FROM brew.registry.redhat.io/rh-osbs/openshift-golang-builder:rhel_9_1.24 as go-builder | ||
| FROM registry.ci.openshift.org/ocp/builder:rhel-9-golang-1.25-openshift-4.22 AS go-builder |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find all Dockerfiles in the repo
echo "=== Dockerfile inventory ==="
fd -type f -name 'Dockerfile*' | head -20
echo -e "\n=== Dockerfile.ci builder image ==="
if [ -f Dockerfile.ci ]; then
rg -n 'FROM.*golang-1\.(24|25)-openshift-4\.(20|22)' Dockerfile.ci || echo "No match found"
# Show broader context around builder image
sed -n '25,35p' Dockerfile.ci 2>/dev/null || echo "File read failed or too short"
fi
echo -e "\n=== go.mod Go version declaration ==="
if [ -f go.mod ]; then
head -20 go.mod | rg -n '^go\s+[0-9]+\.[0-9]+' || echo "No match in first 20 lines"
fi
echo -e "\n=== Dockerfile.ci references in codebase ==="
rg -l 'Dockerfile\.ci' | head -10
echo -e "\n=== CI config references to Dockerfile.ci ==="
find . -type f \( -name '*.yml' -o -name '*.yaml' -o -name '*.sh' \) -exec rg -l 'Dockerfile\.ci' {} + 2>/dev/null | head -10Repository: openshift/logging-view-plugin
Length of output: 289
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find all Dockerfiles in the repo (corrected syntax)
echo "=== Dockerfile inventory ==="
fd -type file -name 'Dockerfile*' | head -20
echo -e "\n=== Dockerfile.ci builder image ==="
if [ -f Dockerfile.ci ]; then
echo "File exists. Checking builder image..."
rg -n 'FROM.*golang' Dockerfile.ci || echo "No golang builder found"
else
echo "Dockerfile.ci not found in root"
fi
echo -e "\n=== Dockerfile main builder image ==="
if [ -f Dockerfile ]; then
rg -n 'FROM.*golang' Dockerfile || echo "No golang builder found"
fi
echo -e "\n=== go.mod Go version declaration ==="
if [ -f go.mod ]; then
head -20 go.mod
fi
echo -e "\n=== Search for Dockerfile.ci references ==="
rg 'Dockerfile\.ci' 2>/dev/null | head -20 || echo "No references found"Repository: openshift/logging-view-plugin
Length of output: 289
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find all Dockerfiles in the repo
echo "=== Dockerfile files in root ==="
ls -la Dockerfile* 2>/dev/null || echo "No Dockerfile* files found"
echo -e "\n=== Dockerfile main builder image ==="
if [ -f Dockerfile ]; then
grep -n 'FROM.*golang' Dockerfile || echo "No golang builder found"
fi
echo -e "\n=== Dockerfile.ci builder image ==="
if [ -f Dockerfile.ci ]; then
grep -n 'FROM.*golang' Dockerfile.ci || echo "No golang builder found"
else
echo "Dockerfile.ci not found"
fi
echo -e "\n=== go.mod Go version ==="
if [ -f go.mod ]; then
head -20 go.mod
fi
echo -e "\n=== Search for Dockerfile.ci references ==="
grep -r 'Dockerfile\.ci' . --include='*.yml' --include='*.yaml' --include='*.sh' --include='*.md' 2>/dev/null | head -20 || echo "No references found"Repository: openshift/logging-view-plugin
Length of output: 1403
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== References to main Dockerfile in CI/build config ==="
grep -r 'Dockerfile[^.]' . --include='*.yml' --include='*.yaml' --include='*.sh' --include='*.md' 2>/dev/null | grep -v Dockerfile.ci | head -20 || echo "No references found"
echo -e "\n=== Full Dockerfile.ci content (first 35 lines) ==="
head -35 Dockerfile.ci
echo -e "\n=== Full Dockerfile content (first 20 lines) ==="
head -20 DockerfileRepository: openshift/logging-view-plugin
Length of output: 1604
Update go.mod to match main Dockerfile Go version.
Line 15 upgrades the main builder to Go 1.25/OCP 4.22, but go.mod still declares go 1.24.0. This mismatch should be resolved by updating go.mod to 1.25 or reverting the Dockerfile to Go 1.24/OCP 4.20 if the earlier version is intentionally required. Dockerfile.ci (Line 31) uses Go 1.24, but no CI references to it were found, suggesting it may be legacy.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Dockerfile` at line 15, The project has a Go version mismatch: the Dockerfile
uses Go 1.25 (FROM ... golang-1.25 ...) while go.mod declares "go 1.24.0";
update the go directive in go.mod to "1.25" to match the Dockerfile (or
alternatively change the Dockerfile FROM back to golang-1.24 if you must remain
on 1.24), and ensure Dockerfile.ci’s Go version is aligned as well so all build
descriptors (go.mod 'go' directive, Dockerfile FROM tag, and Dockerfile.ci)
reference the same Go minor version.
|
@jgbernalp: all tests passed! 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. |
|
/label qe-approved |
openshift-ci
bot
added
the
qe-approved
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jgbernalp, PeterYurkovich 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 |
|
@jgbernalp: #355 failed to apply on top of branch "release-6.1": 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 kubernetes-sigs/prow repository. |
5 participants
Summary by CodeRabbit