ee69caa270
Phase 1 gate step 4 failed: setting Enabled = 0 to take a node out of service returns 422 ClusterEnabledNodeCountMismatch, because DraftValidator.ValidateClusterTopology requires the enabled-node count to equal ServerCluster.NodeCount. On a Warm/Hot pair — every cluster on the rig, and every cluster in the target topology — Enabled can therefore never be the maintenance hatch. The plan called step 4 "the check that makes step 3 acceptable to ship", so Task 3's behaviour change was not shippable as it stood: a node down for maintenance would block every deployment to its cluster. The two rules were each reasonable and contradictory together — the validator reads Enabled as "part of the declared topology", Phase 1 additionally read it as "expect an ack". Split the meanings rather than weaken either rule: Enabled part of the declared topology (validator, untouched) MaintenanceMode expected to participate now (coordinator + reconciler) Rejected alternatives: counting configured rather than enabled nodes (drops the guard against booting a pair into InvalidTopology); downgrading the rule to a warning (weakens a deploy gate for everyone); shipping with no hatch. Sabotage: dropping !n.MaintenanceMode from the coordinator query turns the new test red. It also asserts both nodes remain Enabled, so the fix cannot quietly regress to disabling the row after all. Configuration.Tests 95/95, ControlPlane.Tests 101/101, solution builds clean. Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
223 lines
12 KiB
Markdown
223 lines
12 KiB
Markdown
# Per-Cluster Mesh — Phase 1 plan
|
|
|
|
> **For Claude:** REQUIRED SUB-SKILL: Use superpowers-extended-cc:executing-plans to implement this plan task-by-task.
|
|
|
|
**Parent design:** [`2026-07-21-per-cluster-mesh-design.md`](2026-07-21-per-cluster-mesh-design.md).
|
|
Phases 0a and 0b are done and merged; this is the next phase.
|
|
|
|
**Goal:** Give `ClusterNode` the transport addresses central will need to reach each cluster
|
|
directly, and cut `ConfigPublishCoordinator`'s last dependency on shared cluster membership by
|
|
sourcing its expected-ack set from those rows.
|
|
|
|
**Architecture:** Additive schema change plus one substitution inside the coordinator. Nothing else
|
|
moves; the deploy channel stays on DistributedPubSub until Phase 2.
|
|
|
|
**Status: DONE 2026-07-22** (branch `feat/mesh-phase1`). Live gate:
|
|
[`2026-07-22-mesh-phase1-live-gate.md`](2026-07-22-mesh-phase1-live-gate.md) — **PASSED**, after gate
|
|
step 4 found that the escape hatch this phase depends on did not exist. Three deviations from the
|
|
plan below, all decided before implementing or forced by the gate:
|
|
|
|
1. **Task 3's cluster-scope filtering was dropped** — there is no cluster-scoped deployment to filter
|
|
on. See the gate doc, step 2.
|
|
2. **No per-node role column.** Every `ClusterNode` row is now *defined* to be a driver node.
|
|
3. **Task 7 added:** `ClusterNode.MaintenanceMode`. `Enabled = 0` is rejected by
|
|
`DraftValidator.ValidateClusterTopology` on any 2-node pair, so it could never be the maintenance
|
|
hatch — which the plan called "the check that makes step 3 acceptable to ship".
|
|
|
|
---
|
|
|
|
## Why this is smaller than the design implies
|
|
|
|
The design says "`ClusterNode` gains Akka + gRPC address columns; coordinator sources its
|
|
expected-ack set from the DB", which reads as though the two are one change. They are not, and the
|
|
second does not depend on the first.
|
|
|
|
`ConfigPublishCoordinator.DiscoverDriverNodes` builds `NodeId.Parse($"{host}:{port}")` from
|
|
`Cluster.State.Members`. `ClusterNode.NodeId` is **already in that identifier space** — the rig seeds
|
|
`central-1:4053`, `site-a-1:4053` and so on (`docker-dev/seed/seed-clusters.sql:65`), and its comment
|
|
says so explicitly, because `NodeDeploymentState.NodeId` is FK-bound to `ClusterNode.NodeId` and a
|
|
mismatch throws FK 547 on deploy. The coordinator already writes `NodeDeploymentState` rows keyed by
|
|
the membership-derived id and they already satisfy that FK — which is standing proof the two sets
|
|
agree today.
|
|
|
|
So the coordinator switch is a substitution inside one private method, and the address columns are
|
|
purely forward-looking groundwork for Phase 2's ClusterClient and Phase 5's gRPC dial.
|
|
|
|
## The decision this phase forces
|
|
|
|
**Membership-derived and DB-derived expected-ack sets are not the same set, and the difference is a
|
|
behaviour change on the deploy path.**
|
|
|
|
| Situation | Today (membership) | After (DB rows) |
|
|
|---|---|---|
|
|
| Configured node is running | expected, acks, seals | same |
|
|
| Configured node is **stopped** | not expected — deploy seals **successfully** without it | expected — deploy waits, then **fails at the apply deadline** |
|
|
| Node running but not in `ClusterNode` | expected (and then FK 547 on the state row) | not expected; its ack is discarded |
|
|
|
|
The middle row is the one that matters. Today a deployment can seal green while a node that should
|
|
have received it was down and did not — the operator is told the fleet is deployed when it is not.
|
|
Failing instead is arguably the correct behaviour and is the reason the design moves this to the DB
|
|
(central cannot see cluster membership once the meshes split).
|
|
|
|
But it is a behaviour change, and it needs an operator escape hatch or a node taken down for
|
|
maintenance blocks every deployment. `ClusterNode.Enabled` already exists and is exactly that hatch,
|
|
so the query filters on it: **`Enabled = 1` rows are expected to ack; disabled rows are not.**
|
|
|
|
**Confirmed 2026-07-22 before executing Task 3** — DB-derived with the hatch. (The hatch turned out to be `MaintenanceMode`, not `Enabled`; see Task 7.) If the preference is to keep sealing green past a down
|
|
node, the alternative is to intersect the DB set with current membership, which preserves today's
|
|
behaviour but does not survive the mesh split — it would have to be revisited in Phase 2 anyway.
|
|
|
|
---
|
|
|
|
## Task 1: Address columns on `ClusterNode`
|
|
|
|
**Classification:** small
|
|
**Estimated implement time:** ~4 min
|
|
**Parallelizable with:** none (Task 2 migrates what this declares)
|
|
|
|
**Files:**
|
|
- Modify: `src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Entities/ClusterNode.cs`
|
|
- Modify: the `ClusterNode` entity configuration in `src/Core/ZB.MOM.WW.OtOpcUa.Configuration/` (find the `IEntityTypeConfiguration<ClusterNode>` or the `OnModelCreating` block)
|
|
- Test: `tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/` (mirror an existing entity-mapping test)
|
|
|
|
Add two nullable columns beside the existing `OpcUaPort` / `DashboardPort`:
|
|
|
|
- `int AkkaPort` — default `4053`. The remoting port central will use to build the ClusterClient
|
|
contact point. Non-nullable with a default, because every node has one.
|
|
- `int? GrpcPort` — nullable, no default. The Phase 5 telemetry-stream port. Nullable is deliberate:
|
|
it does not exist yet, and a non-null default would assert a port nothing listens on.
|
|
|
|
Document on each property that these are **central's dial targets**, not the node's own binding
|
|
configuration — the node binds from `Cluster:Port` in its own appsettings, and these columns
|
|
duplicate that value for a reader that cannot see the node's config. That duplication is the reason
|
|
Task 4 exists.
|
|
|
|
**Step 1:** Write a mapping test asserting a round-tripped `ClusterNode` preserves both values and
|
|
that `AkkaPort` defaults to 4053.
|
|
**Step 2:** Run it; expect failure to compile.
|
|
**Step 3:** Add the properties + mapping.
|
|
**Step 4:** Run; expect pass.
|
|
**Step 5:** Commit.
|
|
|
|
## Task 2: EF migration
|
|
|
|
**Classification:** small
|
|
**Estimated implement time:** ~3 min
|
|
**Parallelizable with:** none
|
|
|
|
**Files:**
|
|
- Create: a new migration under the Configuration project's `Migrations/` folder
|
|
|
|
```bash
|
|
dotnet ef migrations add AddClusterNodeTransportPorts \
|
|
--project src/Core/ZB.MOM.WW.OtOpcUa.Configuration
|
|
```
|
|
|
|
Verify the generated `Up` adds `AkkaPort` with `defaultValue: 4053` and `GrpcPort` as nullable, and
|
|
that `Down` drops both. **Check the migration is additive only** — an `AlterColumn` on anything else
|
|
means the model had drifted from the last migration and that drift must be understood before this
|
|
ships, not carried along by it.
|
|
|
|
## Task 3: Coordinator sources the expected-ack set from `ClusterNode`
|
|
|
|
**Classification:** high-risk — this is the deploy path
|
|
**Estimated implement time:** ~5 min
|
|
**Parallelizable with:** none
|
|
|
|
**Files:**
|
|
- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/Coordinators/ConfigPublishCoordinator.cs` (`DiscoverDriverNodes`, ~line 262, and the class doc comment at line 21 which states the membership rule)
|
|
- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/ConfigPublishCoordinatorTests.cs`
|
|
|
|
Replace the `Cluster.State.Members` scan with a query over enabled `ClusterNode` rows. Keep
|
|
`NodeIdComparer` — the case-insensitivity rationale in its comment applies unchanged, and SQL
|
|
collation makes it more relevant, not less.
|
|
|
|
Points to get right:
|
|
|
|
- **Filter on `Enabled`.** See "The decision this phase forces".
|
|
- **Scope to the deployment's cluster if the deployment is cluster-scoped.** `DeploymentArtifact.ResolveClusterScope`
|
|
already defines that scoping; a fleet-wide deployment expects every enabled node, a cluster-scoped
|
|
one expects only that cluster's. Getting this wrong makes every cluster-scoped deploy hang on nodes
|
|
that were never sent it. **Read `ResolveClusterScope` before writing the query.**
|
|
- **The empty-set branch still matters.** `HandleDispatch` seals immediately when the set is empty,
|
|
and its log message says "no driver-role members in cluster" — reword it, because the reason is now
|
|
"no enabled ClusterNode rows", which is a configuration error rather than a topology observation.
|
|
- The DB is read inside `HandleDispatch`, which already opens a context; reuse it rather than opening
|
|
a second.
|
|
|
|
**Tests to add:** enabled-only filtering; cluster-scope filtering; empty set seals with the new
|
|
reason; an ack from a node absent from `ClusterNode` is discarded. **Positive control on each:**
|
|
delete the filter and confirm the test goes red — the existing suite passed throughout the
|
|
membership era, so a test that also passes against the old derivation is testing nothing.
|
|
|
|
## Task 4: Reconcile the duplicated port with the node's own config
|
|
|
|
**Classification:** standard
|
|
**Estimated implement time:** ~4 min
|
|
**Parallelizable with:** none
|
|
|
|
`ClusterNode.AkkaPort` and the node's own `Cluster:Port` are the same fact in two places, and nothing
|
|
makes them agree. A node that binds 4054 while its row says 4053 is unreachable from central in Phase
|
|
2, and the symptom will be a silent absence of acks rather than an error.
|
|
|
|
Options, in preference order:
|
|
|
|
1. **Have the node assert its row on startup** — on join, compare `Cluster:Port` /
|
|
`Cluster:PublicHostname` to its own `ClusterNode` row and log an Error (not a Warning) on
|
|
mismatch. Cheap, no schema change, no write path from driver nodes to the ConfigDb (which Phase 4
|
|
removes anyway — so put this check on an **admin** node, reading the membership it can already see).
|
|
2. Deploy-time validation in `DraftValidator`, alongside the existing gates.
|
|
|
|
Pick one and implement it. Do not skip this task: an unenforced duplicated address is the exact shape
|
|
of the `Modbus`/`ModbusTcp` drift and the `TwinCat`/`Focas` drift already recorded in this repo.
|
|
|
|
## Task 5: Rig seed + docs
|
|
|
|
**Classification:** small
|
|
**Estimated implement time:** ~3 min
|
|
|
|
**Files:**
|
|
- Modify: `docker-dev/seed/seed-clusters.sql` — add `AkkaPort` to the six INSERTs (all 4053) so a
|
|
freshly-seeded rig matches a migrated one. Leave `GrpcPort` null.
|
|
- Modify: `docs/Configuration.md` — document both columns.
|
|
- Modify: `docs/plans/2026-07-21-per-cluster-mesh-design.md` — mark Phase 1 done in §7.
|
|
|
|
## Task 6: Live gate
|
|
|
|
**Classification:** high-risk
|
|
**Estimated implement time:** n/a — operator-driven
|
|
|
|
Offline tests cannot cover the substitution's real risk, which is that the DB set and the live set
|
|
disagree on the rig. Run on `docker-dev`:
|
|
|
|
1. Deploy with all six nodes up → seals green, six `NodeDeploymentState` rows, no FK 547.
|
|
2. Deploy a **cluster-scoped** config → expected set is that cluster's nodes only.
|
|
3. `docker stop` one node, deploy → **fails at the apply deadline** naming the missing node. This is
|
|
the new behaviour; confirm the failure is legible, because an operator will meet it.
|
|
4. Set that node's `Enabled = 0`, deploy again → seals green without it. This proves the escape
|
|
hatch, and is the check that makes step 3 acceptable to ship.
|
|
|
|
Record results in a gate doc beside this plan, per the Phase 1/Phase 2 precedent.
|
|
|
|
## Task 7: `MaintenanceMode` — the escape hatch step 4 proved missing (ADDED 2026-07-22)
|
|
|
|
**Classification:** high-risk — deploy path + schema
|
|
**Why:** gate step 4 returned `422 ClusterEnabledNodeCountMismatch`. `Enabled = 0` on one node of a
|
|
Warm/Hot pair fails `DraftValidator.ValidateClusterTopology` (enabled count must equal `NodeCount`),
|
|
so the hatch Task 3 relies on cannot be used on any 2-node cluster — i.e. every cluster in the target
|
|
topology. Task 3's behaviour change is only shippable with a hatch that works.
|
|
|
|
**Decision:** split the meanings rather than weaken either rule.
|
|
|
|
| Flag | Question | Enforced by |
|
|
|---|---|---|
|
|
| `Enabled` | part of the declared topology? | `DraftValidator` vs `ServerCluster.NodeCount` |
|
|
| `MaintenanceMode` | expected to participate right now? | `ConfigPublishCoordinator`, `ClusterNodeAddressReconciler` |
|
|
|
|
Rejected: counting configured rather than enabled nodes (removes the InvalidTopology guard);
|
|
downgrading the rule to a warning (weakens a deploy gate for everyone); shipping without a hatch.
|
|
|
|
**Done:** column + migration `AddClusterNodeMaintenanceMode`; both consumers filter on
|
|
`Enabled && !MaintenanceMode`; deadline + reconciler messages name the right flag; docs; gate step 4
|
|
re-run green.
|