From a0dd7dae9b7340b8dfc10f0234596ae6b2d2b3ff Mon Sep 17 00:00:00 2001
From: Feng Ruohang <rh@vonng.com>
Date: Tue, 15 Sep 2026 22:16:12 +0800
Subject: [PATCH] fix(iam): resume site healing after leadership changes

Keep one healing loop per process across replication configuration reloads. Reacquire leadership after a lease is canceled and allow shutdown while waiting, so temporary quorum loss cannot permanently stop revocation propagation.

Signed-off-by: Feng Ruohang <rh@vonng.com>
---
 cmd/iam-healing-leadership_test.go       | 75 ++++++++++++++++++++++++
 cmd/site-replication.go                  | 23 +++++++-
 docs/site-replication/iam-revocations.md |  6 ++
 3 files changed, 101 insertions(+), 3 deletions(-)
 create mode 100644 cmd/iam-healing-leadership_test.go

diff --git a/cmd/iam-healing-leadership_test.go b/cmd/iam-healing-leadership_test.go
new file mode 100644
index 000000000..5c30010f2
--- /dev/null
+++ b/cmd/iam-healing-leadership_test.go
@@ -0,0 +1,75 @@
+// Copyright (c) 2026 PGSTY
+// SPDX-License-Identifier: AGPL-3.0-only
+
+package cmd
+
+import (
+	"context"
+	"testing"
+	"time"
+)
+
+func TestIAMHealingResumesAfterLeadershipLoss(t *testing.T) {
+	previous := globalLeaderLock
+	locks := make(chan LockContext)
+	globalLeaderLock = &sharedLock{lockContext: locks}
+	ctx, cancel := context.WithCancel(context.Background())
+	done := make(chan struct{})
+	c := &SiteReplicationSys{}
+	go func() { c.startHealRoutine(ctx, nil); close(done) }()
+	t.Cleanup(func() {
+		cancel()
+		select {
+		case <-done:
+		case locks <- LockContext{ctx: ctx}:
+		}
+		<-done
+		globalLeaderLock = previous
+	})
+	first, loseFirst := context.WithCancel(ctx)
+	defer loseFirst()
+	select {
+	case locks <- LockContext{ctx: first}:
+	case <-time.After(time.Second):
+		t.Fatal("healer did not acquire its first leader context")
+	}
+	loseFirst() // A transient quorum loss cancels the distributed lease.
+	select {
+	case <-done:
+		t.Fatal("healer permanently exited after temporary leadership loss")
+	case locks <- LockContext{ctx: ctx}:
+	case <-time.After(time.Second):
+		t.Fatal("healer did not wait for reacquired leadership")
+	}
+	// Shutdown must also interrupt the wait for leadership after lease loss.
+	cancel()
+	select {
+	case <-done:
+	case <-time.After(time.Second):
+		t.Fatal("healer did not stop with its owning context")
+	}
+}
+
+func TestIAMHealingLeadershipWaitCancels(t *testing.T) {
+	previous := globalLeaderLock
+	locks := make(chan LockContext)
+	globalLeaderLock = &sharedLock{lockContext: locks}
+	ctx, cancel := context.WithCancel(context.Background())
+	done := make(chan struct{})
+	c := &SiteReplicationSys{}
+	go func() { c.startHealRoutine(ctx, nil); close(done) }()
+	cancel()
+	t.Cleanup(func() {
+		select {
+		case <-done:
+		case locks <- LockContext{ctx: ctx}:
+		}
+		<-done
+		globalLeaderLock = previous
+	})
+	select {
+	case <-done:
+	case <-time.After(time.Second):
+		t.Fatal("healer ignored shutdown while waiting for leadership")
+	}
+}
diff --git a/cmd/site-replication.go b/cmd/site-replication.go
index 6592dd14e..6c82fa52b 100644
--- a/cmd/site-replication.go
+++ b/cmd/site-replication.go
@@ -211,6 +211,7 @@ type SiteReplicationSys struct {
 	state srState
 
 	iamMetaCache        srIAMCache
+	healOnce            sync.Once // Configuration reloads must not spawn more healing loops.
 	iamHealMu           sync.Mutex
 	iamRevisionProgress map[string]iamRevisionProgress
 	iamRevisionMetrics  iamRevisionMetrics
@@ -238,7 +239,7 @@ type srStateData struct {
 
 // Init - initialize the site replication manager.
 func (c *SiteReplicationSys) Init(ctx context.Context, objAPI ObjectLayer) error {
-	go c.startHealRoutine(ctx, objAPI)
+	c.healOnce.Do(func() { go c.startHealRoutine(ctx, objAPI) })
 	r := rand.New(rand.NewSource(time.Now().UnixNano()))
 	for {
 		err := c.loadFromDisk(ctx, objAPI)
@@ -4577,9 +4578,25 @@ func (c *SiteReplicationSys) PeerStateEditReq(ctx context.Context, arg madmin.SR
 const siteHealTimeInterval = 30 * time.Second
 
 func (c *SiteReplicationSys) startHealRoutine(ctx context.Context, objAPI ObjectLayer) {
-	ctx, cancel := globalLeaderLock.GetLock(ctx)
-	defer cancel()
+	for ctx.Err() == nil {
+		var leadership LockContext
+		select {
+		case <-ctx.Done():
+			return
+		case leadership = <-globalLeaderLock.lockContext:
+		}
+		if leadership.Context().Err() != nil {
+			continue
+		}
+		leaderCtx, cancel := mergeContext(leadership.Context(), ctx)
+		c.healWithLeadership(leaderCtx, objAPI)
+		cancel()
+		// Quorum loss cancels a leadership lease, not the subsystem. Wait
+		// for a new lease so revocations can still reach offline sites.
+	}
+}
 
+func (c *SiteReplicationSys) healWithLeadership(ctx context.Context, objAPI ObjectLayer) {
 	healTimer := time.NewTimer(siteHealTimeInterval)
 	defer healTimer.Stop()
 
diff --git a/docs/site-replication/iam-revocations.md b/docs/site-replication/iam-revocations.md
index 6908d88b8..d2ee4303c 100644
--- a/docs/site-replication/iam-revocations.md
+++ b/docs/site-replication/iam-revocations.md
@@ -114,6 +114,12 @@ cache.
 
 ## Healing, failures and operational cost
 
+Each process starts one healing loop. Losing its distributed leadership lease
+pauses work until leadership is reacquired; it does not permanently terminate
+healing. Configuration reloads do not create extra loops. The 30-second interval
+starts after leadership is acquired and after each completed pass. Initial lock
+retries and endpoint recovery can add further delay; it is not a convergence SLA.
+
 The normal IAM loaders maintain an in-memory index of deletion records and
 retained boundaries, without secrets. Healing uses this index; it does not add a
 second full walk of `config/iam/` every cycle. Existing full IAM loading still