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

H4HIP: Adding a HIP on charts v3 #381

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

Conversation

mattfarina
Copy link
Contributor

This HIP outlines creating a new chart version of v3 to enable some more radical changes without disrupting current charts and the Helm 4 timeline.

hips/hip-0000.md Outdated

## Abstract

This HIP proposes the creation of charts v3, updating Helm to handle charts v2 and v3, and a
Copy link
Member

@gjenkins8 gjenkins8 Jan 28, 2025

Choose a reason for hiding this comment

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

The only problem, is we don't quite know what v3 would look like yet I think.

So we should aim to support both v2 and a future (or "placeholder") v3?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This HIP proposes adding a new apiVersion for charts without removing any existing versions. The design enables multiple apiVersions of charts, with significant differences, to be handled by Helm.

We must ensure existing charts continue to work so we cannot remove the old apiVersion without breaking that. If the existing code for the existing apiVersion is massively changed it will be difficult to ensure charts don't break across apiVersions.

hips/hip-0000.md Outdated

Describe the syntax and semantics of any new feature.


Copy link
Member

@scottrigby scottrigby Jan 31, 2025

Choose a reason for hiding this comment

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

Agree with Geroge's comment here #381 (comment), as well as Matt's rationale for getting a HIP on this in place. Perhaps we should say that outright here. Something like this?

Suggested change
TBD while this HIP is in draft mode. This allows us to acknowledge and make a promise for charts API v2 backwards compatibility in Helm 4 while leaving space for the upcoming radical changes we have not yet decided on.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I massively updated the HIP. The whole idea is to enable major changes to charts while ensuring existing charts continue to work without a problem.

@pull-request-size pull-request-size bot added size/L and removed size/M labels Feb 7, 2025
@mattfarina mattfarina marked this pull request as ready for review February 7, 2025 20:50
@mattfarina
Copy link
Contributor Author

Some things the new apiVersion for charts can allow without breaking the existing charts apiVersion is things like embedded WASM, different engines (maybe tied to WASM), resource ordering, changes to CRD handling, etc.

To illustrate this, while in development the `chart` package will have the following structure:

```
internal/chart
Copy link
Member

Choose a reason for hiding this comment

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

I suspect a new chart version, will also warrant a new release version.

As such, we will also want pkg/release/{v2,v3}

(Eventually, also code which will turn a chart v3 into a release v3. But I think the structure of that code can be left for another day)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call. Updated

hips/hip-0020.md Outdated
## Backwards compatibility

This development and process is designed with backwards compatibility in mind. The package
locations will change, which is ok in a major version of Rancher. The existing charts will be preserved
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
locations will change, which is ok in a major version of Rancher. The existing charts will be preserved
locations will change, which is ok in a major version of Helm. The existing charts will be preserved

:)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops. You have no idea how often I type that.

Signed-off-by: Matt Farina <[email protected]>
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.

3 participants