-
Notifications
You must be signed in to change notification settings - Fork 115
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
Helm Chart v4 (Component) #2947
Conversation
Does the PR have any schema changes?Looking good! No breaking changes found. New resources:
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2947 +/- ##
==========================================
+ Coverage 29.99% 32.29% +2.29%
==========================================
Files 63 69 +6
Lines 8338 8947 +609
==========================================
+ Hits 2501 2889 +388
- Misses 5615 5791 +176
- Partials 222 267 +45 ☔ View full report in Codecov by Sentry. |
a8f65ff
to
2186e1d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Crazy thought experiment, partly out of curiosity and partly to help me better understand our wrapping:
Let's pretend newRootCmd
is public, so we have a NewRootCmd
that gives us a cobra.Command
encapsulating all of helm's functionality.
We can feed that cobra.Command
raw arguments with something like cmd.SetArgs([]string{"install", "--values", ...})
), and we can execute it like any other command.
I'm curious what functionality we would lose, or what would no longer be expressible, if we reduced everything to parsing Pulumi's inputs → mapping those to CLI args → feeding that to the CLI command?
Obviously the command's outputs would be awkward to work with, but there's also the really appealing upside of always matching upstream...
d797696
to
5342b6b
Compare
limitations under the License. | ||
*/ | ||
|
||
// package helm contains code vendored from the upstream Helm project. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering what our maintenance strategy is for this vendored fork? Are we going to try and track upstream changes closely? If so, maybe a quick doc.go that describes how this code was vendored and the releavant changes made, if any?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also it appears that we have our own code within the vendored code in this file? If so, should we also include a Pulumi license to this file? Not sure what the correct procedure here is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I too was wondering about that but I'm pretty sure we've done enough with the headers. The vendoring was done simply by copy-paste of certain functions. I think we'll keep iterating on the tool soon, e.g. for a Release v4 resource.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 to headers being sufficient.
Part of the motivation behind my suggestion here was to help reduce non-upstream code, because otherwise things will start to get intermingled and harder to keep in sync.
It might make sense to break out our pulumi-specific stuff into its own package (pulumihelm or something) to make tracking upstream easier, but we don't need to do that now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still really curious if we can remove the custom MergeValues but that doesn't need to block. Let's do it!
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@pulumi/kubernetes](https://pulumi.com) ([source](https://togithub.com/pulumi/pulumi-kubernetes)) | dependencies | minor | [`4.11.0` -> `4.12.0`](https://renovatebot.com/diffs/npm/@pulumi%2fkubernetes/4.11.0/4.12.0) | --- > [!WARNING] > Some dependencies could not be looked up. Check the Dependency Dashboard for more information. --- ### Release Notes <details> <summary>pulumi/pulumi-kubernetes (@​pulumi/kubernetes)</summary> ### [`v4.12.0`](https://togithub.com/pulumi/pulumi-kubernetes/blob/HEAD/CHANGELOG.md#4120-May-21-2024) [Compare Source](https://togithub.com/pulumi/pulumi-kubernetes/compare/v4.11.0...v4.12.0) ##### Added - Added a new Helm Chart v4 resource. ([https://github.com/pulumi/pulumi-kubernetes/pull/2947](https://togithub.com/pulumi/pulumi-kubernetes/pull/2947)) - Added support for deletion propagation policies (e.g. Orphan). ([https://github.com/pulumi/pulumi-kubernetes/pull/3011](https://togithub.com/pulumi/pulumi-kubernetes/pull/3011)) - Server-side apply conflict errors now include the original field manager's name. ([https://github.com/pulumi/pulumi-kubernetes/pull/2983](https://togithub.com/pulumi/pulumi-kubernetes/pull/2983)) ##### Changed - Pulumi will now wait for DaemonSets to become ready. ([https://github.com/pulumi/pulumi-kubernetes/pull/2953](https://togithub.com/pulumi/pulumi-kubernetes/pull/2953)) - The Release resource's merge behavior for `valueYamlFiles` now more closely matches Helm's behavior. ([https://github.com/pulumi/pulumi-kubernetes/pull/2963](https://togithub.com/pulumi/pulumi-kubernetes/pull/2963)) ##### Fixed - Helm Chart V3 previews no longer fail when the cluster is unreachable. ([https://github.com/pulumi/pulumi-kubernetes/pull/2992](https://togithub.com/pulumi/pulumi-kubernetes/pull/2992)) - Fixed a panic that could occur when a missing field became `null`. ([https://github.com/pulumi/pulumi-kubernetes/issues/1970](https://togithub.com/pulumi/pulumi-kubernetes/issues/1970)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://togithub.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zNzEuMSIsInVwZGF0ZWRJblZlciI6IjM3LjM3MS4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJ0eXBlL21pbm9yIl19--> Co-authored-by: lumiere-bot[bot] <98047013+lumiere-bot[bot]@users.noreply.github.com>
Proposed changes
This PR implements an MLC-based
Chart
resource as per design doc.Notable improvements over v3:Chart:
config.kubernetes.io/depends-on
--values
) and as individual values (--set-file
).Detailed changes:
helm.sh/v4:Chart
resource plusv4:PostRenderer,v4:RepositoryOpts
PreRegisterF
hook to be able to mutate child resource options, specificallyRetainOnDelete
HaveSkipAwaitAnnotation()
Tests:
Examples
See the examples in the API docs:
https://github.com/pulumi/pulumi-kubernetes/blob/47a6ae4fceb8339c49d4f13b5f765af798c0308b/provider/pkg/gen/examples/overlays/chartV4.md#example-usage
Local Chart:
Repository Chart:
Remote Chart:
OCI Chart:
Custom Values:
Chart Verification w/ Keyring:
Post-Rendering:
Related issues (optional)
Fixes #2847