feat: Add opt-in DSX Exchange DevSpace profile#3795
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (8)
💤 Files with no reviewable changes (1)
Summary by CodeRabbit
WalkthroughAdds optional DSX Exchange event bus configuration to the NICO API Helm chart, a DevSpace deployment profile for a local NATS MQTT service, configuration parsing tests, documentation, and REST integration environment updates. ChangesDSX Exchange integration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant DevSpace
participant Helm
participant NICOAPI
participant DSXExchange
DevSpace->>Helm: Apply dsx-exchange profile
Helm->>DSXExchange: Deploy NATS MQTT service
Helm->>NICOAPI: Inject event bus configuration
NICOAPI->>DSXExchange: Publish managed host state every 10 seconds
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Manual TestingManual testing was performed on the branch Setupgit checkout codex/devspace-dsx-exchange
test "$(git rev-parse HEAD)" = "1f59be25338f57cb78a737ef0f01a9e83bf0cf5d"
work_dir="$(mktemp -d)"
export KUBECONFIG="${work_dir}/kubeconfig"
kind create cluster \
--name pr3795 \
--image kindest/node:v1.31.6@sha256:28b7cbb993dfe093c76641a0c95807637213c9109b761f1d422c2400e22b8e87 \
--kubeconfig "${KUBECONFIG}"
dev/deployment/devspace/bootstrap-prereqs.sh
devspace use context kind-pr3795 -n nico-systemRunbookdevspace print --skip-info --no-colors -n nico-system
devspace print --skip-info --no-colors -n nico-system --profile dsx-exchange
devspace deploy --profile dsx-exchange -n nico-system --no-colors
kubectl get pods -A
kubectl run mqtt-subscriber \
-n nico-system \
--image=eclipse-mosquitto:2.0.21 \
--restart=Never \
--rm -i \
--pod-running-timeout=120s \
-- mosquitto_sub \
-h dsx-exchange \
-p 1883 \
-t NICO/v1/machine/+/state \
-C 1 \
-W 30 \
-vVerificationThe default DevSpace render contained neither the DSX Exchange deployment nor The exact-head deployment completed successfully. Its post-deploy check reported the registered site online with both Core hosts Ready and synchronized from a current REST inventory, and all deployed workloads were Ready with zero restarts. The one-shot subscriber received a message on |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f59be2533
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| value: | ||
| dsxExchangeEventBusConfig: '{enabled=true,mqtt_endpoint="dsx-exchange",mqtt_broker_port=1883,auth={auth_mode="none"},periodic_state_republish={interval="10s"}}' | ||
| - name: core-only | ||
| patches: |
There was a problem hiding this comment.
Nit: the path targets the entire nico-api object rather than just the new key. This works today since nico-api isn't in the inline values block, but if a future profile also patches deployments.carbide-local.helm.values.nico-api the second op: add would clobber this one. Safer to be surgical: path: deployments.carbide-local.helm.values.nico-api.dsxExchangeEventBusConfig with the TOML string as the value directly. Non-blocking.
| @@ -9,8 +9,6 @@ nico-api: | |||
| requests: | |||
There was a problem hiding this comment.
Nit: removing legacyAlias.enabled: false silently re-enables the carbide-api ExternalName Service in dev (chart default is true). PR description doesn't mention this. If intentional after #3596 clarified that the alias only covers DNS not TLS, worth a one-line note. If accidental, revert. Non-blocking.
shayan1995
left a comment
shayan1995
left a comment
There was a problem hiding this comment.
Two nits left inline (both non-blocking) — otherwise LGTM. The opt-in profile is clean, the default path is unaffected, and the machine-a-tron/nico-admin-cli fixes are correct.
3 participants
Adds an opt-in DSX Exchange DevSpace profile that deploys a local NATS MQTT service alongside NICo and passes the event-bus configuration to nico-api. The default DevSpace and Helm paths remain disabled unless the profile is selected. It also corrects the post-deploy integration checks to use the current Machine-a-Tron resource names and the local nico-api endpoint so a clean deployment can complete verification.
Related issues
None.
Type of Change
Breaking Changes
Testing
Additional Notes
Verified at
1f59be25338f57cb78a737ef0f01a9e83bf0cf5din an isolated local Kind cluster. The default render omitted DSX Exchange, the opt-in profile deployed NATS with MQTT, both Core hosts reached Ready and synced into a current REST inventory, and a subscriber received a managed-hostreadystate. Full reproduction steps are in the manual-testing comment.