Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update helm-project-operator template values/helpers #121

Merged
merged 13 commits into from
Nov 25, 2024

Conversation

mallardduck
Copy link
Member

@mallardduck mallardduck commented Nov 22, 2024

Related Issue:
#120

In an effort to improve the above issue, this PR makes a few changes:

  • Makes image tags based on Chart.yaml file'sappVersion,
  • Removes the image tag from each values.yaml file,
  • Corrects image name for HPO dev-chart

The net result is that end users can still override tag values as they wish. However from a Dev POV we only have to maintain updating the Chart.yaml version as necessary.

@mallardduck mallardduck requested a review from a team as a code owner November 22, 2024 16:54
@mallardduck mallardduck force-pushed the fix-default-tag branch 2 times, most recently from 93eea81 to 33b69fc Compare November 22, 2024 18:51
@@ -0,0 +1,64 @@
#!/bin/bash
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice de-duplication!

@@ -20,7 +20,7 @@ spec:
spec:
containers:
- name: {{ template "helm-project-operator.name" . }}
image: "{{ template "system_default_registry" . }}{{ .Values.image.repository }}:{{ .Values.image.tag }}"
image: "{{ template "helm-project-operator.imageRegistry" . }}{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }}"
Copy link
Member Author

Choose a reason for hiding this comment

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

@joshmeranda - Felt like a function over complicated it, so I simplified to an inline default.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm happy with either way as long as the right values are being redered. Since this is all done at render time anyway its not gonna have a real impact. Thanks!

@mallardduck mallardduck merged commit a8cead8 into rancher:main Nov 25, 2024
17 checks passed
@mallardduck mallardduck deleted the fix-default-tag branch November 25, 2024 20:35
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