Skip to content

Commit

Permalink
chore: added changes requested from review
Browse files Browse the repository at this point in the history
  • Loading branch information
keithgg committed Mar 2, 2023
1 parent b8e4cbe commit 104568c
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 25 deletions.
8 changes: 6 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ always upgrading to the corresponding version before updating the chart. This is
in the management of CRDs by Helm.
If you already installed cert-manager by different means, make sure set `cert-manager.enabled: false` for this chart.


## Central Database/Monitoring/etc

In the future, this chart could also be made to install monitoring, databases, ElasticSearch, or other shared resources
Expand Down Expand Up @@ -158,10 +157,15 @@ K8S_HARMONY_ENABLE_SHARED_ELASTICSEARCH: true
RUN_ELASTICSEARCH: false
```
- And create the user on the cluster with `k8s harmony create-elasticsearch-user`.
- And create the user on the cluster with `tutor k8s harmony create-elasticsearch-user`.
- Rebuild your Open edX image `tutor images build openedx`.
- Finally, redeploy your changes: `tutor k8s start && tutor k8s init`.

#### Caveats

- In order for SSL to work without warnings the CA certificate needs to be mounted in the relevant pods. This is out of scope for this PR as it needs to be an instance level change, which requires more work than intended.
- It's not possible to mount the CA certificate for the forum due to an [outstanding issue in tutor](https://github.com/overhangio/tutor/issues/791).

## Appendix : how to uninstall this chart

Just run `helm uninstall --namespace tutor-multi tutor-multi` to uninstall this.
Expand Down
4 changes: 2 additions & 2 deletions infra-example/k8s-cluster/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ resource "digitalocean_kubernetes_cluster" "cluster" {
node_pool {
name = "${var.cluster_name}-nodes"
size = "s-4vcpu-8gb"
# At this size, at least 4 nodes are recommended to run 2 Open edX instances using the default Tutor images, because
# At this size, at least 3 nodes are recommended to run 2 Open edX instances using the default Tutor images, because
# resources like MySQL/MongoDB are not shared.
min_nodes = 4
min_nodes = 3
max_nodes = 4
auto_scale = true
}
Expand Down
38 changes: 33 additions & 5 deletions tutor-contrib-multi-plugin/tutor_multi_k8s_plugin/elasticsearch.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,15 @@
import json
import typing

from tutor import utils


class ElasticSearchAPI:
"""
Helper class to interact with the ElasticSearch
API on the deployed cluster.
"""

def __init__(self, namespace):
self._command_base = [
"kubectl",
Expand All @@ -17,19 +25,39 @@ def __init__(self, namespace):
]
self._curl_base = ["curl", "--insecure", "-u", "elastic:${ELASTIC_PASSWORD}"]

def run_command(self, curl_options):
def run_command(self, curl_options) -> typing.Union[dict, bytes]:
"""
Invokes a curl command on the first Elasticsearch pod.
If possible returns the parsed json from the Elasticsearch response.
Otherwise, the raw bytes from the curl command are returned.
"""
response = utils.check_output(
*self._command_base, " ".join(self._curl_base + curl_options)
)
try:
return json.loads(response)
except (TypeError, ValueError) as e:
except (TypeError, ValueError):
return response

def get(self, url):
return self.run_command(["-XGET", f"https://localhost:9200/{url}"])
def get(self, endpoint):
"""
Runs a GET request on the Elasticsearch cluster with the specified
endpoint.
If possible returns the parsed json from the Elasticsearch response.
Otherwise, the raw bytes from the curl command are returned.
"""
return self.run_command(["-XGET", f"https://localhost:9200/{endpoint}"])

def post(self, endpoint: str, data: dict) -> typing.Union[dict, bytes]:
"""
Runs a POST request on the Elasticsearch cluster with the specified
endpoint.
def post(self, endpoint, data):
If possible returns the parsed json from the Elasticsearch response.
Otherwise, the raw bytes from the curl command are returned.
"""
return self.run_command(
[
"-XPOST",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# For multi-instance clusters. Allow one central load balancer on the cluster to
# handle HTTPS certs and forward traffic to each Open edX instance's Caddy
# instance.
{%- set HOSTS = [LMS_HOST, CMS_HOST, PREVIEW_HOST, MFE_HOST] + MULTI_K8S_INGRESS_HOST_LIST %}
{%- set HOSTS = [LMS_HOST, CMS_HOST, PREVIEW_HOST, MFE_HOST] + K8S_HARMONY_INGRESS_HOST_LIST %}
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
Expand Down
12 changes: 5 additions & 7 deletions tutor-contrib-multi-plugin/tutor_multi_k8s_plugin/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@
# when installing additional plugins such as tutor-ecommerce or tutor-minio.
# The workaround is to manually add a list of hosts to be routed to the caddy
# instance.
"INGRESS_HOST_LIST": [ ]
"INGRESS_HOST_LIST": [],
"ENABLE_SHARED_ELASTICSEARCH": False,
},
"overrides": {
# Don't use Caddy as a per-instance external web proxy, but do still use it
Expand All @@ -31,8 +32,8 @@
},
"unique": {
"ELASTICSEARCH_HTTP_AUTH": "{{K8S_NAMESPACE}}:{{ 24|random_string }}",
"ELASTICSEARCH_INDEX_PREFIX": "{{K8S_NAMESPACE}}-{{ 4|random_string }}",
}
"ELASTICSEARCH_INDEX_PREFIX": "{{K8S_NAMESPACE}}-{{ 4|random_string|lower }}-",
},
}

# Load all configuration entries
Expand All @@ -41,9 +42,7 @@
)

hooks.Filters.CONFIG_OVERRIDES.add_items(list(config["overrides"].items()))
hooks.Filters.CONFIG_UNIQUE.add_items(
list(config["unique"].items())
)
hooks.Filters.CONFIG_UNIQUE.add_items(list(config["unique"].items()))

# Load all patches from the "patches" folder
for path in glob(
Expand All @@ -55,5 +54,4 @@
with open(path, encoding="utf-8") as patch_file:
hooks.Filters.ENV_PATCHES.add_item((os.path.basename(path), patch_file.read()))


hooks.Filters.CLI_COMMANDS.add_item(commands.harmony)
7 changes: 2 additions & 5 deletions tutor-multi-chart/Chart.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,8 @@ dependencies:
- name: cert-manager
repository: https://charts.jetstack.io
version: v1.11.0
- name: traefik
repository: https://traefik.github.io/charts
version: 20.3.0
- name: elasticsearch
repository: https://helm.elastic.co
version: 7.17.3
digest: sha256:7e80a648180d0fe471a9f45d2a47edd5da8894ef544b562911fac2e11d7fd411
generated: "2023-02-02T14:28:19.400979073-04:00"
digest: sha256:833041ca860a77cc220bdc97f7b6af8ff8b6da0c0a021615dc2858138a29bbbd
generated: "2023-02-26T12:17:23.507774503+02:00"
6 changes: 3 additions & 3 deletions tutor-multi-chart/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ elasticsearch:
# ```
# RUN_ELASTICSEARCH: false
# ELASTICSEARCH_PREFIX_INDEX: "username-"
# MULTI_K8S_USE_SHARED_ELASTICSEARCH: true
# K8S_HARMONY_USE_SHARED_ELASTICSEARCH: true
# ELASTICSEARCH_AUTH: "username:actual_password"

# The chart will handle certificate creation so that the CA can be used
# in other pods/namespaces.
# We will create the relevant certs, because they need to shared
# with pods in other namespaces.
createCert: false
# Authentication is only available in https
protocol: https
Expand Down

0 comments on commit 104568c

Please sign in to comment.