Skip to content

internal/rest/resources/control: retry remaining join addresses on client connect err#760

Draft
claudiubelu wants to merge 1 commit into
canonical:v4from
claudiubelu:v4-join-with-token-err
Draft

internal/rest/resources/control: retry remaining join addresses on client connect err#760
claudiubelu wants to merge 1 commit into
canonical:v4from
claudiubelu:v4-join-with-token-err

Conversation

@claudiubelu

Copy link
Copy Markdown
Contributor

When joining a cluster, joinWithToken iterates over all addresses in the token to find a reachable cluster member.

However, if state.Connect().Member fails for an address, the function returns immediately instead of trying the next address. This is inconsistent with how other failures in the same loop are handled (GetRemoteCertificate and AddClusterMember both continue to the next address on error).

The function now iterates over all the join addresses in this scenario.

…ient connect err

When joining a cluster, joinWithToken iterates over all addresses in the
token to find a reachable cluster member.

However, if state.Connect().Member fails for an address, the function returns
immediately instead of trying the next address. This is inconsistent with
how other failures in the same loop are handled (GetRemoteCertificate and
AddClusterMember both continue to the next address on error).

The function now iterates over all the join addresses in this scenario.

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

Copy link
Copy Markdown
Contributor Author

@roosterfish, could you please take a quick glance?

client, err := state.Connect().Member(&url.URL, false, cert)
if err != nil {
return nil, nil, err
logger.Warn("Failed to get client for cluster member", slog.String("address", url.String()), slog.String("error", err.Error()))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect we currently simply return the error as the call to shared.GetRemoteCertificate should have already failed if it's not reachable.

Would you mind sharing a reproducer how to trigger this?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@claudiubelu any update on this?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves cluster join robustness in the internal REST control resource by ensuring join attempts continue across all addresses in a join token when connecting to a member fails, aligning this failure mode with the existing “try next address” behavior for other join-step errors.

Changes:

  • Continue iterating over token.JoinAddresses when state.Connect().Member(...) fails, rather than returning immediately.
  • Emit a warning log for connection/client creation failures and preserve the last error for final reporting.

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

Comment on lines 248 to +252
client, err := state.Connect().Member(&url.URL, false, cert)
if err != nil {
return nil, nil, err
logger.Warn("Failed to get client for cluster member", slog.String("address", url.String()), slog.String("error", err.Error()))
lastErr = err
continue
@roosterfish roosterfish marked this pull request as draft July 8, 2026 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants