feat: add readiness probe and read only root filesystem - #2515
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds read-only filesystem settings, temporary ChangesNetwork-observer chart hardening and probes
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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.
🧹 Nitpick comments (1)
charts/network-observer/templates/_deployment.yaml (1)
25-26: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider bounding the
proxy-tmpemptyDir size.No
sizeLimitis set, so this volume can grow unbounded and consume node ephemeral storage.♻️ Proposed fix
- name: proxy-tmp - emptyDir: {} + emptyDir: + sizeLimit: 64Mi
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 17615b9f-a6e7-4a6e-85fc-208b54bf44b4
📒 Files selected for processing (2)
charts/network-observer/templates/_deployment.yamlcharts/network-observer/values.yaml
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
charts/network-observer/templates/deployment.yaml (1)
50-50: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTrailing whitespace after the
iftag.Line 50 has trailing spaces after
}}, inconsistent with the other conditional blocks in this file (e.g. Line 15's{{- if .Values.prometheus.persistence.enabled }}has no trailing whitespace). This introduces a stray whitespace-only line in rendered output and may trip YAML/lint checks.Proposed fix
- {{- if .Values.readinessProbe.enabled }} + {{- if .Values.readinessProbe.enabled }}
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 34f4f26f-e3fa-4792-932c-4ba22e2e79de
📒 Files selected for processing (1)
charts/network-observer/templates/deployment.yaml
|
|
||
| ## Controller containers' readiness probe | ||
| ## ref: https://kubernetes.io/docs/concepts/workloads/pods/pod-lifecycle/#container-probes | ||
| readinessProbe: |
There was a problem hiding this comment.
@CarolinaRFerreira thank you for the pull request. I am curious if you had a plan for these probes already or have been able to test this change with enabled: true. As it stands, I do not believe the commented example: GET :9191/healthz would not work.
There was a problem hiding this comment.
I've made some changes to the values. I already tested them and they seem to be correct, both the probes and the readOnlyFilesystem.
c-kruse
left a comment
There was a problem hiding this comment.
Thank you for the contribution! Looks good to me.
Summary by CodeRabbit