feat(cluster): report and converge a Raft voter set that disagrees with node roles (#880)

RELEASE_NOTES_2026.09.2.md

@@ -448,6 +448,33 @@ Nothing stopped it afterwards either. The callback that shuts a compaction sched
 
 The check now runs on every tick, which is what the cluster-operations rule in this repository has always required and what the retention, continuous-query, Iceberg and reconciliation schedulers already did. A lease change now takes effect without a restart, which is the other half of the same rule.
 
+### An upgraded cluster could keep a reader or compactor voting in Raft, with nothing reporting it ([#880](https://github.com/Basekick-Labs/arc/issues/880))
+
+Arc now grants a Raft vote only to nodes that can accept writes, so a reader or compactor cannot win leadership and stall every task that runs on exactly one node. That rule applies when a node **joins**. It does not change a vote a node already holds, because adding a server that is already a voter as a non-voter updates its address and leaves its suffrage alone — that is how the underlying Raft library works, deliberately.
+
+Most clusters converge anyway. A node that shuts down gracefully announces it, the leader drops it from the Raft configuration, and its next start is a clean add with the right suffrage; a rolling upgrade does that for most of a cluster as a side effect. What does not converge is a node killed ungracefully, a node whose departure arrived while there was no leader to process it, the departing leader itself, and any node re-added while it was still in the configuration.
+
+So a cluster could sit indefinitely with a reader holding a vote, and there was nothing to look at: the only record of a server's suffrage was inside a diagnostic string in the Raft statistics block.
+
+**`GET /api/v1/cluster` now reports the voter set.** A new `raft.membership` block lists every server with its suffrage and the role the cluster has on record for it; where that role is one Arc recognises, it also carries the suffrage the role calls for and whether the two agree. A server whose role cannot be determined — an entry left by a version that recorded something Arc no longer knows, or one present in the Raft configuration but in no node record — is reported as unresolved, with no verdict attached, and is never acted on. The block also carries counts of voting servers, disagreements and unresolved entries, and names the node it came from, because a follower's view of the Raft configuration can lag the leader's.
+
+When disagreements persist, the leader now says so in the log, once a minute, pointing at the endpoint below.
+
+**`POST /api/v1/cluster/voters/converge` fixes it.** Admin-only. It plans by default: send `{"dry_run": false}` to act. It only ever **revokes** votes, never grants them — granting one raises the number of nodes that must agree before anything can change, and if the newly-voting node is down or behind, nothing can commit, no further membership change is possible, and the cluster loses its leader with no way back. Revoking is safe in the other direction because it is self-repairing: a node that should vote gets its vote back the next time it joins.
+
+Revoking is not automatically safe either, and the endpoint refuses rather than guesses:
+
+- It will not revoke a vote if doing so would leave the remaining voters without a healthy majority. A configuration change takes effect the moment it is made, so removing a live voter from a group whose survivors are down strands the change permanently — it can never be agreed, and the cluster cannot elect a leader again.
+- It will not drop a cluster to a single voter unless you say so explicitly with `{"allow_single_voter": true}`. One voter works until that node dies, after which nothing can ever elect a leader.
+- A server whose role the cluster cannot determine is reported and left alone. Those entries are disproportionately nodes that are already gone, which is exactly what must not be counted on for a majority.
+- If the node you call is itself voting when its role says it should not — which is common on a cluster upgraded from before the rule existed, where every node was given a vote regardless of role — it hands leadership to a node whose role does vote and tells you to re-run against the new leader. Without that step it would revoke everyone else's vote, report success, and leave the cluster in the state you were trying to fix.
+
+If a revocation fails, the endpoint stops there rather than continuing down the list, and reports what it did and what it did not. Carrying on would apply the remaining changes to a voter set that was never checked, which is the one way the safety rules above can be defeated.
+
+An unreadable or absent request body is treated as a dry run rather than rejected: the safe reading of an unclear instruction to change cluster membership is to not change it.
+
+Convergence is deliberately never automatic. Doing it on its own when a node takes leadership was considered and rejected for this release: it is the kind of change that is safest when an operator chooses the moment, can see the plan first, and is watching when it lands.
+
 ### The compactor lease never moved to a dedicated compactor, and there was no way to move it by hand ([#876](https://github.com/Basekick-Labs/arc/issues/876))
 
 The other half of the fix above. A cluster hands the compactor lease to the best candidate it can see when it first assigns one, and prefers a node whose role is `compactor` — but if no such node is visible at that moment it falls back to a writer, and there it stayed. The lease was only ever moved again when its holder became *unhealthy*. A healthy writer kept it for the life of the cluster.

internal/api/cluster.go

@@ -2,6 +2,7 @@ package api
 
 import (
 	"context"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"sort"
@@ -163,6 +164,138 @@ func (h *ClusterHandler) RegisterRoutes(app *fiber.App) {
 		compactorGroup.Use(auth.RequireAdmin(h.authManager))
 	}
 	compactorGroup.Post("/assign", h.handleAssignCompactor)
+
+	// Admin-only: converge the Raft voter set onto the role-based rule
+	// (#880). Named "converge" rather than "reconcile" because
+	// internal/reconciliation is already the manifest-vs-storage reconciler,
+	// and an operator reading a log line should not have to work out which
+	// one fired.
+	votersGroup := app.Group("/api/v1/cluster/voters")
+	if h.authManager != nil {
+		votersGroup.Use(auth.RequireAdmin(h.authManager))
+	}
+	votersGroup.Post("/converge", h.handleConvergeVoters)
+}
+
+// convergeVotersRequest is the body of POST /api/v1/cluster/voters/converge.
+//
+// dry_run is a *bool so that "absent" is distinguishable from "false". Absent
+// means a dry run: this endpoint changes Raft membership, and the failure mode
+// of an unintended demotion is a cluster that cannot elect a leader, which
+// Arc cannot recover from. Acting requires saying so.
+type convergeVotersRequest struct {
+	DryRun           *bool `json:"dry_run"`
+	AllowSingleVoter bool  `json:"allow_single_voter"`
+}
+
+// parseConvergeRequest reads the request body, defaulting to a DRY RUN.
+//
+// Separated from the handler so the default is testable on its own: a handler
+// test with no coordinator refuses before it ever reaches this, so it asserts
+// nothing about the parse — which is how the first version of that test came
+// to pass no matter what this returned.
+//
+// An absent, empty or unparseable body all mean "plan, do not act". This
+// endpoint changes Raft membership, and the failure mode of an unintended
+// demotion is a cluster that cannot elect a leader with no recovery path, so
+// the safe reading of an instruction Arc cannot parse is to not follow it.
+func parseConvergeRequest(body []byte) (dryRun, allowSingleVoter bool) {
+	if len(body) == 0 {
+		return true, false
+	}
+	var req convergeVotersRequest
+	if err := json.Unmarshal(body, &req); err != nil {
+		return true, false
+	}
+	if req.DryRun != nil {
+		dryRun = *req.DryRun
+	} else {
+		dryRun = true
+	}
+	return dryRun, req.AllowSingleVoter
+}
+
+// convergeIsPartial reports whether a converge stopped part-way.
+//
+// A partial result is not a success. Converge stops at the first failed
+// revocation, so a populated Failed map means the voter set is somewhere
+// between where it was and where it was asked to be, and the caller has to
+// look before deciding what to do next.
+func convergeIsPartial(result *cluster.VoterConvergeResult) bool {
+	return result != nil && len(result.Failed) > 0
+}
+
+// handleConvergeVoters demotes Raft servers holding a suffrage the role-based
+// rule would not grant them today.
+//
+// #862 made suffrage follow the role at join time and left existing clusters
+// alone, because AddNonvoter on an existing voter is a no-op on suffrage. This
+// is the operator lever that converges one, and it only ever demotes.
+func (h *ClusterHandler) handleConvergeVoters(c *fiber.Ctx) error {
+	dryRun, allowSingleVoter := parseConvergeRequest(c.Body())
+	req := convergeVotersRequest{AllowSingleVoter: allowSingleVoter}
+	_ = req
+
+	if h.coordinator == nil {
+		return c.Status(fiber.StatusConflict).JSON(fiber.Map{
+			"success": false,
+			"error":   "clustering is not enabled on this node, so there is no Raft voter set to converge",
+		})
+	}
+
+	result, err := h.coordinator.ConvergeVoters(dryRun, allowSingleVoter)
+	if err != nil {
+		if errors.Is(err, cluster.ErrVoterConvergeLeadershipMoved) {
+			// Not a failure in the sense of "nothing happened" — the first
+			// necessary step did happen — but success:false is correct,
+			// because the thing the caller asked for is not finished.
+			return c.Status(fiber.StatusConflict).JSON(fiber.Map{
+				"success":             false,
+				"leadership_moved_to": result.LeadershipMovedTo,
+				"error":               err.Error(),
+				"message":             "this node held a vote its role does not grant, so leadership was moved first; re-run this request against the new leader to finish converging",
+			})
+		}
+		status := fiber.StatusInternalServerError
+		switch {
+		case errors.Is(err, cluster.ErrNotLeaderForTopology),
+			errors.Is(err, cluster.ErrClusterRaftNotConfigured),
+			errors.Is(err, cluster.ErrVoterConvergeUnsafe),
+			errors.Is(err, cluster.ErrVoterConvergeNeedsLeadershipMove):
+			status = fiber.StatusConflict
+		case errors.Is(err, cluster.ErrVoterConvergeNotReady):
+			status = fiber.StatusServiceUnavailable
+		}
+		if status == fiber.StatusInternalServerError {
+			h.logger.Error().Err(err).Msg("Failed to converge the Raft voter set")
+		}
+		return c.Status(status).JSON(fiber.Map{
+			"success": false,
+			"error":   err.Error(),
+		})
+	}
+
+	if !dryRun {
+		ev := h.logger.Info()
+		if len(result.Failed) > 0 {
+			ev = h.logger.Warn()
+		}
+		ev.
+			Strs("demoted", result.Demoted).
+			Strs("skipped", result.Skipped).
+			Int("failed", len(result.Failed)).
+			Int("voting_servers_before", result.VotingServersBefore).
+			Int("voting_servers_after", result.VotingServersAfter).
+			Msg("Raft voter set converge finished")
+	}
+	if convergeIsPartial(result) {
+		return c.Status(fiber.StatusConflict).JSON(fiber.Map{
+			"success": false,
+			"result":  result,
+			"error":   "converge stopped at the first failed demotion; inspect raft.membership and re-run",
+		})
+	}
+	return c.JSON(fiber.Map{"success": true, "result": result})
 }
 
 // assignCompactorRequest is the body of POST /api/v1/cluster/compactor/assign.

internal/api/cluster_converge_voters_test.go

@@ -0,0 +1,156 @@
+package api
+
+import (
+	"encoding/json"
+	"net/http/httptest"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/basekick-labs/arc/internal/auth"
+	"github.com/basekick-labs/arc/internal/cluster"
+	"github.com/gofiber/fiber/v2"
+	"github.com/rs/zerolog"
+)
+
+// #880: the operator lever that converges an existing cluster's Raft voter set
+// onto the role-based rule #862 introduced at join time. Demote-only, dry-run
+// by default.
+
+func convergeTestApp(t *testing.T) *fiber.App {
+	t.Helper()
+	h := NewClusterHandler(nil, nil, nil, zerolog.Nop())
+	app := fiber.New()
+	h.RegisterRoutes(app)
+	return app
+}
+
+func postConverge(t *testing.T, app *fiber.App, body string) *httptest.ResponseRecorder {
+	t.Helper()
+	req := httptest.NewRequest("POST", "/api/v1/cluster/voters/converge", strings.NewReader(body))
+	req.Header.Set("Content-Type", "application/json")
+	resp, err := app.Test(req)
+	if err != nil {
+		t.Fatalf("app.Test: %v", err)
+	}
+	rec := httptest.NewRecorder()
+	rec.Code = resp.StatusCode
+	_, _ = rec.Body.ReadFrom(resp.Body)
+	return rec
+}
+
+func TestConvergeVotersRouteIsRegistered(t *testing.T) {
+	rec := postConverge(t, convergeTestApp(t), `{"dry_run":true}`)
+	if rec.Code == fiber.StatusNotFound || rec.Code == fiber.StatusMethodNotAllowed {
+		t.Fatalf("POST /api/v1/cluster/voters/converge is not routed (status %d)", rec.Code)
+	}
+}
+
+// It must not collide with any existing /api/v1/cluster/* route, and must not
+// be reachable by GET — this changes Raft membership.
+func TestConvergeVotersRejectsGet(t *testing.T) {
+	app := convergeTestApp(t)
+	resp, err := app.Test(httptest.NewRequest("GET", "/api/v1/cluster/voters/converge", nil))
+	if err != nil {
+		t.Fatalf("app.Test: %v", err)
+	}
+	if resp.StatusCode != fiber.StatusMethodNotAllowed && resp.StatusCode != fiber.StatusNotFound {
+		t.Errorf("GET returned %d; this endpoint must not be reachable by GET", resp.StatusCode)
+	}
+}
+
+// Without clustering it must refuse with a non-2xx, not answer 200 with
+// enabled=false the way the read endpoints do.
+func TestConvergeVotersWithoutClustering(t *testing.T) {
+	rec := postConverge(t, convergeTestApp(t), `{"dry_run":true}`)
+	if rec.Code != fiber.StatusConflict {
+		t.Fatalf("status %d, want 409", rec.Code)
+	}
+	var body map[string]any
+	if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil {
+		t.Fatalf("response is not JSON: %v", err)
+	}
+	if ok, _ := body["success"].(bool); ok {
+		t.Error("success=true on a node with no coordinator")
+	}
+}
+
+// The endpoint changes Raft membership, and an unintended demotion can leave a
+// cluster unable to elect a leader with no recovery path. Admin auth is not
+// optional, and nothing else in this file asserts it — every other case builds
+// the handler with a nil auth manager, which disables the middleware.
+func TestConvergeVotersRequiresAuth(t *testing.T) {
+	am, err := auth.NewAuthManager(t.TempDir()+"/auth.db", time.Second, 100, zerolog.Nop())
+	if err != nil {
+		t.Fatalf("NewAuthManager: %v", err)
+	}
+	defer am.Close()
+
+	h := NewClusterHandler(nil, am, nil, zerolog.Nop())
+	app := fiber.New()
+	h.RegisterRoutes(app)
+
+	rec := postConverge(t, app, `{"dry_run":false}`)
+	if rec.Code != fiber.StatusUnauthorized {
+		t.Fatalf("an unauthenticated converge returned %d, want 401", rec.Code)
+	}
+}
+
+// dry_run defaults to TRUE. Tested against the parser directly: a handler
+// test with no coordinator refuses before the parse result is ever used, so it
+// asserts nothing — which is exactly how the first version of this test came
+// to pass regardless of the default.
+func TestConvergeVotersDefaultsToDryRun(t *testing.T) {
+	cases := []struct {
+		name string
+		body string
+		want bool
+	}{
+		{"absent body", "", true},
+		{"empty object", "{}", true},
+		{"malformed", "not json", true},
+		{"only allow_single_voter", `{"allow_single_voter":true}`, true},
+		{"explicit true", `{"dry_run":true}`, true},
+		{"explicit false is the ONLY way to act", `{"dry_run":false}`, false},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			got, _ := parseConvergeRequest([]byte(tc.body))
+			if got != tc.want {
+				t.Errorf("parseConvergeRequest(%q) dry_run = %v, want %v", tc.body, got, tc.want)
+			}
+		})
+	}
+}
+
+// allow_single_voter is carried through, and is false unless asked for.
+func TestConvergeVotersAllowSingleVoterIsOptIn(t *testing.T) {
+	if _, allow := parseConvergeRequest([]byte(`{"dry_run":false}`)); allow {
+		t.Error("allow_single_voter defaulted to true")
+	}
+	if _, allow := parseConvergeRequest([]byte(`{"dry_run":false,"allow_single_voter":true}`)); !allow {
+		t.Error("allow_single_voter was not carried through")
+	}
+	// A malformed body must not smuggle it in either.
+	if _, allow := parseConvergeRequest([]byte(`{"allow_single_voter":true,`)); allow {
+		t.Error("a malformed body enabled allow_single_voter")
+	}
+}
+
+// A converge that stopped part-way must not be reported as a success. It stops
+// at the first failed revocation, so a populated failure map means the voter
+// set is between where it was and where it was asked to be.
+func TestPartialConvergeIsNotASuccess(t *testing.T) {
+	if convergeIsPartial(&cluster.VoterConvergeResult{Demoted: []string{"a"}}) {
+		t.Error("a clean run was reported as partial")
+	}
+	if !convergeIsPartial(&cluster.VoterConvergeResult{
+		Demoted: []string{"a"},
+		Failed:  map[string]string{"b": "no longer the leader"},
+	}) {
+		t.Error("a run that stopped at a failed revocation was reported as a success")
+	}
+	if convergeIsPartial(nil) {
+		t.Error("nil result treated as partial")
+	}
+}