Restructure (4): HTTP client interface with cluster wide queries by roosterfish · Pull Request #582 · canonical/microcluster

roosterfish · 2026-01-12T13:58:00Z

This is the fourth PR taken from a single commit in #534.
The final package layout will be different but to allow reviewing the changes in smaller chunks I will split this PR by its commits.

In this PR two new Client and Connector interfaces are added to address all use cases when it comes to cluster wide HTTP connections.
In addition we now have app level funcs for all user facing operations which previously required going to a separate client package.
Furthermore the specific query functions are now moved to internal/ as they should not be exposed directly to the end user.

Copilot

Pull request overview

This pull request restructures the microcluster HTTP client interface by introducing new Client and Connector interfaces that provide a cleaner abstraction for cluster-wide HTTP connections. The refactoring moves internal query functions from the public client package to internal/rest/client, making the API boundaries clearer.

Changes:

Introduces Client and Connector interfaces in microcluster/types/client.go for standardized cluster communication
Adds new application-level functions (GetClusterMembers, RemoveClusterMember, Shutdown) to MicroCluster struct for common operations
Refactors internal client functions to accept types.Client interface instead of concrete types, improving testability and flexibility

Reviewed changes

Copilot reviewed 30 out of 30 changed files in this pull request and generated 11 comments.

Show a summary per file

File	Description
microcluster/types/client.go	New file defining `Client` and `Connector` interfaces and `Clients` type with query methods
microcluster/types/addrport.go	Removes `SelectRandom` method (moved to state package)
microcluster/app.go	Updates to use new `types.Client` interface and adds new public methods for cluster operations
internal/state/state.go	Implements `Connector` interface methods and adds `RandomMember` function
internal/rest/client/*.go	Refactors all client functions to accept `types.Client` interface parameter
internal/rest/resources/*.go	Updates resource handlers to use new `Connect()` method and interface types
internal/trust/remotes.go	Removes `SelectRandom` method and updates `Cluster` to return `types.Clients`
internal/daemon/daemon.go	Updates to use new client interface throughout
internal/db/dqlite.go	Updates `SendHeartbeat` to accept `types.Client` interface
internal/recover/recover.go	Updates to use `types.Clients` instead of `client.Cluster`
example/cmd/microctl/*.go	Simplifies CLI commands by using new MicroCluster methods
example/client/client.go	Updates to use `microTypes.Client` interface
example/api/extended.go	Updates to use new `types.IsNotification` and interface methods
client/cluster.go	Deleted - functionality moved to `types.Clients`
client/client.go	Deleted - functionality moved to `types.Client` interface

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

microcluster/types/client.go

internal/rest/client/tokens.go

internal/rest/resources/cluster.go

internal/daemon/daemon.go

example/cmd/microctl/cluster_members.go

microcluster/app.go

internal/state/state.go

microcluster/types/client.go

Client is an actual client to a cluster member whereas Connector is a more general concept allowing to reach out to specific members, the cluster leader or multiple members of a cluster at once. Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>

…kage Also switch the concurrent Query implementation to use errgroup instead of waitgroup. Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>

Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>

…plement Client interface Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>

Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>

markylaing · 2026-01-13T10:53:25Z

internal/state/state.go

+	return c, nil
+}
+
+func (s *InternalState) Member(url *url.URL, isNotification bool, cert *x509.Certificate) (types.Client, error) {


Comment missing

tugbataluy · 2026-01-13T12:30:26Z

example/cmd/microctl/cluster_members.go

 type cmdClusterMembers struct {
 	common *CmdControl
 }



I believe the other example files (example/api and example/client) contain comment lines explaining the functions. However, these comments are not present for the functions in this file. Was this done intentionally? Or was it overlooked?

Putting comments on lowercase/unexported funcs is optional.
However the linter doesn't pick this rule up consistently, it failed at least for one occasion in this PR, see #582 (comment).

markylaing

Just a couple of nits.

Overall I think this is a good simplification - exemplified by the example package!

internal/rest/client/cluster.go

internal/rest/client/tokens.go

Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>

This allows easy access for cluster internal connections through the state interface without having to access connection functions from another client package wich should provide a more smooth user experience. Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>

Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>

…lable through client funcs We should take the same approach for all operations and not require the user to switch between app level funcs and client funcs inside another package. Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>

Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>

…Connector interface Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>

Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>

…e others Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>

As we have detached the client funcs from the client type, there should be an app level func for both server and certificate update too. Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>

markylaing

Looks great, thanks :)

roosterfish changed the title ~~Restructure (3): HTTP client interface with cluster wide queries~~ Restructure (4): HTTP client interface with cluster wide queries Jan 12, 2026

roosterfish marked this pull request as draft January 12, 2026 13:58

roosterfish force-pushed the restructure_client_package branch from d2aa5c2 to e1d4466 Compare January 12, 2026 14:01

roosterfish marked this pull request as ready for review January 12, 2026 14:30

roosterfish requested a review from Copilot January 12, 2026 14:30

Copilot started reviewing on behalf of roosterfish January 12, 2026 14:31 View session

Copilot AI reviewed Jan 12, 2026

View reviewed changes

roosterfish added 7 commits January 12, 2026 16:25

microcluster/types: Move SelectRandom and Query to the respective pac…

b9f9ec4

…kage Also switch the concurrent Query implementation to use errgroup instead of waitgroup. Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>

microcluster/types: Move IsNotification to the client

78904b1

Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>

microcluster/types/addport: Drop SelectRandom as it's unused

3423396

Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>

internal/rest/client: Move public facing API funcs to internal and im…

c8e28e9

…plement Client interface Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>

internal/daemon: Use new client interface

ba7f6d2

Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>

internal/recover: Use new client interface

209225c

Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>

roosterfish force-pushed the restructure_client_package branch from 56c12bf to ddc43cf Compare January 12, 2026 15:25

roosterfish requested review from markylaing and tugbataluy January 12, 2026 15:45

markylaing reviewed Jan 13, 2026

View reviewed changes

tugbataluy reviewed Jan 13, 2026

View reviewed changes

markylaing requested changes Jan 15, 2026

View reviewed changes

internal/rest/client/cluster.go Show resolved Hide resolved

internal/rest/client/tokens.go Show resolved Hide resolved

roosterfish added 10 commits January 15, 2026 14:17

internal/rest/client: Use new client interface

00a9204

Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>

internal/rest/resources: Use new client interface

86b880b

Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>

internal/db: Use new client interface

1ff0b38

Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>

internal/trust: Drop unused SelectRandom func

018aa6d

Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>

internal/trust: Use new client interface

7139e0d

Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>

microcluster: Use new client interface

b3c5ced

Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>

example/api: Use new client interface

583d512

Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>

example/client: Use new client interface

a860c9c

Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>

roosterfish added 5 commits January 15, 2026 14:30

example/cmd/microctl: Use new client interface

5acfcf3

Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>

microcluster/types/client: Move SelectRandom to RandomMember func on …

fd2f3ce

…Connector interface Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>

internal/rest/client: Fix typo in docstring

6afc470

Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>

internal/rest/client: Detach UpdateServers func from the client as th…

a08c9db

…e others Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>

microcluster: Add missing app level update funcs

48681f7

As we have detached the client funcs from the client type, there should be an app level func for both server and certificate update too. Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>

roosterfish force-pushed the restructure_client_package branch from ddc43cf to 48681f7 Compare January 15, 2026 13:30

roosterfish requested review from markylaing and tugbataluy January 15, 2026 13:33

markylaing approved these changes Jan 15, 2026

View reviewed changes

roosterfish merged commit a822744 into canonical:v3 Jan 15, 2026
5 checks passed

roosterfish deleted the restructure_client_package branch January 15, 2026 16:07

roosterfish mentioned this pull request Jan 15, 2026

fix: add consistency checks across core_cluster_members, truststore, and dqlite #515

Merged

Conversation

roosterfish commented Jan 12, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

markylaing Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

roosterfish Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

tugbataluy Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

roosterfish Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

markylaing left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

markylaing left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants