feat: Make it possible to customize k8s resources

[foadlind]

Currently there is no way for plugins to customize Kubernetes resources
defined in Tutor deployment manifests.
This change makes that possible by taking advantage of the strategic
merge patching mechanism in kustomization.yml.
Any resource definition in a k8s-override patch in a plugin will
override the resource defined by Tutor, provided that their names match.

Based on this comment from @regisb in #675.

[foadlind]

@regisb while testing this with a new plugin, I noticed a case that we need to think about: If two plugins override the same Kubernetes resource.
In that case, the order of the override is determined based on the order of the plugins in config.yml, which is alphabetical at the moment.

In my case, I have two plugins that are overriding the Caddy PVC size. The first one is overriding it to 5Gi, the second one to 7Gi. When I run tutor config save, I get this in my $(tutor config printroot)/env/k8s/override.yml:

# $(tutor config printroot)/env/k8s/override.yml
---
apiVersion: v1
kind: PersistentVolumeClaim
metadata:
  name: caddy
  labels:
    app.kubernetes.io/component: volume
    app.kubernetes.io/name: caddy
spec:
  accessModes:
    - ReadWriteOnce
  resources:
    requests:
      storage: 5Gi
 
---
apiVersion: v1
kind: PersistentVolumeClaim
metadata:
  name: caddy
  labels:
    app.kubernetes.io/component: volume
    app.kubernetes.io/name: caddy
spec:
  accessModes:
    - ReadWriteOnce
  resources:
    requests:
      storage: 7Gi
 

What should Tutor do in such a case?

(@fghaas This is an example of the issues you mentioned in this comment)

[foadlind]

Also there is another issue here. Let's say we have 5 plugins that are changing 5 different properties of the same Kubernetes resource. One plugin changes its volume, one changes its label, one changes its docker image, and so on.
Then, instead of just reading one yaml file to know how the resource is configured, we have to:

• first read the resource's base definition,
• then look at all the overrides that touch that resource,
• and then deduce what the end result will be in Kubernetes after all the overrides.

This is not really a desirable situation. The good thing about Tutor's current setup is that we can just read one file (for example $(tutor config printroot)/env/k8s/volumes.yml) and know how a resource will look like.

[regisb]

In my case, I have two plugins that are overriding the Caddy PVC size. The first one is overriding it to 5Gi, the second one to 7Gi. When I run tutor config save, I get this in my $(tutor config printroot)/env/k8s/override.yml.
What should Tutor do in such a case?

I don't think that Tutor should be doing anything. It's not Tutor's job to decide which plugin is right. Instead, Tutor's job is to provide a consistent experience and ensure that users have the same result across multiple runs. (thus: load plugins in alphabetical order)

Consider the alternative: what if multiple plugins override the same Tutor setting? Users will have to chase which plugin is setting each config value. The situation is worse.

• It is the plugin developer's role to add a # <my plugin> customizations comment line at the top of their k8s override patches to make it crystal clear which plugin is introducing a patch.
• It is the plugin developer's role to describe what the plugin is doing in the plugin README.
• It is the Tutor user's responsibility to read this README before they enable the plugin.

Note that it's possible for developers to modify the order of patches. Actions support a priority argument. Currently, there is no similar argument for filters, but it would be easy enough to add it. I initially added it to the v1 before removing it because I had no use for it.

Also there is another issue here. Let's say we have 5 plugins that are changing 5 different properties of the same Kubernetes resource. One plugin changes its volume, one changes its label, one changes its docker image, and so on.
Then, instead of just reading one yaml file to know how the resource is configured, we have to: ...

Yes, the definition of k8s resources will be spread over two files instead of just one. But it's easy enough to display a merged view using:

kubectl kustomize "$(tutor config printroot)/env"

We could even add that command as tutor k8s printresources.

Does this answer your questions?

[fghaas]

I don't think that Tutor should be doing anything. It's not Tutor's job to decide which plugin is right. Instead, Tutor's job is to provide a consistent experience and ensure that users have the same result across multiple runs. (thus: load plugins in alphabetical order)

If I may interject, I don't quite agree with that premise, based on how Tutor behaves today. If someone's config.yml contains

PLUGINS:
  - foo
  - bar

... then if Tutor truly didn't "decide which plugin is right", it's reasonable to expect that foo will be loaded before bar. But Tutor doesn't do that, instead it reorders this list to load bar before foo.¹

Could I ask how loading the plugins in alphabetical order, rather than the order in which they are listed in PLUGINS in config.yml, is anything other than Tutor already "deciding which plugin is right"?

Footnotes

1. I may be missing something but as far as I can make out thus far, this happens completely silently — there is no CLI warning that the order of plugins is alphabetised, it is not reflected in tutor config save, and the plugin documentation does not include this detail. Thus, I am not sure how many plugin developers, let alone users, are even aware of it. ↩

[regisb]

it's reasonable to expect that foo will be loaded before bar

I disagree with this premise. If it did, then the order with which we enable plugins would be important. This would be a source of confusion. In order to debug other people's platforms, we would have to ask them to run tutor config printvalue PLUGINS in addition to tutor plugins list.

[fghaas]

I disagree with this premise. If it did, then the order with which we enable plugins would be important.

Yes but right now absolutely nothing except one line of code indicates that it isn't. And a YAML sequence is an ordered series of zero or more nodes (emphasis mine; a mapping in contrast is unordered), so in my humble opinion it is perfectly reasonable to expect that the order in a sequence is preserved.

[regisb]

Loading plugins in alphabetical order is one of the many design decisions on which I made a judgment call during development. While I stand by my choice, if you're interested I encourage you to open a new topic about this on the forum. Since this matter is only tangentially related to the current pull request, it would be better if we moved the conversation elsewhere. If we have this conversation, then I'd like to ask you to take into account the two arguments that I listed above:

1. It would be pretty easy to make it possible to execute patches/filter callbacks in order of priority, where plugin developers have full control over this parameter.
2. Loading plugins in the order they were enabled would make it more difficult to troubleshoot user installations.

[regisb]

Oh dammit I wrote these comments before the weekend and forgot to press "submit review"... Sorry about the delay.

[regisb]

Looks good! I think this is a great step forward. Just rebase your changes and we'll merge.

[foadlind]

Thanks @regisb for the review. I rebased the PR now.

[regisb]

Thanks for your patience @foadlind!

[fghaas]

@regisb Could you please lay out your suggestions here for using this functionality in a Maple environment? As far as I can tell there is currently no branch that would allow this to be backported and/or added to a future 13.x.x release. I imagine that, based on prior experience with Open edX, many users would be reluctant to upgrade edx‑platform any earlier than its open‑release/nutmeg.3 point release (which has been our policy as well, considering the fact that database migrations usually failed prior to that). And also with Tutor's plugin system, users now have to contend with the fact that plugins take a while to update — particularly given the fact that Tutor has moved from JSON to YAML files for configuring Open edX services with v14, and it's also prudent for plugin authors to update from the v0 to the v1 plugin API rather sooner than later.

For what it's worth, the particular change covered in this PR would be a clean cherry-pick onto the v13.3.1 tag, so this would enable you to cleanly cut a 13.4.0 release if you opened up a separate maple branch based on that tag.

What do you think?

[regisb]

Unfortunately I'm not willing to maintain releases older than the current one. If you need Tutor v13 with some of the v14 features, I suggest that you fork Tutor and cherry-pick the changes you need.