Skip to content

add modules workaround docs - #214

Merged
jdheyburn merged 5 commits into
valkey-io:mainfrom
hieu2102:add-module-workaround-docs
Jun 6, 2026
Merged

add modules workaround docs#214
jdheyburn merged 5 commits into
valkey-io:mainfrom
hieu2102:add-module-workaround-docs

Conversation

@hieu2102

@hieu2102 hieu2102 commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

This PR closes #213

Summary

Add docs/using-module.md describing a workaround to deploy a ValkeyCluster with module.

Features / Behaviour Changes

  • Add docs/using-module.md
  • Add link to docs/using-module.md from docs/quickstart.md

Implementation

Limitations

Testing

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one issue.
  • Commit message explains what changed and why
  • Tests are added or updated.
  • Documentation files are updated.
  • I have run pre-commit locally (pre-commit run --all-files or hooks on commit)

Signed-off-by: hieu2102 <hieund2102@gmail.com>
@greptile-apps

greptile-apps Bot commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Adds docs/using-modules.md, a workaround guide for deploying a ValkeyCluster with a custom module (valkey-json), and links to it from docs/quickstart.md.

  • The guide walks through building a single-stage custom Valkey image with build tooling baked in, deploying a ValkeyCluster that references it, and verifying the module loaded.
  • The ValkeyCluster YAML example passes --loadmodule /opt/valkey-json/build/src/libjson.so as a single args list entry, which Kubernetes delivers to the Valkey process as one argv element; Valkey's argument parser will not recognize it as a valid directive, so the module will not load.

Confidence Score: 4/5

Documentation-only PR; the args format bug in the ValkeyCluster example would cause readers to produce a non-functional deployment, but no operator code is changed.

The args list in the ValkeyCluster YAML passes --loadmodule /opt/valkey-json/build/src/libjson.so as a single string. Kubernetes delivers each list item as a discrete argv element, so Valkey's argument parser receives the whole --loadmodule /path string as one token and cannot interpret it as a loadmodule directive. Any reader following the guide as written will get a cluster where the module silently never loads.

docs/using-modules.md — the ValkeyCluster YAML example at lines 70-71 needs the args split into two separate list items.

Important Files Changed

Filename Overview
docs/using-modules.md New guide for module usage workaround; contains an args-format bug that would prevent the module from loading, and a missing registry push step.
docs/quickstart.md Adds a correctly-referenced link to the new using-modules guide; no issues.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Developer machine] -->|docker build| B[valkey-customized:1.0 local image]
    B -->|missing step: docker push to registry| C[Container Registry]
    C -->|image pull| D[Kubernetes Nodes]
    A -->|kubectl apply| E[ValkeyCluster CR]
    E -->|operator reconciles| F[ValkeyNode pods]
    F -->|strategic merge patch containers spec| G[server container Command: valkey-server /config/valkey.conf Args: --loadmodule /path/to/libjson.so as ONE string]
    G -->|single argv element not parsed correctly| H[Module fails to load]
    G2[Corrected args: two separate list items] -->|two separate argv elements| H2[Module loads successfully]
Loading

Reviews (4): Last reviewed commit: "update wording" | Re-trigger Greptile

Comment thread docs/quickstart.md Outdated
Comment thread docs/using-modules.md Outdated
Comment thread docs/using-modules.md
Signed-off-by: hieu2102 <hieund2102@gmail.com>
@bjosv

bjosv commented Jun 1, 2026

Copy link
Copy Markdown
Collaborator

CI hit the problem in #215, I trigger a CI rerun since this is only docs

@bjosv bjosv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Very nice!

Comment thread docs/quickstart.md Outdated
Comment thread docs/using-modules.md
hieu2102 added 2 commits June 2, 2026 09:44
Signed-off-by: hieu2102 <hieund2102@gmail.com>
Signed-off-by: hieu2102 <hieund2102@gmail.com>
bjosv
bjosv previously approved these changes Jun 2, 2026
jdheyburn
jdheyburn previously approved these changes Jun 2, 2026

@jdheyburn jdheyburn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, thank you for raising! cc @stockholmux to see if this is what you had in mind from chatting on Friday

@stockholmux stockholmux left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A couple of nits but I think this makes a lot of sense for where we are today.

Comment thread docs/using-modules.md Outdated
Signed-off-by: hieu2102 <hieund2102@gmail.com>
@hieu2102
hieu2102 dismissed stale reviews from jdheyburn and bjosv via 91459bb June 5, 2026 08:41

@jdheyburn jdheyburn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for raising this! I'll get this merged now, we can refine it if we need to.

@jdheyburn
jdheyburn merged commit 69b1b1a into valkey-io:main Jun 6, 2026
8 checks passed
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.

Add documentation for workaround to using modules

4 participants