internal/rest/resources/control: Use independent context for join faiure cleanup by claudiubelu · Pull Request #777 · canonical/microcluster

claudiubelu · 2026-06-29T12:40:05Z

The controlPost reverter used r.Context() for all cleanup operations (PreRemove hook, certificate fetch, client creation).

When a join fails due to context deadline exceeded, r.Context() may already be expired, causing the entire cleanup to silently fail and leaving a ghost node in the dqlite cluster. In particular, shared.GetRemoteCertificate was silently returning due an error returned (due to r.Context() being expired), which means that internalClient.DeleteClusterMember was never called.

Use an independent context with a 2-minute timeout for cleanup operations so they can complete regardless of the original request context state.

Also add error logging to GetRemoteCertificate and client creation failures which previously returned silently.

Copilot

Pull request overview

This PR adjusts controlPost’s failure cleanup logic so that cleanup work isn’t blocked by an already-expired request context (e.g., join timeouts), and adds missing error logs for previously silent cleanup failures.

Changes:

Introduces a dedicated cleanup context with a 2-minute timeout for failure reverter operations.
Uses that cleanup context for pre-remove hook execution, certificate retrieval, and client creation during join cleanup.
Adds error logging when remote certificate fetch or client creation fails during cleanup.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

claudiubelu · 2026-10-20T12:26:27Z

+		// Use an independent context for cleanup so that it is not tied
+		// to the request context, which may already be expired (e.g. the
+		// join timed out).
+		cleanupCtx, cleanupCancel := context.WithTimeout(context.Background(), 2*time.Minute)
+		defer cleanupCancel()


Done, by deriving from the daemon's shutdown context instead.

claudiubelu · 2026-10-20T12:26:48Z

 		if joinInfo == nil || req.JoinToken == "" {
-			reExec, err := resetClusterMember(r.Context(), state, true)
+			reExec, err := resetClusterMember(cleanupCtx, state, true)
 			if err != nil {
 				logger.Error("Failed to reset cluster member on bootstrap error", slog.String("error", err.Error()))
 				return


Done, it uses r.Context() once again.

roosterfish

Thanks, could you please also add a test that triggers this?

roosterfish · 2026-07-01T10:12:24Z

+		// Use an independent context for cleanup so that it is not tied
+		// to the request context, which may already be expired (e.g. the
+		// join timed out).
+		cleanupCtx, cleanupCancel := context.WithTimeout(context.Background(), 2*time.Minute)


Instead of deriving a new context, should we rather use the daemon's shutdownCtx here to be independent of the join request but still account for the health of the daemon when running the hooks?

Changed to cleanupCtx, cleanupCancel := context.WithTimeout(intState.Context, 2*time.Minute)

roosterfish · 2026-07-08T12:41:38Z

Marking as draft until you get a chance to get back to it @claudiubelu.

…tstrap or join When Bootstrap or Join fails, the reverter closes the dqlite app but keeps the accept channel open. The go-dqlite reader ranging over it keeps receiving connections from the remaining cluster members and panics when it cannot dial the closed app's local socket, crashing the daemon before the join failure cleanup can run. Rotate the accept channel before closing the dqlite app, like Restart already does. Signed-off-by: Claudiu Belu <cbelu@cloudbasesolutions.com>

…ffline On a failed join, the cluster member handling the cleanup resets the joiner through PUT /cluster/{name}. That endpoint required an open database, which a joiner whose dqlite join failed never has, so the reset was refused and the joiner could not be initialized again until its daemon was restarted. Signed-off-by: Claudiu Belu <cbelu@cloudbasesolutions.com>

Signed-off-by: Claudiu Belu <cbelu@cloudbasesolutions.com>

claudiubelu

Updated, as required by the comments. Note that this now depends on: #850 . The commits from that PR are contained in this PR, so the tests pass.

claudiubelu · 2026-10-20T12:26:27Z

+		// Use an independent context for cleanup so that it is not tied
+		// to the request context, which may already be expired (e.g. the
+		// join timed out).
+		cleanupCtx, cleanupCancel := context.WithTimeout(context.Background(), 2*time.Minute)
+		defer cleanupCancel()


Done, by deriving from the daemon's shutdown context instead.

claudiubelu · 2026-10-20T12:26:48Z

 		if joinInfo == nil || req.JoinToken == "" {
-			reExec, err := resetClusterMember(r.Context(), state, true)
+			reExec, err := resetClusterMember(cleanupCtx, state, true)
 			if err != nil {
 				logger.Error("Failed to reset cluster member on bootstrap error", slog.String("error", err.Error()))
 				return


Done, it uses r.Context() once again.

…lure cleanup The controlPost reverter used r.Context() for all cleanup operations (PreRemove hook, certificate fetch, client creation). When a join fails due to context deadline exceeded, r.Context() may already be expired, causing the entire cleanup to silently fail and leaving a ghost node in the dqlite cluster. In particular, shared.GetRemoteCertificate was silently returning due an error returned (due to r.Context() being expired), which means that internalClient.DeleteClusterMember was never called. Use an independent context with a 2-minute timeout for cleanup operations so they can complete regardless of the original request context state. Also add error logging to GetRemoteCertificate and client creation failures which previously returned silently. Signed-off-by: Claudiu Belu <cbelu@cloudbasesolutions.com>

For cleanup, we now use daemon's context instead of an independent context. On the `resetClusterMember` call, `r.Context()` is used (only used for the logger and waiting for the request to finish). Signed-off-by: Claudiu Belu <cbelu@cloudbasesolutions.com>

Signed-off-by: Claudiu Belu <cbelu@cloudbasesolutions.com>

roosterfish requested a review from Copilot July 1, 2026 10:06

Copilot started reviewing on behalf of roosterfish July 1, 2026 10:06

View session

Copilot AI reviewed Jul 1, 2026

View reviewed changes

roosterfish requested changes Jul 1, 2026

View reviewed changes

roosterfish marked this pull request as draft July 8, 2026 12:41

claudiubelu added 3 commits October 19, 2026 13:35

example: Add failed join system test

5e5274a

Signed-off-by: Claudiu Belu <cbelu@cloudbasesolutions.com>

claudiubelu force-pushed the v4-new-cleanup-context branch from 92fce9c to dcd1e68 Compare October 20, 2026 11:37

claudiubelu commented Oct 20, 2026

View reviewed changes

claudiubelu added 3 commits October 20, 2026 12:49

example: Adds failed join after client timeout test

b98266a

Signed-off-by: Claudiu Belu <cbelu@cloudbasesolutions.com>

claudiubelu force-pushed the v4-new-cleanup-context branch from dcd1e68 to b98266a Compare October 20, 2026 12:49

Navigation Menu

Repository navigation

internal/rest/resources/control: Use independent context for join faiure cleanup - #777

internal/rest/resources/control: Use independent context for join faiure cleanup#777
claudiubelu wants to merge 6 commits into
canonical:v4from
claudiubelu:v4-new-cleanup-context

claudiubelu commented Jun 29, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

claudiubelu Oct 20, 2026

Uh oh!

claudiubelu Oct 20, 2026

Uh oh!

roosterfish left a comment

Uh oh!

roosterfish Jul 1, 2026

Uh oh!

claudiubelu Oct 20, 2026

Uh oh!

roosterfish commented Jul 8, 2026

Uh oh!

claudiubelu left a comment

Uh oh!

claudiubelu Oct 20, 2026

Uh oh!

claudiubelu Oct 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

claudiubelu commented Jun 29, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

claudiubelu Oct 20, 2026

Choose a reason for hiding this comment

Uh oh!

claudiubelu Oct 20, 2026

Choose a reason for hiding this comment

Uh oh!

roosterfish left a comment

Choose a reason for hiding this comment

Uh oh!

roosterfish Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

claudiubelu Oct 20, 2026

Choose a reason for hiding this comment

Uh oh!

roosterfish commented Jul 8, 2026

Uh oh!

claudiubelu left a comment

Choose a reason for hiding this comment

Uh oh!

claudiubelu Oct 20, 2026

Choose a reason for hiding this comment

Uh oh!

claudiubelu Oct 20, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants