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

feat: Add config parameters for k8s volume sizes #675

Closed
wants to merge 1 commit into from

Conversation

foadlind
Copy link
Contributor

Today, the storage size for Persistent Volume Claims for services like
MongoDB and MySQL are hard-coded in Tutor. Make those configurable by
setting template variables in defaults.yaml.
This gives Tutor users the option to define volume sizes in their
config.yml.

Reference discussion: https://discuss.openedx.org/t/adding-configuration-parameters-for-persistent-volume-claim-sizes-in-tutor-k8s/7327

Today, the storage size for Persistent Volume Claims for services like
MongoDB and MySQL are hard-coded in Tutor. Make those configurable by
setting template variables in defaults.yaml.
This gives Tutor users the option to define volume sizes in their
config.yml.
Copy link
Contributor

@fghaas fghaas left a comment

Choose a reason for hiding this comment

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

+1 on this change. The 5 GiB size for MongoDB in particular will be a problem for anyone migrating an existing Open edX codebase into tutor k8s, so making this configurable is absolutely a welcome addition.

@regisb
Copy link
Contributor

regisb commented May 30, 2022

@foadlind I agree that the hardcoded volume size is problematic, but I disagree with the implementation that you are suggesting. When possible, I do not want to add extra settings to the Tutor configuration. Here's my reasoning:

  1. Will a large proportion (~30-50%) of users actually want to fiddle with these settings? If yes then add a setting.
  2. If no, we need to provide a good default value AND a way for people to override that value with a plugin.

(I admit that these decision criteria should be added to the docs or to the config/defaults.yml file)

The k8s volume size is "good enough" for most people, so I'm not willing to create (and maintain) 5 extra settings to the tutor configuration. But we do need to make it easy for anyone to override these values.

Currently there is no way to override values from the deployment manifests. When we run tutor local it will automatically load any docker-compose.override.yml file. I'd like to introduce a similar mechanism in tutor k8s. For that, maybe we can make use of the "strategic merge" patching mechanism from kustomization.yml: https://kubernetes.io/docs/tasks/manage-kubernetes-objects/kustomization/#customizing

In kustomization.yml we would add the following:

patchesStrategicMerge:
- k8s/override.yaml
# add a patch for good measure (not strictly necessary today but we might need it one day)
{{ patch("kustomization-patches-strategic-merge") }}

And k8s/override.yml would be just an empty file with a patch statement:

{{ patch("k8s-override") }}

To override the volume size for a specific volume, all you would have to do is to implement the "k8s-override" patch:

---
apiVersion: v1
kind: PersistentVolumeClaim
metadata:
  name: caddy
spec:
  resources:
    requests:
      storage: 5Gi

This approach is much more generic than introducing new settings, and it opens the door to more extensive and powerful customizations for k8s operators. @fghaas @foadlind what do you think?

@foadlind
Copy link
Contributor Author

foadlind commented Jun 7, 2022

@regisb That will certainly make it easier for users to change K8s objects with plugins. Is anyone working on making these changes in Tutor? Otherwise I can take it on.

@regisb
Copy link
Contributor

regisb commented Jun 7, 2022

It would be fantastic if you could work on it @foadlind.

foadlind added a commit to foadlind/tutor that referenced this pull request Jun 7, 2022
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.

Reference: overhangio#675
foadlind added a commit to foadlind/tutor that referenced this pull request Jun 7, 2022
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.

Reference: overhangio#675
foadlind added a commit to foadlind/tutor that referenced this pull request Jun 21, 2022
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.

Reference: overhangio#675
foadlind added a commit to foadlind/tutor that referenced this pull request Jun 27, 2022
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.

Reference: overhangio#675
foadlind added a commit to foadlind/tutor that referenced this pull request Jun 28, 2022
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.

Reference: overhangio#675
regisb pushed a commit that referenced this pull request Jun 28, 2022
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.

Reference: #675
@CodeWithEmad
Copy link
Member

is this PR abandoned?
It's almost time to celebrate its first anniversary!

@regisb
Copy link
Contributor

regisb commented Jun 6, 2023

The "k8s-override" patch now exists, so k8s volume size can be easily overridden by a plugin. I think this is the "right" way of overriding the volume size.

@CodeWithEmad
Copy link
Member

I completely agree. Therefore, we can consider this matter resolved.

@fghaas
Copy link
Contributor

fghaas commented Jun 6, 2023

Yes, we do use a YAML plugin with a k8s-override patch for this purpose now. Side note: please see kubernetes-sigs/kustomize#5052 for an issue related to patchesStrategicMerge deprecation/removal.

@kumarviresh25
Copy link

hi i have created a plugin k8s-override and after saving i can see the increased volume in override. i want to ask i have to apply override.yml file and thn restart thn my mongo volume will be increased . one more thing i have created another plugin for memory increment of lms so i have used k8s-deployments i can see in deployments.yml with update after this i am running tutor k8s start it is throwing error Namespace already exists: skipping creation.
kubectl apply --kustomize /home/ubuntu/eks/envtutor/env --wait --selector app.kubernetes.io/component=volume
error: map[string]interface {}(nil): yaml: unmarshal errors:
line 20: mapping key "apiVersion" already defined at line 1
line 21: mapping key "kind" already defined at line 2
line 22: mapping key "metadata" already defined at line 3
line 27: mapping key "spec" already defined at line 14
Error: Command failed with status 1: kubectl apply --kustomize /home/ubuntu/eks/envtutor/env --wait --selector app.kubernetes.io/component=volume
@regisb @foadlind pl check once.

@regisb
Copy link
Contributor

regisb commented Sep 22, 2023

Resolved by the "k8s-override" patch. Please re-open if you think this PR is still necessary.

@regisb regisb closed this Sep 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Won't fix
Development

Successfully merging this pull request may close these issues.

5 participants