opensource/coder
3
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2026-09-22 05:05:20 +08:00
Code Issues Packages Projects Releases Wiki Activity

chore: modify replicasync to handle NATS explicitly (#26666)

Browse Source 
relates to GRU-69

Modifies replicasync to handle discovering NATS enabled primary replicas explicitly, and passing that info to the NATS Pubsub.

This PR adds a new deployment value to explicitly represent the host or IP that the replica can be reached on. It isn't wired up to the CLI, but piggybacks on the DERP config for now.

We learn the NATS port directly from NATS at runtime, and propagate it thru replicasync to learn all peers for clustering.
This commit is contained in:
Spike Curtis
2026-06-26 08:36:32 -04:00
committed by GitHub
parent 9f211ce5ae
commit 98e1ce133c
16 changed files with 219 additions and 132 deletions
Download Patch File Download Diff File Expand all files Collapse all files
coderd/apidoc/docs.go
Generated
+11
View File
@@ -17838,6 +17838,14 @@ const docTemplate = `{
"ChatWatchEventKindContextDirty"
]
},
"codersdk.ClusterConfig": {
"type": "object",
"properties": {
"host": {
"type": "string"
}
}
},
"codersdk.ConnectionLatency": {
"type": "object",
"properties": {
@@ -19174,6 +19182,9 @@ const docTemplate = `{
"cli_upgrade_message": {
"type": "string"
},
"cluster": {
"$ref": "#/definitions/codersdk.ClusterConfig"
},
"config": {
"type": "string"
},
coderd/apidoc/swagger.json
Generated
+11
View File
@@ -16091,6 +16091,14 @@
"ChatWatchEventKindContextDirty"
]
},
"codersdk.ClusterConfig": {
"type": "object",
"properties": {
"host": {
"type": "string"
}
}
},
"codersdk.ConnectionLatency": {
"type": "object",
"properties": {
@@ -17376,6 +17384,9 @@
"cli_upgrade_message": {
"type": "string"
},
"cluster": {
"$ref": "#/definitions/codersdk.ClusterConfig"
},
"config": {
"type": "string"
},
coderd/x/nats/cluster.go
+19 -15
View File
@@ -17,12 +17,15 @@ const defaultClusterTokenUsername = "coder"
// PeerFetcher fetches NATS peer route addresses.
type PeerFetcher interface {
PrimaryPeerAddresses() []string
FetchNATSPeers() []string
SetSelfNATSPort(port int32)
}
type NopPeerFetcher struct{}
func (NopPeerFetcher) PrimaryPeerAddresses() []string {
func (NopPeerFetcher) SetSelfNATSPort(int32) {}
func (NopPeerFetcher) FetchNATSPeers() []string {
return nil
}
@@ -35,6 +38,14 @@ func (p *Pubsub) SetPeerFetcher(fetcher PeerFetcher) {
}
p.peerFetcher = fetcher
p.mu.Unlock()
if ca := p.Server.ClusterAddr(); ca != nil {
if ca.Port >= 1 && ca.Port <= 65535 {
//nolint:gosec // range checked above so conversion is safe.
fetcher.SetSelfNATSPort(int32(ca.Port))
} else {
p.logger.Warn(p.ctx, "unexpected NATS cluster port", slog.F("port", ca.Port))
}
}
p.RefreshPeers()
}
@@ -53,7 +64,7 @@ func (p *Pubsub) runPeerRefresh() {
fetcher := p.peerFetcher
p.mu.Unlock()
addrs := fetcher.PrimaryPeerAddresses()
addrs := fetcher.FetchNATSPeers()
if err := p.setPeerAddresses(addrs); err != nil {
if errors.Is(err, errClosed) && p.ctx.Err() != nil {
return
@@ -81,7 +92,7 @@ func (p *Pubsub) setPeerAddresses(addresses []string) error {
return xerrors.New("nats pubsub was not started with clustering enabled")
}
routes, err := p.parsePeerAddresses(addresses)
routes, err := parsePeerAddresses(addresses)
if err != nil {
return err
}
@@ -109,7 +120,7 @@ func (p *Pubsub) setPeerAddresses(addresses []string) error {
return nil
}
func (p *Pubsub) parsePeerAddresses(addresses []string) ([]*url.URL, error) {
func parsePeerAddresses(addresses []string) ([]*url.URL, error) {
routesByAddress := make(map[string]*url.URL, len(addresses))
for i, address := range addresses {
trimmed := strings.TrimSpace(address)
@@ -122,16 +133,6 @@ func (p *Pubsub) parsePeerAddresses(addresses []string) ([]*url.URL, error) {
return nil, err
}
// This is a hack to enable testing with an arbitrary port. The logic here
// is to presume if the default port is being used then we are running in prod
// and all peers are using the same port. If the port is not the default then
// we are running a test in which case we should pass through the custom port.
// This hack will be removed when https://github.com/coder/scaletest/issues/149
// is resolved.
if p.opts.ClusterPort == defaultClusterPort {
port = defaultClusterPort
}
hostPort := net.JoinHostPort(host, strconv.Itoa(port))
routesByAddress[hostPort] = &url.URL{
Scheme: "nats",
@@ -168,6 +169,9 @@ func normalizeHostPort(address string) (string, int, error) {
if route.Path != "" || route.RawQuery != "" || route.Fragment != "" {
return "", 0, xerrors.Errorf("peer address %q must not include path, query, or fragment", address)
}
if route.Scheme != "nats" {
return "", 0, xerrors.Errorf("peer address %q must use nats scheme", address)
}
host, port, err := net.SplitHostPort(route.Host)
if err != nil {
coderd/x/nats/cluster_internal_test.go
+26 -62
View File
@@ -10,15 +10,19 @@ import (
"github.com/coder/coder/v2/testutil"
)
const (
minTCPPort int32 = 1
maxTCPPort int32 = 65535
)
func Test_parsePeerAddresses(t *testing.T) {
t.Parallel()
t.Run("Valid", func(t *testing.T) {
t.Parallel()
ps := &Pubsub{}
routes, err := ps.parsePeerAddresses([]string{
"whatever://127.0.0.1:4222 ",
"http://[::1]:7222",
routes, err := parsePeerAddresses([]string{
"nats://127.0.0.1:4222 ",
"nats://[::1]:7222",
"nats://example.com:6222",
})
require.NoError(t, err)
@@ -29,51 +33,16 @@ func Test_parsePeerAddresses(t *testing.T) {
}, routeStrings(routes))
})
// Test that when a pubsub is running with the default port, it assumes all peers are also using
// the default port.
t.Run("PrefersDefaultPort", func(t *testing.T) {
t.Parallel()
ps := &Pubsub{}
ps.opts.ClusterPort = defaultClusterPort
routes, err := ps.parsePeerAddresses([]string{
"whatever://127.0.0.1:4222 ",
"http://[::1]:7222",
"nats://example.com:1234",
})
require.NoError(t, err)
require.ElementsMatch(t, []string{
"nats://127.0.0.1:6222",
"nats://[::1]:6222",
"nats://example.com:6222",
}, routeStrings(routes))
})
// Regression: in production the relay URL host carries the coderd HTTP
// port (e.g. 8080), and routes must be rewritten to the NATS cluster
// port. This only works because New defaults ClusterPort to
// defaultClusterPort; if it were left at the zero value the rewrite
// would be skipped and routes would dial the HTTP port.
t.Run("RewritesRelayHTTPPort", func(t *testing.T) {
t.Parallel()
ps := &Pubsub{}
ps.opts.ClusterPort = defaultClusterPort
routes, err := ps.parsePeerAddresses([]string{"http://10.0.0.7:8080"})
require.NoError(t, err)
require.Equal(t, []string{"nats://10.0.0.7:6222"}, routeStrings(routes))
})
t.Run("Empty", func(t *testing.T) {
t.Parallel()
ps := &Pubsub{}
routes, err := ps.parsePeerAddresses(nil)
routes, err := parsePeerAddresses(nil)
require.NoError(t, err)
require.Empty(t, routes)
})
t.Run("Dedupes", func(t *testing.T) {
t.Parallel()
ps := &Pubsub{}
routes, err := ps.parsePeerAddresses([]string{
routes, err := parsePeerAddresses([]string{
"nats://b.example:6222",
"nats://a.example:6222",
"nats://b.example:6222",
@@ -103,11 +72,12 @@ func Test_parsePeerAddresses(t *testing.T) {
"nats://127.0.0.1:4222/path",
"nats://127.0.0.1:4222?x=1",
"nats://127.0.0.1:4222#frag",
"whatever://127.0.0.1:4222 ",
"http://[::1]:7222",
} {
t.Run(address, func(t *testing.T) {
t.Parallel()
ps := &Pubsub{}
_, err := ps.parsePeerAddresses([]string{address})
_, err := parsePeerAddresses([]string{address})
require.Error(t, err)
})
}
@@ -117,10 +87,9 @@ func Test_parsePeerAddresses(t *testing.T) {
func Test_filterSelfRoutes(t *testing.T) {
t.Parallel()
ps := &Pubsub{}
routes, err := ps.parsePeerAddresses([]string{
routes, err := parsePeerAddresses([]string{
"nats://b.example:6222",
"http://self.example:6222",
"nats://self.example:6222",
})
require.NoError(t, err)
@@ -141,6 +110,8 @@ func TestPubsub_RefreshPeers(t *testing.T) {
opts := clusterTestOptions(t)
opts.PeerFetcher = fetcher
a := newTestPubsub(t, opts)
require.GreaterOrEqual(t, fetcher.port, minTCPPort)
require.LessOrEqual(t, fetcher.port, maxTCPPort)
require.Eventually(t, func() bool {
routes := currentRouteURLs(a)
@@ -159,11 +130,13 @@ func TestPubsub_RefreshPeers(t *testing.T) {
"nats://127.0.0.1:1234",
"nats://127.0.0.1:1235",
}
fetcher := &testPeerFetcher{routes}
fetcher := &testPeerFetcher{addresses: routes}
expectedRoutes := routesWithAuth(mustParsePeerAddresses(t, fetcher.addresses...), opts.ClusterAuthToken)
a.SetPeerFetcher(fetcher)
require.GreaterOrEqual(t, fetcher.port, minTCPPort)
require.LessOrEqual(t, fetcher.port, maxTCPPort)
require.Eventually(t, func() bool {
return sortedURLsEqual(currentRouteURLs(a), sortRouteURLs(expectedRoutes))
}, testutil.WaitShort, testutil.IntervalFast)
@@ -194,26 +167,17 @@ func currentRouteURLs(ps *Pubsub) []*url.URL {
type testPeerFetcher struct {
addresses []string
port int32
}
func (f *testPeerFetcher) PrimaryPeerAddresses() []string {
func (f *testPeerFetcher) SetSelfNATSPort(port int32) {
f.port = port
}
func (f *testPeerFetcher) FetchNATSPeers() []string {
return f.addresses
}
// TestPubsub_New_DefaultsClusterPort guards the production wiring: New
// must persist the default cluster port onto opts so the peer route
// rewrite in parsePeerAddresses recognizes prod and forces routes to the
// NATS port. The cli constructs Options without a ClusterPort, so leaving
// it at the zero value made every replica dial peers at the relay URL's
// HTTP port instead of the NATS route port.
func TestPubsub_New_DefaultsClusterPort(t *testing.T) {
t.Parallel()
// defaultTestOptions disables clustering (no fixed-port listener to
// collide with parallel tests) and leaves ClusterPort unset.
ps := newTestPubsub(t, defaultTestOptions())
require.Equal(t, defaultClusterPort, ps.opts.ClusterPort)
}
func TestPubsub_setPeerAddresses(t *testing.T) {
t.Parallel()
t.Run("OK", func(t *testing.T) {
coderd/x/nats/natsbench/topology.go
+3 -1
View File
@@ -34,9 +34,11 @@ type staticPeerFetcher struct {
addrs []string
}
func (*staticPeerFetcher) SetSelfNATSPort(int32) {}
var _ nats.PeerFetcher = (*staticPeerFetcher)(nil)
func (f *staticPeerFetcher) PrimaryPeerAddresses() []string {
func (f *staticPeerFetcher) FetchNATSPeers() []string {
f.mu.Lock()
defer f.mu.Unlock()
return slices.Clone(f.addrs)
coderd/x/nats/pubsub.go
+39 -24
View File
@@ -113,7 +113,8 @@ type Options struct {
ClusterHost string
// ClusterPort is the embedded NATS route listener port. Zero means
// 6222 when cluster mode is enabled.
// 6222 when cluster mode is enabled. NATS `server.RANDOM_PORT` can be
// used to select a random port.
ClusterPort int
// ClusterAuthToken is the shared route authentication token for
@@ -297,18 +298,7 @@ func (p *Pubsub) buildConnHandlers() connHandlers {
// New creates an embedded NATS Pubsub. The returned *Pubsub owns the
// embedded server and the publisher and subscriber connection pools.
// Close shuts down all owned resources.
func New(ctx context.Context, logger slog.Logger, opts Options) (*Pubsub, error) {
// Persist the default cluster port onto opts so it is the same value the
// listener (buildServerOptions) binds and the value parsePeerAddresses
// compares against. parsePeerAddresses overwrites each peer's parsed port
// with defaultClusterPort, but only when opts.ClusterPort already equals
// defaultClusterPort. Callers like the cli leave ClusterPort at 0, so
// without this that branch is skipped and peers are dialed on the relay
// URL's port (e.g. 8080) instead of the NATS route port (6222).
if opts.ClusterPort == 0 {
opts.ClusterPort = defaultClusterPort
}
func New(ctx context.Context, logger slog.Logger, opts Options) (pubSub *Pubsub, retErr error) {
sopts, err := buildServerOptions(opts)
if err != nil {
return nil, err
@@ -318,6 +308,12 @@ func New(ctx context.Context, logger slog.Logger, opts Options) (*Pubsub, error)
if err != nil {
return nil, err
}
defer func() {
if retErr != nil {
ns.Shutdown()
ns.WaitForShutdown()
}
}()
logger.Info(context.Background(), "embedded nats server started",
slog.F("client_url", ns.ClientURL()),
@@ -328,6 +324,11 @@ func New(ctx context.Context, logger slog.Logger, opts Options) (*Pubsub, error)
}
p := newPubsub(ctx, logger, opts)
defer func() {
if retErr != nil {
p.cancel()
}
}()
p.Server = ns
p.clustered = !opts.disableCluster
p.serverOpts = sopts.Clone()
@@ -336,29 +337,43 @@ func New(ctx context.Context, logger slog.Logger, opts Options) (*Pubsub, error)
publishPool, err := newConnPool(ns, opts, handlers, opts.PublishConns, "coder-pubsub-pub")
if err != nil {
p.cancel()
ns.Shutdown()
ns.WaitForShutdown()
return nil, err
}
defer func() {
if retErr != nil {
for _, c := range publishPool {
c.Close()
}
}
}()
p.publishPool = publishPool
subscribePool, err := newConnPool(ns, opts, handlers, opts.SubscribeConns, "coder-pubsub-sub")
if err != nil {
p.cancel()
for _, c := range publishPool {
c.Close()
}
ns.Shutdown()
ns.WaitForShutdown()
return nil, err
}
p.publishPool = publishPool
defer func() {
if retErr != nil {
for _, c := range subscribePool {
c.Close()
}
}
}()
p.subscribePool = subscribePool
// All owned connections dialed successfully above.
p.metrics.markConnected(len(publishPool) + len(subscribePool))
if p.clustered {
ca := ns.ClusterAddr()
if ca == nil {
return nil, xerrors.New("no cluster address")
}
// sec checks, just to be sure
if ca.Port < 0 || ca.Port > 65535 {
return nil, xerrors.Errorf("invalid cluster port: %d", ca.Port)
}
//nolint:gosec // range checked above so conversion is safe.
opts.PeerFetcher.SetSelfNATSPort(int32(ca.Port))
go p.runPeerRefresh()
}
go func() {