fix: ensure ES/OS cluster setup for shared usage (#73)

Signed-off-by: Gabor Boros <[email protected]> Co-authored-by: Matjaz Gregoric <[email protected]> Co-authored-by: Max Sokolski <[email protected]>
openedx · Jul 31, 2024 · 8767df2 · 8767df2
1 parent 92f7a0e
commit 8767df2
Show file tree

Hide file tree

Showing 18 changed files with 274 additions and 73 deletions.
diff --git a/README.md b/README.md
@@ -32,7 +32,7 @@ In particular, this project aims to provide the following benefits to Open edX o
    * Ingress controller: [ingress-nginx](https://kubernetes.github.io/ingress-nginx/)
    * Automatic HTTPS cert provisioning: [cert-manager](https://cert-manager.io/)
    * Autoscaling: `metrics-server` and `vertical-pod-autoscaler`
-   * Search index: ElasticSearch (support for OpenSearch is planned)
+   * Search index: ElasticSearch or OpenSearch
    * Monitoring: TODO
    * Database clusters: TODO (for now we recommend provisioning managed MySQL/MongoDB database clusters from your cloud provider using OpenTofu or a tool like [Grove](https://grove.opencraft.com/).)
    * Where possible, we try to configure these systems to **auto-detect** newly deployed Open edX instances and adapt to them automatically; where that isn't possible, Tutor plugins are used so that the instances self-register or self-provision the shared resources as needed.
@@ -282,21 +282,33 @@ from this job, run:") in a separate terminal in order to monitor the status.
 
 ### Multi-tenant Elasticsearch
 
-Tutor creates an Elasticsearch pod as part of the Kubernetes deployment. Depending on the number of instances
-Memory and CPU use can be lowered by running a central ES cluster instead of an ES pod for every instance.
+Tutor creates an Elasticsearch pod as part of the Kubernetes deployment. Depending on the number of instances, memory
+and CPU use can be lowered by running a central ES cluster instead of an ES pod for every instance.
 
-**Please note that this will only work for "Palm" version and later.**
+**Please note that this will only work for "Redwood" version and later. The OpenSearch implementation is not yet confirmed to work as expected.**
 
 To enable set `elasticsearch.enabled=true` in your `values.yaml` and deploy the chart.
 
 For each instance you would like to enable this on, set the configuration values in the respective `config.yml`:
 
 ```yaml
-K8S_HARMONY_ENABLE_SHARED_HARMONY_SEARCH: true
 RUN_ELASTICSEARCH: false
+K8S_HARMONY_ENABLE_SHARED_SEARCH_CLUSTER: true
+K8S_HARMONY_SEARCH_CLUSTER_HTTP_AUTH: instance-name:desired-password
+K8S_HARMONY_SEARCH_CLUSTER_INDEX_PREFIX: prefix-
 ```
 
-* And create the user on the cluster with `tutor k8s harmony create-elasticsearch-user`.
+If the `K8S_HARMONY_SEARCH_CLUSTER_HTTP_AUTH` or `K8S_HARMONY_SEARCH_CLUSTER_INDEX_PREFIX` is not set, the settings are
+populated with random value to ensure uniqueness.
+
+* Create the user on the cluster with `tutor harmony create-elasticsearch-user`.
+* Copy the Elasticsearch CA certificate to the instance's namespace where `$INSTANCE_NAMESPACE` is where the instance is installed in. The `$HARMONY_NAMESPACE` should be set to the namespace where the Harmony is installed to.
+  ```shell
+  kubectl get secret "search-cluster-certificates-elasticsearch" -n "$HARMONY_NAMESPACE" -o "yaml" | \
+      grep -v '^\s*namespace:\s' | \
+      sed s/-elasticsearch//g |\
+      kubectl apply -n "$INSTANCE_NAMESPACE" --force -f -
+  ```
 * Rebuild your Open edX image `tutor images build openedx`.
 * Finally, redeploy your changes: `tutor k8s start && tutor k8s init`.
 

diff --git a/charts/harmony-chart/Chart.lock b/charts/harmony-chart/Chart.lock
@@ -33,4 +33,4 @@ dependencies:
   repository: https://openfaas.github.io/faas-netes
   version: 14.2.34
 digest: sha256:b636bd16d732d51544ca7223f460e22f45a7132e31e874a789c5fc0cac460a45
-generated: "2024-04-26T06:09:47.906542+04:00"
+generated: "2024-05-02T12:32:49.796635+04:00"
diff --git a/charts/harmony-chart/Chart.yaml b/charts/harmony-chart/Chart.yaml
@@ -5,7 +5,7 @@ type: application
 # This is the chart version. This version number should be incremented each time you make changes to the chart and its
 # templates, including the app version.
 # Versions are expected to follow Semantic Versioning (https://semver.org/)
-version: 0.7.3
+version: 0.7.4
 # This is the version number of the application being deployed. This version number should be incremented each time you
 # make changes to the application. Versions are not expected to follow Semantic Versioning. They should reflect the
 # version the application is using. It is recommended to use it with quotes.

diff --git a/charts/harmony-chart/templates/elasticsearch/secrets.yaml b/charts/harmony-chart/templates/elasticsearch/secrets.yaml
@@ -5,12 +5,13 @@
 apiVersion: v1
 kind: Secret
 metadata:
-  name: elasticsearch-certificates
+  name: search-cluster-certificates-elasticsearch
 type: Opaque
 data:
   "ca.crt": {{ $ca.Cert | b64enc | toYaml | indent 4}}
   "tls.key": {{ $cert.Key | b64enc | toYaml | indent 4}}
-  "tls.crt": {{ print $cert.Cert $ca.Cert | b64enc | toYaml | indent 4}}
+  "tls.crt": {{ print $cert.Cert | b64enc | toYaml | indent 4}}
+  "chain.crt": {{ print $cert.Cert $ca.Cert | b64enc | toYaml | indent 4}}
 ---
 apiVersion: v1
 kind: Secret

diff --git a/charts/harmony-chart/templates/opensearch/secrets.yaml b/charts/harmony-chart/templates/opensearch/secrets.yaml
@@ -1,16 +1,17 @@
 ---
 {{- $ca := genCA "opensearchca" 1825 }}
-{{- $cn := printf "opensearch-master.%s.local" .Release.Namespace }}
+{{- $cn := printf "harmony-search-cluster.%s.svc.cluster.local" .Release.Namespace }}
 {{- $cert := genSignedCert $cn nil (list $cn) 1825 $ca }}
 apiVersion: v1
 kind: Secret
 metadata:
-  name: opensearch-certificates
+  name: search-cluster-certificates-opensearch
 type: Opaque
 data:
   "ca.crt": {{ $ca.Cert | b64enc | toYaml | indent 4}}
   "tls.key": {{ $cert.Key | b64enc | toYaml | indent 4}}
-  "tls.crt": {{ print $cert.Cert $ca.Cert | b64enc | toYaml | indent 4}}
+  "tls.crt": {{ print $cert.Cert | b64enc | toYaml | indent 4}}
+  "chain.crt": {{ print $cert.Cert $ca.Cert | b64enc | toYaml | indent 4}}
 ---
 {{- $password := randAlphaNum 32 }}
 {{- $password_bcrypt := $password | bcrypt }}
@@ -20,5 +21,5 @@ metadata:
   name: opensearch-credentials
 type: Opaque
 data:
-  harmony_password: {{ $password | b64enc | quote }}
+  password: {{ $password | b64enc | quote }}
   internal_users.yml: {{ printf "---\n_meta:\n  type: \"internalusers\"\n  config_version: 2\n\nharmony:\n  hash: \"%s\"\n  reserved: true\n  backend_roles:\n    - \"admin\"\n  description: \"Harmony admin user\"\n" $password_bcrypt | b64enc | quote }}
diff --git a/charts/harmony-chart/values.yaml b/charts/harmony-chart/values.yaml
@@ -47,9 +47,7 @@ elasticsearch:
   # tutor harmony create-elasticsearch-user
   # ```
   # RUN_ELASTICSEARCH: false
-  # HARMONY_SEARCH_INDEX_PREFIX: "username-"
   # K8S_HARMONY_ENABLE_SHARED_HARMONY_SEARCH: true
-  # HARMONY_SEARCH_HTTP_AUTH: "username:actual_password"
 
   # We will create the relevant certs, because they need to shared
   # with pods in other namespaces.
@@ -60,7 +58,7 @@ elasticsearch:
   # This secret will contain the http certificates.
   secretMounts:
     - name: elasticsearch-certificates
-      secretName: elasticsearch-certificates
+      secretName: search-cluster-certificates-elasticsearch
       path: /usr/share/elasticsearch/config/certs
       defaultMode: 0777
 
@@ -78,7 +76,7 @@ elasticsearch:
       xpack.security.enabled: true
       xpack.security.http.ssl.enabled: true
       xpack.security.http.ssl.key: /usr/share/elasticsearch/config/certs/tls.key
-      xpack.security.http.ssl.certificate: /usr/share/elasticsearch/config/certs/tls.crt
+      xpack.security.http.ssl.certificate: /usr/share/elasticsearch/config/certs/chain.crt
       xpack.security.transport.ssl.enabled: true
       xpack.security.transport.ssl.key: /usr/share/elasticsearch/config/certs/tls.key
       xpack.security.transport.ssl.certificate: /usr/share/elasticsearch/config/certs/tls.crt
@@ -98,14 +96,12 @@ opensearch:
   # tutor harmony create-opensearch-user
   # ```
   # RUN_ELASTICSEARCH: false
-  # HARMONY_SEARCH_INDEX_PREFIX: "username-"
   # K8S_HARMONY_USE_SHARED_OPENSEARCH: true
-  # HARMONY_SEARCH_HTTP_AUTH: "username:actual_password"
 
   # # This secret will contain the ssl certificates.
   secretMounts:
     - name: opensearch-certificates
-      secretName: opensearch-certificates
+      secretName: search-cluster-certificates-elasticsearch
       path: /usr/share/opensearch/config/certs
       defaultMode: 0777
 
@@ -125,11 +121,6 @@ opensearch:
   extraEnvs:
     - name: DISABLE_INSTALL_DEMO_CONFIG
       value: "true"
-    - name: HARMONY_PASSWORD
-      valueFrom:
-        secretKeyRef:
-          name: opensearch-credentials
-          key: harmony_password
 
   # Allows you to add any config files in {{ .Values.opensearchHome }}/config
   opensearchHome: /usr/share/opensearch

diff --git a/tutor-contrib-harmony-plugin/tutor_k8s_harmony_plugin/__about__.py b/tutor-contrib-harmony-plugin/tutor_k8s_harmony_plugin/__about__.py
@@ -1 +1 @@
-__version__ = "17.0.0"
+__version__ = "18.0.0"
diff --git a/tutor-contrib-harmony-plugin/tutor_k8s_harmony_plugin/commands.py b/tutor-contrib-harmony-plugin/tutor_k8s_harmony_plugin/commands.py
@@ -21,13 +21,21 @@ def create_elasticsearch_user(context: click.Context):
     config = tutor_config.load(context.root)
     namespace = config["K8S_HARMONY_NAMESPACE"]
     api = ElasticSearchAPI(namespace)
-    username, password = config["HARMONY_SEARCH_HTTP_AUTH"].split(":", 1)
+    username, password = config["K8S_HARMONY_SEARCH_CLUSTER_HTTP_AUTH"].split(":", 1)
     role_name = f"{username}_role"
 
-    prefix = config["HARMONY_SEARCH_INDEX_PREFIX"]
+    prefix = config["K8S_HARMONY_SEARCH_CLUSTER_INDEX_PREFIX"]
     api.post(
         f"_security/role/{role_name}",
-        {"indices": [{"names": [f"{prefix}*"], "privileges": ["all"]}]},
+        {
+            "cluster": ["monitor"], 
+            "indices": [
+                {
+                    "names": [f"{prefix}*"],
+                    "privileges": ["all"],
+                },
+            ],
+        },
     )
 
     api.post(
@@ -50,10 +58,10 @@ def create_opensearch_user(context: click.Context):
     config = tutor_config.load(context.root)
     namespace = config["K8S_HARMONY_NAMESPACE"]
     api = OpenSearchAPI(namespace)
-    username, password = config["HARMONY_SEARCH_HTTP_AUTH"].split(":", 1)
+    username, password = config["K8S_HARMONY_SEARCH_CLUSTER_HTTP_AUTH"].split(":", 1)
     role_name = f"{username}_role"
 
-    prefix = config["HARMONY_SEARCH_INDEX_PREFIX"]
+    prefix = config["K8S_HARMONY_SEARCH_CLUSTER_INDEX_PREFIX"]
     api.put(
         f"_plugins/_security/api/roles/{role_name}",
         {

diff --git a/tutor-contrib-harmony-plugin/tutor_k8s_harmony_plugin/harmony_search/base.py b/tutor-contrib-harmony-plugin/tutor_k8s_harmony_plugin/harmony_search/base.py
@@ -1,5 +1,6 @@
 import json
 import typing
+import base64
 
 from tutor import utils
 
@@ -11,36 +12,63 @@ class BaseSearchAPI:
     """
 
     def __init__(self, namespace):
-        self._command_base = [
-            "kubectl",
-            "exec",
-            "--stdin",
-            "--tty",
-            "--namespace",
-            namespace,
-            "harmony-search-cluster-master-0",
-            "--",
-            "bash",
-            "-c",
-        ]
+        self._command_base = ["kubectl", "--namespace", namespace]
+        self._exec_command = [*self._command_base, "exec", "--stdin", "--tty"]
+
         # Must be specified by subclasses
         self._curl_base = None
 
-    def run_command(self, curl_options) -> typing.Union[dict, bytes]:
+    def run_kubectl_command(self, cmd: list = None, opts: list = None) -> bytes:
+        """
+        Invokes a kubectl command in a pre-defined namespace.
+        """
+        if cmd is None:
+            cmd = self._command_base
+
+        if opts is None:
+            opts = list()
+
+        call_args = list([x for x in [*cmd, " ".join(opts)] if x])
+        return utils.check_output(*call_args)
+
+    def run_curl_command(self, curl_options) -> typing.Union[dict, bytes]:
         """
         Invokes a curl command on the first HarmonySearch pod.
 
         If possible returns the parsed json from the HarmonySearch response.
         Otherwise, the raw bytes from the curl command are returned.
         """
-        response = utils.check_output(
-            *self._command_base, " ".join(self._curl_base + curl_options)
+        container = "harmony-search-cluster-master-0"
+        response = self.run_kubectl_command(
+            cmd=[*self._exec_command, container, "--", "bash", "-c"],
+            opts=self._curl_base + curl_options,
         )
+
         try:
             return json.loads(response)
         except (TypeError, ValueError):
             return response
 
+    def get_cluster_password(self, secret_name: str, field: str = "password") -> str:
+        """
+        Returns the search admin password for the cluster.
+
+        Read the kubernetes opaque secret and return the value at the given
+        `field` from the `secret_name`.
+        """
+        password = self.run_kubectl_command(
+            [
+                *self._command_base,
+                "get",
+                "secret",
+                secret_name,
+                "-o",
+                f"jsonpath={{.data.{field}}}",
+            ]
+        )
+
+        return base64.b64decode(password).decode()
+
     def get(self, endpoint):
         """
         Runs a GET request on the HarmonySearch cluster with the specified
@@ -49,7 +77,7 @@ def get(self, endpoint):
         If possible returns the parsed json from the HarmonySearch response.
         Otherwise, the raw bytes from the curl command are returned.
         """
-        return self.run_command(["-XGET", f"https://localhost:9200/{endpoint}"])
+        return self.run_curl_command(["-XGET", f"https://localhost:9200/{endpoint}"])
 
     def post(self, endpoint: str, data: dict) -> typing.Union[dict, bytes]:
         """
@@ -59,7 +87,7 @@ def post(self, endpoint: str, data: dict) -> typing.Union[dict, bytes]:
         If possible returns the parsed json from the HarmonySearch response.
         Otherwise, the raw bytes from the curl command are returned.
         """
-        return self.run_command(
+        return self.run_curl_command(
             [
                 "-XPOST",
                 f"https://localhost:9200/{endpoint}",
@@ -78,7 +106,7 @@ def put(self, endpoint: str, data: dict) -> typing.Union[dict, bytes]:
         If possible returns the parsed json from the HarmonySearch response.
         Otherwise, the raw bytes from the curl command are returned.
         """
-        return self.run_command(
+        return self.run_curl_command(
             [
                 "-XPUT",
                 f"https://localhost:9200/{endpoint}",

diff --git a/tutor-contrib-harmony-plugin/tutor_k8s_harmony_plugin/harmony_search/elasticsearch.py b/tutor-contrib-harmony-plugin/tutor_k8s_harmony_plugin/harmony_search/elasticsearch.py
@@ -9,4 +9,5 @@ class ElasticSearchAPI(BaseSearchAPI):
 
     def __init__(self, namespace):
         super().__init__(namespace)
-        self._curl_base = ["curl", "--insecure", "-u", "elastic:${ELASTIC_PASSWORD}"]
+        cluster_password = self.get_cluster_password("elasticsearch-credentials")
+        self._curl_base = ["curl", "--insecure", "-u", f"elastic:{cluster_password}"]
diff --git a/tutor-contrib-harmony-plugin/tutor_k8s_harmony_plugin/harmony_search/opensearch.py b/tutor-contrib-harmony-plugin/tutor_k8s_harmony_plugin/harmony_search/opensearch.py
@@ -9,5 +9,5 @@ class OpenSearchAPI(BaseSearchAPI):
 
     def __init__(self, namespace):
         super().__init__(namespace)
-        # TODO: Make this configurable
-        self._curl_base = ["curl", "--insecure", "-u", "harmony:${HARMONY_PASSWORD}"]
+        cluster_password = self.get_cluster_password("opensearch-credentials")
+        self._curl_base = ["curl", "--insecure", "-u", f"harmony:{cluster_password}"]
diff --git a/tutor-contrib-harmony-plugin/tutor_k8s_harmony_plugin/patches/discovery-common-settings b/tutor-contrib-harmony-plugin/tutor_k8s_harmony_plugin/patches/discovery-common-settings
@@ -0,0 +1,14 @@
+{%- if is_plugin_loaded("discovery") and K8S_HARMONY_ENABLE_SHARED_SEARCH_CLUSTER %}
+import os
+import ssl
+
+with open(os.getenv("ELASTICSEARCH_CA_PATH")) as ca_cert:
+    ELASTICSEARCH_CA_CERT = ca_cert.read()
+
+ELASTICSEARCH_DSL['default'].update({
+  "use_ssl": True,
+  "hosts": "{{ ELASTICSEARCH_HOST }}",
+  "http_auth": "{{ K8S_HARMONY_SEARCH_CLUSTER_HTTP_AUTH }}",
+  "ssl_context": ssl.create_default_context(cadata=ELASTICSEARCH_CA_CERT),
+})
+{% endif %}
diff --git a/tutor-contrib-harmony-plugin/tutor_k8s_harmony_plugin/patches/kustomization b/tutor-contrib-harmony-plugin/tutor_k8s_harmony_plugin/patches/kustomization
@@ -0,0 +1,33 @@
+# Most of the open edX resources have volumes and volumeMounts, which means that
+# we can add an extra volume to their volume list. However, in some cases, like
+# the forum and forum-job, the resource does not define any volumes. This means,
+# we cannot have a common solution as adding extra volumes to non-existing list
+# breaks, while adding a volume definition to existing volumes replaces the
+# original set of volumes. We have to manually distinguish between resources.
+{%- set HAS_VOLUME = ["lms", "cms", "lms-job", "cms-job"] %}
+{%- set VOLUMELESS = [] %}
+
+{%- if is_plugin_loaded("discovery") %}
+  {%- set HAS_VOLUME = HAS_VOLUME + ["discovery", "discovery-job"] %}
+{% endif %}
+
+{%- if is_plugin_loaded("forum") %}
+  {%- set VOLUMELESS = VOLUMELESS + ["forum", "forum-job"] %}
+{% endif %}
+
+patches:
+- path: plugins/k8s_harmony/k8s/deployment-revision-history.yaml
+  target:
+    kind: Deployment
+{%- for res in HAS_VOLUME %}
+- path: plugins/k8s_harmony/k8s/shared-search-cert-patch.yaml
+  target:
+    kind: {% if "-job" in res %}Job{% else %}Deployment{% endif %}
+    name: {{ res }}.*
+{%- endfor %}
+{%- for res in VOLUMELESS %}
+- path: plugins/k8s_harmony/k8s/shared-search-cert-patch-volumeless.yaml
+  target:
+    kind: {% if "-job" in res %}Job{% else %}Deployment{% endif %}
+    name: {{ res }}.*
+{%- endfor %}