-
Notifications
You must be signed in to change notification settings - Fork 42
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 ElasticSearch support in tutor-discovery #89
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
- [Feature] Add Elasticsearch support in tutor-discovery. As Tutor and Open edX have shifted to Meilisearch, and course-discovery still depends on Elasticsearch, running the Elasticsearch container with tutor-discovery will facilitate smoother operation for the course-discovery service. (by @Faraz32123) | ||
- It's related PR from tutor: https://github.com/overhangio/tutor/pull/1141. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
DISCOVERY_RUN_ELASTICSEARCH = {{ DISCOVERY_RUN_ELASTICSEARCH }} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,4 +23,3 @@ spec: | |
- name: settings | ||
configMap: | ||
name: discovery-settings | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
{% if DISCOVERY_RUN_ELASTICSEARCH %} | ||
--- | ||
apiVersion: v1 | ||
kind: PersistentVolumeClaim | ||
metadata: | ||
name: elasticsearch | ||
labels: | ||
app.kubernetes.io/component: volume | ||
app.kubernetes.io/name: elasticsearch | ||
spec: | ||
accessModes: | ||
- ReadWriteOnce | ||
resources: | ||
requests: | ||
storage: 2Gi | ||
{% endif %} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
{% if DISCOVERY_RUN_ELASTICSEARCH %}setowner 1000 /mounts/elasticsearch{% endif %} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am seeing |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
{% if DISCOVERY_RUN_ELASTICSEARCH %}- ../../data/elasticsearch:/mounts/elasticsearch{% endif %} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,8 +20,14 @@ | |
} | ||
} | ||
|
||
DISCOVERY_DOCKER_IMAGE_ELASTICSEARCH = "{{ DISCOVERY_DOCKER_IMAGE_ELASTICSEARCH }}" | ||
DISCOVERY_ELASTICSEARCH_HOST = "{{ DISCOVERY_ELASTICSEARCH_HOST }}" | ||
DISCOVERY_ELASTICSEARCH_PORT = "{{ DISCOVERY_ELASTICSEARCH_PORT }}" | ||
DISCOVERY_ELASTICSEARCH_SCHEME = "{{ DISCOVERY_ELASTICSEARCH_SCHEME }}" | ||
DISCOVERY_ELASTICSEARCH_HEAP_SIZE = "{{ DISCOVERY_ELASTICSEARCH_HEAP_SIZE }}" | ||
Comment on lines
+23
to
+27
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are these needed now? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As we are running elasticsearch container for/from discovery plugin now, these settings will be need in various patches to run elasticsearch same as it was running through tutor. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But if the patches were expecting the config with DISCOVERY_ prefix, won't those patches need to an update as well? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah, these settings are already being used in all patches in this PR with |
||
|
||
ELASTICSEARCH_DSL['default'].update({ | ||
'hosts': "{{ ELASTICSEARCH_SCHEME }}://{{ ELASTICSEARCH_HOST }}:{{ ELASTICSEARCH_PORT }}/" | ||
'hosts': "{{ DISCOVERY_ELASTICSEARCH_SCHEME }}://{{ DISCOVERY_ELASTICSEARCH_HOST }}:{{ DISCOVERY_ELASTICSEARCH_PORT }}/" | ||
}) | ||
|
||
{% for name, index in DISCOVERY_INDEX_OVERRIDES.items() %} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these settings are necessary for elasticsearch i think, when we are running docker in rootless mode to avoid memory issues for elasticsearch.