OCPBUGS-83863: Drop rhel8 builds, strip debug info#405
Open
sdodson wants to merge 2 commits into
Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
WalkthroughBuild artifacts only in the rhel9 builder using stripped Go binaries, then copy all three binaries into a shared /usr/src/whereabouts/bin in the base-rhel9 image; LABEL formatting adjusted. ChangesDocker Build Configuration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 10 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (10 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: sdodson The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
sdodson
changed the title
openshift-ci-robot
added
the
jira/valid-reference
Contributor
|
@sdodson: This pull request references Jira Issue OCPBUGS-83863, which is valid. 3 validation(s) were run on this bug
The bug has been updated to refer to the pull request using the external bug tracker. 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-bug
![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
🤖 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.openshift`:
- Around line 22-26: The Dockerfile removed the rhel8 bin path but entrypoint.sh
still may set rhelmajor=8 and try to run
/usr/src/whereabouts/rhel8/bin/ip-control-loop, causing a runtime "No such file"
error; fix by either (A) adding a compatibility hardlink directory
/usr/src/whereabouts/rhel8/bin with the same links as rhel10 (create symlinks to
whereabouts, ip-control-loop, node-slice-controller) so entrypoint.sh can find
binaries, or (B) update entrypoint.sh logic that sets rhelmajor to stop
selecting 8 (change the branch that computes rhelmajor or the path resolution)
so it never references rhel8; adjust whichever file you choose (Dockerfile for
adding links, or entrypoint.sh for changing rhelmajor resolution) and ensure
tests/containers exercise the Fedora CoreOS path.
🪄 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: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 32bc904a-6a13-4d0d-a6c1-6230d8100f04
📒 Files selected for processing (1)
Dockerfile.openshift
There was a problem hiding this comment.
♻️ Duplicate comments (1)
Dockerfile.openshift (1)
22-26:⚠️ Potential issue | 🟠 MajorRuntime path mismatch for Fedora CoreOS is still possible
entrypoint.sh(Line 12-35 inentrypoint.sh) can still resolverhelmajor=8and execute/usr/src/whereabouts/rhel8/bin/ip-control-loop. With onlyrhel9/binandrhel10/bincreated here, that path can fail at runtime.Minimal compatibility patch in this Dockerfile
RUN mkdir -p /usr/src/whereabouts/rhel10/bin && \ ln /usr/src/whereabouts/bin/whereabouts /usr/src/whereabouts/rhel10/bin/whereabouts && \ ln /usr/src/whereabouts/bin/ip-control-loop /usr/src/whereabouts/rhel10/bin/ip-control-loop && \ ln /usr/src/whereabouts/bin/node-slice-controller /usr/src/whereabouts/rhel10/bin/node-slice-controller +RUN mkdir -p /usr/src/whereabouts/rhel8/bin && \ + ln /usr/src/whereabouts/bin/whereabouts /usr/src/whereabouts/rhel8/bin/whereabouts && \ + ln /usr/src/whereabouts/bin/ip-control-loop /usr/src/whereabouts/rhel8/bin/ip-control-loop && \ + ln /usr/src/whereabouts/bin/node-slice-controller /usr/src/whereabouts/rhel8/bin/node-slice-controller🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile.openshift` around lines 22 - 26, The Dockerfile creates rhel9 and rhel10 runtime bin paths but not rhel8, so entrypoint.sh resolving rhelmajor=8 can fail when it tries to run /usr/src/whereabouts/rhel8/bin/ip-control-loop; update the RUN block that creates /usr/src/whereabouts/rhel*/bin to also create /usr/src/whereabouts/rhel8/bin and symlink the same binaries (whereabouts, ip-control-loop, node-slice-controller) into it (or create a rhel8 -> rhel9/rhel10 redirect) so entrypoint.sh’s resolution of /usr/src/whereabouts/rhel8/bin/ip-control-loop always finds the executable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@Dockerfile.openshift`:
- Around line 22-26: The Dockerfile creates rhel9 and rhel10 runtime bin paths
but not rhel8, so entrypoint.sh resolving rhelmajor=8 can fail when it tries to
run /usr/src/whereabouts/rhel8/bin/ip-control-loop; update the RUN block that
creates /usr/src/whereabouts/rhel*/bin to also create
/usr/src/whereabouts/rhel8/bin and symlink the same binaries (whereabouts,
ip-control-loop, node-slice-controller) into it (or create a rhel8 ->
rhel9/rhel10 redirect) so entrypoint.sh’s resolution of
/usr/src/whereabouts/rhel8/bin/ip-control-loop always finds the executable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 9a6a5f5f-2b5f-43b6-b007-51c28cfe563f
📒 Files selected for processing (1)
Dockerfile.openshift
sdodson
force-pushed
the
drop-rhel8-add-rhel10
branch
from
5156041 to
26eb379
Compare
sdodson
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@Dockerfile.openshift`:
- Around line 12-17: The Dockerfile currently creates /usr/src/whereabouts/bin
but fails to create the rhel-specific hardlink directories that entrypoint.sh
expects (it resolves binaries at /usr/src/whereabouts/rhel${rhelmajor}/bin/*);
update the Dockerfile to mkdir -p for /usr/src/whereabouts/rhel9/bin and
/usr/src/whereabouts/rhel10/bin (and rhel8 or fedora equivalents if required)
and create hardlinks (or copy) from /usr/src/whereabouts/bin/<binary> into those
rhelN/bin directories so that entrypoint.sh can find ip-control-loop,
whereabouts, node-slice-controller, etc.; ensure the same set of files copied
from the rhel9 stage (entrypoint.sh, whereabouts, ip-control-loop,
node-slice-controller) are hardlinked into each rhelN/bin directory referenced
by entrypoint.sh.
🪄 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: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 90c297fb-3c3a-4bbd-8887-ae26fd0b0852
📒 Files selected for processing (1)
Dockerfile.openshift
Comment on lines
12
to
+17
| RUN mkdir -p /usr/src/whereabouts/images && \ | ||
| mkdir -p /usr/src/whereabouts/bin && \ | ||
| mkdir -p /usr/src/whereabouts/rhel9/bin && \ | ||
| mkdir -p /usr/src/whereabouts/rhel8/bin | ||
| COPY --from=rhel8 /go/src/github.com/openshift/whereabouts/entrypoint.sh /usr/src/whereabouts/bin | ||
| COPY --from=rhel8 /go/src/github.com/openshift/whereabouts/bin/whereabouts /usr/src/whereabouts/bin | ||
| COPY --from=rhel8 /go/src/github.com/openshift/whereabouts/bin/ip-control-loop /usr/src/whereabouts/bin | ||
| COPY --from=rhel8 /go/src/github.com/openshift/whereabouts/bin/node-slice-controller /usr/src/whereabouts/bin | ||
| COPY --from=rhel9 /go/src/github.com/openshift/whereabouts/bin/whereabouts /usr/src/whereabouts/rhel9/bin | ||
| COPY --from=rhel9 /go/src/github.com/openshift/whereabouts/bin/ip-control-loop /usr/src/whereabouts/rhel9/bin | ||
| COPY --from=rhel9 /go/src/github.com/openshift/whereabouts/bin/node-slice-controller /usr/src/whereabouts/rhel9/bin | ||
| COPY --from=rhel8 /go/src/github.com/openshift/whereabouts/bin/whereabouts /usr/src/whereabouts/rhel8/bin | ||
| COPY --from=rhel8 /go/src/github.com/openshift/whereabouts/bin/ip-control-loop /usr/src/whereabouts/rhel8/bin | ||
| COPY --from=rhel8 /go/src/github.com/openshift/whereabouts/bin/node-slice-controller /usr/src/whereabouts/rhel8/bin | ||
| mkdir -p /usr/src/whereabouts/bin | ||
| COPY --from=rhel9 /go/src/github.com/openshift/whereabouts/entrypoint.sh /usr/src/whereabouts/bin | ||
| COPY --from=rhel9 /go/src/github.com/openshift/whereabouts/bin/whereabouts /usr/src/whereabouts/bin | ||
| COPY --from=rhel9 /go/src/github.com/openshift/whereabouts/bin/ip-control-loop /usr/src/whereabouts/bin | ||
| COPY --from=rhel9 /go/src/github.com/openshift/whereabouts/bin/node-slice-controller /usr/src/whereabouts/bin |
There was a problem hiding this comment.
Missing rhel9/bin and rhel10/bin hardlink directories causes runtime failure.
The entrypoint.sh resolves the binary path as /usr/src/whereabouts/rhel${rhelmajor}/bin/ip-control-loop, but this Dockerfile only creates /usr/src/whereabouts/bin/. The container will fail at runtime with "No such file or directory" for all RHEL versions (9, 10, and 8 for Fedora CoreOS).
The PR objectives and test plan mention creating hardlinks for rhel9/bin and rhel10/bin, but this is not implemented.
🐛 Proposed fix to add hardlink directories
RUN mkdir -p /usr/src/whereabouts/images && \
mkdir -p /usr/src/whereabouts/bin
COPY --from=rhel9 /go/src/github.com/openshift/whereabouts/entrypoint.sh /usr/src/whereabouts/bin
COPY --from=rhel9 /go/src/github.com/openshift/whereabouts/bin/whereabouts /usr/src/whereabouts/bin
COPY --from=rhel9 /go/src/github.com/openshift/whereabouts/bin/ip-control-loop /usr/src/whereabouts/bin
COPY --from=rhel9 /go/src/github.com/openshift/whereabouts/bin/node-slice-controller /usr/src/whereabouts/bin
+RUN mkdir -p /usr/src/whereabouts/rhel9/bin && \
+ ln /usr/src/whereabouts/bin/whereabouts /usr/src/whereabouts/rhel9/bin/whereabouts && \
+ ln /usr/src/whereabouts/bin/ip-control-loop /usr/src/whereabouts/rhel9/bin/ip-control-loop && \
+ ln /usr/src/whereabouts/bin/node-slice-controller /usr/src/whereabouts/rhel9/bin/node-slice-controller
+RUN mkdir -p /usr/src/whereabouts/rhel10/bin && \
+ ln /usr/src/whereabouts/bin/whereabouts /usr/src/whereabouts/rhel10/bin/whereabouts && \
+ ln /usr/src/whereabouts/bin/ip-control-loop /usr/src/whereabouts/rhel10/bin/ip-control-loop && \
+ ln /usr/src/whereabouts/bin/node-slice-controller /usr/src/whereabouts/rhel10/bin/node-slice-controllerFor rhel8/Fedora CoreOS compatibility (as flagged in a previous review), also add:
+RUN mkdir -p /usr/src/whereabouts/rhel8/bin && \
+ ln /usr/src/whereabouts/bin/whereabouts /usr/src/whereabouts/rhel8/bin/whereabouts && \
+ ln /usr/src/whereabouts/bin/ip-control-loop /usr/src/whereabouts/rhel8/bin/ip-control-loop && \
+ ln /usr/src/whereabouts/bin/node-slice-controller /usr/src/whereabouts/rhel8/bin/node-slice-controller🤖 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 `@Dockerfile.openshift` around lines 12 - 17, The Dockerfile currently creates
/usr/src/whereabouts/bin but fails to create the rhel-specific hardlink
directories that entrypoint.sh expects (it resolves binaries at
/usr/src/whereabouts/rhel${rhelmajor}/bin/*); update the Dockerfile to mkdir -p
for /usr/src/whereabouts/rhel9/bin and /usr/src/whereabouts/rhel10/bin (and
rhel8 or fedora equivalents if required) and create hardlinks (or copy) from
/usr/src/whereabouts/bin/<binary> into those rhelN/bin directories so that
entrypoint.sh can find ip-control-loop, whereabouts, node-slice-controller,
etc.; ensure the same set of files copied from the rhel9 stage (entrypoint.sh,
whereabouts, ip-control-loop, node-slice-controller) are hardlinked into each
rhelN/bin directory referenced by entrypoint.sh.
…ctories The version-specific binary directories (rhel8/, rhel9/) are no longer needed since cluster-network-operator#2967 removed the OS detection logic from cnibincopy and now uses a single SOURCE_DIRECTORY. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
sdodson
force-pushed
the
drop-rhel8-add-rhel10
branch
from
26eb379 to
42aaab5
Compare
Member
Author
|
/retest-required |
Member
Author
|
/hold cancel |
Member
Author
|
/test e2e-aws e2e-aws-upgrade iamges verify-deps okd-scos-images |
Member
Author
|
/retest-required |
Contributor
|
@sdodson: 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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Remove the rhel8 build stage and strip debug symbols from binaries.
The rhel9/rhel10 version-specific subdirectories are no longer needed
since openshift/cluster-network-operator#2967 removed the OS detection
logic and now copies binaries directly from the base directory.
Summary by CodeRabbit