Skip to content

fix: Helm chart install guidance and deployment defaults#2976

Merged
hubcio merged 10 commits intoapache:masterfrom
avirajkhare00:helm-readme-fix
Mar 21, 2026
Merged

fix: Helm chart install guidance and deployment defaults#2976
hubcio merged 10 commits intoapache:masterfrom
avirajkhare00:helm-readme-fix

Conversation

@avirajkhare00
Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Closes #

#2975

This PR is mainly for #2975, but it also partially cleans up #2932 by making the README ingress example controller-neutral.

Rationale

Local kind testing showed two real Helm chart bugs: the server deployment was not getting the documented default podSecurityContext, and the UI health probes were hitting a route
that returns 404.

What changed?

On a clean Helm install, the server deployment rendered the wrong pod security context because the template looked for server.podSecurityContext even though the chart defines
podSecurityContext at the root. The UI deployment also used / for liveness and readiness probes, but the web UI returns 404 on that path, so Kubernetes kept restarting the pod.

The fix makes the server deployment use the root podSecurityContext value and changes the UI probe path to /auth/sign-in, which returns 200. I also updated the chart README to
clarify the ServiceMonitor prerequisite, note the local io_uring caveat seen during testing, and use more controller-neutral ingress wording.

Local Execution

  • Partially passed.
  • helm lint ./helm/charts/iggy passed.
  • helm install iggy ./helm/charts/iggy -n iggy-test --create-namespace --set server.serviceMonitor.enabled=false was run on a local kind cluster.
  • After the template changes, the server deployment rendered the expected root podSecurityContext, and the UI became healthy with /auth/sign-in as the probe path.
  • Full local startup still did not fully pass because the server hit a separate io_uring / runtime Invalid argument failure on kind. That looks like a different issue and is not
    fixed in this change.
  • Pre-commit hooks not ran. prek was not installed/configured in this checkout, and only the sample git hooks were present.

AI Usage

  1. Tool used: OpenAI Codex / ChatGPT.
  2. Scope: investigation, small Helm template changes, README updates, and drafting the issue/PR text.
  3. Verification: I ran helm lint, installed the chart on local kind, inspected rendered pod specs, checked pod logs and Kubernetes events, and verified the UI route behavior with
    local HTTP checks.
  4. Yes, I can explain every line of the change if asked.

@avirajkhare00 avirajkhare00 changed the title Fix Helm chart install guidance and deployment defaults fix: Helm chart install guidance and deployment defaults Mar 19, 2026
@avirajkhare00
Copy link
Copy Markdown
Contributor Author

avirajkhare00 commented Mar 19, 2026

#2977

@avirajkhare00
Copy link
Copy Markdown
Contributor Author

Also, should we clean up comments from values.yaml, otherwise there will always be documentation drift between two

Copy link
Copy Markdown
Contributor

@atharvalade atharvalade left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm now!

@atharvalade
Copy link
Copy Markdown
Contributor

Also, should we clean up comments from values.yaml, otherwise there will always be documentation drift between two

Good point. For this PR, the values.yaml comment and README are consistent (both describe serviceMonitor as opt-in for Prometheus Operator users), so there's no immediate drift. But you're right that maintaining descriptions in both places long-term is fragile. The # -- comments in values.yaml follow the helm-docs convention -- the standard approach is to auto-generate the README parameter table from those comments so there's a single source of truth. That would be a nice follow-up but shouldn't block this PR. @avirajkhare00

@avirajkhare00
Copy link
Copy Markdown
Contributor Author

let me think on auto generation of readme part, will create different issue for that

@avirajkhare00 avirajkhare00 requested a review from hubcio March 21, 2026 03:29
@avirajkhare00
Copy link
Copy Markdown
Contributor Author

@hubcio kindly review

#2983 too

@hubcio hubcio merged commit f665400 into apache:master Mar 21, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants