feat: support auto assign node port for dp-manager#271
Conversation
Add nodeport configuration support for dp-manager service: - Add autoAssignNodePort option to automatically set nodePort to same value as port - Add manual nodePort/tlsNodePort options for custom nodePort values - Update chart version to 0.17.48 - Update README documentation Reference: PR #258
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughBumps the api7 Helm chart to 0.17.48 and adds dp_manager_service NodePort controls: new values ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
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)
charts/api7/README.md (1)
3-3:⚠️ Potential issue | 🟡 MinorUpdate README version badge to match chart version.
Line 3 still shows
0.17.47, butcharts/api7/Chart.yamlwas bumped to0.17.48.📝 Suggested doc fix
-   +  🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/api7/README.md` at line 3, Update the README version badge so it matches the Chart.yaml chart version bump: replace the "Version: 0.17.47" badge text in the README line containing the version badge with "Version: 0.17.48" (the badge alt/text and query value that currently reads 0.17.47). Ensure the string containing "Version-0.17.47" is updated to "Version-0.17.48" so the README reflects the Chart.yaml version change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@charts/api7/templates/dp-manager-service.yaml`:
- Around line 22-39: The template currently uses dp_manager_service.port and
dp_manager_service.tlsPort as nodePort when
dp_manager_service.autoAssignNodePort is true, which can produce out-of-range
NodePorts; update dp-manager-service.yaml to fail-fast: when
dp_manager_service.autoAssignNodePort is true, do not inject nodePort from
port/tlsPort (let Kubernetes auto-assign) or validate that port/tlsPort are
within 30000-32767 and call Helm's fail with a clear message; specifically add
checks around dp_manager_service.autoAssignNodePort, dp_manager_service.port,
and dp_manager_service.tlsPort (and keep existing dp_manager_service.nodePort /
dp_manager_service.tlsNodePort handling unchanged) and use the Helm fail(...)
function to error during template rendering if values fall outside the
30000-32767 range.
---
Outside diff comments:
In `@charts/api7/README.md`:
- Line 3: Update the README version badge so it matches the Chart.yaml chart
version bump: replace the "Version: 0.17.47" badge text in the README line
containing the version badge with "Version: 0.17.48" (the badge alt/text and
query value that currently reads 0.17.47). Ensure the string containing
"Version-0.17.47" is updated to "Version-0.17.48" so the README reflects the
Chart.yaml version change.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a115c67e-6fa9-40a7-9a3c-b69dec459909
📒 Files selected for processing (4)
charts/api7/Chart.yamlcharts/api7/README.mdcharts/api7/templates/dp-manager-service.yamlcharts/api7/values.yaml
- Update README version badge to 0.17.48 - Add port range validation for autoAssignNodePort: fail if port/tlsPort is outside 30000-32767 when autoAssignNodePort is true
Summary
Add nodeport configuration support for dp-manager service, similar to gateway stream's autoAssignNodePort feature.
Changes
autoAssignNodePortoption to automatically set nodePort to same value as HTTP/HTTPS portnodePortoption for custom HTTP nodePort valuetlsNodePortoption for custom HTTPS nodePort valueConfiguration
Reference
Related: #258
Summary by CodeRabbit
Chores
New Features
Documentation