-
Notifications
You must be signed in to change notification settings - Fork 4
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
Support two Elasticsearch clusters #115
Conversation
@@ -56,9 +56,9 @@ Common lists of environment variables | |||
value: {{ .Values.wbstack.wikiDbProvisionVersion | quote }} | |||
- name: WBSTACK_WIKI_DB_USE_VERSION | |||
value: {{ .Values.wbstack.wikiDbUseVersion | quote }} | |||
{{- if .Values.wbstack.maxWikisPerUser }} |
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.
Not sure I follow this part of the change. Is this inadvertently removing this env var?
value: {{ .Values.mw.elasticsearch.host }} | ||
- name: MW_ELASTICSEARCH_PORT | ||
value: {{ .Values.mw.elasticsearch.port | quote }} | ||
- name: MW_PRIMARY_ELASTICSEARCH_HOST |
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.
I was thinking this could be less intrusive if we'd keep the MW_ELASTICSEARCH_HOST
for the primary and then add something optional like MW_ELASTICSEARCH_SECONDARY_HOST
. That way we could roll out changes more gradually, e.g. merge this chart change without having to sync it with anything else.
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.
Alternatively keep all the existing env vars and allow them to be comma separated lists of values for multiple hosts (this might get hairy when using though).
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.
Similar to what I had in mind but still moving from MW_ELASTICSEARCH_HOST
to MW_<new something>_ELASTICSEARCH_HOST
via deprecation.
The reason for this being that we also want to have control over whether or not to use the ES 6 compatibility layer rather than requiring an image rebuild to do this but we'd also probably rather not bake in using the compat layer as the default.
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.
Similar to what I had in mind but still moving from MW_ELASTICSEARCH_HOST to MW__ELASTICSEARCH_HOST via deprecation.
Mind elaborating what you mean by "via deprecation"?
If we want to be flexible on the versions, we can also have something like MW_ELASTICSEARCH_VERSION
and MW_ELASTICSEARCH_SECONDARY_VERSION
I guess?
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.
I was meaning that the image would work using both MW_ELASTICSEARCH_HOST
and MW_ELASTICSEARCH_<new>_HOST
however we'd mark MW_ELASTICSEARCH_HOST
as deprecated.
However the functionality of these two host configurations slightly differ. The old one remains functioning like it does now: it always adds the ES6 compatibility layer. The new one defaults to no compatibility layer unless it is specified.
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.
I pushed a PR to the mediawiki repository that may make these comments more concrete: wbstack/mediawiki#362
Closing this in favor of #122 |
No description provided.