Skip to content

Update values.yaml - #50

Open
the-imperator wants to merge 1 commit into
caas-team:mainfrom
the-imperator:main
Open

Update values.yaml#50
the-imperator wants to merge 1 commit into
caas-team:mainfrom
the-imperator:main

Conversation

@the-imperator

Copy link
Copy Markdown

Add

  • Example for SMTP with Username and Password
  • Fix Helm-Deploy-Warnings in case of duplicated Ports
  • Add PVC to store Grafana Userchanges and keep it after Helm-Delete

Add
- Example for SMTP with Username and Password
- Fix Helm-Deploy-Warnings in case of duplicated Ports
- Add PVC to store Grafana Userchanges and keep it after Helm-Delete
@puffitos puffitos self-assigned this Dec 12, 2025

@puffitos puffitos left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I've tested the changes myself and suprisingly eveything still works. I dug a bit deeper and I think there's no point in updating this here, as this is the default, unfortunately expected behavior.

We can keep the other changes though. Thank you very much for your pull request!

Comment thread values.yaml
# Persists a PVC and keep it by Helm-Delete so you can Update and your local Changes should be available
persistence:
type: pvc
enabled: true

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should be false please. We don't want to create PVCs per default. If a user wants to persist their grafana dashboards they should be doing it declaratively and not by imperatively saving everything in the dashboard itself and then relying on a PV being there.

The size of the PVC is also way too large, if it's only for dashboards and grafana configuration.

Or ist there some other reason for this change?

Comment thread values.yaml
Comment on lines +148 to +149
# Change Container-Port in Case of Helm Duplicate Port Warnings
- containerPort: 8081

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for working on this! I wanted to clarify what's happening with the port configuration to help explain why this
change works (and what it actually does) and why we just need to accept the warning for the time being.

What's Actually Happening

The duplicate port warning occurs because of how the upstream Grafana Helm chart works. When we set
service.targetPort: 8080 in the values.yaml, the Grafana chart automatically adds containerPort: 8080 to the Grafana container itself
(see line 1023 in the chart's _pod.tpl - https://github.com/grafana/helm-charts/blob/main/charts/grafana/templates/_pod.tpl). We then add the additional containerPort for the additional proxy container, which is exactly how the upstream chart does it (https://github.com/grafana/helm-charts/blob/main/charts/grafana/values.yaml#L388-L403).

Why Your Change "Works"

Changing the extraContainer port to 8081 removes the warning, but it's important to understand what's actually
changed:

  • You removed the duplicate port warning (no more two containers claiming port 8080)
  • The nginx proxy still listens on port 8080 (defined in the nginx.conf ConfigMap)
  • The Service still targets port 8080 (defined in service.targetPort )

The containerPort field is mostly cosmetic/documentary - Kubernetes doesn't enforce it. So now:

  • The Grafana container claims port 8081 (but doesn't actually use it)
  • The nginx-proxy container claims port 8081 (but actually listens on 8080)
  • The Service routes to port 8080 (where nginx is really listening)

Everything works because the Service routing is what matters, not the port declarations.

The Root Cause

This is a limitation of the upstream Grafana chart - it assumes the Service targets Grafana directly, not a sidecar
proxy. When using the proxy pattern, there's no clean way to avoid this mismatch without modifying the nginx.conf and service.targetPort together.

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.

2 participants