Add Cluster Mode Enabled (CME) testing support - #12
Conversation
Signed-off-by: Tracy <yuningt@amazon.com>
Signed-off-by: Tracy <yuningt@amazon.com>
zackcam
left a comment
zackcam
left a comment
There was a problem hiding this comment.
Great start! One other thing could we update the readme to explain how to use this as well
| def start(self, connect_client=True): | ||
| super(ClusterNodeHandle, self).start(connect_client=connect_client) | ||
| if connect_client: | ||
| self._set_node_id() |
There was a problem hiding this comment.
I think we call this twice, we can most likely remove this call.
| def knows(): | ||
| nodesInfo = self.client.cluster("NODES") | ||
| for key in nodesInfo: | ||
| if re.match(nodeid, nodesInfo[key]["node_id"]): |
There was a problem hiding this comment.
Does the nodeid differ here, do we need to do a regex match or can we just do an equals? This works but equals might be cleaner
There was a problem hiding this comment.
Done! Use == instead of re.match
| ) | ||
| return | ||
|
|
||
| def start(self, connect_client=True): |
There was a problem hiding this comment.
Do we need the wait_for_ping=True parameter here? We use the start all nodes but might want compatibility incase some people are using the wait_for_ping
There was a problem hiding this comment.
Forward wait_for_ping / connect_client args in start_all_node in commit 2e09c8a
| timeout=TEST_MAX_WAIT_TIME_SECONDS, | ||
| ) | ||
|
|
||
| def wait_for_cluster_know_node(self, nodeid): |
There was a problem hiding this comment.
Nit: Typo should be known I think
|
|
||
| def get_cluster_client(self): | ||
| """Return a cluster-aware client that follows MOVED/ASK redirections.""" | ||
| from valkey.cluster import ValkeyCluster |
There was a problem hiding this comment.
Should we import this at the top?
| self.nodes[i].client.readonly() | ||
| except Exception: | ||
| logging.warning( | ||
| "READONLY failed on replica port {}".format(self.nodes[i].port) |
There was a problem hiding this comment.
What do we need the try catch here for?
Signed-off-by: Tracy <yuningt@amazon.com>
zackcam
left a comment
zackcam
left a comment
There was a problem hiding this comment.
Looks good to me, can we get a quick update of the README
Signed-off-by: Tracy <yuningt@amazon.com>
2 participants
This PR implements CME (Cluster Mode Enabled) testing support, addressing issue #3.
What's added
Three classes in
src/valkey_test_case.py:ClusterInfo— exposes typed accessors over aCLUSTER INFOreply (is_cluster_ok,cluster_known_nodes,cluster_slots_assigned).ClusterNodeHandle— a single cluster-mode node; wraps the standardCLUSTERcommands (MEET/REPLICATE/ADDSLOTSRANGE) and the waiters for the cluster to converge.ClusterTestCase— orchestrates bootstrap.setup_cluster(num_shards, num_replicas_per_shard)starts the nodes, gossips them together, evenly distributes the 16384 hash slots across primaries, optionally attaches replicas, waits forcluster_state:ok, and returns a topology-awareValkeyClusterclient.Cluster formation is done purely through standard
CLUSTERcommands, and it reuses the framework's existing port reservation, server lifecycle, andnodes.confcleanup.Tests
tests/test_cluster_mode.py:CLUSTER SLOTScovers 0..16383 exactly once, no overlap.