openedx / openedx-k8s-harmony

A Prototype Helm Chart for deploying multiple Open edX instances (via Tutor) onto a cluster.
GNU Affero General Public License v3.0
10 stars 14 forks source link

Deploy shared ElasticSearch onto the cluster #4

Closed bradenmacdonald closed 1 year ago

bradenmacdonald commented 1 year ago

Can use a Helm subchart.

Note: we should consider the use case where multiple Open edX platforms are run in the same cluster with different elasticsearch versions.

antoviaque commented 1 year ago

@keithgg Could you post a status update on this task here? This way we could follow & discuss here async, ahead of the next meeting.

keithgg commented 1 year ago

Sure @antoviaque.

There was a minor (I hope) issue I ran into, which is that when running multiple ElasticSearch versions it might make sense to have them in different namespaces. es7 vs es8 for instance. Because both helm chart set up similar containers there's some conflicts.

AFAICT helm doesn't allow deploying to multiple namespaces, but I'm still a helm newbie so I need to investigate it a bit to see what options we have here.

bradenmacdonald commented 1 year ago

@keithgg Yeah, Helm doesn't really support deploying to multiple namespaces with a single chart (or at least it's highly discouraged). You can always run a separate command to deploy a chart or subchart to a separate namespace, but that's not ideal.

Do we really need to run both es7 and es8 on the cluster though? Why is that?

antoviaque commented 1 year ago

@keithgg Thanks! Btw if I understand correctly, the issue was a blocker to the completion of the current task? What is the work remaining currently to complete the current ticket, and ETA?

keithgg commented 1 year ago

Do we really need to run both es7 and es8 on the cluster though?

Note: we should consider the use case where multiple Open edX platforms are run in the same cluster with different elasticsearch versions.

@bradenmacdonald I didn't think we did, but your note suggested otherwise. Did you mean something else?

Thanks! Btw if I understand correctly, the issue was a blocker to the completion of the current task? What is the work remaining currently to complete the current ticket, and ETA?

@antoviaque yep. There's not that much to do here, besides adding the helm chart with some light configuration. I should be able to complete the work in a couple hours, but testing it is problematic, because I'm incorporating multiple changes to make it work.

I'll be working on this Monday with the aim of having everything completed by Wednesday next week.

bradenmacdonald commented 1 year ago

@keithgg I meant to think about it, which you have now done, but it doesn't need to be a blocker if it's not something we can easily support. Just a potential worry for the future so it's helpful to plan now in case there's a workaround we can include from the beginning. But if not, that's fine.

bradenmacdonald commented 1 year ago

@keithgg Actually there may be a workaround that we can use [in the future], though it depends on whether or not the ElasticSearch chart supports it:

https://cert-manager.io/docs/installation/helm/#installing-cert-manager-as-subchart

There may be a situation where you want to specify the namespace to install [something] different to the umbrella chart's namespace.

This is a known issue with helm and subcharts, that you can't specify the namespace for the subchart and is being solved by most public charts by allowing users to set the namespace via the values file, but needs to be a capability added to the chart by the maintainers.

keithgg commented 1 year ago

Update: I got delayed a bit with the forum changes (had to brush up on my Ruby), but I managed to get ES deployed onto the cluster. If all goes well, I should be able to wrap this up tomorrow.

antoviaque commented 1 year ago

@keithgg How is this going? Sorry if this has been mentioned during the last meeting that I missed, but I wasn't sure after reading the notes if this was explicitly discussed.

bradenmacdonald commented 1 year ago

@antoviaque It's ready and @keithgg is just waiting for our review on https://github.com/openedx/tutor-contrib-multi/pull/13

antoviaque commented 1 year ago

Update from the meeting: there is one last item from the review that @keithgg need to address, and this should then hopefully be ready to merge.