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

Memgraph platform #13

Closed
wants to merge 14 commits into from
Closed

Memgraph platform #13

wants to merge 14 commits into from

Conversation

tewnut
Copy link
Contributor

@tewnut tewnut commented Jan 24, 2024

  • Add memgraph-platform chart.
  • Upgrade memgraph chart default version to 2.13.0

@antejavor
Copy link
Collaborator

antejavor commented Jan 24, 2024

Hi @tewnut, thanks a lot for opening a PR 🚀 . I will review this in the next few days!

| `autoscaling.targetCPUUtilizationPercentage` | Target CPU utilization percentage for autoscaling | `80` |
| `volumes` | Additional volumes on the output Deployment definition | `[]` |
| `volumeMounts` | Additional volumeMounts on the output Deployment definition | `[]` |
| `nodeSelector` | Node selector for pods | `{}` |

Choose a reason for hiding this comment

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

nodeSelector, tolerations and affinity doesn't seem to be used in the chart.

@CarlEkerot
Copy link

Been using this somewhat successfully the last week or so. I am not knowledgeable enough in Helm charts to give a proper review, but here are some pointers as a user:

  • Deploying this from IaC, it would be nice to (optionally) provide existing volume claims for storage
  • I didn't manage to get the ingress to work properly. Haven't spent too much time debugging the issue.
  • See review comment regarding nodeSelector, tolerations and affinity

@karmenrabar
Copy link

Hello @CarlEkerot, thanks so much for the review! @antejavor is currently on his AL, but he'll get back to you as soon as he's back. I'm glad to hear that it's going successfully for you. Could you tell us what you are working on? :)

@CarlEkerot
Copy link

Hello @CarlEkerot, thanks so much for the review! @antejavor is currently on his AL, but he'll get back to you as soon as he's back. I'm glad to hear that it's going successfully for you. Could you tell us what you are working on? :)

Mostly poking around with memgraph and a few other graph DBs :) Looking forward to using this chart once it's in a stable state 👍

@karmenrabar
Copy link

Nice to hear that you're exploring Memgraph, @CarlEkerot :) Feel free to ask if you have more questions, and don't hesitate to reach out on our discord or book an office hours call !

@tewnut
Copy link
Contributor Author

tewnut commented Feb 14, 2024

Been using this somewhat successfully the last week or so. I am not knowledgeable enough in Helm charts to give a proper review, but here are some pointers as a user:

  • Deploying this from IaC, it would be nice to (optionally) provide existing volume claims for storage
  • I didn't manage to get the ingress to work properly. Haven't spent too much time debugging the issue.
  • See review comment regarding nodeSelector, tolerations and affinity

#1. Existing chart included PVC. Do you mean we should be able to specify the existing volume, instead of naming the volume in this format {{ include "memgraph-platform.fullname" . }}-lib-storage? I have used this intentionally to be consistent with the memgraph chart in this same repo.
#2. Can you share the ingress section of your values.yaml file?

@katarinasupe
Copy link
Collaborator

Hi @tewnut, thank you so much for providing the chart for the Memgraph Platform. With the Memgraph Lab image being available on the Docker hub now, we have been working on a new way of running the Memgraph Platform that should ease the deployment process, and that's why we still need to review this. With Memgraph 2.15, we will make these changes public, and then we'll see what to do with Helm Charts (how to approach the Memgraph Platform deployment properly).

@CarlEkerot thank you for the review as well! 👏 Stay tuned for the updates 😄

@tewnut
Copy link
Contributor Author

tewnut commented Feb 24, 2024

Hi @tewnut, thank you so much for providing the chart for the Memgraph Platform. With the Memgraph Lab image being available on the Docker hub now, we have been working on a new way of running the Memgraph Platform that should ease the deployment process, and that's why we still need to review this. With Memgraph 2.15, we will make these changes public, and then we'll see what to do with Helm Charts (how to approach the Memgraph Platform deployment properly).

@CarlEkerot thank you for the review as well! 👏 Stay tuned for the updates 😄

Thanks, looking forward to the new release...

@antejavor antejavor self-assigned this Apr 22, 2024
@antejavor antejavor modified the milestones: v0.1.2, v0.1.3. Apr 22, 2024
Copy link
Collaborator

@antejavor antejavor left a comment

Choose a reason for hiding this comment

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

Hi @tewnut,

First of all, thanks for the contribution and effort. 🚀

As @katarinasupe has said previously and
as you can read in release notes, we have changed how Memgraph platform works.

Now, it is a Docker compose (based on the Memgraph mage and Memgraph lab. Those are two separate containers now; there is no longer a single container image that has both products.

Unfortunately, this means this chart is not valid. Approximately three months ago, when you opened PR, we started planning the transition to the new platform. Due to upcoming changes and other priorities, this PR didn't receive proper attention.

As you can see in our roadmap, we started working on improving our Kubernetes experience, and this repository will receive more changes and updates in the following weeks.

One of the updates is to create the helm charts for replication and high-availability support.

This will be my main focus, and having a platform chart would be a great addition to this repository.

The question is, are you willing to adopt this chart to base the deployment on the two images (mage and lab) so we can wrap up and merge this PR?

@tewnut
Copy link
Contributor Author

tewnut commented Apr 25, 2024

Yeah, I think this is altogether a better approach. Thanks for the update.

Let me try creating the charts for my k8s cluster. When things are in good shape, I will create a PR in this repo, should be a couple of days...

@antejavor
Copy link
Collaborator

Yeah, I think this is a better approach altogether. Thanks for the update.

Let me try creating the charts for my k8s cluster. When things are in good shape, I will create a PR in this repo, should be a couple of days...

Keep in mind we updated the contribution guide a bit: c1cc606

If you are going to create a new PR, please use the latest release branch and create a PR on a specific release branch.

If you are going to use this PR it is also fine.

@antejavor
Copy link
Collaborator

antejavor commented May 3, 2024

Hi @tewnut can we close this PR now? Since you have open another for Memgraph Lab.

@tewnut tewnut closed this May 4, 2024
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.

5 participants