fix: let a standalone ValkeyNode start

[Alfex4936]

This PR closes #438

Summary

A ValkeyNode created without a ValkeyCluster never starts. Its container asks for
internal--system-passwords, a Secret that cannot exist, and the pod sits in
CreateContainerConfigError.

I ran into this while checking whether a single non-cluster node was possible today. I applied a
bare ValkeyNode, the StatefulSet came up fine, and then the pod just never went Ready:

Error: secret "internal--system-passwords" not found

The double dash is the giveaway. buildContainersDef wires PRIMARY_AUTH to
getSystemPasswordSecretName(node.Labels[LabelCluster]), and with no cluster label that name is built
from an empty string.

What made me think this was a bug and not intended is that the code around it already handles this
case. operatorUserPasswordSecret returns nil when the cluster name is empty, with the comment
"this can be empty only when valkeynode is created independently without ValkeyCluster CR". The
exporter sidecar is gated on an explicit enabled: true for the same reason, "so a standalone node
must not gain one by default". And ensureConfigMap builds the node its own ConfigMap when
ServerConfigMapName is empty, rendered by generateValkeyNodeConfig, which deliberately leaves
cluster-enabled off. So a cluster-less node is provided for in three places. PRIMARY_AUTH looks
like the one site that was missed.

Features / Behaviour Changes

For a node with no valkey.io/cluster label, --primaryuser, --primaryauth and the PRIMARY_AUTH
env are no longer set. Such a node has no primary to replicate from, so the flags had nothing to do
anyway.

Cluster-owned nodes are unchanged.

Implementation

Pulled the replication flags into replicationArgsAndEnv, a sibling of the existing
valkeyAnnounceArgsAndEnv, returning (nil, nil) when the cluster label is absent. That keeps
buildContainersDef readable, since the args and env were being appended inline.

Limitations

This only makes a bare ValkeyNode start. It is still a single non-cluster server with no Service,
no replica and no failover, which is what the Valkey CRD in #388 / #390 is for. I expect whatever
controller backs that CRD will create ValkeyNodes with no cluster label, so it lands on this path
too.

Testing

make test and make lint pass.

TestBuildValkeyNodePodTemplateSpec was asserting the broken shape: its fixture sets no labels, so it
was a standalone node expecting --primaryauth. Updated it to the standalone expectation, and added
TestBuildValkeyNodePodTemplateSpec_ClusterNodeHasReplicationAuth for the other branch, so both sides
of the guard are covered.

Also ran both sides for real, on Kubernetes 1.35, with the manager built from main (3ec092d) and
its cache restricted to one namespace with --watch-namespace.

Before, applying the bare ValkeyNode from #438:

NAME              READY   STATUS                       RESTARTS   AGE
valkey-mynode-0   0/1     CreateContainerConfigError   0          25s

Warning  Failed  pod/valkey-mynode-0  Error: secret "internal--system-passwords" not found

After, with this patch (the existing pod has to be deleted once, since the StatefulSet rollout is
blocked behind a pod that will never go Ready):

NAME              READY   STATUS    RESTARTS   AGE
valkey-mynode-0   1/1     Running   0          13s

$ kubectl get valkeynode mynode -o jsonpath='{range .status.conditions[*]}{.type}={.status}/{.reason} {end}'
Ready=True/PodRunning ACLApplied=True/Applied

The node reports cluster_enabled:0, and the operations cluster mode rejects all work:

Operation	ValkeyCluster, any shard count	standalone ValkeyNode
MGET across slots	CROSSSLOT	1 2
MSET across slots	CROSSSLOT	OK
multi-key DEL	CROSSSLOT	2
SUNION across slots	CROSSSLOT	a b
MULTI/EXEC spanning keys	CROSSSLOT	OK OK
SELECT 1	ERR DB index is out of range	OK

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)

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: e3aa8552-68a7-4625-80a6-716af95f72f0

📥 Commits

Reviewing files that changed from the base of the PR and between 9564ac8 and 41ad0c6.

📒 Files selected for processing (2)

• internal/controller/valkeynode_resources.go
• internal/controller/valkeynode_resources_test.go

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.

---

📝 Walkthrough

Walkthrough

Changes

The container resource builder now adds --primaryuser, --primaryauth, and PRIMARY_AUTH only when a ValkeyNode has a LabelCluster label. Tests cover standalone nodes without replication authentication and cluster nodes with the expected Secret reference.

Replication authentication

Layer / File(s)	Summary
Conditional replication authentication internal/controller/valkeynode_resources.go, internal/controller/valkeynode_resources_test.go	The new helper returns replication arguments and environment data for cluster nodes and no data for standalone nodes. Container construction appends the returned values. Tests validate both node types.

Suggested reviewers: jdheyburn

Priority: ➖ Normal

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 41ad0

Standalone nodes no longer reference cluster credentials, while cluster-owned nodes retain replication authentication. No actionable merge risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main change: allowing a standalone ValkeyNode to start.
Description check	✅ Passed	The description is complete and directly related to the change. It covers the issue, behavior changes, implementation, limitations, testing, and checklist. The documentation and pre-commit checklist i…
Linked Issues check	✅ Passed	Issue #438 requires standalone ValkeyNode resources to start without the cluster-owned Secret and without replication configuration. The new replicationArgsAndEnv helper returns no flags or environm…
Out of Scope Changes check	✅ Passed	The pull request changes only server-container replication argument and environment construction, plus focused tests for standalone and cluster-owned ValkeyNode resources. These changes directly imple…

• Fix all pre-merge checks with AI

Warning

Some tools did not complete. Review the errors below.

🔧 golangci-lint (2.13.2)

Error: build linters: plugin(logcheck): plugin "logcheck" not found
The command is terminated due to an error: build linters: plugin(logcheck): plugin "logcheck" not found

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Greptile Summary

Summary

• Standalone ValkeyNodes now omit the cluster-managed replication credential reference, allowing them to start without a ValkeyCluster system-password Secret.
• Cluster-owned ValkeyNodes continue to include the replication credentials required for authenticated replication.
• The focused controller behavior check passed for both node types. No issues were found, and the change is safe to merge.

Confidence Score: 5/5

Safe to merge: the changed standalone and cluster-owned ValkeyNode construction paths behave as intended.

No final findings were identified. The focused controller check confirmed that standalone nodes omit replication credentials while cluster-owned nodes retain the expected flags and Secret reference.

Files Needing Attention: None.

T-Rex Logs

What T-Rex did

• Executed the valkeynode replication-auth evidence script in the PR workspace and captured the complete observed output in valkeynode-replication-auth-02-after.log, with exit code 0.
• Compared the standalone node and the cluster-owned node results and confirmed that the standalone node had no replication authentication, while the cluster-owned node retained the expected flags and a system-password Secret reference.
• Validated that this test run did not alter product code and used only the existing PR workspace sources and outputs.
• Created artifacts documenting the run, including the valkeynode-replication-auth-evidence.sh script and the accompanying log, to enable review of the exact commands and the observed results.

_{Ran code and verified through T-Rex}

_{Reviews (1): Last reviewed commit: "fix: let a standalone ValkeyNode start" | Re-trigger Greptile}

[deepakpunjabi]

@Alfex4936 ValkeyNode is meant for internal use and should not ideally be used by end users. Would your use-case be served by upcomingValkey CR instead?

[Alfex4936]

Yeah, Valkey CR is what I want. #387's motivation section describes my case pretty much exactly (single keyspace, multi-key ops and transactions, SELECT). Not asking for ValkeyNode to be user facing.

One question though, since it decides whether this PR is useful to you at all: will a ValkeyNode created by a Valkey carry valkey.io/cluster?

#387 says data pods reuse ValkeyNode unchanged, and the topology sketch has Valkey "auth-cache" owning ValkeyNode auth-cache-0..2 plus Secret internal-auth-cache-system-passwords with _replication in it. But buildContainersDef can only get that Secret's name from node.Labels["valkey.io/cluster"]. If the Valkey controller stamps that label then this is a no-op for you and I'll close it. If it doesn't, the name has to come from somewhere owner agnostic, and replicationArgsAndEnv is where that would go.

Also, separate from all that: TestBuildValkeyNodePodTemplateSpec's fixture sets no labels, so it's a standalone node, and it currently asserts --primaryauth with a Secret name built from "". Probably worth fixing either way.

Happy to cut this down to just the test change, or close it, whichever you prefer.