Use helm dependency build in release build #563

Merged
justusbunsi merged 2 commits from justusbunsi/helm-chart:build-vs-update into main 2023-11-27 18:35:42 +00:00
Member

Using helm dependency update may result in unwillingly updating the
dependencies while cutting a release. I wasn't able to do so. Most
likely due to the dependency pinning in Chart.yaml and Chart.lock.

Based on Helm documentation, update uses Chart.yaml1 while build
uses Chart.lock2.
All in all it is safer to use helm dependency build. :D

Using `helm dependency update` may result in unwillingly updating the dependencies while cutting a release. I wasn't able to do so. Most likely due to the dependency pinning in Chart.yaml and Chart.lock. Based on Helm documentation, `update` uses Chart.yaml[^1] while `build` uses Chart.lock[^2]. All in all it is safer to use `helm dependency build`. :D [^1]: https://helm.sh/docs/helm/helm_dependency_update/ [^2]: https://helm.sh/docs/helm/helm_dependency_build/
justusbunsi added 1 commit 2023-11-14 17:24:19 +00:00
All checks were successful
check-and-test / check-and-test (pull_request) Successful in 27s
e8aaad527b
Use `helm dependency build` in release build
Using `helm dependency update` may result in unwillingly updating the
dependencies while cutting a release. I wasn't able to do so. Most
likely due to the dependency pinning in Chart.yaml and Chart.lock.

Based on Helm documentation, `update` uses Chart.yaml[^1] while `build`
uses Chart.lock[^2].
All in all it is safer to use `helm dependency build`. :D

[^1]: https://helm.sh/docs/helm/helm_dependency_update/
[^2]: https://helm.sh/docs/helm/helm_dependency_build/

Signed-off-by: justusbunsi <sk.bunsenbrenner@gmail.com>
justusbunsi requested review from pat-s 2023-11-14 17:24:26 +00:00
justusbunsi requested review from techknowlogick 2023-11-14 17:24:31 +00:00
justusbunsi added the
kind
build
label 2023-11-14 18:21:58 +00:00
Member

Interesting. Shouldn't Chart.lock and Chart.yaml be in sync at all times? The former is created from the latter and I don't see how helm dependency update would change Chart.yaml if a fixed version is used.

However, if you make a manual change to Chart.yaml (e.g. to prepare an update) and run helm dependency build, you will get an error:

Error: the lock file (Chart.lock) is out of sync with the dependencies file (Chart.yaml). Please update the dependencies

which essentially means: "Use helm dependency update".

So I wonder: should we just stick to helm dependency update? What event motivated you too look into this in the first place?

Interesting. Shouldn't `Chart.lock` and `Chart.yaml` be in sync at all times? The former is created from the latter and I don't see how `helm dependency update` would change `Chart.yaml` if a fixed version is used. However, if you make a manual change to `Chart.yaml` (e.g. to prepare an update) and run `helm dependency build`, you will get an error: > Error: the lock file (Chart.lock) is out of sync with the dependencies file (Chart.yaml). Please update the dependencies which essentially means: "Use `helm dependency update`". So I wonder: should we just stick to `helm dependency update`? What event motivated you too look into this in the first place?
Author
Member

@pat-s I probably confused you with the PR description. What I wanted to say:

Regenerating the charts folder is possible with both commands helm dependency build and helm dependency update.

  • For the build command: This is its sole purpose.
  • For the upgrade command: The charts folder will be build, but you are usually using this command for updating dependencies.

That's similar to npm install and npm update.

The intention of this PR is consistency within our builds. For releasing the chart, we simply need to regenerate the charts folder. There is no intention to update dependencies. So it would be consistent to use helm dependency build, not update.

So I wonder: should we just stick to helm dependency update? What event motivated you too look into this in the first place?

IMO, the continued usage of helm dependency update in our release workflow is risky. Maybe not right now as we have versions in bot Chart.yaml and Chart.lock preventing an unintended update.
I got there because I looked up what the release build does - in the context of #564. And then downloaded the released tar.gz file to make sure there was no implicit dependency update.

Using helm dependency build eliminates any doubts in the future. 🙂

@pat-s I probably confused you with the PR description. What I wanted to say: Regenerating the `charts` folder is possible with both commands `helm dependency build` and `helm dependency update`. - For the `build` command: This is its sole purpose. - For the `upgrade` command: The `charts` folder will be build, but you are usually using this command for updating dependencies. That's similar to `npm install` and `npm update`. The intention of this PR is consistency within our builds. For releasing the chart, we simply need to regenerate the `charts` folder. There is no intention to update dependencies. So it would be consistent to use `helm dependency build`, not `update`. > So I wonder: should we just stick to `helm dependency update`? What event motivated you too look into this in the first place? IMO, the continued usage of `helm dependency update` in our release workflow is risky. Maybe not right now as we have versions in bot Chart.yaml and Chart.lock preventing an unintended update. I got there because I looked up what the release build does - in the context of #564. And then downloaded the released tar.gz file to make sure there was no implicit dependency update. Using `helm dependency build` eliminates any doubts in the future. 🙂
pat-s approved these changes 2023-11-16 19:20:49 +00:00
pat-s left a comment
Member

Thanks for the detailed clarification. All clear now!

Thanks for the detailed clarification. All clear now!
Author
Member

@techknowlogick You wanna review or shall we merge?

@techknowlogick You wanna review or shall we merge?
Author
Member

Merging now.

Merging now.
justusbunsi added 1 commit 2023-11-27 18:34:51 +00:00
All checks were successful
check-and-test / check-and-test (pull_request) Successful in 31s
745ddd8821
Merge branch 'main' into build-vs-update
justusbunsi merged commit 34c1212939 into main 2023-11-27 18:35:42 +00:00
justusbunsi deleted branch build-vs-update 2023-11-27 18:35:43 +00:00
Sign in to join this conversation.
No description provided.