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

Enable "helm upgrade --install" equivalent behavior #1247

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

n-oden
Copy link

@n-oden n-oden commented Sep 7, 2023

Description

This PR adds a (potentially) idempotent "upgrade mode" to the provider, mimicking the behavior of helm upgrade --install as defined in https://github.com/helm/helm/blob/main/cmd/helm/upgrade.go

To wit: an upgrade block is added to the resource attributes, consisting of two boolean values: enable and install.

If enabled is true, this causes the provider to create a *action.Upgrade client, and attempts to perform an upgrade on the named chart. If install is true, it will first create an *action.History client to determine if a release already exists; if it does not find one it creates an *action.Install client and attempts to install the release from scratch. If a release is found, an upgrade is performed. This allows the resource to potentially co-exist with, e.g., CI/CD systems that could potentially install the release out-of-band from terraform's viewpoint.

Acceptance tests

  • Have you added an acceptance test for the functionality being added?

Release Note

Release note for CHANGELOG:

ENHANCEMENT:
* `resource/helm_release`: add `upgrade` map attribute to enable idempotent release installation, addressing components of [GH-425](https://github.com/hashicorp/terraform-provider-helm/issues/425)

References

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

@n-oden n-oden requested a review from a team as a code owner September 7, 2023 16:44
@hashicorp-cla
Copy link

hashicorp-cla commented Sep 7, 2023

CLA assistant check
All committers have signed the CLA.

@n-oden
Copy link
Author

n-oden commented Sep 17, 2023

As I noted over in the #425, I've released a test build of this for people to kick the tires on:

https://registry.terraform.io/providers/n-oden/helm/latest

@n-oden
Copy link
Author

n-oden commented Sep 28, 2023

Bump: could someone re-approve the checks? I've updated the PR to address the failing ones.

@bd-spl

This comment was marked as resolved.

helm/resource_release.go Outdated Show resolved Hide resolved
@n-oden n-oden requested a review from bd-spl October 24, 2023 14:16
@n-oden
Copy link
Author

n-oden commented Oct 24, 2023

Updated, all tests passing locally.

@n-oden
Copy link
Author

n-oden commented Oct 31, 2023

@bd-spl gentle nudge? :)

@bd-spl
Copy link

bd-spl commented Nov 8, 2023

@bd-spl gentle nudge? :)

I welcome this change, albeit have no merging powers here. Please try reaching out the owners of this repository.
I think the 'copywrite' check is blocking this effort?

@n-oden
Copy link
Author

n-oden commented Nov 9, 2023

@alexsomesan can I prevail on you to take a look here? I think this is ready to go.

@n-oden
Copy link
Author

n-oden commented Nov 29, 2023

@alexsomesan nudge? :)

@n-oden
Copy link
Author

n-oden commented Dec 13, 2023

@alexsomesan this is now rebased on top of the current main branch. Is there any chance this could be reviewed before EOY? I've got an internal project that's rather blocked by this and would like to avoid the temptation to use a forked version without a formal 👍 on the approach from your team.

@n-oden
Copy link
Author

n-oden commented Jan 3, 2024

Happy New Year everyone! @alexsomesan my apologies for being a bother, but would it be possible to get the test workflows approved? The tests are passing locally and I believe absent any other feedback that this is ready for submission.

@n-oden n-oden force-pushed the upgrade-mode branch 2 times, most recently from 3c6f6e8 to 599bca0 Compare January 17, 2024 20:02
@n-oden n-oden changed the title Proposal: upgrade mode Enable "helm upgrade --install" equivalent behavior Jan 17, 2024
@lodmfjord
Copy link

Is there any news on this?

@n-oden
Copy link
Author

n-oden commented Jan 26, 2024

@lodmfjord I'm honestly not sure what's going on here; we seem to have fallen into a bit of a hole. In the meantime if you'd like to kick the tires on this you can use https://registry.terraform.io/providers/n-oden/helm/0.0.2 which is a fork of the hashicorp helm provider with this PR cherry-picked on top. Obviously the caveat there is that there's no guarantee that they'll ever merge this back upstream, or hashicorp might ask for incompatible state schema changes before they do, so using it in production is very much an at your own risk proposition and I probably can't personally commit to keeping a fork up to date in perpetuity if this PR never gets accepted.

@n-oden
Copy link
Author

n-oden commented Feb 20, 2024

@arybolovlev @SarahFrench @appilon @jrhouston my apologies for the spam, but I'm hoping to get someone to approve running the checks on this PR, which seems to have gotten a little lost.

@alexsomesan
Copy link
Member

We're having a look at this, but I personally have to familiarise myself with the helm upgrade mechanism itself before I can evaluate this PR. I'm doing that, in the background of other tasks.

@n-oden
Copy link
Author

n-oden commented Apr 6, 2024

@alexsomesan many thanks, and if I can answer any questions please don't hesitate to ask

@n-oden
Copy link
Author

n-oden commented Apr 23, 2024

@alexsomesan just checking in: I've rebased on the current upstream main branch and dealt with the merge conflicts. Could you approve the testing workflows?

@n-oden
Copy link
Author

n-oden commented Apr 24, 2024

@alexsomesan thanks! I'm a little baffled as to why the 0.12 test is failing, and the output gives no hint. Can you re-run that with debug logging enabled?

@n-oden
Copy link
Author

n-oden commented Apr 24, 2024

FWIW I cobbled together a local test environment and was unable to reproduce the test failure using tf v0.12.31:

https://gist.github.com/n-oden/99afa11625eee21c49efddf6ebc896db

(It did eventually fail, down in TestAccResourceRelease_delete_regression but I suspect this is due to my jerry-rigged test container and not any actual code issues.)

@n-oden
Copy link
Author

n-oden commented May 2, 2024

After some more experimentationI'm gonna strongly assert that whatever was going on with the v0.12.31 test in the previous run was not a result of this PR. I set up a parallel PR in my own repo to make the tests run there and you can see here that it's passing: odenio#1

@alexsomesan I've rebased this against the current main/HEAD -- can you re-approve the test runs?

This PR adds a (potentially) idempotent "upgrade mode" to the provider,
mimicing the behavior of `helm upgrade --install` as defined in
https://github.com/helm/helm/blob/main/cmd/helm/upgrade.go

To wit: an `upgrade` block is added to the resource attributes,
consisting of tool boolean values: `enable` and `install`.

If `enable` is true, this causes the provider to create a
`*action.Upgrade` client, and attempts to perform an upgrade on the
named chart.  If `install` is true, it will first create an
`*action.History` client to determine if a release already exists; if it
does not find one it creates an `*action.Install` client and attempts to
install the release from scratch. If a release _is_ found, an upgrade is
performed.  This allows the resource to potentially co-exist with, e.g.,
CI/CD systems that could potentially install the release out-of-band
from terraform's viewpoint.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants