cortex-helm-chart icon indicating copy to clipboard operation
cortex-helm-chart copied to clipboard

Add monolithic mode options

Open TaylorMutch opened this issue 2 years ago • 4 comments

What this PR does:

Adds the ability to deploy the Cortex helm chart in a "monolithic" mode

Which issue(s) this PR fixes: Fixes https://github.com/cortexproject/cortex-helm-chart/issues/378

Checklist

  • [x] CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

TaylorMutch avatar Jul 11 '22 01:07 TaylorMutch

What you have prepared looks great and should work fine.

I am conflicted on whether this should just be its own chart. It disables almost the whole chart, so maybe it would be simpler to just have two variations of the chart, one for distributed and one for monolith.

I also wondered this as well; I somewhat took some inspiration from the bitnami memcached chart which has two architectures available in the same helm chart.

I would be open to having this as a separate chart; having the ability to run the same helm chart but with a different architecture is very useful (my use case is primarily for standing up integration/local dev environments)

TaylorMutch avatar Jul 12 '22 20:07 TaylorMutch

  1. Consolidate the pod template to be common in the Deployment and Statefulset (or help me understand the value in maintaining them separately -- I'm trying to keep it DRY)

I think a pod template that is shared makes sense; I've seen that implemented elsewhere as well. I followed the pattern for the ingester templates which has them separated. I can implement a shared pod template for the monolith, and if we like that we could do that in the rest of the chart in a separate PR. What do you think?

TaylorMutch avatar Jul 12 '22 22:07 TaylorMutch

if we like that we could do that in the rest of the chart in a separate PR. What do you think?

That would be nice, but it will depend on whether it can be done in a backwards compatible way. I haven't looked closely at the difference between the ingester deployment and statefulset to know yet if they could be updated to the same template without breaking anything. If something like the selector labels don't match it could present a problem.

kd7lxl avatar Jul 16 '22 23:07 kd7lxl

This issue has been automatically marked as stale because it has not had any activity in the past 30 days. It will be closed in 15 days if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Sep 20 '22 21:09 stale[bot]