Published helm chart release 1.97.0 does not match code tagged (v1.98)

[cwitthaus]

Describe the bug
The helm chart that is packaged and distributed as version 1.97.0 via https://kubecost.github.io/cost-analyzer/ does not match the code at tag 1.97.0. Specifically, https://github.com/kubecost/cost-analyzer-helm-chart/blob/v1.97.0/cost-analyzer/templates/cost-analyzer-cluster-role-binding-template.yaml has a different value for the name of the RoleBinding. In the repository, the name is set to {{ template "cost-analyzer.serviceAccountName" . }}, while the pulled chart uses {{ .Release.Name }}.

To Reproduce
Steps to reproduce the behavior:

1. Add the helm repository to your local environment via the commands in the README:

   helm repo add kubecost https://kubecost.github.io/cost-analyzer/

2. Download the helm chart from the repository:

   helm pull kubecost/cost-analyzer --version 1.97.0

3. Unpack the downloaded helm chart.
4. View the templates/cost-analyzer-cluster-role-binding-template.yaml file:

   >> cat cost-analyzer/templates/cost-analyzer-cluster-role-binding-template.yaml
   {{- if .Values.reporting }}
   {{- if .Values.reporting.logCollection }}
   apiVersion: rbac.authorization.k8s.io/v1
   kind: RoleBinding
   metadata:
   name: {{ .Release.Name }}
   labels:
   {{ include "cost-analyzer.commonLabels" . | nindent 4 }}
   ...

5. Note that this does not match the state of the repository at tag 1.97.0.

Expected behavior
I would expect the code packaged in the helm chart to match the code in the release tag.

[AjayTripathy]

Hi @cwitthaus thank you for this report. @nealormsbee did the last build, any ideas what might have happened here? Did the tag not update here or is it pointed to the wrong place/tarball?

[nealormsbee]

@cwitthaus thank you! I'm able to reproduce the issue and I see the discrepancy you're talking about.

Looking at command history and logs from when I built the tarball, I built it from master, immediately after merging the v1.97 branch into master. It seems likely that I resolved a merge conflict incorrectly, or should have built from the v1.97.0 tag directly. My sincere apologies here -- it was my first time running this process end-to-end on my own.

@AjayTripathy can you advise a course of action? Leaving the tag and tarball contents out of sync seems non-ideal, but so does changing the contents of either.

Thanks again, @cwitthaus. We can prevent this in the upcoming v1.98 release and future releases.

[AjayTripathy]

IMO I would update the tag to reflect reality and then make sure we fix the build/buildscript processes to make this work seamlessly in v1.98.

[cwitthaus]

I didn't do a full investigation of everything that was in the pushed chart, but it seems to me like it would be possible for the release to not actually include changes that are detailed in the release notes if the wrong version of the code was used for the chart. I think trying to make sure the helm chart contains the features and fixes outlined there seems best as a consumer of the chart.

[AjayTripathy]

Hi @cwitthaus sorry for the late response-- @nealormsbee is OOO. I'll go through patch notes to make sure they reflect reality. Swapping in the right helm chart into the tarball is probably wrong at this point-- we are going to fix our build process in the upcoming releases (scheduled for next week) to ensure this doesn't happen again. Expect a postmortem to follow.

[kaelanspatel]

Tested with the newest rc, looks like this specific example no longer happens.

metadata:
  name: {{ template "cost-analyzer.serviceAccountName" . }}
  labels:
    {{ include "cost-analyzer.commonLabels" . | nindent 4 }}

We probably want to understand what exactly went wrong here (cc @nealormsbee, maybe just some Github Action weirdness?) but for now it seems to be working as intended.

[nealormsbee]

@kaelanspatel I believe the discrepancy came from the fact that, after the GH Actions are done opening PRs, creating tags, etc, we built the tarball from the helm chart, using a script that is not part of a GH action. For that step, I used the Helm chart's master branch rather than the v1.97.0 tag. I had thought they should be identical, but there's some long-standing strangeness where we get merge conflicts when merging our release branches into master.

Is the tarball build in an Action now?

[kaelanspatel]

After talking with Neal, this was probably just a one-off issue in the v1.97.0 build. I'll take a few steps to verify correctness for the v1.98.0 release, but it seems to be working in general.

[AjayTripathy]

@kaelanspatel , would you mind documenting the verification steps when you can under the release process? Going to close this for now then.