Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow reuse-values on upgrade to be configurable #531

Closed
wants to merge 1 commit into from

Conversation

joejulian
Copy link
Contributor

What this PR does / why we need it:

many charts add new values between chart versions. When using the reuse-values flag, those charts fail because reuse-values ignores the default values file.

By making this configurable, it allows the chart maintainer to determine whether it's appropriate for their chart to add new values when upgrading.

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #525

Special notes for your reviewer:

many charts add new values between chart versions. When using the
reuse-values flag, those charts fail because reuse-values ignores the
default values file.

By making this configurable, it allows the chart maintainer to determine
whether it's appropriate for their chart to add new values when
upgrading.

Signed-off-by: Joe Julian <[email protected]>
@joejulian joejulian force-pushed the configurable-reuse-values branch from ecb72b3 to aa94b9a Compare March 11, 2023 00:44
Copy link
Member

@cpanato cpanato left a comment

Choose a reason for hiding this comment

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

lgtm, thanks for this PR
but will need to update the docs and fix the test, can you do that?

@joejulian
Copy link
Contributor Author

Yes, I've also noticed another inconsistency this can cause so I'll address that in this PR also

@stgrace
Copy link

stgrace commented Jun 14, 2023

Hey @joejulian,

Have you gotten a chance to look at this yet? Would like for this PR to get merged, because any chart adding new values basically fails on testing because of this at the moment.
I could also take a look at it if you want me to.

Kind regards

@joejulian
Copy link
Contributor Author

I'm going to close this for now. I'll re-do it when the --reset-and-reuse flag is added to helm.

@AndrewSirenko
Copy link

AndrewSirenko commented Jan 10, 2024

@joejulian now that feat(helm): Add --reset-then-reuse-values flag to 'helm upgrade' #9653 was merged, would you be able to re-open the PR? Or would you prefer me to help update docs + add test?

Edit: I mistakenly thought new helm update has released 😅 apologies

@joejulian
Copy link
Contributor Author

@joejulian now that feat(helm): Add --reset-then-reuse-values flag to 'helm upgrade' #9653 was merged, would you be able to re-open the PR? Or would you prefer me to help update docs + add test?

Yes, I'll work on that in the next week.

@ppawlowski
Copy link

Is there a plan to bring this up to the table again?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

helm upgrade uses reuse-values by default
5 participants