[BUG]: ValkeyNode with unparseable topology labels is never cleaned up and wedges the cluster in Reconciling

[jdheyburn]

Pre-flight checks

• I have searched existing issues and discussions and could not find a duplicate.
• I am running a recent version of the operator (latest release or main) and the issue still reproduces.

Bug Description

A ValkeyNode that carries a cluster's valkey.io/cluster label but has an unparseable valkey.io/shard-index (or valkey.io/node-index) label is never cleaned up, and its presence wedges the owning ValkeyCluster in Reconciling indefinitely. The cluster only recovers when the node is deleted by hand.

Two pieces of code interact:

• The cluster controller lists its nodes by the valkey.io/cluster label alone, so the malformed node is included in every reconcile.
• deleteExcessValkeyNodes calls strconv.Atoi on the topology labels and continues on error, so the node is never considered excess and never deleted.

The scale-in path does count it as an extra node, so every reconcile requeues with Ready=False/Reconciling "Scaling in cluster" / Progressing=RebalancingSlots and the healthy path is never reached again.

Found while validating #400 (prompted by the Greptile comment there). The false Degraded/ACLApplyFailed that comment predicts does not reproduce — the node controller rewrites the condition to Applied within seconds, and the wedge described here keeps the cluster off the healthy path where the aggregation runs. The wedge itself is pre-existing: none of the implicated paths (deleteExcessValkeyNodes, handleScaleIn, the node listing) are touched by #400.

Steps to Reproduce

1. Create a 1-shard, 0-replica cluster and wait for Ready/ClusterHealthy.

2. Create a ValkeyNode labelled for the cluster but with a malformed shard index:

   apiVersion: valkey.io/v1alpha1
   kind: ValkeyNode
   metadata:
     name: aclfail-bogus
     namespace: default
     labels:
       valkey.io/cluster: aclfail
       valkey.io/node-index: "99"
       valkey.io/shard-index: not-an-integer
   spec:
     workloadType: StatefulSet

3. Watch the ValkeyCluster status.

Expected Behaviour

Either the operator deletes the out-of-topology node (it is labelled as owned by the cluster and is not part of the desired topology), or it excludes it from reconciliation and reports something actionable. In no case should the cluster stay in Reconciling forever with no event or condition naming the offending node.

Actual Behaviour

Sampled every 5s for 2 minutes, one state throughout, no convergence:

15:42:16  state=Reconciling/RebalancingSlots  Degraded=absent
   ... identical until observation stopped at 15:44:14

Ready=False/Reconciling msg=Scaling in cluster
Progressing=True/RebalancingSlots msg=Rebalancing slots for scale-in
ClusterFormed=True/TopologyComplete
SlotsAssigned=True/AllSlotsAssigned

The valkeynode controller meanwhile reconciles the bogus CR as a normal node: it creates valkey-aclfail-bogus-0 (which comes up as a standalone, non-cluster instance) and loops on it:

ERROR  failed to create valkey client  {"ValkeyNode": {"name":"aclfail-bogus"}, "error": "WRONGPASS invalid username-password pair or user is disabled."}
ERROR  command failed: CLUSTER MYID    {"ValkeyCluster": {"name":"aclfail"}, "error": "This instance has cluster support disabled"}

kubectl delete valkeynode aclfail-bogus returns the cluster to Ready/ClusterHealthy immediately.

Operator version

main (reproduced on the #400 branch @ b9914dd; the implicated code is unchanged from main @ 7a09b34)

Kubernetes version

v1.36.1

Kubernetes distribution / environment

kind

Additional context

A shared "in desired topology" predicate — parseable non-negative indices, shard < spec.shards, node < 1 + spec.replicas — used by both cleanup and any status aggregation would close this class of problem: malformed nodes either get deleted (they carry the cluster's ownership label) or are excluded everywhere consistently. Deletion needs an ownership-safety check so the operator never removes a CR a user created independently that merely reuses the label.

Low severity in practice: the operator never creates such labels itself, so this needs a hand-crafted or corrupted CR. The bad part is the failure mode — a permanent, silent wedge with no event, condition, or log line pointing at the node.

[melancholictheory]

Reproduced this on k3d with your exact manifest. The wedge is as you describe, Ready=False/Reconciling "Scaling in cluster" with nothing naming the offending node. Two things the scenario does beyond that.

The malformed node is not inert. The node controller provisions it like any other: valkey-wedge-bogus StatefulSet, valkey-wedge-bogus-0 running 1/1, status.ready=true and a pod IP. So the operator starts an extra Valkey server that belongs to no cluster and keeps it running for as long as the CR exists. So the cost is an orphan pod on top of the stuck status.

The cluster controller then cannot talk to it. Every node reconcile logs failed to create valkey client ... WRONGPASS invalid username-password pair or user is disabled at ERROR, once a minute, indefinitely, with no condition or event pointing at the cause. Someone reading the logs sees a permanent authentication error against a pod they did not create.

It does not join the cluster, which is worth recording since that is the first thing you would fear. cluster_known_nodes stayed at 1 throughout. The reason is that a hand-made node has no ACL wiring, so the operator's credentials are refused, getNodeState returns nothing, and the node never reaches PendingNodes where meetIsolatedNodes would CLUSTER MEET it.

That last part is incidental rather than designed, and it points at the case worth checking. A malformed node that does carry working credentials, for example one left behind by an older operator version and still wired to the cluster's own secrets, would be dialled successfully and would land in PendingNodes. The auth failure is the only thing standing between this scenario and a foreign server being met into the cluster.

Worth folding into the in-scope predicate you sketched: whatever decides which nodes count should gate the dial list in getValkeyClusterState too, not only cleanup and scale-in.