feat(ngxdig): add Helm chart for the ngxdig eBPF diagnosis agent#315
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 48 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughA new Changesngxdig Helm chart
Sequence Diagram(s)sequenceDiagram
participant Helm
participant Kubernetes
participant ngxdig
Helm->>Kubernetes: Render DaemonSet, Service, and ServiceAccount
Kubernetes->>ngxdig: Start one collector Pod per node
ngxdig->>Kubernetes: Expose debug UI through the Service
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
charts/ngxdig/Chart.yaml (1)
1-6: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winDeclare or verify the chart’s minimum Kubernetes version.
The defaults render
securityContext.appArmorProfile; Kubernetes versions before 1.30 used AppArmor annotations instead. Without akubeVersionconstraint, older clusters may reject or ignore this field and fail to apply the intended runtime configuration. Confirm the supported range, then either declare the minimum version or render a legacy compatibility path. (kubernetes.io)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@charts/ngxdig/Chart.yaml` around lines 1 - 6, Update the ngxdig Helm chart metadata to explicitly define the supported Kubernetes version range based on its use of securityContext.appArmorProfile. Set an appropriate kubeVersion minimum for Kubernetes 1.30+; if older clusters must remain supported, add conditional rendering for the legacy AppArmor annotation path instead.charts/ngxdig/README.md.gotmpl (1)
25-43: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument optional NodePort exposure.
The PR stack adds optional NodePort exposure for the debug UI Service, but this doc only covers the default ClusterIP + port-forward flow. Consider a short section (e.g.
--set service.type=NodePort) so users know how to reach the UI without port-forwarding.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@charts/ngxdig/README.md.gotmpl` around lines 25 - 43, Update the “Accessing the debug UI” section in the README template to document optional NodePort exposure using the service.type=NodePort configuration and explain how users can obtain and access the assigned node port. Keep the existing ClusterIP and port-forward instructions unchanged, and retain the warning that the debug UI should only be exposed on a trusted network.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yaml:
- Around line 80-81: Update the ct install command’s chart list to include
charts/aisix-cp alongside the existing charts in the workflow, ensuring it
receives install-time validation while preserving the current chart entries.
In `@AGENTS.md`:
- Line 12: Expand the ngxdig entry in AGENTS.md, or add a dedicated ngxdig
section, to document its key capabilities and typical usage patterns while
retaining the existing purpose, chart location, and api7/ngx-flame
source-of-truth details.
In `@charts/ngxdig/README.md`:
- Around line 33-48: The README access guidance should require selecting the
target collector Pod by node rather than using arbitrary `.items[0]` selection.
Update the port-forward command to include the `spec.nodeName=<node>` field
selector, and state that the selector-based ClusterIP Service is
non-node-specific and may load-balance across collector Pods.
- Around line 39-41: Update the README port-forward command’s pod selector to
use the Helm release label app.kubernetes.io/instance=ngxdig, matching the label
used in NOTES.txt, instead of selecting by app.kubernetes.io/name.
In `@charts/ngxdig/templates/daemonset.yaml`:
- Around line 33-60: Update the chart defaults in values.yaml to disable
hostPID, CAP_SYS_ADMIN, and unconfined seccomp/AppArmor settings unless
explicitly opted in, and enable readOnlyRootFilesystem by default. Ensure the
daemonset template continues propagating these security settings through
hostPID, podSecurityContext, and securityContext while preserving user-provided
opt-ins.
In `@charts/ngxdig/values.yaml`:
- Around line 91-101: Make the debug UI Service opt-in by default in the chart’s
service configuration, disabling its creation unless explicitly enabled through
a values key. Update the related deployment/templates and NOTES so they only
reference or expose the Service when enabled, while preserving direct Pod
port-forwarding as the documented alternative.
---
Nitpick comments:
In `@charts/ngxdig/Chart.yaml`:
- Around line 1-6: Update the ngxdig Helm chart metadata to explicitly define
the supported Kubernetes version range based on its use of
securityContext.appArmorProfile. Set an appropriate kubeVersion minimum for
Kubernetes 1.30+; if older clusters must remain supported, add conditional
rendering for the legacy AppArmor annotation path instead.
In `@charts/ngxdig/README.md.gotmpl`:
- Around line 25-43: Update the “Accessing the debug UI” section in the README
template to document optional NodePort exposure using the service.type=NodePort
configuration and explain how users can obtain and access the assigned node
port. Keep the existing ClusterIP and port-forward instructions unchanged, and
retain the warning that the debug UI should only be exposed on a trusted
network.
🪄 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: 548fb512-f613-467d-8855-b1b89faf1eba
📒 Files selected for processing (13)
.github/workflows/ci.yamlAGENTS.mdcharts/ngxdig/.helmignorecharts/ngxdig/Chart.yamlcharts/ngxdig/README.mdcharts/ngxdig/README.md.gotmplcharts/ngxdig/ci/ct-values.yamlcharts/ngxdig/templates/NOTES.txtcharts/ngxdig/templates/_helpers.tplcharts/ngxdig/templates/daemonset.yamlcharts/ngxdig/templates/service.yamlcharts/ngxdig/templates/serviceaccount.yamlcharts/ngxdig/values.yaml
…vice is not node-specific
2 participants
Adds a public Helm chart for
ngxdig(source:api7/ngx-flame), the eBPF on-CPU flame-graph and CPU/memory/latency diagnosis agent for OpenResty/Nginx.Because ngxdig profiles the OpenResty/Nginx worker processes running on a node, it is packaged as a DaemonSet (one collector per node), not a plain Deployment — mirroring how node-level eBPF profilers deploy. The chart wires up exactly what the collector needs to work:
hostPID: trueso the collector sees target worker processes on the node.CAP_BPF/CAP_PERFMON/CAP_SYS_PTRACE,runAsUser: 0, and unconfined seccomp/AppArmor./sys/kernel/btf), and an emptyDir for run artifacts.ngxdig ui; a ClusterIP Service exposes the debug UI (reached viakubectl port-forward, since the UI is not a hardened multi-user service).Default image is
api7/ngxdig:latest. Chart is independently versioned (not forked per EE line); added a row toAGENTS.md.CI: added
charts/ngxdigto the existingct lintandct installsteps in.github/workflows/ci.yaml. Validated locally against a kind v1.31 cluster —ct lintandct installpass, the DaemonSet Pod becomes Ready, andngxdig doctorinside the Pod confirms BPF features, kernel BTF, and root are all available (host looks ready).Summary by CodeRabbit
New Features
ngxdigHelm chart for deploying a node-level diagnostics collector and debug UI.Documentation
Chores
ngxdig.