[mattlord]

Idle=True lets the keyspace controller turn a shard down, so it must only be set once every cell has been observed. Both ways of computing it enumerated cells with GetCellInfoNames, which lists the global cells/ directory. A cell's CellInfo is its only entry there, so a lost CellInfo dropped the cell from the listing entirely and a shard still serving in that cell could be reported idle. With the shard record present this went through topo.Server.GetShardServingCells, which additionally counted a listed cell without a CellInfo as not serving; without the record, #831's shardServingCells had the same blind spot for unlisted cells.

Both paths now use shardServingCells, which also checks every cell in the VitessCluster (the shard's ZoneMap), so a configured cell whose CellInfo can't be read leaves Idle Unknown instead of being skipped.

Fixes #863

[Copilot]

🟢 Approval recommended

The focused change prevents unsafe idle decisions and includes regression coverage for both affected paths, with no unresolved findings.

No open issues 🎉

What changed in this PR

Fixes #863 by checking configured cluster cells before declaring a shard idle, preventing unsafe turndown when a serving cell’s CellInfo is missing.

Changes:

• Uses shardServingCells for both present and missing shard records.
• Checks the union of topology-listed cells and ZoneMap cells, leaving Idle=Unknown when a cell cannot be observed.
• Adds regression tests for lost CellInfo records on both reconciliation paths.

File	Description
pkg/​controller/​vitessshard/​reconcile_topo.go	Unifies serving-cell checks and includes configured cluster cells.
pkg/​controller/​vitessshard/​reconcile_topo_test.go	Tests missing, unlisted CellInfo records with and without a shard record.

_{Review effort: Balanced}

[mattlord]

Closing this, since it trades a very unlikely failure for a more likely one.

The case it guards against is narrow. A cell has to lose its CellInfo while it still serves the shard, which takes a forced DeleteCellInfo (the operator never forces it) or a lost etcd key. With the default registerCells: true, the cluster controller recreates a missing CellInfo for any cell in the spec on its next reconcile. By then the cell is already broken: no new vtgate or vttablet there can find its local topo.

The fix also adds a new failure under updateStrategy: External. ZoneMap only reaches a VitessShard through a released rollout (updateVitessShardInPlace doesn't copy it). In a routine cell removal (remove the cell's tablet pools and release, then remove the cell), the VitessCell becomes idle and PruneCells deletes the cell's CellInfo. The VitessShards still list the cell until that rollout is released. With this change, every shard in the cluster would then report Idle=Unknown, which blocks all shard turndowns including reshard cleanups. They would also skip pruneShardCells, and emit a TopoGetFailed event and requeue every 5 seconds, until someone releases the rollout. It fails safe, but it's a visible regression in a routine operation, to protect against a fault that needs a topo problem plus a mistaken spec change.

A narrower version that requires only the cells where the shard has tablet pools (vts.Spec.GetCells()) seems to avoid that window. But it doesn't cover vtgate-only cells, and its safety depends on how three controllers order their updates. That doesn't seem worth it for a case this rare. Details are in #863, which stays open as a known, low-priority gap.