DNM, test test remove docker.io #2068
Conversation
WalkthroughReplaced several container images and updated init-container readiness checks across sample-app manifests (curl/nc-based checks → UBI and bash /dev/tcp checks); swapped multiple Mongo images to quay.io/community builds; added PVC block-mode manifests; updated one nginx image/command and appended Velero plugins when VMFR enabled. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Comment |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: weshayutin 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
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@tests/e2e/sample-applications/mongo-persistent/mongo-persistent-csi.yaml`:
- Around line 125-127: The curl-tool container uses the UBI base image which
lacks curl, causing e2e tests (which exec curl via
tests/e2e/lib/common_helpers.go) to fail; update the container spec for the
curl-tool (container name "curl-tool") in the mongo-persistent-csi.yaml (and
other affected sample YAMLs) to either use a curl-capable image such as
docker.io/curlimages/curl:8.5.0 or add installation of curl to the UBI image
(e.g., via package install in the image or init step) so the container can run
curl commands during tests.
In
`@tests/e2e/sample-applications/mysql-persistent/mysql-persistent-twovol-csi.yaml`:
- Around line 150-152: The curl-tool container used by e2e tests (container name
"curl-tool") lacks curl causing MakeRequest calls in common_helpers.go to fail;
fix by switching the image to one that includes curl (for example use
curlimages/curl:latest or a distro image with curl) or ensure curl is installed
at container startup (add an init command or modify the Dockerfile to install
curl) so the curl-tool container can execute the test requests successfully.
In `@tests/e2e/sample-applications/mysql-persistent/mysql-persistent.yaml`:
- Around line 242-243: The loop currently relies on $attempt to decide failure
and thus misreports success if the connection succeeds on the final attempt;
update the command string so that after the until loop you explicitly re-check
the TCP connection (using the same (echo > /dev/tcp/mysql/3306) >/dev/null 2>&1
test) and only report an error if that explicit connection test fails, rather
than basing the failure solely on $attempt and $max_attempts; keep the same
variables (max_attempts, attempt) and the existing tcp check token so you can
locate and modify the command in the YAML.
🧹 Nitpick comments (3)
tests/e2e/sample-applications/mongo-persistent/mongo-persistent-block.yaml (1)
168-168: Pin the UBI image to a specific version/digest instead of:latest.Using
:latestmakes the e2e environment non-deterministic and increases supply-chain risk from unreviewed upstream changes. Please pin to a specific version or digest.♻️ Example update (use a pinned tag or digest)
- - image: registry.access.redhat.com/ubi8/ubi:latest + - image: registry.access.redhat.com/ubi8/ubi:<pinned-tag-or-digest> ... - image: registry.access.redhat.com/ubi8/ubi:latest + image: registry.access.redhat.com/ubi8/ubi:<pinned-tag-or-digest>Also applies to: 239-240
tests/e2e/sample-applications/mongo-persistent/mongo-persistent-csi.yaml (1)
125-127: Pin the UBI images instead of using:latest.
Line 125 and Line 196 pull a floating tag, which can change over time and make e2e runs non-reproducible. Please pin to a fixed tag or digest for stability and security.Also applies to: 196-197
tests/e2e/sample-applications/mongo-persistent/mongo-persistent.yaml (1)
138-140: Pin the UBI image tag for determinism in sample application.The
registry.access.redhat.com/ubi8/ubi:latesttag should be pinned to a specific version or digest for reproducibility. However, curl is included by default in the standard UBI8 image and is not actually invoked by either container (curl-tool only sleeps, and the init container uses bash /dev/tcp). Since this is a test sample application, the determinism concern is less critical than in production, but pinning the tag remains a best practice.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (6)
tests/e2e/sample-applications/mongo-persistent/mongo-persistent-block.yamltests/e2e/sample-applications/mongo-persistent/mongo-persistent-csi.yamltests/e2e/sample-applications/mongo-persistent/mongo-persistent.yamltests/e2e/sample-applications/mysql-persistent/mysql-persistent-csi.yamltests/e2e/sample-applications/mysql-persistent/mysql-persistent-twovol-csi.yamltests/e2e/sample-applications/mysql-persistent/mysql-persistent.yaml
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
tests/e2e/sample-applications/mongo-persistent/mongo-persistent.yamltests/e2e/sample-applications/mysql-persistent/mysql-persistent.yamltests/e2e/sample-applications/mongo-persistent/mongo-persistent-block.yamltests/e2e/sample-applications/mysql-persistent/mysql-persistent-twovol-csi.yamltests/e2e/sample-applications/mongo-persistent/mongo-persistent-csi.yamltests/e2e/sample-applications/mysql-persistent/mysql-persistent-csi.yaml
🔇 Additional comments (5)
tests/e2e/sample-applications/mysql-persistent/mysql-persistent.yaml (1)
171-173: Image change looks good.Switching to
registry.access.redhat.com/ubi8/ubi:latestaddresses the Docker Hub rate limiting issue. Consider renaming the container fromcurl-toolsince UBI8 doesn't include curl, though this is minor for a test sample.tests/e2e/sample-applications/mysql-persistent/mysql-persistent-csi.yaml (2)
158-160: Verifycurlis available in the newcurl-toolimage.Line 158 replaces the curl-specific image with UBI; if tests exec into this container expecting
curl, this could break. Please confirm the binary is present (or bake it into the image).
229-230: Ensure the init container hasbashwith/dev/tcpsupport.The new probe relies on
bash -cand/dev/tcp. If the UBI image lacks bash or has/dev/tcpdisabled, the init container will fail. Please confirm.tests/e2e/sample-applications/mysql-persistent/mysql-persistent-twovol-csi.yaml (1)
227-228: LGTM!The switch to bash's
/dev/tcpfor TCP connectivity checking is a valid approach that removes the docker.io dependency. The retry logic correctly handles both success and timeout scenarios.tests/e2e/sample-applications/mongo-persistent/mongo-persistent.yaml (1)
208-210: Verify/dev/tcpreadiness check works in the new base image.This init container now depends on bash’s
/dev/tcpfeature. If the UBI image lacks bash net‑redirections orbashisn’t on PATH, startup will fail. Please confirm the feature is available, or switch to an explicit tool (nc/curl) with timeouts.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| - image: registry.access.redhat.com/ubi8/ubi:latest | ||
| name: curl-tool | ||
| command: ["/bin/sleep", "infinity"] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for references to curl-tool container
echo "=== Searching for curl-tool references ==="
rg -n -C3 '\bcurl-tool\b' tests/
echo ""
echo "=== Searching for oc exec commands targeting curl-tool ==="
rg -n -C3 'exec.*curl-tool|--container[= ]curl-tool' tests/
echo ""
echo "=== Searching for curl command invocations in test files ==="
rg -n 'curl ' tests/e2e/ | head -30Repository: openshift/oadp-operator
Length of output: 13254
Add curl to the curl-tool container or use a curl-capable base image.
The e2e tests execute curl commands within the curl-tool container (see tests/e2e/lib/common_helpers.go), but registry.access.redhat.com/ubi8/ubi:latest does not include curl. Compare with fedora-todolist.yaml which uses docker.io/curlimages/curl:8.5.0. Either install curl in the UBI container or use the proper curl image. This affects multiple sample applications (mongo-persistent, mysql-persistent variants) and will cause test failures.
🤖 Prompt for AI Agents
In `@tests/e2e/sample-applications/mongo-persistent/mongo-persistent-csi.yaml`
around lines 125 - 127, The curl-tool container uses the UBI base image which
lacks curl, causing e2e tests (which exec curl via
tests/e2e/lib/common_helpers.go) to fail; update the container spec for the
curl-tool (container name "curl-tool") in the mongo-persistent-csi.yaml (and
other affected sample YAMLs) to either use a curl-capable image such as
docker.io/curlimages/curl:8.5.0 or add installation of curl to the UBI image
(e.g., via package install in the image or init step) so the container can run
curl commands during tests.
| - image: registry.access.redhat.com/ubi8/ubi:latest | ||
| name: curl-tool | ||
| command: ["/bin/sleep", "infinity"] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for test files that reference curl-tool or the mysql-persistent manifest
find tests -type f \( -name "*.go" -o -name "*.sh" \) | head -20Repository: openshift/oadp-operator
Length of output: 691
🏁 Script executed:
#!/bin/bash
# Search for references to curl-tool in the repository
rg -r "curl-tool" --type go --type sh -i -C 2Repository: openshift/oadp-operator
Length of output: 463
🏁 Script executed:
#!/bin/bash
# Search for references to mysql-persistent-twovol-csi test or manifest usage
rg -r "mysql-persistent-twovol-csi\|mysql-persistent" --type go -C 3 | head -100Repository: openshift/oadp-operator
Length of output: 463
🏁 Script executed:
#!/bin/bash
# Check the actual file to understand the context better
head -250 tests/e2e/sample-applications/mysql-persistent/mysql-persistent-twovol-csi.yaml | tail -110Repository: openshift/oadp-operator
Length of output: 3416
🏁 Script executed:
#!/bin/bash
# Search for references to mysql-persistent in go test files
rg "mysql-persistent" tests/e2e --type go -C 3Repository: openshift/oadp-operator
Length of output: 14177
🏁 Script executed:
#!/bin/bash
# Search for curl-tool references in the entire repository
rg "curl-tool" tests/ -C 2Repository: openshift/oadp-operator
Length of output: 8693
🏁 Script executed:
#!/bin/bash
# Check apps.go which likely defines test applications
grep -n "mysql-persistent\|curl-tool" tests/e2e/lib/apps.goRepository: openshift/oadp-operator
Length of output: 338
🏁 Script executed:
#!/bin/bash
# Search for exec or kubectl commands that might interact with containers
rg "exec.*curl-tool|kubectl exec" tests/e2e --type go -C 2 | head -50Repository: openshift/oadp-operator
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Search for the MakeRequest function definition
rg "func MakeRequest" tests/e2e -A 30Repository: openshift/oadp-operator
Length of output: 2197
🏁 Script executed:
#!/bin/bash
# Search for curl command usage in test files
rg "curl\s+http" tests/e2e --type go | head -30Repository: openshift/oadp-operator
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Check apps.go more carefully for how it makes requests through curl-tool
sed -n '530,600p' tests/e2e/lib/apps.goRepository: openshift/oadp-operator
Length of output: 2430
🏁 Script executed:
#!/bin/bash
# Get the full MakeRequest function from common_helpers.go
rg "func MakeRequest" tests/e2e/lib/common_helpers.go -A 100 | head -120Repository: openshift/oadp-operator
Length of output: 3553
🏁 Script executed:
#!/bin/bash
# Search for curlInProxyCmd to see what curl command is constructed
rg "curlInProxyCmd" tests/e2e/lib/common_helpers.go -B 5 -A 5Repository: openshift/oadp-operator
Length of output: 1098
Container named "curl-tool" requires curl for e2e tests.
The e2e tests execute curl commands directly in the curl-tool container (via MakeRequest in common_helpers.go). UBI8 does not include curl by default, causing test failures when the proxy pod is used as a fallback for service connectivity.
Either:
- Include curl in the container image (update the Dockerfile or use an image that includes curl)
- Install curl as part of container initialization
- Switch to an image that includes curl (e.g., curlimages/curl or a standard distro image)
🤖 Prompt for AI Agents
In
`@tests/e2e/sample-applications/mysql-persistent/mysql-persistent-twovol-csi.yaml`
around lines 150 - 152, The curl-tool container used by e2e tests (container
name "curl-tool") lacks curl causing MakeRequest calls in common_helpers.go to
fail; fix by switching the image to one that includes curl (for example use
curlimages/curl:latest or a distro image with curl) or ensure curl is installed
at container startup (add an init command or modify the Dockerfile to install
curl) so the curl-tool container can execute the test requests successfully.
| image: registry.access.redhat.com/ubi8/ubi:latest | ||
| command: ['bash', '-c', 'sleep 30; max_attempts=180; attempt=0; until (echo > /dev/tcp/mysql/3306) >/dev/null 2>&1 || [ $attempt -ge $max_attempts ]; do attempt=$((attempt+1)); echo "Attempt $attempt/$max_attempts: Trying to connect to mysql DB port"; sleep 5; done; if [ $attempt -ge $max_attempts ]; then echo "ERROR: Failed to connect to mysql after $max_attempts attempts"; exit 1; fi; echo "mysql DB port reachable"'] |
There was a problem hiding this comment.
Edge-case logic bug in connectivity check.
The until condition (echo > /dev/tcp/mysql/3306) || [ $attempt -ge $max_attempts ] exits when either condition is true. However, if connection succeeds exactly on the 180th attempt, $attempt will equal $max_attempts, and the post-loop check will incorrectly report failure.
Suggested fix
- command: ['bash', '-c', 'sleep 30; max_attempts=180; attempt=0; until (echo > /dev/tcp/mysql/3306) >/dev/null 2>&1 || [ $attempt -ge $max_attempts ]; do attempt=$((attempt+1)); echo "Attempt $attempt/$max_attempts: Trying to connect to mysql DB port"; sleep 5; done; if [ $attempt -ge $max_attempts ]; then echo "ERROR: Failed to connect to mysql after $max_attempts attempts"; exit 1; fi; echo "mysql DB port reachable"']
+ command: ['bash', '-c', 'sleep 30; max_attempts=180; attempt=0; until (echo > /dev/tcp/mysql/3306) >/dev/null 2>&1; do attempt=$((attempt+1)); if [ $attempt -ge $max_attempts ]; then echo "ERROR: Failed to connect to mysql after $max_attempts attempts"; exit 1; fi; echo "Attempt $attempt/$max_attempts: Trying to connect to mysql DB port"; sleep 5; done; echo "mysql DB port reachable"']📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| image: registry.access.redhat.com/ubi8/ubi:latest | |
| command: ['bash', '-c', 'sleep 30; max_attempts=180; attempt=0; until (echo > /dev/tcp/mysql/3306) >/dev/null 2>&1 || [ $attempt -ge $max_attempts ]; do attempt=$((attempt+1)); echo "Attempt $attempt/$max_attempts: Trying to connect to mysql DB port"; sleep 5; done; if [ $attempt -ge $max_attempts ]; then echo "ERROR: Failed to connect to mysql after $max_attempts attempts"; exit 1; fi; echo "mysql DB port reachable"'] | |
| image: registry.access.redhat.com/ubi8/ubi:latest | |
| command: ['bash', '-c', 'sleep 30; max_attempts=180; attempt=0; until (echo > /dev/tcp/mysql/3306) >/dev/null 2>&1; do attempt=$((attempt+1)); if [ $attempt -ge $max_attempts ]; then echo "ERROR: Failed to connect to mysql after $max_attempts attempts"; exit 1; fi; echo "Attempt $attempt/$max_attempts: Trying to connect to mysql DB port"; sleep 5; done; echo "mysql DB port reachable"'] |
🤖 Prompt for AI Agents
In `@tests/e2e/sample-applications/mysql-persistent/mysql-persistent.yaml` around
lines 242 - 243, The loop currently relies on $attempt to decide failure and
thus misreports success if the connection succeeds on the final attempt; update
the command string so that after the until loop you explicitly re-check the TCP
connection (using the same (echo > /dev/tcp/mysql/3306) >/dev/null 2>&1 test)
and only report an error if that explicit connection test fails, rather than
basing the failure solely on $attempt and $max_attempts; keep the same variables
(max_attempts, attempt) and the existing tcp check token so you can locate and
modify the command in the YAML.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@tests/e2e/sample-applications/mongo-persistent/mongo-persistent.yaml`:
- Around line 127-131: The curl-tool container uses
registry.access.redhat.com/ubi8/ubi:latest which lacks curl; update the
curl-tool container spec (container name "curl-tool") to use a curl-capable
image such as quay.io/curl/curl:latest or modify its startup command to install
curl (e.g., run a package install before sleeping) so the MakeRequest calls in
common_helpers.go can execute curl successfully.
♻️ Duplicate comments (2)
tests/e2e/sample-applications/mysql-persistent/mysql-persistent-twovol-csi.yaml (1)
150-152: curl-tool container lacks curl.This issue has already been flagged in a previous review comment.
tests/e2e/sample-applications/mongo-persistent/mongo-persistent-csi.yaml (1)
125-127: curl-tool container lacks curl.This issue has already been flagged in a previous review comment.
🧹 Nitpick comments (1)
tests/e2e/sample-applications/mongo-persistent/mongo-persistent-block.yaml (1)
17-35: Privileged container access is intentional but should be documented.The static analysis flags for privileged containers (CKV_K8S_16, CKV_K8S_20, CKV_K8S_23) are expected for block device access. The setup-block-device init container requires privileges to format (
mkfs.ext4) and mount the raw block device. Consider adding a comment in the manifest explaining why privileged mode is required for this block-mode use case.Also, line 66 labels the pod with
curl-tool: 'true'but the sidecar (lines 161-165) now uses UBI8 instead of curl image. This label may be stale.Also applies to: 67-95
| - image: registry.access.redhat.com/ubi8/ubi:latest | ||
| name: curl-tool | ||
| command: | ||
| - /bin/sleep | ||
| - infinity |
There was a problem hiding this comment.
Container named curl-tool requires curl for e2e tests.
The e2e tests execute curl commands directly in the curl-tool container via MakeRequest in common_helpers.go. registry.access.redhat.com/ubi8/ubi:latest does not include curl by default.
Options:
- Use a curl-capable image (e.g.,
quay.io/curl/curl:latestto stay off docker.io) - Install curl at container startup
🤖 Prompt for AI Agents
In `@tests/e2e/sample-applications/mongo-persistent/mongo-persistent.yaml` around
lines 127 - 131, The curl-tool container uses
registry.access.redhat.com/ubi8/ubi:latest which lacks curl; update the
curl-tool container spec (container name "curl-tool") to use a curl-capable
image such as quay.io/curl/curl:latest or modify its startup command to install
curl (e.g., run a package install before sleeping) so the MakeRequest calls in
common_helpers.go can execute curl successfully.
|
@weshayutin: 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. |
1 participant
Why the changes were made
BAH.. containers are not starting due to hidden registry pull errors and rate limiting
How to test the changes made
CHECK CHECK CHECK