fix: enable cluster shard/node switching and improve dropdown usability#253
fix: enable cluster shard/node switching and improve dropdown usability#253alexey-temnikov wants to merge 1 commit intovalkey-io:mainfrom
Conversation
- Fix cluster node switching in header dropdown - Fix RequireConnection guard for cluster routes - Fix server-side client resolution for non-entry cluster nodes - Fix credential passing for empty username (PR valkey-io#252) - Add ca-certificates to Dockerfile for TLS - Make entire badge clickable for dropdown toggle Signed-off-by: Alexey Temnikov <[email protected]>
| const navigate = useNavigate() | ||
| const { id, clusterId } = useParams<{ id: string; clusterId: string }>() | ||
| const { host, port, username, alias } = useSelector(selectConnectionDetails(id!)) | ||
| const connectionDetails = useSelector(selectConnectionDetails(id!)) ?? {} as Partial<ReturnType<ReturnType<typeof selectConnectionDetails>>> |
There was a problem hiding this comment.
I think, this change is unnecessary, let's keep the original (line 24)
|
|
||
| // For cluster mode, consider connected if any node in the cluster is connected | ||
| const isClusterConnected = clusterId | ||
| ? Object.values(allConnections ?? {}).some( |
There was a problem hiding this comment.
let's move the nullish coalescing operator higher where (line 35) where allConnections is initialized using useSelector
| <li className="flex flex-col gap-1" key={primaryKey}> | ||
| <button className="flex items-center cursor-pointer hover:bg-primary/20" | ||
| disabled={!nodeIsConnected} | ||
| <button className={`flex items-center cursor-pointer hover:bg-primary/20 ${isCurrentNode ? "bg-primary/10" : ""}`} |
There was a problem hiding this comment.
let's use cn util instead of raw templates
| const connections = useSelector(selectConnections) | ||
|
|
||
| // For cluster routes, consider connected if ANY node in the same cluster is connected | ||
| if (clusterId && status !== CONNECTED) { |
There was a problem hiding this comment.
This hook is a convenience util to get connection status by URL params effectively. This change breaks the abstraction boundary — part of the job is done in the selector, another part — in the hook. What effectively this change is doing — is making clusterId an argument for the selector that has only one argument. So the selector now needs to accept two parameters (this is a minimal change surface) or accept an object { id/nodeId, clusterId } (which is a much bigger surface for the change). But in any case, derived state should be still produced in the selector, not hook.
Another option is (also within the selector) is to flatten.filter connections — this way we don't even need clusterId.
| variant="bodySm" | ||
| > | ||
| <Dot className={isConnected ? "text-green-500" : "text-gray-400"} size={45} /> | ||
| <Dot className={effectiveConnected ? "text-green-500" : "text-gray-400"} size={45} /> |
There was a problem hiding this comment.
Doesn't this change the semantic of the Dot? I.e. it used to mean that "you, user, are connected to this node in the cluster" but now it means "you, user, are connected to SOME node in this cluster" — if so, this is not a desired outcome — the users connecting from Electron app will hate it because they cannot even physically connect to all nodes, they actually need to see what instances (of 12 max) they are connected to. Instead of "connected", we can think of it as "have active glide client + ws connections".
2 participants
Summary
Fixes the inability to switch between Valkey cluster shards/nodes when connected to a cluster environment (e.g., ElastiCache with 6 shards, 18 nodes). Also includes credential fix from #252 and a dropdown usability improvement.
Bug Fixes
1. Cluster node switching disabled in header dropdown (
app-header.tsx)Root cause: The dropdown iterated over all cluster nodes discovered via
CLUSTER SLOTS, but checkedallConnections[primaryKey].status === CONNECTEDper node. Since only the entry node getsCONNECTEDstatus in the Redux store, all other shard buttons weredisabled.Fix: Removed per-node disabled check. All nodes in the dropdown are now clickable when any node in the cluster is connected. Added
isCurrentNodehighlighting and cluster-wideeffectiveConnectedcheck for the toggle button.2. Route guard redirecting to
/connecton shard switch (useIsConnected.ts)Root cause:
RequireConnection→useIsConnectedcheckedselectStatus(id)for the specific node in the URL. When navigating to a different shard, that node's status wasNot Connected, causing a redirect.Fix: For cluster routes (when
clusterIdis present), check if ANY node in the same cluster is connected, not just the specific:idnode.3. Server-side client lookup failure for non-entry nodes (
utils.ts, action handlers)Root cause:
clients.get(connectionId)returnedundefinedfor shard nodes that weren't the original entry point, because only the entry node's connectionId is stored in the server'sclientsmap. TheGlideClusterClientcan route to any node, but the lookup failed.Fix: Added
resolveClient()utility that falls back to finding the cluster client viaclusterNodesMapwhen direct lookup fails. Updatedstats.ts,keys.ts,cluster.ts, andcommand.tshandlers to use it.4. Crash when navigating to undiscovered shard (
app-header.tsx)Root cause:
selectConnectionDetails(id!)returnedundefinedfor nodes that had no entry invalkeyConnectionstate (e.g., shard 6 never individually connected), causing destructuring to throw.Fix: Added safe fallback:
?? {}with proper type casting.5. Credential passing for empty username (from #252) (
cluster-node.tsx)Root cause:
primary.username && primary.passwordshort-circuits tofalsewhen username is""(e.g., ElastiCachedefaultuser), dropping both credentials and causing NOAUTH errors.Fix: Gate only on
primary.passwordand default username toprimary.username ?? "".6. Missing CA certificates for TLS (
Dockerfile.app)Fix: Install
ca-certificatesin the production runtime image for TLS connections to ElastiCache.UX Enhancement
7. Dropdown triggered by entire badge, not just chevron icon (
app-header.tsx)Problem: The node switcher dropdown could only be opened by clicking the tiny chevron icon — the node name text was not clickable.
Fix: Moved
onClickhandler to theBadgecomponent so the entire area (text + icon) toggles the dropdown. AddedbadgeRefto the outside-click handler to prevent mousedown/click event conflicts.Files Changed
apps/frontend/src/hooks/useIsConnected.tsapps/frontend/src/components/ui/app-header.tsxapps/frontend/src/components/cluster-topology/cluster-node.tsxapps/server/src/utils.tsresolveClient()helperapps/server/src/actions/stats.tsresolveClientapps/server/src/actions/keys.tsresolveClientapps/server/src/actions/cluster.tsresolveClientapps/server/src/actions/command.tsresolveClientdocker/Dockerfile.appTesting
Tested against a live ElastiCache Valkey cluster (6 shards, 2 replicas per shard, TLS enabled):