chore: support specifying nodeport for the file-server service#282
chore: support specifying nodeport for the file-server service#282
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 28 minutes and 50 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis pull request bumps the Helm chart version from 0.17.52 to 0.17.53 and introduces support for configuring a Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 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.
Pull request overview
This PR adds Helm chart support for optionally setting a fixed nodePort on the file-server Service when file_server_service.type is NodePort.
Changes:
- Add conditional
nodePortrendering to the file-server Service template when explicitly configured. - Document
file_server_service.nodePortin the chart README. - Bump chart version from
0.17.52to0.17.53.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| charts/api7/values.yaml | Adds a commented example for file_server_service.nodePort. |
| charts/api7/templates/file-server-service.yaml | Conditionally sets spec.ports[].nodePort for the file-server Service when configured. |
| charts/api7/README.md | Documents the new file_server_service.nodePort value. |
| charts/api7/Chart.yaml | Increments chart version to reflect template/values changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 | 🟡 MinorStale version badge — regenerate README via
helm-docs.The version badge still shows
0.17.52whileChart.yamlwas bumped to0.17.53. Runhelm-docsto regenerate this header so the published documentation reflects the released chart version.📝 Proposed 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, The README header contains a stale version badge (shows 0.17.52) that no longer matches Chart.yaml (bumped to 0.17.53); regenerate the documentation by running helm-docs so the badge and header are updated to the current chart version (i.e., update the README badge at the top of charts/api7/README.md by running helm-docs to refresh the Version/AppVersion badges).
🧹 Nitpick comments (1)
charts/api7/values.yaml (1)
217-220: Optional: align documentation style withdp_manager_service.
dp_manager_service(lines 190–195) uses explicit# --annotations and anodePort: nulldefault which makes the value discoverable byhelm-docsand clearly communicates the contract. Consider mirroring that style here for consistency:♻️ Proposed refactor
file_server_service: type: ClusterIP port: 8080 - # nodePort: 30080 + # -- (int) The nodePort for HTTP service, only used if file_server_service.type is NodePort. If not set, a random port will be assigned by Kubernetes. + nodePort: nullNote: the README already documents
file_server_service.nodePortwith defaultnil(line 244), which suggests the docs were hand-edited rather than generated from values.yaml comments. Aligning the styles would lethelm-docsregenerate the table cleanly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/api7/values.yaml` around lines 217 - 220, The file_server_service block should mirror dp_manager_service's documented style so helm-docs can pick up the nodePort contract; update the file_server_service entry (the file_server_service map and its nodePort key) to include the same explicit comment annotations and a nodePort: null default (or commented nodePort: null with the same "# --" separator/comment style used in dp_manager_service) so the value becomes discoverable and consistent with README generation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@charts/api7/README.md`:
- Line 3: The README header contains a stale version badge (shows 0.17.52) that
no longer matches Chart.yaml (bumped to 0.17.53); regenerate the documentation
by running helm-docs so the badge and header are updated to the current chart
version (i.e., update the README badge at the top of charts/api7/README.md by
running helm-docs to refresh the Version/AppVersion badges).
---
Nitpick comments:
In `@charts/api7/values.yaml`:
- Around line 217-220: The file_server_service block should mirror
dp_manager_service's documented style so helm-docs can pick up the nodePort
contract; update the file_server_service entry (the file_server_service map and
its nodePort key) to include the same explicit comment annotations and a
nodePort: null default (or commented nodePort: null with the same "# --"
separator/comment style used in dp_manager_service) so the value becomes
discoverable and consistent with README generation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f9b7ef81-a0a8-4998-af21-364b2dcfb55d
📒 Files selected for processing (4)
charts/api7/Chart.yamlcharts/api7/README.mdcharts/api7/templates/file-server-service.yamlcharts/api7/values.yaml
Summary by CodeRabbit
New Features
NodePort.Chores