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

Next-gen (3.0) chart layout #977

Open
3 tasks
mflendrich opened this issue Dec 19, 2023 · 2 comments
Open
3 tasks

Next-gen (3.0) chart layout #977

mflendrich opened this issue Dec 19, 2023 · 2 comments

Comments

@mflendrich
Copy link
Member

Problem Statement

We've decided that the 3.0 version of Helm charts will come with a complete revamp of the user experience: instead of starting from the "implementation" (1-1 relationship with the deployment that is either Kong or KIC or both) we will start from the "use case" (KIC, no KIC, Konnect DP, Operator, etc.)

Proposed Solution

First step: merge #976 (3.0.0-alpha.1)
Second step: reach agreement on the desired UX. We have two options on the table

  • Option 1: targeted small charts (kong/ingress, kong/konnect-dp, kong/operator - perhaps more) where default values have a reasonable "batteries included" capability
  • Option 2: single chart with different values.yaml examples for different use cases

We currently believe that Option 1 is more promising but more research needs to go into it.

Third step: 3.0.0-alpha.2 is implemented in the chosen option out of the above

Fourth step: 3.0.0-alpha.N promoted to 3.0.0

Acceptance Criteria

  • @mheap and @team-k8s jointly choose the option to proceed out of Option 1 and Option 2
  • 3.0.0 is released in the new shape
  • if multiple charts come into existence, their build system is configured to release all in one go, with the same version number
@rainest
Copy link
Contributor

rainest commented Dec 19, 2023

I would like to do a GA release of the chart in #976 sooner. The scope there
(create a chart that deploys the controller and Kong separately, with upgrade
paths from both kong/kong 2.x and kong/ingress 0.x) is already known, we have
code ready to do it, and it being out has no bearing on the other changes
proposed here.

Is there some reason we believe a GA release would prevent us from taking
either approach mentioned here later?

Keeping it unreleased requires continuing to deal with the problems associated
with umbrella charts requires redoing any change we make to kong/kong 2.x to
handle the revised templates and values.yaml structure in the consolidated
chart. Enough of the design and implementation of the requests here in #977 are
unknown that we'd probably be stuck with that extra burden for a while.

That said, some preliminary reviews of the options:

Single chart, separate values.yamls

This seeks to allow installing kong/kong, kong/foo, kong/etc for
different standard configurations using a single chart packaged with a
different base values.yaml. This is, IMO, a bit of overkill for something that
could be handled with a -f https://raw.githubusercontent.com/Kong/charts/main/charts/kong/example-values/minimal-kong-standalone.yaml,
but it's probably doable with CD hacks.

chart-releaser's package step appears to just be a wrapper around the helm package command before uploading its output to GH pages. The loader and saver
helm package uses
don't appear to have much any intelligence around chart structure specifically,
and just serve to package the whole of a chart dir's contents into a tar.gz.

Actually modifying these tools to provide some sort of values.yaml injection
would probably require maintaining forks of helm (for a custom package
command), chart-releaser (it doesn't let you bring your own helm binary AFAIK),
and chart-releaser-action (ditto re BYO chart-releaser binary), which isn't
particularly advisable.

The simplicity of the packager does allow us to just copy a chart directory
repeatedly and sub out values.yaml before point chart-releaser at that location
instead of the default. We also need to update the name in Chart.yaml, though
we wouldn't avoid that with modified Helm tooling (Chart.yaml is the apparent
source of truth for the name--there's no obvious separately-determined name in
the code).

Injecting a complete values.yaml rather than overlaying overrides requires some
method of maintaining separate complete values.yaml. Maintaining these
independently would unfortunaely be a massive pain, as we'd need to update any
key that doesn't need to change between configurations N times for each
variant. Although merging these is simple in Go,
we have no way of invoking Helm's merge tool alone (it only runs as part of
install or template) without building our own binary. Using yq
(https://mikefarah.gitbook.io/yq/v/v2.x/merge#overwrite-values) is probably a
viable alternative.

Separate charts with shared templates

We'd ideally have some clearer expectation around what we cannot do now (or
with default values.yaml hacks) before going this route.

Our topologies generally all involve installing a Kong Deployment, with the
specifics of its role determined by environment variables. That's an outgrowth
of how Kong itself is designed: there is a single Kong image that fulfills
different roles based on kong.conf. The surrounding Kubernetes resources
generally remain the same regardless of role. If we want to separate them per
role, we should know what needs to differ and is difficult or cumbersome to
handle via template logic.

The operator is an exception because its chart would presumably not deploy
anything the existing charts deploy, since those charts deploy roughly the same
set of resources the operator is supposed to manage. However, if we plan to
continue supporting installing Kong directly via Helm, an operator chart would
presumably be entirely separate. We could share some common functionality
similar to what's in Bitnami common, but it's hard to say what the overlap
would be without actually trying to write the operator chart.

The one example I have for using a library dependency (Bitnami's chart library)
generally uses it for generic utility functionality shared across charts with
significant differences elsewhere (no matter how you deploy Kong, it's going to
be pretty different than how you deploy Postgres). For example, they use the
library functions to determine a full release name and shared labels for all
resources

within a chart-specific deployment template.

Dependency charts load the same way regardless of whether they're rendered in
an unbrella chart or simply being used as storage for common templates. For
example. the inclusion of the common chart
in most Bitnami charts comes along withe same Chart.lock pinning
that we currently have in kong/ingress. We'd still have the same multi-step
releases we do now.

Symlinking a library template file may be a way to circumvent the need for a
separate dependency release. We don't have an obvious need to use a separate
repo, which would require using Helm's dependency system.

@czeslavo
Copy link
Contributor

I think that the first option @rainest describes (Single chart, separate values.yamls) doesn’t look promising. It would mean we’d have non-standard tooling that would be doing the job of releasing the very same chart but with different names and default values. Maintaining forks of helm or any other dependency looks like a no-go for me. 😟

Picking the second option has its issues but I think it is cleaner and closer to the standard approach (e.g. to how bitnami does that). I’d see that with the following steps to be executed:

  1. Rename kong/kong to kong/ingress in Release consolidated chart as kong 3.0.0-alpha1 #976 and release it as 3.0 (after ensuring all breaking changes we’d like to incorporate into the 3.0 release are already in, e.g. changing the default router type, etc.). I remember @mheap saying that renaming would satisfy the basic requirements we have for now.
  2. Start creating separate charts to cover the missing use cases (kong/konnect-dp, kong/operator), taking into account that if we find we have to unnecessarily duplicate templates that are already in kong/ingress, we extract such templates to kong/common chart. We use the extracted templates from kong/common in both kong/ingress and kong/* new charts.

We won’t be able to predict up-front all templates that are really to be extracted to a common chart so I think that trying to create new charts would be the best method to find out what we can extract from kong/ingress. I think we should make sure we have a clear dependency tree where kong/common is the only common dependency. No user-facing chart should depend on another user-facing chart (e.g. kong/konnect-dp should not rely on kong/ingress). kong/common should not render any resources by default but it should only export templates to be used (i.e. if I define kong/common as a dependency in Chart.yaml it doesn’t make any new resources to be rendered unless I use a common.someTemplateName in my chart’s templates).

Regarding the issue with multi-step releases in case of modifying something in kong/common - I believe we could mitigate that by using file:// prefix in the dependencies[].repository in Chart.yaml of our user-facing charts. That would allow us to modify kong/common and release every dependency in one step together with kong/common. We could write a helper script that would bump the dependency on kong/common and bump their minor/patch (it could be the script's parameter) in user-facing charts so we don’t have to go one-by-one and manually bump the version in their respective Chart.yaml. That would mean just a make update-deps or similar before pushing changes in kong/common chart.

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

No branches or pull requests

3 participants