The subcommand network-test was not working due to /sbin/init command#180
The subcommand network-test was not working due to /sbin/init command#180rjhowe wants to merge 1 commit into
Conversation
- systemd container was only needed for nginx server which can be
replaced with python http server
- For network tests changing all pods created by script to run python
server by default
- Remove nginx package from Dockerfile and Dockerfile.fedora as its
not needed for testing network
- Made changes to Dockerfile.fedora updating to 4.22 build and
changing yum to dnf
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: rjhowe 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 |
WalkthroughThe PR replaces nginx with a lightweight Python HTTP server across the network-tools image and test infrastructure. It removes nginx from container image package lists, updates test pods to run ChangesNetwork Tools Image and Test Script Updates
🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
debug-scripts/test-networking/common (1)
19-39:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse an unprivileged HTTP port to avoid pod startup failures
python3 -m http.server 80(Line 19) can fail when the container runs as non-root, which breaks readiness and leaves the service without endpoints. Use an unprivileged port (for example8080) and expose the same port.Suggested patch
- oc run "$POD_NAME" --image=quay.io/openshift/origin-network-tools:latest \ - --overrides='{ "spec": { "nodeSelector": {"'"$POD_NAME"'": "network-tools-debug-role"}, "tolerations": [{"key": "node-role.kubernetes.io/master", "effect": "NoSchedule", "operator": "Exists"}] }}' \ - -- python3 -m http.server 80 + oc run "$POD_NAME" --image=quay.io/openshift/origin-network-tools:latest \ + --overrides='{ "spec": { "nodeSelector": {"'"$POD_NAME"'": "network-tools-debug-role"}, "tolerations": [{"key": "node-role.kubernetes.io/master", "effect": "NoSchedule", "operator": "Exists"}] }}' \ + -- python3 -m http.server 8080 @@ - oc expose pod/"$SVC_NAME" --port=80 + oc expose pod/"$SVC_NAME" --port=8080🤖 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 `@debug-scripts/test-networking/common` around lines 19 - 39, The pod startup uses a privileged port (80) which fails for non-root containers; update create_pod_on_node (the command that starts the server) to run the HTTP server on an unprivileged port (e.g. 8080) and make create_svc expose that same port (change the oc expose invocation and any port references from 80 to 8080) so readiness succeeds and endpoints are created; ensure any wait logic (oc get ep) still references the service name only and no port-specific jsonpath changes are required.
🤖 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.fedora`:
- Around line 8-12: The Dockerfile uses an unpinned non-RH runtime base ("FROM
fedora:latest") which violates the base image policy; update the final runtime
stage to either use a UBI minimal or distroless image from catalog.redhat.com
(preferred) or, if using a non-Red Hat image, pin it by digest instead of
:latest—modify the final FROM line accordingly and ensure the change applies to
the runtime stage that follows the ovnkube-trace and tools stages so provenance
and reproducibility are preserved.
---
Outside diff comments:
In `@debug-scripts/test-networking/common`:
- Around line 19-39: The pod startup uses a privileged port (80) which fails for
non-root containers; update create_pod_on_node (the command that starts the
server) to run the HTTP server on an unprivileged port (e.g. 8080) and make
create_svc expose that same port (change the oc expose invocation and any port
references from 80 to 8080) so readiness succeeds and endpoints are created;
ensure any wait logic (oc get ep) still references the service name only and no
port-specific jsonpath changes are required.
🪄 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: c554a272-4ea1-4b30-9fcb-c4bc15d56af1
📒 Files selected for processing (4)
DockerfileDockerfile.fedoradebug-scripts/test-networking/commondocs/user.md
💤 Files with no reviewable changes (2)
- Dockerfile
- docs/user.md
| FROM registry.ci.openshift.org/ocp/4.22:ovn-kubernetes AS ovnkube-trace | ||
|
|
||
| FROM registry.ci.openshift.org/ocp/4.12:tools AS tools | ||
| FROM registry.ci.openshift.org/ocp/4.22:tools AS tools | ||
|
|
||
| FROM fedora:latest |
There was a problem hiding this comment.
Runtime base image violates container hardening policy
The runtime stage still uses fedora:latest (Line 12), which is neither a UBI minimal/distroless base nor pinned by digest for a non-Red Hat image. This weakens provenance and reproducibility guarantees.
As per coding guidelines, "Base image: UBI minimal or distroless from catalog.redhat.com" and "non-RH images: pin by digest".
🤖 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.fedora` around lines 8 - 12, The Dockerfile uses an unpinned
non-RH runtime base ("FROM fedora:latest") which violates the base image policy;
update the final runtime stage to either use a UBI minimal or distroless image
from catalog.redhat.com (preferred) or, if using a non-Red Hat image, pin it by
digest instead of :latest—modify the final FROM line accordingly and ensure the
change applies to the runtime stage that follows the ovnkube-trace and tools
stages so provenance and reproducibility are preserved.
There was a problem hiding this comment.
Only used for local testing this fedora image is not shipped or pushed to RH repos.
There was a problem hiding this comment.
@rjhowe, understood — the Dockerfile.fedora is explicitly a development-only image (noted in its header comment as well) and is never shipped to any registry. The hardening policy applies to production/shipped images. I'll dismiss this concern.
✏️ Learnings added
Learnt from: rjhowe
Repo: openshift/network-tools PR: 180
File: Dockerfile.fedora:8-12
Timestamp: 2026-06-03T20:41:39.450Z
Learning: In the `openshift/network-tools` repository, `Dockerfile.fedora` is a **development-only** image used exclusively for local testing. It is never pushed to Red Hat or any other production registry. Container hardening policy requirements (UBI base image, digest pinning, etc.) do not apply to this file.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
|
/retest |
|
@rjhowe: 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. |
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores