Terraform-provider-helm: helm_release: Suggestion to split chart attribute to local and remote

[legal90]

Hi, that topic is just a suggestion. Feel free to share your thoughts regarding this.

What's issue?

In the current implementation of helm_release resource (v0.7.0, v0.8.0) we have a getChart() function which is called one time for _each_ declared resource (by resourceDiff func) and one more time for each newly created or updated resource (by resourceReleaseCreate and resourceReleaseUpdate appropriately).

This function is pretty heavy because it downloads & unarchives the chart every time when it's called. That slows down the terraform run on big configurations and/or pure network connection. We should minimize these calls as possible.

Suggested Solution

The obvious solution is to do not call getChart() in the CustomizeDiff func in case of the remote chart (e.q. not the local directory or URL), assuming that the chart version is not changed since we already "lock" it with repository and version attributes.

I suggest to split the chart attribute to two:

• The existing attribute chart, use it for remote charts only. Requires repository and version to be set. In this case CustomizeDiff func should neither download the chart, nor override its version.
• Some new attribute, like chart_uri - use it only for local paths or URI. Conflicts with repository and chart (those should not be set in the configuration). In this case CustomizeDiff should behave as it does now - fetch the chart and override the desired version from the metadata.

For the smooth migration we can implement the StateUpgrader func, so the state will be modified transparently for the most of users.

Please let me know what you think about it.
If it's OK, I can work on the implementation & send a PR later this week, or the next one.

P.s. Relevant links:

• https://github.com/terraform-providers/terraform-provider-helm/issues/173
• https://github.com/terraform-providers/terraform-provider-helm/pull/161#discussion_r255358762

[meyskens]

I have been thinking about this and I think it would be maybe better to detect from the current values if this is local or not or if the version is pinned or not. Helm or our provider is doing this anyway so we could use the same logic inside CustomizeDiff.

Would this solve it or did I misunderstand the issue here?

[jpreese]

Just wanted to chime in and say this is something that we would be interested in. Dealing with https://kubernetes-charts.storage.googleapis.com/ using the provider can be a little painful due to its size.

I think there's value in detecting whether or not the index needs to be re-downloaded, or provide a way to use a local chart.

[mcuadros]

@legal90 still applies?

[amitsehgal]

yes... i have been getting the same error

2020/05/14 12:04:03 [TRACE] dag/walk: vertex "root" is waiting for "provider.helm (close)"
2020/05/14 12:04:04 [TRACE] dag/walk: vertex "provider.helm (close)" is waiting for "module.eks_addons.helm_release.cluster_autoscaler"
2020/05/14 12:04:08 [TRACE] dag/walk: vertex "root" is waiting for "provider.helm (close)"
2020/05/14 12:04:09 [TRACE] dag/walk: vertex "provider.helm (close)" is waiting for "module.eks_addons.helm_release.cluster_autoscaler"
2020/05/14 12:04:13 [TRACE] dag/walk: vertex "root" is waiting for "provider.helm (close)"
2020/05/14 12:04:14 [TRACE] dag/walk: vertex "provider.helm (close)" is waiting for "module.eks_addons.helm_release.cluster_autoscaler"
2020/05/14 12:04:18 [TRACE] dag/walk: vertex "root" is waiting for "provider.helm (close)"
2020/05/14 12:04:19 [TRACE] dag/walk: vertex "provider.helm (close)" is waiting for "module.eks_addons.helm_release.cluster_autoscaler"