refactor: use filter to define workers command

overhangio · Oct 18, 2024 · 86864b7 · 86864b7
1 parent f0b2bdd
commit 86864b7
Show file tree

Hide file tree

Showing 5 changed files with 140 additions and 77 deletions.
diff --git a/tutor/hooks/catalog.py b/tutor/hooks/catalog.py
@@ -514,8 +514,9 @@ def your_filter_callback(some_data):
     #: :param str file_path: The path to the file being checked.
     IS_FILE_RENDERED: Filter[bool, [str]] = Filter()
 
-    #: List of workers
-    CELERY_WORKERS_CONFIG: Filter[dict[str, dict[str, Any]], []] = Filter()
+
+    LMS_WORKER_COMMAND: Filter[list[str]] = Filter
+    CMS_WORKER_COMMAND: Filter[dict[str, str]] = Filter
 
 
 class Contexts:

diff --git a/tutor/plugins/openedx.py b/tutor/plugins/openedx.py
@@ -144,60 +144,65 @@ def is_directory_mounted(image_name: str, dirname: str) -> bool:
 )
 
 
-@hooks.Filters.CELERY_WORKERS_CONFIG.add(priority=hooks.priorities.HIGH)
-def _add_default_celery_queues(worker_configs):
+@hooks.Filters.LMS_WORKER_COMMAND.add(priority=hooks.priorities.HIGH)
+def _add_default_lms_worker_parameters(worker_configs):
     worker_configs = {
-        "lms": {
-            "default": {
-                "concurrency": 100,
-                "pool": "gevent",
-            },
-            "high": {
-                "concurrency": 1,
-                "pool": "prefork",
-            },
-            "high_mem": { # Not sure what's the difference this one and high
-                "concurrency": 1,
-                "pool": "prefork",
-            },
-        },
-        "cms": {
-            "default": {
-                "concurrency": 100,
-                "pool": "gevent",
-            },
-            "high": {
-                "concurrency": 1,
-                "pool": "prefork",
-            },
-            "low": { # Not sure how to is this used
-                "concurrency": 100,
-                "pool": "gevent",
-            },
-        },
+        "loglevel": "info",
+        "hostname": "edx.lms.core.default.%%h",
+        "queues": "edx.lms.core.default,edx.lms.core.high,edx.lms.core.high_mem",
+        "max-tasks-per-child": "100",
     }
     return worker_configs
 
 
+@hooks.Filters.CMS_WORKER_COMMAND.add(priority=hooks.priorities.HIGH)
+def _add_default_cms_worker_parameters(worker_configs):
+    worker_configs = {
+        "loglevel": "info",
+        "hostname": "edx.cms.core.default.%%h",
+        "queues": "edx.cms.core.default,edx.cms.core.high,edx.cms.core.low",
+        "max-tasks-per-child": "100"
+    }
+    return worker_configs
+
+
+@hooks.lru_cache
+def get_cms_celery_parameters() -> dict[str, dict[str, t.Any]]:
+    """
+    This function is cached for performance.
+    """
+    return hooks.Filters.CMS_WORKER_COMMAND.apply({})
+
+
 @hooks.lru_cache
-def get_celery_workers_config() -> dict[str, dict[str, t.Any]]:
+def get_lms_celery_parameters() -> dict[str, dict[str, t.Any]]:
     """
     This function is cached for performance.
     """
-    return hooks.Filters.CELERY_WORKERS_CONFIG.apply({})
+    return hooks.Filters.LMS_WORKER_COMMAND.apply({})
+
+
+def iter_cms_celery_parameters() -> dict[str, dict[str, t.Any]]:
+    """
+    Yield:
+
+        (name, dict)
+    """
+    return {name: config for name, config in get_cms_celery_parameters().items()}
 
 
-def iter_celery_workers_config() -> dict[str, dict[str, t.Any]]:
+def iter_lms_celery_parameters() -> dict[str, dict[str, t.Any]]:
     """
     Yield:
 
         (name, dict)
     """
-    return {name: config for name, config in get_celery_workers_config().items()}
+    return {name: config for name, config in get_lms_celery_parameters().items()}
 
 
 hooks.Filters.ENV_TEMPLATE_VARIABLES.add_items(
     [
-        ("iter_celery_workers_config", iter_celery_workers_config),
+        ("iter_cms_celery_parameters", iter_cms_celery_parameters),
+        ("iter_lms_celery_parameters", iter_lms_celery_parameters),
     ]
 )
diff --git a/tutor/templates/apps/openedx/settings/partials/common_all.py b/tutor/templates/apps/openedx/settings/partials/common_all.py
@@ -246,8 +246,5 @@
     "user": None,
 }
 
-# Prevents losing tasks when workers are shutdown
-CELERY_ACKS_LATE = True
-
 {{ patch("openedx-common-settings") }}
 ######## End of settings common to LMS and CMS
diff --git a/tutor/templates/k8s/deployments.yml b/tutor/templates/k8s/deployments.yml
@@ -122,6 +122,57 @@ spec:
 ---
 apiVersion: apps/v1
 kind: Deployment
+metadata:
+  name: cms-worker
+  labels:
+    app.kubernetes.io/name: cms-worker
+spec:
+  selector:
+    matchLabels:
+      app.kubernetes.io/name: cms-worker
+  template:
+    metadata:
+      labels:
+        app.kubernetes.io/name: cms-worker
+    spec:
+      securityContext:
+        runAsUser: 1000
+        runAsGroup: 1000
+      containers:
+        - name: cms-worker
+          image: {{ DOCKER_IMAGE_OPENEDX }}
+          args:
+          - "celery"
+          - "worker"
+          - "--app=cms.celery"{% for setting, value in iter_cms_celery_parameters().items() %}
+          - "--{{setting}}={{value}}"{% endfor %}
+          env:
+          - name: SERVICE_VARIANT
+            value: cms
+          - name: DJANGO_SETTINGS_MODULE
+            value: cms.envs.tutor.production
+          volumeMounts:
+            - mountPath: /openedx/edx-platform/lms/envs/tutor/
+              name: settings-lms
+            - mountPath: /openedx/edx-platform/cms/envs/tutor/
+              name: settings-cms
+            - mountPath: /openedx/config
+              name: config
+          securityContext:
+            allowPrivilegeEscalation: false
+      volumes:
+        - name: settings-lms
+          configMap:
+            name: openedx-settings-lms
+        - name: settings-cms
+          configMap:
+            name: openedx-settings-cms
+        - name: config
+          configMap:
+            name: openedx-config
+---
+apiVersion: apps/v1
+kind: Deployment
 metadata:
   name: lms
   labels:
@@ -182,60 +233,57 @@ spec:
             - key: uwsgi.ini
               path: uwsgi.ini
 ---
-{% for service, variants in iter_celery_workers_config().items() %}
-{% for variant, config in variants.items() %}
-{% set deployment = service + "-" + "worker" + "-" + variant.replace("_", "-") %}
----
 apiVersion: apps/v1
 kind: Deployment
 metadata:
-  name: {{deployment}}
+  name: lms-worker
   labels:
-    app.kubernetes.io/name: {{deployment}}
+    app.kubernetes.io/name: lms-worker
 spec:
   selector:
     matchLabels:
-      app.kubernetes.io/name: {{deployment}}
+      app.kubernetes.io/name: lms-worker
   template:
     metadata:
       labels:
-        app.kubernetes.io/name: {{deployment}}
+        app.kubernetes.io/name: lms-worker
     spec:
       securityContext:
         runAsUser: 1000
         runAsGroup: 1000
       containers:
-        - name: {{deployment}}
+        - name: lms-worker
           image: {{ DOCKER_IMAGE_OPENEDX }}
           args:
-            - "celery"
-            - "--app={{service}}.celery"
-            - "worker"
-            - "--loglevel=info"
-            - "--hostname=edx.{{service}}.core.{{variant}}.%%h"
-            - "--queues=edx.{{service}}.core.{{variant}}"{% for setting, value in config.items() %}
-            - "--{{setting}}={{value}}"{% endfor %}
+          - "celery"
+          - "worker"
+          - "--app=lms.celery"{% for setting, value in iter_cms_celery_parameters().items() %}
+          - "--{{setting}}={{value}}"{% endfor %}
           env:
           - name: SERVICE_VARIANT
-            value: {{service}}
+            value: lms
           - name: DJANGO_SETTINGS_MODULE
-            value: {{service}}.envs.tutor.production
+            value: lms.envs.tutor.production
           volumeMounts:
-            - mountPath: /openedx/edx-platform/{{service}}/envs/tutor/
-              name: settings-{{service}}
+            - mountPath: /openedx/edx-platform/lms/envs/tutor/
+              name: settings-lms
+            - mountPath: /openedx/edx-platform/cms/envs/tutor/
+              name: settings-cms
             - mountPath: /openedx/config
               name: config
           securityContext:
             allowPrivilegeEscalation: false
       volumes:
-        - name: settings-{{service}}
+        - name: settings-lms
+          configMap:
+            name: openedx-settings-lms
+        - name: settings-cms
           configMap:
-            name: openedx-settings-{{service}}
+            name: openedx-settings-cms
         - name: config
           configMap:
             name: openedx-config
-{% endfor %}
-{% endfor %}
+---
 {% if RUN_ELASTICSEARCH %}
 ---
 apiVersion: apps/v1

diff --git a/tutor/templates/local/docker-compose.prod.yml b/tutor/templates/local/docker-compose.prod.yml
@@ -26,28 +26,40 @@ services:
     {% endif %}
 
   ############# LMS and CMS workers
-  {% for service, variants in iter_celery_workers_config().items() %}
-  {% for variant, config in variants.items() %}
-  {% set deployment = service + "-" + "worker" + "-" + variant.replace("_", "-") %}
-  {{deployment}}:
+  lms-worker:
     image: {{ DOCKER_IMAGE_OPENEDX }}
     environment:
-      SERVICE_VARIANT: {{service}}
-      DJANGO_SETTINGS_MODULE: {{service}}.envs.tutor.production
-    command: celery --app={{service}}.celery worker --loglevel=info --hostname=edx.{{service}}.core.default.%%h --queues=edx.{{service}}.core.{{variant}}{% for param, value in config.items() %} --{{param}}={{value}}{%endfor%}
+      SERVICE_VARIANT: lms
+      DJANGO_SETTINGS_MODULE: lms.envs.tutor.production
+    command: celery worker --app=lms.celery{% for setting, value in iter_lms_celery_parameters().items() %} "--{{setting}}={{value}}"{% endfor %}
     restart: unless-stopped
     volumes:
       - ../apps/openedx/settings/lms:/openedx/edx-platform/lms/envs/tutor:ro
       - ../apps/openedx/settings/cms:/openedx/edx-platform/cms/envs/tutor:ro
       - ../apps/openedx/config:/openedx/config:ro
-      - ../../data/{{service}}:/openedx/data
+      - ../../data/lms:/openedx/data
       - ../../data/openedx-media:/openedx/media
-      {%- for mount in iter_mounts(MOUNTS, "openedx", service + "-worker") %}
+      {%- for mount in iter_mounts(MOUNTS, "openedx", "lms-worker") %}
       - {{ mount }}
       {%- endfor %}
     depends_on:
-      - {{service}}
-    {% endfor %}
-    {% endfor %}
+      - lms
 
-  {{ patch("local-docker-compose-prod-services")|indent(2) }}
+  cms-worker:
+    image: {{ DOCKER_IMAGE_OPENEDX }}
+    environment:
+      SERVICE_VARIANT: cms
+      DJANGO_SETTINGS_MODULE: cms.envs.tutor.production
+    command: celery worker --app=cms.celery{% for setting, value in iter_cms_celery_parameters().items() %} "--{{setting}}={{value}}"{% endfor %}
+    restart: unless-stopped
+    volumes:
+      - ../apps/openedx/settings/lms:/openedx/edx-platform/lms/envs/tutor:ro
+      - ../apps/openedx/settings/cms:/openedx/edx-platform/cms/envs/tutor:ro
+      - ../apps/openedx/config:/openedx/config:ro
+      - ../../data/cms:/openedx/data
+      - ../../data/openedx-media:/openedx/media
+      {%- for mount in iter_mounts(MOUNTS, "openedx", "cms-worker") %}
+      - {{ mount }}
+      {%- endfor %}
+    depends_on:
+      - cms