Skip to content

Conversation

Sahaquielxo
Copy link

  • introduce cluster.env and cluster.envFrom in values.yaml
  • render them conditionally in templates/cluster.yaml under the Postgres container

Fix #609 issue

@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. chart( cluster ) Related to the cluster chart labels Jun 12, 2025
@Sahaquielxo Sahaquielxo reopened this Jun 12, 2025
@Sahaquielxo
Copy link
Author

@itay-grudev hello. Any ideas how can I force PR review, please?

@itay-grudev
Copy link
Collaborator

Could you please include an example in the non-default configuration test?

Copy link
Collaborator

@itay-grudev itay-grudev left a comment

Choose a reason for hiding this comment

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

Please update the non-default configuration test.

- introduce `cluster.env` and `cluster.envFrom` in values.yaml
- render them conditionally in templates/cluster.yaml under the Postgres container

Signed-off-by: Sahaquielxo <[email protected]>
@Sahaquielxo
Copy link
Author

@itay-grudev I hope this will be enough. Let me know if I have to add anything else.

@Sahaquielxo Sahaquielxo requested a review from itay-grudev June 20, 2025 20:53
Comment on lines +45 to +52
{{- if .Values.cluster.env }}
env:
{{ toYaml .Values.cluster.env | indent 4 }}
{{- end }}
{{- if .Values.cluster.envFrom }}
envFrom:
{{ toYaml .Values.cluster.envFrom | indent 4 }}
{{- end }}
Copy link

Choose a reason for hiding this comment

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

Thank you for your contribution.

The yamllint fails, can you update With this changes:

Suggested change
{{- if .Values.cluster.env }}
env:
{{ toYaml .Values.cluster.env | indent 4 }}
{{- end }}
{{- if .Values.cluster.envFrom }}
envFrom:
{{ toYaml .Values.cluster.envFrom | indent 4 }}
{{- end }}
env:
{{ toYaml .Values.cluster.env | nindent 4 }}
{{- end }}
{{- if .Values.cluster.envFrom }}
envFrom:
{{ toYaml .Values.cluster.envFrom | nindent 4 }}
{{- end }}

The chainsaw test succeed after the fix.

Comment on lines +84 to +85
----------------------------------------------
Autogenerated from chart metadata using [helm-docs v1.14.2](https://github.com/norwoodj/helm-docs/releases/v1.14.2)
Copy link

Choose a reason for hiding this comment

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

Can you also remove these lines 84-85
Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chart( cluster ) Related to the cluster chart size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Helm chart doesn't support custom environment variables
3 participants