🌱 update the mqtt image #346
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: skeeey The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
WalkthroughThis PR modifies two deployment configuration files in the examples directory. One updates the MQTT broker container image registry source, and another simplifies the readiness probe for the klusterlet-work-agent by replacing an iterative loop-based check with a fixed 60-second delay. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
Signed-off-by: Wei Liu <[email protected]>
|
/hold |
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
examples/deploy/mqtt/mqtt-broker.yaml(1 hunks)examples/deploy/ocm-cloudevents/install.sh(1 hunks)
🔇 Additional comments (1)
examples/deploy/ocm-cloudevents/install.sh (1)
71-72: The proposed fix is logically flawed—it adds arollout statuscheck that already exists.The review's suggestion is problematic because:
Line 68 already has the rollout status check:
${KUBECTL} -n open-cluster-management-agent rollout status deployment/klusterlet-work-agent --timeout=120sruns before the sleep.The proposed diff adds it again after the sleep, which is ineffective. If rollout status confirms readiness at line 68, the subsequent 60-second sleep serves no purpose, and running rollout status again after the sleep is redundant.
The real pattern issue: The logic should be either:
- Remove the
sleep 60(line 72) since rollout status already verifies readiness, OR- Replace
sleep 60with a more targeted readiness check if the TODO's "live probe" requirement implies checking MQTT broker connectivity specifically—which requires actual Kubernetes probe configuration on the deployment, not shell script logic.The current code is correct to have the rollout status check; the sleep is the questionable addition.
Likely an incorrect or invalid review comment.
| containers: | ||
| - name: mosquitto | ||
| image: eclipse-mosquitto:2.0.18 | ||
| image: quay.io/maestro/eclipse-mosquitto:2.0.18 |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the new image registry and document the rationale for the registry change.
The image registry was changed from Docker Hub (eclipse-mosquitto:2.0.18) to Quay (quay.io/maestro/eclipse-mosquitto:2.0.18). While the version remains the same, this is a significant change that requires verification and documentation.
Before merging, please:
- Confirm image availability: Verify that
quay.io/maestro/eclipse-mosquitto:2.0.18exists and is publicly accessible (or provide pull-secret configuration if private). - Document the rationale: Update the PR description to explain why the registry was changed (e.g., security, compliance, performance, upstream deprecation).
- Test in CI/CD: Ensure your pipeline successfully pulls and deploys from the new registry without ImagePullBackOff errors.
Run the following script to verify image accessibility:
🏁 Script executed:
#!/bin/bash
# Verify image exists and is accessible
skopeo inspect docker://quay.io/maestro/eclipse-mosquitto:2.0.18 2>&1 | head -20Length of output: 44
Let me try an alternative approach using the Quay.io REST API:
---
🏁 Script executed:
```shell
#!/bin/bash
# Query Quay.io REST API for image metadata
curl -s "https://quay.io/api/v1/repositories/maestro/eclipse-mosquitto/tags/?specificTag=2.0.18" | head -50
Length of output: 1726
🌐 Web query:
eclipse-mosquitto 2.0.18 quay.io maestro image registry
💡 Result:
I couldn't find an eclipse-mosquitto:2.0.18 image hosted on quay.io (Maestro) public registry. The official eclipse-mosquitto images (including tag 2.0.18) are published on Docker Hub (docker.io/eclipse-mosquitto:2.0.18) and are available via other registries (e.g., Chainguard/cgr.dev). [1][2]
Sources:
- Docker image releases for eclipse-mosquitto (shows 2.0.18 tag on Docker Hub). [1]
- Chainguard container directory listing (eclipse-mosquitto 2.0.18 available via cgr.dev/Chainguard). [2]
Image registry change breaks deployment—verify or revert to Docker Hub.
The image quay.io/maestro/eclipse-mosquitto:2.0.18 does not exist on the Quay.io (Maestro) registry. Web verification confirms that official eclipse-mosquitto 2.0.18 images are published on Docker Hub, not Quay. This change will cause deployments to fail with ImagePullBackOff errors.
Required actions:
- Either revert to
eclipse-mosquitto:2.0.18(Docker Hub) or provide the correct Quay registry path if intentionally using a custom/private image. - If a registry change is necessary, document the rationale and ensure the image exists and is accessible.
Line 42: image: quay.io/maestro/eclipse-mosquitto:2.0.18
🤖 Prompt for AI Agents
In examples/deploy/mqtt/mqtt-broker.yaml around line 42, the image reference
`quay.io/maestro/eclipse-mosquitto:2.0.18` does not exist on that Quay registry
and will cause ImagePullBackOff; either change the image back to the official
Docker Hub name `eclipse-mosquitto:2.0.18` or replace it with the correct Quay
path for a private/custom image, and if you keep a non-Docker-Hub registry
ensure the image is published there, update any required
imagePullSecrets/permissions, and add a short comment in the YAML explaining why
the non-default registry is used and confirming accessibility.
There was a problem hiding this comment.
let's use this image to avoid sometimes the eclipse-mosquitto images cannot be pulled from docker hub
There was a problem hiding this comment.
🧩 Analysis chain
@skeeey, let me verify the image accessibility one more time to ensure the deployment will work correctly.
🏁 Script executed:
#!/bin/bash
# Verify if the image is now accessible
echo "=== Checking image accessibility via Quay.io API ==="
curl -s "https://quay.io/api/v1/repository/maestro/eclipse-mosquitto/tag/2.0.18/images" 2>&1 | head -30
echo -e "\n=== Checking repository visibility ==="
curl -s "https://quay.io/api/v1/repository/maestro/eclipse-mosquitto" 2>&1 | head -30
Length of output: 1008
@skeeey, perfect! I've verified that the image is indeed accessible. The repository quay.io/maestro/eclipse-mosquitto is public, and the tag 2.0.18 exists with manifest digest sha256:9d3b8c89ac16e97abba919e86642e1f7c2340a4a6e8fcf61e711f16c70cd7360.
My apologies for the initial concern - the image is valid and will deploy successfully. The deployment will work as expected with this Quay.io registry image. ✅
In the open-cluster-management-io/addon-framework repository, the MQTT broker deployment in examples/deploy/mqtt/mqtt-broker.yaml uses the custom image quay.io/maestro/eclipse-mosquitto:2.0.18 from the maestro organization's Quay.io registry, which is publicly accessible and intentionally chosen over the Docker Hub official image.
|
/unhold |
1 participant
Summary
Related issue(s)
Fixes #
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.