.Release.Revision not working correctly in diff

[aianus]

Hi, we have a pod label as follows that includes the helm revision number as a value:

tags.datadoghq.com/version: {{ .Release.Revision | default 0 | quote }}

This seems to work fine with helm, but with helm diff, the value is always displayed as "1" in the diff:

Observed output in diff:

-     tags.datadoghq.com/version: "5"
+     tags.datadoghq.com/version: "1"

Expected output in diff:

-     tags.datadoghq.com/version: "5"
+     tags.datadoghq.com/version: "6"

[david92rl]

I just had a quick look and it seems that the issue isn't really related to the plugin itself.
helm-diff relies on the template function from helm (see this)

I ran a quick test and I was able to reproduce. All helm template commands are returning 1 where .Release.Revision is defined

Helm versions I used for this test: 3.4.1 and 3.5.0

[david92rl]

And.... this is why:

The template command uses the install method to generate the templates. This method sets the revision to 1 as it thinks it's the first deployment of the release

[david92rl]

Ok, so apparently helm template works as intended.
One way to fix this could be replacing the command used to render templates, so we use upgrade --dry-run instead of template

[aianus]

Any update on whether helm-diff will switch to upgrade --dry-run?

[strainovic]

It is very annoying behavior. We have one-time jobs for database migrations and it is triggered each time, no matter if we changed something on not.

As far as I can see, helm-diff is not maintained actively?

[mumoshu]

As far as I can see, helm-diff is not maintained actively?

@strainovic Hey! What is the point to calling it not maintained actively? Are you willing to contribute?

[mumoshu]

I've never realized upgrade --dry-run can be used for the replacement to helm template!

As we need to handle diffing on first install too, probably we'd need to use helm upgrade --install --dry-run?

[mumoshu]

I was blindly beliving helm template --is-upgrade we're currently using would magically handle the revision number as well, but apparently not.

      --is-upgrade                   set .Release.IsUpgrade instead of .Release.IsInstall

helm-diff/cmd/helm3.go

Line 134 in 0882d8c

flags = append(flags, "--is-upgrade")

I'm not fully sure if it's safe to fully migrate helm-diff to use helm upgrade --dry-run instead of helm template.

Should we add some envvar to toggle helm-diff to use upgrade --dry-run in

helm-diff/cmd/helm3.go

Line 141 in 0882d8c

args := []string{"template", d.release, d.chart}

?

[strainovic]

As far as I can see, helm-diff is not maintained actively?

@strainovic Hey! What is the point to calling it not maintained actively? Are you willing to contribute?

Hi @mumoshu, I told you that as I haven't seen any commit almost for a year, while a lot of things have been changed since then.

My knowledge in Golang is very limited, but if I find any spot where I can contribute, I would do it :)

Thank you @mumoshu for your amazing work!

[mumoshu]

@strainovic Thanks! Gotcha. I was rather in an impression that that kind of inactivity means maturity of the project :)

I use helm-diff and helmfile and haven't seen big blockers for a year or so.

I occasionally review issues and pull requests but most of the time (1) they look like not necessities but nice-to-haves, good workarounds exist, or (2)no preceding issues are written to clarify the goals of the changes so I'm unsure if I should merge it or (3)have no tests or manual test report so I'm not sure if I can safely merge it or (4) I responded/asked question(s) but no one answers, or (5)many people talk about big dreams but no one actually contributes code 😢

Regarding this issue, I already took some time to test helm upgrade --dry-run myself, wrote my thought and concern in #253 (comment), and I'm now awaiting replies and feedbacks.

And I thought it's reasonable and forgivable to wait a few weeks until anyone tries to contribute, as this is annoying but not critical as you can just omit .Release.Revision from the chart template... I actually don't understand why one wants to have it in k8s resource labels.

a lot of things have been changed since then.

If you've specific/critical things helm-diff needs to address, please feel free to raise issues.

I can't promise to do everyone's all works myself, but I'll try keeping my best to maintain the project!

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[woodcockjosh]

/please fix /pretty-please fix

[woodcockjosh]

@mumoshu thanks for thinking about this. In my opinion risky features should be rolled out with a feature flag. Eg --use-experimental-upgrade and or HELM_DIFF_USE_EXPERIMENTAL_UPGRADE_FOR_DIFF

I'm not a great go programmer but this issue is really plaguing me. I will literally pay money for someone to fix this.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[Prometej]

/please fix /pretty-please fix

[lucasslima]

Hey, in my current position we are using are setting a label with .Release.Revision for selecting which resources we should way for, since we rely on helm install for deploying and need to make sure they are healthy on our pipelines.

So if diff could return the correct revision, would be great to not have a misleading output.