Skip to content

Commit d6ce216

Browse files
kylecarbskylecarbs
committed
Ensure close kills replica
1 parent abff96b commit d6ce216

File tree

9 files changed

+32
-40
lines changed

9 files changed

+32
-40
lines changed

‎cli/root.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"flag"
66
"fmt"
7+
"io"
78
"net/http"
89
"net/url"
910
"os"
@@ -100,8 +101,9 @@ func Core() []*cobra.Command {
100101
}
101102

102103
func AGPL() []*cobra.Command {
103-
all := append(Core(), Server(deployment.Flags(), func(_ context.Context, _ config.Root, o *coderd.Options) (*coderd.API, error) {
104-
return coderd.New(o), nil
104+
all := append(Core(), Server(deployment.Flags(), func(_ context.Context, o *coderd.Options) (*coderd.API, io.Closer, error) {
105+
api := coderd.New(o)
106+
return api, api, nil
105107
}))
106108
return all
107109
}

‎cli/server.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ import (
6767
)
6868

6969
// nolint:gocyclo
70-
func Server(dflags *codersdk.DeploymentFlags, newAPI func(context.Context, config.Root, *coderd.Options) (*coderd.API, error)) *cobra.Command {
70+
func Server(dflags *codersdk.DeploymentFlags, newAPI func(context.Context, *coderd.Options) (*coderd.API, io.Closer, error)) *cobra.Command {
7171
root := &cobra.Command{
7272
Use: "server",
7373
Short: "Start a Coder server",
@@ -463,11 +463,14 @@ func Server(dflags *codersdk.DeploymentFlags, newAPI func(context.Context, confi
463463
), dflags.PromAddress.Value, "prometheus")()
464464
}
465465

466-
coderAPI, err := newAPI(ctx, config, options)
466+
// We use a separate closer so the Enterprise API
467+
// can have it's own close functions. This is cleaner
468+
// than abstracting the Coder API itself.
469+
coderAPI, closer, err := newAPI(ctx, options)
467470
if err != nil {
468471
return err
469472
}
470-
defer coderAPI.Close()
473+
defer closer.Close()
471474

472475
client := codersdk.New(localURL)
473476
if dflags.TLSEnable.Value {

‎coderd/coderd.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ func New(options *Options) *API {
123123
options.TailnetCoordinator = tailnet.NewCoordinator()
124124
}
125125
if options.DERPServer == nil {
126-
options.DERPServer = derp.NewServer(key.NewNode(), tailnet.Logger(options.Logger.Named("derp").Leveled(slog.LevelDebug)))
126+
options.DERPServer = derp.NewServer(key.NewNode(), tailnet.Logger(options.Logger.Named("derp")))
127127
options.DERPServer.SetMeshKey("todo-kyle-change-this")
128128
}
129129
if options.Auditor == nil {

‎enterprise/cli/server.go

Lines changed: 5 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,12 @@ package cli
22

33
import (
44
"context"
5+
"io"
56
"net/url"
67

7-
"github.com/google/uuid"
88
"github.com/spf13/cobra"
99
"golang.org/x/xerrors"
1010

11-
"cdr.dev/slog"
12-
13-
"github.com/coder/coder/cli/config"
1411
"github.com/coder/coder/cli/deployment"
1512
"github.com/coder/coder/enterprise/coderd"
1613

@@ -20,28 +17,11 @@ import (
2017

2118
func server() *cobra.Command {
2219
dflags := deployment.Flags()
23-
cmd := agpl.Server(dflags, func(ctx context.Context, cfg config.Root, options *agplcoderd.Options) (*agplcoderd.API, error) {
24-
replicaIDRaw, err := cfg.ReplicaID().Read()
25-
generatedReplicaID := false
26-
if err != nil {
27-
replicaIDRaw = uuid.NewString()
28-
generatedReplicaID = true
29-
}
30-
replicaID, err := uuid.Parse(replicaIDRaw)
31-
if err != nil {
32-
options.Logger.Warn(ctx, "failed to parse replica id", slog.Error(err), slog.F("replica_id", replicaIDRaw))
33-
replicaID = uuid.New()
34-
generatedReplicaID = true
35-
}
36-
if generatedReplicaID {
37-
// Make sure we save it to be reused later!
38-
_ = cfg.ReplicaID().Write(replicaID.String())
39-
}
40-
20+
cmd := agpl.Server(dflags, func(ctx context.Context, options *agplcoderd.Options) (*agplcoderd.API, io.Closer, error) {
4121
if dflags.DerpServerRelayAddress.Value != "" {
4222
_, err := url.Parse(dflags.DerpServerRelayAddress.Value)
4323
if err != nil {
44-
return nil, xerrors.Errorf("derp-server-relay-address must be a valid HTTP URL: %w", err)
24+
return nil, nil, xerrors.Errorf("derp-server-relay-address must be a valid HTTP URL: %w", err)
4525
}
4626
}
4727

@@ -51,17 +31,16 @@ func server() *cobra.Command {
5131
SCIMAPIKey: []byte(dflags.SCIMAuthHeader.Value),
5232
UserWorkspaceQuota: dflags.UserWorkspaceQuota.Value,
5333
RBAC: true,
54-
ReplicaID: replicaID,
5534
DERPServerRelayAddress: dflags.DerpServerRelayAddress.Value,
5635
DERPServerRegionID: dflags.DerpServerRegionID.Value,
5736

5837
Options: options,
5938
}
6039
api, err := coderd.New(ctx, o)
6140
if err != nil {
62-
return nil, err
41+
return nil, nil, err
6342
}
64-
return api.AGPL, nil
43+
return api.AGPL, api, nil
6544
})
6645

6746
deployment.AttachFlags(cmd.Flags(), dflags, true)

‎enterprise/coderd/coderd.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,8 @@ func New(ctx context.Context, options *Options) (*API, error) {
123123

124124
var err error
125125
api.replicaManager, err = replicasync.New(ctx, options.Logger, options.Database, options.Pubsub, replicasync.Options{
126-
ID: options.ReplicaID,
126+
// Create a new replica ID for each Coder instance!
127+
ID: uuid.New(),
127128
RelayAddress: options.DERPServerRelayAddress,
128129
RegionID: int32(options.DERPServerRegionID),
129130
})
@@ -154,7 +155,6 @@ type Options struct {
154155
// Used for high availability.
155156
DERPServerRelayAddress string
156157
DERPServerRegionID int
157-
ReplicaID uuid.UUID
158158

159159
EntitlementsUpdateInterval time.Duration
160160
Keys map[string]ed25519.PublicKey
@@ -256,10 +256,15 @@ func (api *API) updateEntitlements(ctx context.Context) error {
256256
addresses = append(addresses, replica.RelayAddress)
257257
}
258258
api.derpMesh.SetAddresses(addresses)
259+
_ = api.updateEntitlements(ctx)
259260
})
260261
} else {
261262
api.derpMesh.SetAddresses([]string{})
262-
api.replicaManager.SetCallback(func() {})
263+
api.replicaManager.SetCallback(func() {
264+
// If the amount of replicas change, so should our entitlements.
265+
// This is to display a warning in the UI if the user is unlicensed.
266+
_ = api.updateEntitlements(ctx)
267+
})
263268
}
264269

265270
// Recheck changed in case the HA coordinator failed to set up.

‎enterprise/coderd/coderdenttest/coderdenttest.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"time"
1010

1111
"github.com/golang-jwt/jwt/v4"
12-
"github.com/google/uuid"
1312
"github.com/stretchr/testify/assert"
1413
"github.com/stretchr/testify/require"
1514

@@ -69,7 +68,6 @@ func NewWithAPI(t *testing.T, options *Options) (*codersdk.Client, io.Closer, *c
6968
SCIMAPIKey: options.SCIMAPIKey,
7069
DERPServerRelayAddress: oop.AccessURL.String(),
7170
DERPServerRegionID: oop.DERPMap.RegionIDs()[0],
72-
ReplicaID: uuid.New(),
7371
UserWorkspaceQuota: options.UserWorkspaceQuota,
7472
Options: oop,
7573
EntitlementsUpdateInterval: options.EntitlementsUpdateInterval,

‎enterprise/coderd/license/license.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ func Entitlements(
176176
"You have multiple replicas but your license is not entitled to high availability.")
177177
} else {
178178
entitlements.Warnings = append(entitlements.Warnings,
179-
"You have multiple replicas but high availability is an Enterprise feature. Contact sales to get a license.")
179+
"You have multiple replicas but high availability is an Enterprise feature.")
180180
}
181181
case codersdk.EntitlementGracePeriod:
182182
entitlements.Warnings = append(entitlements.Warnings,

‎enterprise/coderd/license/license_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ func TestEntitlements(t *testing.T) {
228228
require.NoError(t, err)
229229
require.False(t, entitlements.HasLicense)
230230
require.Len(t, entitlements.Warnings, 1)
231-
require.Equal(t, "You have multiple replicas but high availability is an Enterprise feature. Contact sales to get a license.", entitlements.Warnings[0])
231+
require.Equal(t, "You have multiple replicas but high availability is an Enterprise feature.", entitlements.Warnings[0])
232232
})
233233

234234
t.Run("MultipleReplicasNotEntitled", func(t *testing.T) {

‎enterprise/coderd/replicas_test.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ func TestReplicas(t *testing.T) {
3030
},
3131
})
3232
_ = coderdtest.CreateFirstUser(t, firstClient)
33-
secondClient := coderdenttest.New(t, &coderdenttest.Options{
33+
secondClient, _, secondAPI := coderdenttest.NewWithAPI(t, &coderdenttest.Options{
3434
Options: &coderdtest.Options{
3535
Database: db,
3636
Pubsub: pubsub,
@@ -40,6 +40,11 @@ func TestReplicas(t *testing.T) {
4040
ents, err := secondClient.Entitlements(context.Background())
4141
require.NoError(t, err)
4242
require.Len(t, ents.Warnings, 1)
43+
_ = secondAPI.Close()
44+
45+
ents, err = firstClient.Entitlements(context.Background())
46+
require.NoError(t, err)
47+
require.Len(t, ents.Warnings, 0)
4348
})
4449
t.Run("ConnectAcrossMultiple", func(t *testing.T) {
4550
t.Parallel()

0 commit comments

Comments
 (0)