Skip to content

gorums: add per-server options for local server groups - #361

Merged
meling merged 3 commits into
masterfrom
feat/325/local-server-options
Oct 12, 2026
Merged

meling merged 3 commits into
masterfrom
feat/325/local-server-options

Conversation

@meling

@meling meling commented Oct 11, 2026

Copy link
Copy Markdown
Member

Part of #325; the base of a stack: this PR, then #359 (peer identity), then #360 (refused-stream retry).

Summary

Test infrastructure for local server groups whose servers need different options, for example each its own TLS certificate.

  • gorums: add per-server options to NewLocalServers — WithLocalServerOptionsFunc(fn func(id ID) []ServerOption) and WithLocalDialOptionsFunc(fn func(id ID) []DialOption) apply the options that fn returns for a server's node ID, after the shared options, so per-server transport credentials replace shared ones.
  • gorumstest: accept local server and dial options in LocalServers — LocalServers(t, n, opts ...gorumstest.Option) now routes a ServerOption to every server, a DialOption to every server's outbound configuration after DialOptions(t), and a LocalServerOption (such as WithLocalServerOptionsFunc) as given; other types panic, as for Config and Node.
    Callers that spread a []gorums.ServerOption must use a []gorumstest.Option slice; the four call sites in this repository are updated.
  • gorumstest: register the leak check before the listeners' cleanup — restores the documented cleanup order, so goleak.VerifyNone runs after the listeners' cleanup.

Testing

TestLocalServersAppliesPerServerOptions, TestLocalServersOptions, and TestLocalServersUnsupportedOption; make test and make integrationtest pass on this branch alone.

Copilot AI balanced review requested due to automatic review settings October 11, 2026 20:40

Copilot AI left a comment

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

NewLocalServers applied the same server and dial options to every
server, so a local cluster could not give each server its own TLS
certificate. Add WithLocalServerOptionsFunc and
WithLocalDialOptionsFunc, which return the options for a server's node
ID. They are applied after the shared options, so per-server transport
credentials replace shared ones.
LocalServers took only server options, so a test could not give its
servers per-server options or extra dial options. It now takes the
package's Option type: a ServerOption applies to every server, a
DialOption to every server's outbound configuration after
DialOptions, and a LocalServerOption, such as
WithLocalServerOptionsFunc, as given. Other types panic, as for Config
and Node.
LocalServers built its options, which register the listeners' cleanup
through servers.ListenFunc, before it registered goleak.VerifyNone.
Cleanups run in reverse order, so the leak check ran before the
listeners were cleaned up, contrary to the comment that the check
runs last. Register the leak check first.
@meling
meling force-pushed the feat/325/local-server-options branch from 442e7a9 to 88fcd3e Compare October 12, 2026 20:10
@meling
meling merged commit 00c1b1d into master Oct 12, 2026
3 checks passed
@meling
meling deleted the feat/325/local-server-options branch October 12, 2026 20:22
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.

2 participants