Skip to content
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

Add Helm chart for MongoDB sharded cluster #769

Merged
merged 5 commits into from
Jan 25, 2024

Conversation

sejongk
Copy link
Contributor

@sejongk sejongk commented Jan 19, 2024

What this PR does / why we need it:
This PR adds a Helm chart for MongoDB, supporting both standalone mode and sharded cluster mode.
Shard rules, including shard keys, shard methods, and unique constraints, are defined and managed in the values.yaml.

This chart depends on the Bitnami's mongodb-sharded chart (https://github.com/bitnami/charts/tree/main/bitnami/mongodb-sharded).
There are a few known issues with it:

How to install and uninstall

  1. Create mongodb namespace
kubectl create namespace mongodb
  1. Install helm chart with the set flag and the namespace flag (The reason why the namespace flag is needed to be set is due to the misguided value in the Bitnami chart. In here, it uses .Release.Namespace without offering an option to override it.
helm install mongodb . --namespace mongodb --set=sharded.enabled=true
  1. Uninstall helm chart with the namespace flag
helm install mongodb . --namespace mongodb

Which issue(s) this PR fixes:

Addresses #673

Special notes for your reviewer:

We need to do following tasks in the future:

  • Enable livenessProbe, readinessProbe
  • Connect to Prometheus via PodMonitors

Does this PR introduce a user-facing change?:


Additional documentation:


Checklist:

  • Added relevant tests or not required
  • Didn't break anything

@sejongk sejongk marked this pull request as draft January 19, 2024 13:12
Copy link

codecov bot commented Jan 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (65712ec) 50.14% compared to head (6522060) 50.00%.
Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #769      +/-   ##
==========================================
- Coverage   50.14%   50.00%   -0.15%     
==========================================
  Files          69       69              
  Lines       10182    10224      +42     
==========================================
+ Hits         5106     5112       +6     
- Misses       4534     4570      +36     
  Partials      542      542              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sejongk sejongk marked this pull request as ready for review January 24, 2024 06:51
Copy link
Member

@hackerwins hackerwins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution.
I left a few comments.

build/charts/yorkie-mongodb/values.yaml Show resolved Hide resolved
Copy link
Member

@krapie krapie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution 😄
First off, I have left some small conventional suggestions along with comprehensive questions.

Question 1

It seems like yorkie-cluster Helm chart no longer has mongoDB subshart. This means that we now need to install both yorkie-cluster and yorkie-mongodb Helm chart to fully use yorkie cluster. Is there any reason for this separation?

Question 2

It seems like yorkie-mongodb Helm chart performs sharded mongoDB setup by using Job object. Is there any reason for this approach?

I can see that mongodb-sharded Helm chart provides common.initScriptsCM value to initialize mongoDB instances (Nevertheless I also think using Job is the best approach for this kind of operation, I just want to know the reasons for this approach).

Lack of ARM64 support in the Bitnami mongodb-sharded container
Solution: use the official mongo 6.0 image instead and set up the cluster via a Job.

Also, is there any relations between ARM64 support and setting up cluster via Job?

Suggestion 3

How about adding NOTES.txt to keep consistency with other yorkie Helm charts?

Suggestion 4

We might need to add ArgoCD Application manifest in yorkie-argocd Helm Chart as well.

build/charts/yorkie-mongodb/Chart.yaml Show resolved Hide resolved
build/charts/yorkie-mongodb/values.yaml Show resolved Hide resolved
Copy link
Member

@krapie krapie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍🏼

@hackerwins hackerwins self-requested a review January 25, 2024 10:35
Copy link
Member

@hackerwins hackerwins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@hackerwins hackerwins merged commit 7a0b0b5 into main Jan 25, 2024
3 checks passed
@hackerwins hackerwins deleted the add-mongodb-sharded-cluster-helm-chart branch January 25, 2024 10:39
hackerwins added a commit that referenced this pull request Jan 25, 2024
This PR adds a Helm chart for MongoDB, supporting both standalone mode
and sharded cluster mode.
Shard rules, including shard keys, shard methods, and unique
constraints, are defined and managed in the values.yaml.

This chart depends on Bitnami's mongodb-sharded chart
(https://github.com/bitnami/charts/tree/main/bitnami/mongodb-sharded).

There are a few known issues with it:

A problem with livenessProbe and readinessProbe
  - Issues: bitnami/charts#21957
  - Solution: use custom livenessProbe and readinessProbe instead

Lack of ARM64 support in the Bitnami mongodb-sharded container
- Issues:
  - bitnami/charts#7305 (comment)
  - bitnami/containers#40947
- Solution: use the official Mongo 6.0 image instead and set up the cluster via a Job.

---------

Co-authored-by: Youngteac Hong <susukang98@gmail.com>
@sejongk
Copy link
Contributor Author

sejongk commented Jan 25, 2024

Question 1

It seems like yorkie-cluster Helm chart no longer has mongoDB subshart. This means that we now need to install both yorkie-cluster and yorkie-mongodb Helm chart to fully use yorkie cluster. Is there any reason for this separation?

Question 2

It seems like yorkie-mongodb Helm chart performs sharded mongoDB setup by using Job object. Is there any reason for this approach?

I can see that mongodb-sharded Helm chart provides common.initScriptsCM value to initialize mongoDB instances (Nevertheless I also think using Job is the best approach for this kind of operation, I just want to know the reasons for this approach).

Lack of ARM64 support in the Bitnami mongodb-sharded container
Solution: use the official mongo 6.0 image instead and set up the cluster via a Job.

Also, is there any relations between ARM64 support and setting up cluster via Job?

Thanks for your careful review.
Here are my answers for your questions.

Answer 1

After separating MongoDB from yorkie-cluster, there are now two options for users: self-hosted or managed databases (e.g., MongoDB Atlas, AWS DynamoDB). I believe we can focus on supporting self-hosted clusters by solely maintaining yorkie-mongodb.

Answer 2

I initially explored the approach using Bitnami options and tools. Bitnami charts are designed to be used with Bitnami containers together. However, issues arose, including the lack of ARM64 images in the Bitnami mongodb-sharded container. Instead of incurring the current and potential future engineering costs associated with external tools, I opted to implement and maintain the code ourselves.
I believe the current requirements for this are relatively simple and manageable. If future requirements become more complex and there is a good alternative, that would be the time to consider such an option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants