Skip to content

feat(helm): Add --reset-then-reuse-values flag to 'helm upgrade' - #9653

Merged
scottrigby merged 3 commits into
helm:mainfrom
okhoshi:feat/reset-then-reuse-flag
Nov 4, 2023
Merged

feat(helm): Add --reset-then-reuse-values flag to 'helm upgrade'#9653
scottrigby merged 3 commits into
helm:mainfrom
okhoshi:feat/reset-then-reuse-flag

Conversation

@okhoshi

@okhoshi okhoshi commented Apr 27, 2021

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:
When --reset-then-reuse-values is used on 'helm upgrade', the chart's values will be reset to the values of the deployed chart while the current release's values will be reused and merged with the values passed as argument (is any). --reset-values and --reuse-values flags take precedence over --reset-then-reuse-values, making it ignored if one or the other is also used.

This is a slight improvement over the many times suggested workaround of helm get values > v.yaml; helm upgrade --reset-values --values v.yaml in the sense that this flag allows for atomic operation.

Closes #8085
Closes #3957

If applicable:

  • this PR contains documentation
  • this PR contains unit tests
  • this PR has been tested for backwards compatibility

@helm-bot helm-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Apr 27, 2021
@okhoshi okhoshi changed the title feat(helm): add --reset-then-reuse-values flag to 'helm upgrade' feat(helm): Add --reset-then-reuse-values flag to 'helm upgrade' Apr 27, 2021
When '--reset-then-reuse-values' is used on 'helm upgrade', the chart's values will be
reset to the values of the deployed chart while the current release's values will be
reused and merged with the values passed as argument (is any). '--reset-values' and
'--reuse-values' flags take precedence over `--reset-then-reuse-values', making it
ignored if one or the other is also used.

Closes helm#8085, helm#3957

Signed-off-by: Quentin Devos <quentin@devos.pm>
Comment thread pkg/action/upgrade_test.go
@erickpeirson

Copy link
Copy Markdown

@bacongobbler How can we move this forward?

@bitjson

bitjson commented Dec 15, 2021

Copy link
Copy Markdown

This would really improve our workflow 👍

Tested, and PR works as expected. Is there any additional work or review which needs to be done before this can be merged?

@zombrie

zombrie commented Apr 5, 2022

Copy link
Copy Markdown

This is exactly what we need for our current chart situation... is this going to be merged soon?

@yaron2

yaron2 commented Jun 17, 2022

Copy link
Copy Markdown

Please merge this. I can't stress enough how broken the current experience is.

@Ga13Ou

Ga13Ou commented Sep 8, 2022

Copy link
Copy Markdown

Any idea why this simple, yet useful pull request isn't merged yet?

@voidtek

voidtek commented Nov 23, 2022

Copy link
Copy Markdown

This seems to solve my current headache. Any news?

@kostya2011

Copy link
Copy Markdown

Is there any news on this one?

@ldming

ldming commented Mar 9, 2023

Copy link
Copy Markdown

This seems to solve my current headache. Any news?

@asauber

asauber commented Apr 20, 2023

Copy link
Copy Markdown

This would be a huge help for getting the expected upgrade behavior when using --set. Can we merge this?

@asauber asauber left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mamoit

mamoit commented Jun 13, 2023

Copy link
Copy Markdown

@okhoshi is there anything blocking this PR?

@okhoshi

okhoshi commented Jun 13, 2023

Copy link
Copy Markdown
Contributor Author

@mamoit No idea, not from my side at least. I don't have write access to this repo so I can't merge.

@mamoit

mamoit commented Jun 13, 2023

Copy link
Copy Markdown

Then @jglick, @asauber or @michi-covalent maybe?
Since they are the assigned reviewers.

@michi-covalent

Copy link
Copy Markdown

i don't have write access to the repo either. i just approved it because i love this PR ❤️

@helm helm deleted a comment from yaron2 Aug 1, 2023
@joejulian joejulian added this to the 3.13.0 milestone Aug 1, 2023
Signed-off-by: Joe Julian <me@joejulian.name>

@joejulian joejulian left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this and it works with every scenario I could think of. Approving but this still needs to pass CI.

@joejulian joejulian added the Has One Approval This PR has one approval. It still needs a second approval to be merged. label Aug 1, 2023
@amannijhawan

Copy link
Copy Markdown
Contributor

This is a quite useful PR, I have no special access either.

@0x2b3bfa0

0x2b3bfa0 commented Aug 16, 2023

Copy link
Copy Markdown

@joejulian, it already passes continuous integration checks.

@scottrigby scottrigby left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks @okhoshi for this contribution 👏

A few extra kudos: the linked issues and PR description fully explain the use case for this new feature and the reasoning for this particular solution over existing workarounds. The PR includes automated tests, which are sensible and pass CI automation (since @joejulian last merged in main). This has been peer reviewed by other Helm community members, and reviewed now by two maintainers. Merging now, so this should be included in the next scheduled feature release. 🚀

@scottrigby
scottrigby merged commit 2745909 into helm:main Nov 4, 2023
@jglick

jglick commented Nov 15, 2023

Copy link
Copy Markdown

should be included in the next scheduled feature release

Per https://github.com/helm/helm/releases/tag/v3.13.2

3.14.0 is the next feature release and [will] be on January 17, 2024.

Looking forward to finally having this fixed!

@voidtek

voidtek commented Nov 15, 2023

Copy link
Copy Markdown

Can't wait for it to be merged into the release-3.13 branch.
https://github.com/helm/helm/commits/release-3.13

@scottrigby : any blockers?

@mikael-floden

Copy link
Copy Markdown

OMG finally! Can't believe this is true, I have spent hours and hours explaining how helm is broken to every new developer. How --reuse-values breaks the system, ignoring all new values from the next version/release

People just didn't get it. I don't want to know how many weeks I have spent on users not understanding when you can and when you can't use --reuse-values.

This is how EVERYONE intuitively thinks it works, this parameter should be the default. The lack of this feature has hold back helm for to many years.

This is the "it always works parameter". Thanks you 🙏

mumoshu pushed a commit to databus23/helm-diff that referenced this pull request Oct 23, 2024
…4.0 (#634)

* Allow fetching only user-supplied existing values

* Add support for the --reset-then-reuse-values flag

This flag was added in Helm v3.14.0; for details see
helm/helm#9653.

* Document --reset-then-reuse-values in the README

* Add a comment referencing the new flag's origin

* Move min Helm version check to narrower scope

* Fix unintended name shadowing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Has One Approval This PR has one approval. It still needs a second approval to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet