getporter / helm2-mixin

Helm mixin for Porter
https://porter.sh/mixins/helm
Apache License 2.0
13 stars 7 forks source link

Feature: Support for both Helm v2 and v3 #66

Closed vdice closed 3 years ago

vdice commented 4 years ago

Initial strategy/proposal courtesy @MChorfa :

Strategy:

Originally posted by @MChorfa in https://github.com/deislabs/porter-helm/issues/50#issuecomment-606344338

MChorfa commented 4 years ago

Hi @vdice, @carolynvs, I had a discussion with Ralph and the argument was to maintain separate projects for each Helm client. I would love to know your take on this :) Thank you!

carolynvs commented 4 years ago

I'm aware of the difficulties around implementing the schema command since v2 and v3 support different sets of fields. @squillace Are there any other concerns you have?

squillace commented 4 years ago

From my position, Helm 2 and Helm 3 are different programs and need to present users with a coherent way of understanding the separation. Conceptually, unless you're a Helm master, the two aren't the same because of both the schema differences (while small) but also because of behavior (helm 3 doesn't create namespaces for you, for example).

Thus when I built my mixin for Helm 3, I actually renamed the binary to helm3 so that it couldn't collide with anyone also using the Helm 2 mixin whether intentionally or unintentionally. (https://github.com/squillace/porter-helm3/blob/748066f0aa53af86cec67bd550af71a79dd0da9f/pkg/helm3/build.go#L23 and usage invokes that binary.)

What I'd like to see with the Helm 2/3 mixin is the ability to use them both at once -- OR the forced choice of only one and detection at build if more than one is used.

Simplest way forward is detection of collisions and immediate failure; not concerned with how that is done. Force the choice of one or the other. Makes it very, very simple for the bundle builder.

Perfect would be the schema would be applied when you chose which version of Helm you were going to use (in the version field of the mixin declaration in yaml). but I'm guessing that's really hard if not impossible (given that VS Code sure seems to need a reload with schema changes).

Absent that one, you have to force users to choose explicitly or you have to break the mixins apart to perform the same forcing function. Where does this currently land in this proposal? I'd love to just have @MChorfa do the helm3 mixin alone for the time being, so that it can be used because it's a great experience, but if we can get a good ux for this proposed merged version, I'm in.

What do you think?

f0rmiga commented 3 years ago

The Helm 3 mixin is at https://github.com/MChorfa/porter-helm3.

carolynvs commented 3 years ago

Closing because we went with Ralph's suggestion to keep both mixins.

I think we should rename the helm mixin here to helm2 to remove any confusion with the other helm mixins. Plus update the mixins list to list them both more clearly.

I'll make a new issue to track renaming the mixin.