-
Notifications
You must be signed in to change notification settings - Fork 446
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: Upgrade to Olive #734
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Get Tutor to work on the master branches of Open edX. The corresponding images will have to be rebuilt manually. Note that the process to contribute to the nightly branch is slightly different from the master branch (see the instructions from the corresponding tutorial).
Open edX master now runs elasticsearch 7.10 and mongodb 4.2. Redis also received a minor upgrade.
Nginx and Caddy performed duplicate tasks. It was decided to get rid of the nginx container, for simplification. This is a breaking change for plugin developers. Also, applications that collect nginx logs will have to be modified. See: - Corresponding TEP: https://discuss.overhang.io/t/tep-get-rid-of-the-nginx-container/2024 - the prior discussion: https://discuss.overhang.io/t/why-caddy-nginx/1952
With this change, containers are no longer run as "root" but as unprivileged users. This is necessary in some environments, notably some Kubernetes clusters. To make this possible, we need to manually fix bind-mounted volumes in docker-compose. This is pretty much equivalent to the behaviour in Kubernetes, where permissions are fixed at runtime if the volume owner is incorrect. Thus, we have a consistent behaviour between docker-compose and Kubernetes. We achieve this by bind-mounting some repos inside "*-permissions" services. These services run as root user on docker-compose and will fix the required permissions, as per build/permissions/setowner.sh These services simply do not run on Kubernetes, where we don't rely on bind-mounted volumes. There, we make use of Kubernete's built-in volume ownership feature. With this change, we get rid of the "openedx-dev" Docker image, in the sense that it no longer has its own Dockerfile. Instead, the dev image is now simply a different target in the multi-layer openedx Docker image. This makes it much faster to build the openedx-dev image. Because we declare the APP_USER_ID in the dev/docker-compose.yml file, we need to pass the user ID from the host there. The only way to achieve that is with a tutor config variable. The downside of this approach is that the dev/docker-compose.yml file is no longer portable from one machine to the next. We consider that this is not such a big issue, as it affects the development environment only. We take this opportunity to replace the base image of the "forum" image. There is now no need to re-install ruby inside the image. The total image size is only decreased by 10%, but re-building the image is faster. In order to run the smtp service as non-root, we switch from namshi/smtp to devture/exim-relay. This change should be backward-compatible. Note that the nginx container remains privileged. We could switch to nginxinc/nginx-unprivileged, but it's probably not worth the effort, as we are considering to get rid of the nginx container altogether. Close #323.
Caddy should always be running, even when ENABLE_WEB_PROXY is false. It's the service that should not always be running.
In the past, tutor was installed with "pip install tutor-openedx". For some time (since v12.0.2), "tutor" was installed as a dependency of "tutor-openedx". Now is the time to get rid of that old package. The standard way of installing tutor is now with "pip install tutor".
Forum is an optional feature, and as such it deserves its own plugin. Starting from Maple, users will be able to install the forum from https://github.com/overhangio/tutor-forum/ Close #450.
When nginx was removed in favour of caddy, we decided that plugin implementations of the "caddyfile" patch should make use of the "port" local variable. However, local variables are not available from inside plugin patches, which are rendered outside of the context of the parent templates. For a more extensive description of the problem, see: overhangio/tutor-mfe#23 (comment) We still want to make it easy for developers to decide what should the port be for caddy hosts. To do so, we make use of environment variables that are passed at runtime to the caddy container. Thus, a regular plugin patch should look like this: {{ PLUGIN_HOST }}{$default_site_port} { import proxy "myplugin:8000" }
Adding these volumes was a mistake.
…n Tutor version changes Through the commonLabels directive in kustomization.yml, all resources get a label named "app.kubernetes.io/version", which is being set to the Tutor version at the time of initial deployment. When the user then subsequently progresses to a new Tutor version, Kubernetes attempts to update this label — but for Deployment, ReplicaSet, and DaemonSet resources, this is no longer allowed as of kubernetes/kubernetes#50808. This causes "tutor k8s start" (at the "kubectl apply --kustomize" step) to break with errors such as: Deployment.apps "redis" is invalid: spec.selector: Invalid value: v1.LabelSelector{MatchLabels:map[string]string{"app.kubernetes.io/instance":"openedx-JIONBLbtByCGUYgHgr4tDWu1", "app.kubernetes.io/managed-by":"tutor", "app.kubernetes.io/name":"redis", "app.kubernetes.io/part-of":"openedx", "app.kubernetes.io/version":"12.1.7"}, MatchExpressions:[]v1.LabelSelectorRequirement(nil)}: field is immutable Simply removing the app.kubernetes.io/version label from kustomization.yml will permanently fix this issue for newly created Kubernetes deployments, which will "survive" any future Tutor version changes thereafter. However, *existing* production Open edX deployments will need to throw the affected Deployments away, and re-create them. Also, add the Tutor version as a resource annotation instead, using the commonAnnotations directive. See also: kubernetes/client-go#508 https://kubectl.docs.kubernetes.io/references/kustomize/kustomization/commonlabels/ https://kubectl.docs.kubernetes.io/references/kustomize/kustomization/commonannotations/ Fixes #531.
Project name was incorrectly set to "tutor_dev" instead of "tutor_nightly_dev".
The DJANGO_SETTINGS_MODULE is far from being relevant in all containers.
regisb
force-pushed
the
olive
branch
3 times, most recently
from
December 6, 2022 11:56
34be1bd
to
868f95f
Compare
6 tasks
regisb
force-pushed
the
olive
branch
4 times, most recently
from
December 12, 2022 15:47
6c7cadf
to
fa3cf79
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.