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 support for mounting custom query modules #67

Merged
merged 4 commits into from
Nov 7, 2024

Conversation

matkob
Copy link
Contributor

@matkob matkob commented Oct 28, 2024

Closes #66

# Must be an existing ConfigMap
# - volume: ""
# Must be present in the ConfigMap referenced with `volume`
# file: ""
Copy link
Contributor

Choose a reason for hiding this comment

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

just add empty line here

Copy link
Collaborator

Choose a reason for hiding this comment

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

We need to fix this so the action can be re-run.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh, sorry about that. Added the line

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 @matkob thanks a lot for this contribution. 🚀 Let's just fix the empty space, so we can re-run the test.

If I understood correctly, you would have something like this:

values.yaml

customQueryModules:
  - volume: "module-volume-1"
    file: "module1.py"
  - volume: "module-volume-2"
    file: "module2.py"

Template:

- name: "module-volume-1"
  mountPath: "/var/lib/memgraph/internal_modules/module1.py"
  subPath: "module1.py"
- name: "module-volume-2"
  mountPath: "/var/lib/memgraph/internal_modules/module2.py"
  subPath: "module2.py"

We obviously do not have a clear way of adding this, except maybe user defined volume here.

I am curious if you can comment on what is the benefit of having a volume per QM for you?

Just asking, since I didn't have a chance to use QMs + k8s, so I am curious about the flow and why did you configure it this way.

# Must be an existing ConfigMap
# - volume: ""
# Must be present in the ConfigMap referenced with `volume`
# file: ""
Copy link
Collaborator

Choose a reason for hiding this comment

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

We need to fix this so the action can be re-run.

@matkob
Copy link
Contributor Author

matkob commented Nov 5, 2024

Hi @antejavor, good question! Initially I wanted to create and mount only one ConfigMap with all the custom query modules inside, potentially even stored in multiple files. Unfortunately I bumped into issues related to how Kubernetes mounts the volumes - it adds timestamp to the mountPath and creates a symlink. This would end up in QM not working in memgraph (they would be duplicated and memgraph wouldn't load the code properly).

Mounting each QM separately, using subPath, fixed the issue and the workaround didn't seem "too dirty" to me. Let me know if this seems sound to you, too :)

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.

@matkob cool 🚀 thanks a lot for this.

Let's keep this as a default approach for now, I will take a look at the other options at the future.

@antejavor antejavor merged commit f2ea080 into memgraph:main Nov 7, 2024
2 checks passed
as51340 pushed a commit that referenced this pull request Dec 2, 2024
Mounting each QM via separate volume
---------

Co-authored-by: antejavor <[email protected]>
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.

Injecting custom query modules
3 participants