rainest
commented
1 year ago tl;dr
Using an umbrella chart results in a number of limitations on template development (in general, we cannot share information between the two releases, and thus cannot, for example, automatically configure the controller admin API target based on the proxy's admin API Service configuration) and challenges to release processes. If we no longer wish to support deploying the controller as a sidecar, it makes sense to use a single chart that deploys the controller and Kong each in their own Deployment.
This requires some redesign of the values.yaml to support configuring the Deployments independently. There are several options for this:
- Do not actually require changes or add new settings, and simply apply the same settings to both Deployments equally. This is a bad option.
- Apply existing settings to the proxy Deployment and create new settings for the controller Deployment as needed. This requires fewer changes, but results in confusing configuration design, since like settings are configured in very different places.
- Reorganize settings for consistency and clearing some debt where we poorly structured them in the past. This involves breaking changes. Most of these changes are just moving a key to a new location, however, so a tool to automate it shouldn't be difficult to write.
https://github.com/Kong/charts/pull/923 is the companion to this analysis. It annotates keys of interest in place within values.yaml.
I do not think we should attempt to support both models, as designing good configuration and templates for that would be quite difficult. If we want to keep support for the sidecar in any fashion, I'd recommend doing that in a continued 2.x line while we use 3.x for split Deployments, and work with users to figure out what they'd need to migrate if it's difficult.
Background
The current design of the kong chart uses a single Deployment, which can
contain a Kong container, controller container, or both. values.yaml only has a
single set of Deployment-level (and Pod-level) settings, as these are shared
equally among all containers.
Migrations Jobs often also share many of these settings even though they're a separate workload. Since migrations use a Kong container, settings applicable to the Kong Deployment are often applicable to the migrations, and sharing them allows us to economize on settings.
To split Kong and the controller into separate Deployments, we thus need to convert these settings into two settings in most cases. We may opt to fall back to the original shared setting for both--it's not ideal and limits some functionality (e.g. you probably don't want the same replica count and scaling rules for a controller Deployment as a Kong Deployment), but could allow for a simpler transition for existing users. Defaults for the new settings in the base values.yaml will apply, however, so this option is probably less appealing than it'd seem initially.
values.yaml organization
The chart has grown organically over time, with historically less-informed
decicisions about how to organize its configuration effectively. We have, for
example, root-level .podAnnotations, .deploymentAnnotations, and
.deployment configuration keys. Although it'd make sense to nest these along
the lines of the actual resource structure, like
deployment:
annotations: {}
pod:
annotations: {}we added one of the root-level annotations keys early, before we needed the others, and didn't move it when it became disorganized.
Mirrored and consolidated Kubernetes resource configuration
Since adding a second Deployment will require separate configuration keys for many existing settings, it'd make sense to organize them following our most recent structure. For example:
deployment:
controller:
annotations: {} # Deployment-level annotations
autoscaling: {} # A duplicate of the current root key, for the controller Deployment
pod:
annotations: {} # Pod-level annotations. currently there is a single root-level .podAnnotations used for the single unified Pod
serviceAccount: {}
container:
env: {...} # controller environment
image: {}
sidecarContainers: [] # attach a tcpdump or what have you
kong: {} # same as the controller section
admissionWebhook: {} # the current contents for .ingressController.admissionWebhookThis section should hold all configuration that lives under the Kubernetes Deployment resource.
This section may hold configuration related to Deployment-associated resources,
like HorizontalPodAutoscaler (.deployment.*.autoscaling). We can alternate
similarly split those sections (autoscaling.controller.* and
autoscaling.kong.*).
Application-level configuration
We have several sections dedicated to application-specific configuration
unrelated to Kubernetes resources, such as the ingressController.konnect
settings and enterprise settings.
While these generally manipulate values in a container's environment, they
don't clearly fit into the Kubernetes resource structure from a user POV. As
such I think we should keep them mostly at their current locations, with the
end result that we have deployment.controller (or
deployment.ingressController if we want name consistency) for standard
Kubernetes resource settings and ingressController.konnect,
ingressController.gatewayDiscovery, and so on for our application-specific
configuration.
Migration strategies
Moving keys is a breaking change for existing settings. If we move
configuration currently elsewhere (e.g. .podAnnotations to
.deployment.kong.pod.annotations), users will have to update configuration.
We can provide a helper script, however, as looking up a YAML key and moving it elsewhere isn't particularly hard.
We could alternatively continue to support existing keys, but this comes with
template complexity and non-obvious precedence rules. If you, for example, see
documentation saying "set .deployment.kong.pod.annotations" and do so without
realizing you already have a .podAnnotations and we prefer the latter, your
settings will not take effect and you will be confused. We would probably need
to prefer the older setting if present due to the way values.yaml defaults
work.
We can opt to use the newest organization pattern for new settings only (for
example, creating .deployment.controller.pod.annotations while retaining
.podAnnotations for the proxy, but this prolongs the problem.
Existing release migration
After changes to values.yaml, users will need to upgrade releases.
kong chart users
Going from the kong chart with a single Deployment to one with multiple
should not require much beyond updating values.yaml, and maybe choosing
different settings for the new separate Deployment (optional, but probably
advisable).
AFAIK the main task here is to handle the change from an internal localhost listen for the admin API to discovery, so flipping the admin Service on with the appropriate external listen and enabling discovery. There's some manual effort required, but not much beyond a typical upgrade.
Multiple release kong chart users
Although some classes of separate release (namely separate controller releases) should no longer be necessary, users can continue using them to avoid breaking release continuity.
These users still need to migrate values.yaml and set up discovery if they were using a single-Deployment Kong+controller release, but they do not need to consolidate releases if they do not want to. The procedure to create, for example, a hybrid DP release is still to create a release with the controller disabled and DP settings configured.
Although the multi-Deployment configuration would make single-release hybrid mode more viable, I'm not considering it at present.
ingress chart users
This state has no perfect migration path. Helm, broadly, cannot transfer resources from one release to another (there's technically maybe a path by uninstalling and keeping resources and then manipulating labels to trick it into adopting them into a new release, but that's fiddly as heck and I don't really want to entertain it as a generally acceptable plan of action).
Migrating existing ingress chart releases to the kong chart would thus
require deleting an ingress release and creating a kong release. This
requires an outage window in the simplest case, though it may be possible to
carefully orchestrate a handover with the old release still installed.
Since ingress is DB-less only, this approach should be fine: the new
instances will come up, see the same Ingress resources, and apply the same
configuration. This shouldn't be significantly different from running kubectl rollout restart beyond that there are temporarily no replicas of the proxy
or controller.
Template refactoring
Using mirrored structure for the Kubernetes resource configuration should technically allow us to use a shared template for them that we invoke for each section. However, application-level configuration, though not directly part of the resource configuration, ends up there, mostly in the form of environment variables.
It may be simple enough to just inject the environment variable set generated from another section into the Deployment template. I'd have to try it to be sure.
pmalek
czeslavo
https://github.com/Kong/kubernetes-ingress-controller/issues/4671 talks about chart stuff but isn't in this repo (presumably for milestone reasons). This is a stub to provide charts tracking of the same.