aldoborrero / k8s-usenet

k8s-usenet is a collection of Helm charts related to Usenet services.
Apache License 2.0
76 stars 14 forks source link

Image tag setting not used #1

Closed jeje closed 4 years ago

jeje commented 4 years ago

Hi,

At least in Sabnzbd and Sonarr, although I define a image.tag setting, it is not used.

Example in Sonarr (https://github.com/aldoborrero/k8s-usenet/blob/master/sonarr/templates/deployment.yaml#L30):

image: "{{ .Values.image.repository }}:{{ .Chart.AppVersion }}"

The chart version is used instead of the tag.

Unfortunately this means that the preview version (with the same tag) can't be used instead of the amd64-latest default version :(

jeje commented 4 years ago

The fix should be easy to do. Do you need/want a PR ?

aldoborrero commented 4 years ago

Oh! Good catch!

Yeah let's review all the charts and go ahead and create a PR :)

Thanks

jeje commented 4 years ago

Here is what I suggest: discussion here on Sonarr chart, through a first version of a PR.

When we're okay on the changes, review of all charts having the same "issue" and 2nd version of the PR with all charts fixed.

Would that work for you?

jeje commented 4 years ago

Here is a list of the other charts that would need to be fixed the same way (if that's fine for you):

nzbhydra2/templates/deployment.yaml:30:          image: "{{ .Values.image.repository }}:{{ .Chart.AppVersion }}"
bazarr/templates/deployment.yaml:30:          image: "{{ .Values.image.repository }}:{{ .Chart.AppVersion }}"
lazylibrarian/templates/deployment.yaml:30:          image: "{{ .Values.image.repository }}:{{ .Chart.AppVersion }}"
medusa/templates/deployment.yaml:30:          image: "{{ .Values.image.repository }}:{{ .Chart.AppVersion }}"
sabnzbd/templates/deployment.yaml:30:          image: "{{ .Values.image.repository }}:{{ .Chart.AppVersion }}"
headphones/templates/deployment.yaml:30:          image: "{{ .Values.image.repository }}:{{ .Chart.AppVersion }}"
couchpotato/templates/deployment.yaml:30:          image: "{{ .Values.image.repository }}:{{ .Chart.AppVersion }}"
radarr/templates/deployment.yaml:30:          image: "{{ .Values.image.repository }}:{{ .Chart.AppVersion }}"
mylar/templates/deployment.yaml:30:          image: "{{ .Values.image.repository }}:{{ .Chart.AppVersion }}"
transmission/templates/deployment.yaml:30:          image: "{{ .Values.image.repository }}:{{ .Chart.AppVersion }}"
lidarr/templates/deployment.yaml:30:          image: "{{ .Values.image.repository }}:{{ .Chart.AppVersion }}"
nzbget/templates/deployment.yaml:30:          image: "{{ .Values.image.repository }}:{{ .Chart.AppVersion }}"
organizr/templates/deployment.yaml:30:          image: "{{ .Values.image.repository }}:{{ .Chart.AppVersion }}"
jackett/templates/deployment.yaml:30:          image: "{{ .Values.image.repository }}:{{ .Chart.AppVersion }}"
aldoborrero commented 4 years ago

Ossom @jeje

I already merged the PR (simple and sweet), proceed with the others and many thanks!

jeje commented 4 years ago

I was about to ensure this fix worked fine (although I'm quite confident :)) before doing the same for the other charts, but I assume that the helm repo is not automatically updated from the master?

aldoborrero commented 4 years ago

Up until I don't execute the make action it won't update anything on the repository.

So don't worry on that as I'll double check before pushing the red button :+1:

jeje commented 4 years ago

New PR submitted. I haven't checked though that default tags are available for each chart. I hope there is amd64-latest tag for each container :)

aldoborrero commented 4 years ago

Thanks I'll take a look on the tags and I'll update the PR accordingly (if necessary!)

aldoborrero commented 4 years ago

PR merged. I'll proceed to close this thread! Thanks @jeje feel free to re-open this one if you detect any issue.