Skip to content

Commit 3dfb796

Browse files
committed
Remove ID from replicasync
1 parent 8641e58 commit 3dfb796

File tree

4 files changed

+35
-91
lines changed

4 files changed

+35
-91
lines changed

enterprise/coderd/coderd.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313

1414
"github.com/cenkalti/backoff/v4"
1515
"github.com/go-chi/chi/v5"
16-
"github.com/google/uuid"
1716

1817
"cdr.dev/slog"
1918
"github.com/coder/coder/coderd"
@@ -146,14 +145,13 @@ func New(ctx context.Context, options *Options) (*API, error) {
146145
// internal IP addresses, and if TLS is configured we use the same
147146
// certificates.
148147
meshTLSConfig := &tls.Config{
148+
MinVersion: tls.VersionTLS12,
149149
Certificates: options.TLSCertificates,
150150
RootCAs: meshRootCA,
151151
ServerName: options.AccessURL.Hostname(),
152152
}
153153
var err error
154-
api.replicaManager, err = replicasync.New(ctx, options.Logger, options.Database, options.Pubsub, replicasync.Options{
155-
// Create a new replica ID for each Coder instance!
156-
ID: uuid.New(),
154+
api.replicaManager, err = replicasync.New(ctx, options.Logger, options.Database, options.Pubsub, &replicasync.Options{
157155
RelayAddress: options.DERPServerRelayAddress,
158156
RegionID: int32(options.DERPServerRegionID),
159157
TLSConfig: meshTLSConfig,

enterprise/coderd/replicas_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ func TestReplicas(t *testing.T) {
123123
})
124124
require.NoError(t, err)
125125
require.Eventually(t, func() bool {
126-
ctx, cancelFunc := context.WithTimeout(context.Background(), 3*time.Second)
126+
ctx, cancelFunc := context.WithTimeout(context.Background(), testutil.IntervalMedium)
127127
defer cancelFunc()
128128
_, err = conn.Ping(ctx)
129129
return err == nil

enterprise/replicasync/replicasync.go

Lines changed: 22 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ var (
2626
)
2727

2828
type Options struct {
29-
ID uuid.UUID
3029
UpdateInterval time.Duration
3130
PeerTimeout time.Duration
3231
RelayAddress string
@@ -36,9 +35,9 @@ type Options struct {
3635

3736
// New registers the replica with the database and periodically updates to ensure
3837
// it's healthy. It contacts all other alive replicas to ensure they are reachable.
39-
func New(ctx context.Context, logger slog.Logger, db database.Store, pubsub database.Pubsub, options Options) (*Manager, error) {
40-
if options.ID == uuid.Nil {
41-
panic("An ID must be provided!")
38+
func New(ctx context.Context, logger slog.Logger, db database.Store, pubsub database.Pubsub, options *Options) (*Manager, error) {
39+
if options == nil {
40+
options = &Options{}
4241
}
4342
if options.PeerTimeout == 0 {
4443
options.PeerTimeout = 3 * time.Second
@@ -54,50 +53,29 @@ func New(ctx context.Context, logger slog.Logger, db database.Store, pubsub data
5453
if err != nil {
5554
return nil, xerrors.Errorf("ping database: %w", err)
5655
}
57-
var replica database.Replica
58-
_, err = db.GetReplicaByID(ctx, options.ID)
56+
id := uuid.New()
57+
replica, err := db.InsertReplica(ctx, database.InsertReplicaParams{
58+
ID: id,
59+
CreatedAt: database.Now(),
60+
StartedAt: database.Now(),
61+
UpdatedAt: database.Now(),
62+
Hostname: hostname,
63+
RegionID: options.RegionID,
64+
RelayAddress: options.RelayAddress,
65+
Version: buildinfo.Version(),
66+
DatabaseLatency: int32(databaseLatency.Microseconds()),
67+
})
5968
if err != nil {
60-
if !errors.Is(err, sql.ErrNoRows) {
61-
return nil, xerrors.Errorf("get replica: %w", err)
62-
}
63-
replica, err = db.InsertReplica(ctx, database.InsertReplicaParams{
64-
ID: options.ID,
65-
CreatedAt: database.Now(),
66-
StartedAt: database.Now(),
67-
UpdatedAt: database.Now(),
68-
Hostname: hostname,
69-
RegionID: options.RegionID,
70-
RelayAddress: options.RelayAddress,
71-
Version: buildinfo.Version(),
72-
DatabaseLatency: int32(databaseLatency.Microseconds()),
73-
})
74-
if err != nil {
75-
return nil, xerrors.Errorf("insert replica: %w", err)
76-
}
77-
} else {
78-
replica, err = db.UpdateReplica(ctx, database.UpdateReplicaParams{
79-
ID: options.ID,
80-
UpdatedAt: database.Now(),
81-
StartedAt: database.Now(),
82-
StoppedAt: sql.NullTime{},
83-
RelayAddress: options.RelayAddress,
84-
RegionID: options.RegionID,
85-
Hostname: hostname,
86-
Version: buildinfo.Version(),
87-
Error: sql.NullString{},
88-
DatabaseLatency: int32(databaseLatency.Microseconds()),
89-
})
90-
if err != nil {
91-
return nil, xerrors.Errorf("update replica: %w", err)
92-
}
69+
return nil, xerrors.Errorf("insert replica: %w", err)
9370
}
94-
err = pubsub.Publish(PubsubEvent, []byte(options.ID.String()))
71+
err = pubsub.Publish(PubsubEvent, []byte(id.String()))
9572
if err != nil {
9673
return nil, xerrors.Errorf("publish new replica: %w", err)
9774
}
9875
ctx, cancelFunc := context.WithCancel(ctx)
9976
server := &Manager{
100-
options: &options,
77+
id: id,
78+
options: options,
10179
db: db,
10280
pubsub: pubsub,
10381
self: replica,
@@ -128,6 +106,7 @@ func New(ctx context.Context, logger slog.Logger, db database.Store, pubsub data
128106

129107
// Manager keeps the replica up to date and in sync with other replicas.
130108
type Manager struct {
109+
id uuid.UUID
131110
options *Options
132111
db database.Store
133112
pubsub database.Pubsub
@@ -196,7 +175,7 @@ func (m *Manager) subscribe(ctx context.Context) error {
196175
return
197176
}
198177
// Don't process updates for ourself!
199-
if id == m.options.ID {
178+
if id == m.id {
200179
return
201180
}
202181
if updating {
@@ -233,7 +212,7 @@ func (m *Manager) run(ctx context.Context) error {
233212
m.mutex.Lock()
234213
m.peers = make([]database.Replica, 0, len(replicas))
235214
for _, replica := range replicas {
236-
if replica.ID == m.options.ID {
215+
if replica.ID == m.id {
237216
continue
238217
}
239218
m.peers = append(m.peers, replica)

enterprise/replicasync/replicasync_test.go

Lines changed: 10 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111
"time"
1212

1313
"github.com/google/uuid"
14-
"github.com/stretchr/testify/assert"
1514
"github.com/stretchr/testify/require"
1615
"go.uber.org/goleak"
1716

@@ -32,38 +31,15 @@ func TestReplica(t *testing.T) {
3231
// This ensures that a new replica is created on New.
3332
t.Parallel()
3433
db, pubsub := dbtestutil.NewDB(t)
35-
id := uuid.New()
34+
closeChan := make(chan struct{}, 1)
3635
cancel, err := pubsub.Subscribe(replicasync.PubsubEvent, func(ctx context.Context, message []byte) {
37-
assert.Equal(t, []byte(id.String()), message)
36+
closeChan <- struct{}{}
3837
})
3938
require.NoError(t, err)
4039
defer cancel()
41-
server, err := replicasync.New(context.Background(), slogtest.Make(t, nil), db, pubsub, replicasync.Options{
42-
ID: id,
43-
})
44-
require.NoError(t, err)
45-
_ = server.Close()
46-
require.NoError(t, err)
47-
})
48-
t.Run("UpdatesOnNew", func(t *testing.T) {
49-
// This ensures that a replica is updated when it initially connects
50-
// and immediately publishes it's existence!
51-
t.Parallel()
52-
db, pubsub := dbtestutil.NewDB(t)
53-
id := uuid.New()
54-
_, err := db.InsertReplica(context.Background(), database.InsertReplicaParams{
55-
ID: id,
56-
})
57-
require.NoError(t, err)
58-
cancel, err := pubsub.Subscribe(replicasync.PubsubEvent, func(ctx context.Context, message []byte) {
59-
assert.Equal(t, []byte(id.String()), message)
60-
})
61-
require.NoError(t, err)
62-
defer cancel()
63-
server, err := replicasync.New(context.Background(), slogtest.Make(t, nil), db, pubsub, replicasync.Options{
64-
ID: id,
65-
})
40+
server, err := replicasync.New(context.Background(), slogtest.Make(t, nil), db, pubsub, nil)
6641
require.NoError(t, err)
42+
<-closeChan
6743
_ = server.Close()
6844
require.NoError(t, err)
6945
})
@@ -80,9 +56,7 @@ func TestReplica(t *testing.T) {
8056
Hostname: "something",
8157
})
8258
require.NoError(t, err)
83-
_, err = replicasync.New(context.Background(), slogtest.Make(t, nil), db, pubsub, replicasync.Options{
84-
ID: uuid.New(),
85-
})
59+
_, err = replicasync.New(context.Background(), slogtest.Make(t, nil), db, pubsub, nil)
8660
require.Error(t, err)
8761
require.Equal(t, "a relay address must be specified when running multiple replicas in the same region", err.Error())
8862
})
@@ -104,8 +78,7 @@ func TestReplica(t *testing.T) {
10478
RelayAddress: srv.URL,
10579
})
10680
require.NoError(t, err)
107-
server, err := replicasync.New(context.Background(), slogtest.Make(t, nil), db, pubsub, replicasync.Options{
108-
ID: uuid.New(),
81+
server, err := replicasync.New(context.Background(), slogtest.Make(t, nil), db, pubsub, &replicasync.Options{
10982
RelayAddress: "http://169.254.169.254",
11083
})
11184
require.NoError(t, err)
@@ -145,8 +118,7 @@ func TestReplica(t *testing.T) {
145118
RelayAddress: srv.URL,
146119
})
147120
require.NoError(t, err)
148-
server, err := replicasync.New(context.Background(), slogtest.Make(t, nil), db, pubsub, replicasync.Options{
149-
ID: uuid.New(),
121+
server, err := replicasync.New(context.Background(), slogtest.Make(t, nil), db, pubsub, &replicasync.Options{
150122
RelayAddress: "http://169.254.169.254",
151123
TLSConfig: tlsConfig,
152124
})
@@ -169,8 +141,7 @@ func TestReplica(t *testing.T) {
169141
RelayAddress: "http://169.254.169.254",
170142
})
171143
require.NoError(t, err)
172-
server, err := replicasync.New(context.Background(), slogtest.Make(t, nil), db, pubsub, replicasync.Options{
173-
ID: uuid.New(),
144+
server, err := replicasync.New(context.Background(), slogtest.Make(t, nil), db, pubsub, &replicasync.Options{
174145
PeerTimeout: 1 * time.Millisecond,
175146
RelayAddress: "http://169.254.169.254",
176147
})
@@ -185,10 +156,7 @@ func TestReplica(t *testing.T) {
185156
// Refresh when a new replica appears!
186157
t.Parallel()
187158
db, pubsub := dbtestutil.NewDB(t)
188-
id := uuid.New()
189-
server, err := replicasync.New(context.Background(), slogtest.Make(t, nil), db, pubsub, replicasync.Options{
190-
ID: id,
191-
})
159+
server, err := replicasync.New(context.Background(), slogtest.Make(t, nil), db, pubsub, nil)
192160
require.NoError(t, err)
193161
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
194162
w.WriteHeader(http.StatusOK)
@@ -224,8 +192,7 @@ func TestReplica(t *testing.T) {
224192
count := 20
225193
wg.Add(count)
226194
for i := 0; i < count; i++ {
227-
server, err := replicasync.New(context.Background(), logger, db, pubsub, replicasync.Options{
228-
ID: uuid.New(),
195+
server, err := replicasync.New(context.Background(), logger, db, pubsub, &replicasync.Options{
229196
RelayAddress: srv.URL,
230197
})
231198
require.NoError(t, err)

0 commit comments

Comments
 (0)