[dnm-testing]Dockerfile: Remove redundant iproute and iproute-tc from INSTALL_PKGS#3163
[dnm-testing]Dockerfile: Remove redundant iproute and iproute-tc from INSTALL_PKGS#3163bpickard22 wants to merge 1 commit intoopenshift:masterfrom
Conversation
The base image (ovn-kubernetes-base) already includes iproute as a dependency of the OVS/OVN packages installed from the Fast Datapath repo. Explicitly requesting iproute again in the final image stage causes an intermittent dnf depsolve failure when the FDP version (e.g. 6.11.0) conflicts with the iproute-tc version available in rhel-9-baseos (e.g. 6.2.0). iproute-tc provides the tc binary, which is not used anywhere in ovn-kubernetes and can be safely dropped. Assisted by Claude Opus 4.6 Signed-off-by: Benjamin Pickard <bpickard@redhat.com>
WalkthroughThe Dockerfile's package installation list is modified to remove Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
🚥 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 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: bpickard22 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Dockerfile (1)
42-52:⚠️ Potential issue | 🟡 MinorAdd a build-time guard for the
ipbinary to catch transitive dependency drift.Removing explicit
iproutefrom INSTALL_PKGS is fine—relying on transitive dependencies from base/FDP layers is reasonable for depsolve stability. However,ipis a hard runtime dependency (used extensively byRunIP()for IP route and rule management). Add a simple check so the build fails early if the base image stops providingiroute:♻️ Proposed build-time check
RUN stat /usr/bin/oc +RUN command -v ip >/dev/null(Note: Direct
tccommand usage is absent in the codebase, soiproute-tcremoval is safe.)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` around lines 42 - 52, Add a build-time guard after the package-install step in the Dockerfile to fail fast if the `ip` binary is missing: after the dnf install && dnf clean lines, run a shell check (e.g., command -v ip >/dev/null || { echo "missing ip binary required by RunIP()" >&2; exit 1; }) so any transitive removal of iproute from base layers breaks the build immediately; reference the runtime dependency used by RunIP() in the error message to make the failure clear.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@Dockerfile`:
- Around line 42-52: Add a build-time guard after the package-install step in
the Dockerfile to fail fast if the `ip` binary is missing: after the dnf install
&& dnf clean lines, run a shell check (e.g., command -v ip >/dev/null || { echo
"missing ip binary required by RunIP()" >&2; exit 1; }) so any transitive
removal of iproute from base layers breaks the build immediately; reference the
runtime dependency used by RunIP() in the error message to make the failure
clear.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Enterprise
Run ID: bfafe02a-00b2-4a3b-954c-a51cca5e46b9
📒 Files selected for processing (1)
Dockerfile
|
@bpickard22: The following tests 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. |
bpickard22
changed the title
1 participant
The base image (ovn-kubernetes-base) already includes iproute as a dependency of the OVS/OVN packages installed from the Fast Datapath repo. Explicitly requesting iproute again in the final image stage causes an intermittent dnf depsolve failure when the FDP version (e.g. 6.11.0) conflicts with the iproute-tc version available in rhel-9-baseos (e.g. 6.2.0).
iproute-tc provides the tc binary, which is not used anywhere in ovn-kubernetes and can be safely dropped.
Assisted by Claude Opus 4.6
📑 Description
Fixes #
Additional Information for reviewers
✅ Checks
How to verify it
Summary by CodeRabbit