pulumi-kubernetes
pulumi-kubernetes copied to clipboard
HelmRelease: Path is used as the chart name, triggering unneeded releases
What happened?
When using a local chart in an HelmRelease, the chart's path is used as the chart's name, and is then stored in the state.
This means that, even if the chart and/or value haven't changed, if you move the chart, it will trigger a new release for nothing.
I'm working on a tool that is using the automation api and that I'm packaging as a .pex file (a self-extracting and executing python archive). When it's run, it's decompressed in a temporary folder with a random name. The charts are packaged in that archive, which means everything I run the tool, a new release is created, eating a lot of space on the kube cluster (or replacing olders releases with identical ones, depending on the history configuration)
Steps to reproduce
- Create a local chart, or fetch one from a registry and unpack it
- Create a HelmRelease with this chart (chart="/path/to/chart")
- Run
pulumi up - Move the chart, or rename the chart's folder.
- Update the HelmRelease resource creation
- Run
pulumi up
Expected Behavior
This should be a no-op as no meaningful change was made
Actual Behavior
- The provider considers the resource has changed (
update [diff: ~chart]) - A new helm release is stored on the cluster (as a secret of type
helm.sh/release.v1)
Output of pulumi about
As I'm using the automation API, I don't have a Pulumi.yaml file. I'd love for someone to point me to some doc about how to do that :)
CLI
Version 3.43.1
Go Version go1.19.1
Go Compiler gc
Host
OS debian
Version 11.5
Arch x86_64
Pulumi locates its logs in /tmp by default
warning: Failed to read project: no Pulumi.yaml project file found (searching upwards from /home/lta/code/projects/mainstage/main/ops). If you have not created a project yet, use `pulumi new` to do so: no project file found
warning: A new version of Pulumi is available. To upgrade from version '3.43.1' to '3.48.0', visit https://pulumi.com/docs/reference/install/ for manual instructions and release notes.
Additional context
I hope there's an obvious workaround I haven't found (outside of using relative paths which isn't an option ATM) If not, I'd love some pointers on how to fix this. This isn't really a blocking problem, but it's been annoying me for months, so my annoyance is reaching a sufficiently high level for me to dig into this
Contributing
Vote on this issue by adding a 👍 reaction. To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).
Hi @elthariel -- could you add an example of the helm.Release constructor you are running? If I understand correctly, you might be able to specify the name of the resource rather than using auto-naming to avoid this issue.
The exact call is a bit hard to extract, as it's embedded into an internal framework, so it's a bit split overall. But I'll compute you a relevant call:
return k8s.helm.v3.Release(
self.name,
args=k8s.helm.v3.ReleaseArgs(
# A static sting
name=self.name,
# The source of the issue :)
# /tmp/pex_02hfyhyl/pex-lsbfztnblfNIgJt4c41om7o2RDA/lib/infra/resource/kube/charts/ingress
# The first part of the pex changes are every run in dev mode, and at every build otherwise
chart=self._chart,
namespace=self.kube_namespace,
value_yaml_files=self._values_file_as_file_assets(),
values=self.values(),
),
opts=self.resource_opts(),
)
# The resource options are basic
pulumi.ResourceOptions(
provider=provider,
depends_on=self.depends_on(),
)
I might not be understanding your message correctly, but it seems to me that I don't use auto-naming.
My understanding of the problem is that the chart's name is it's path, as a such is stored in the state. And the change of the chart name triggers an update of the resource. This makes perfect sense when using a chart from a repository, but a little bit less when using a locally developped chart, where the name is the path. In that case, it'd be better to get the actual name from the Chart.yaml.
My go skills are quite bad and I haven't looked enough into the helm API, so I'm not sure if this is a limitation of their API, or something we could work around in this provider :-/
Ah, ok. Thanks for the code and clarification. This seems like an issue we could work around in the provider.
@lblackstone Any pointers on the approach to fix this ? I can give it a try, but I don't really known where to start
Sorry for the delayed response. https://github.com/pulumi/pulumi-kubernetes/blob/v3.23.1/provider/pkg/provider/helm_release.go#L1124 is the relevant code that sets the name. It looks like we aren't doing anything special with it; the value is plumbed directly to the helm client.
I noticed that our helm.v3.Chart resource has separate arguments for path and chart for local charts, which seems like a better solution than using chart for both purposes.
Adding that option would involve adding it to the schema, and then updating the client code I linked above to use the path if provided.
Also sorry for the delayed response :)
I've started to look into this issue, but so far I cannot build because of a java_sdk failure:
FAILURE: Build failed with an exception.
* What went wrong:
kotlin/jvm/internal/Intrinsics
[...]
** Exception is:
java.lang.NoClassDefFoundError: kotlin/jvm/internal/Intrinsics
[...]
I don't know much about Java, and honestly would prefer to keep it that way :p
The CONTRIBUTING.md file doesn't mention the requirements to be able to build the java sdk. Do you know where I could find those ?
Edit: This was due to a bad version of gradle. I've sent a separate MR to update the CONTRIBUTING.md guide
So, after trying to implement this as you suggested, it appears that it's not solving my issue. The change in the chart's path attribute also triggers a new release of the chart :sob:.
Since the path of the chart contains random character, I still get a new release every run, polluting my namespace with many useless secrets :-/
Since https://github.com/pulumi/pulumi-kubernetes/pull/2672 the provider looks to the version within Chart.yaml to determine whether the local chart has substantively changed (thus triggering an upgrade). Imagine that the provider simply ignored the chart input property when computing the diff, it would solve this issue of triggering an upgrade when the path is volatile. However, it would create a different problem, that switching to a truly different chart that happens to use the version number would fail to trigger an upgrade.
Now imagine that the provider were to normalize the chart property such that the checked value is drawn from the name field of Chart.yaml. In other words, the checked value of chart and of version would be the values from Chart.yaml, and the diff would be on that basis. Aside from complications related to migration of existing state, this seems like a practical fix. Thoughts on this?