Skip to content

Commit c0f81d0

Browse files
dannykoppingdannykopping
committed
Enable reconciliator on entitlements change
Signed-off-by: Danny Kopping <dannykopping@gmail.com>
1 parent df743e6 commit c0f81d0

File tree

7 files changed

+154
-94
lines changed

7 files changed

+154
-94
lines changed

‎coderd/prebuilds/api.go

+8-14
Original file line numberDiff line numberDiff line change
@@ -3,24 +3,18 @@ package prebuilds
33
import (
44
"context"
55

6-
"github.com/coder/coder/v2/coderd/database"
76
"github.com/google/uuid"
7+
8+
"github.com/coder/coder/v2/coderd/database"
89
)
910

11+
type Reconciler interface {
12+
RunLoop(ctx context.Context)
13+
Stop(ctx context.Context, cause error)
14+
ReconcileAll(ctx context.Context) error
15+
}
16+
1017
type Claimer interface {
1118
Claim(ctx context.Context, store database.Store, userID uuid.UUID, name string, presetID uuid.UUID) (*uuid.UUID, error)
1219
Initiator() uuid.UUID
1320
}
14-
15-
type AGPLPrebuildClaimer struct{}
16-
17-
func (c AGPLPrebuildClaimer) Claim(context.Context, database.Store, uuid.UUID, string, uuid.UUID) (*uuid.UUID, error) {
18-
// Not entitled to claim prebuilds in AGPL version.
19-
return nil, nil
20-
}
21-
22-
func (c AGPLPrebuildClaimer) Initiator() uuid.UUID {
23-
return uuid.Nil
24-
}
25-
26-
var DefaultClaimer Claimer = AGPLPrebuildClaimer{}

‎coderd/prebuilds/claim.go

+22
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
package prebuilds
2+
3+
import (
4+
"context"
5+
6+
"github.com/google/uuid"
7+
8+
"github.com/coder/coder/v2/coderd/database"
9+
)
10+
11+
type AGPLPrebuildClaimer struct{}
12+
13+
func (c AGPLPrebuildClaimer) Claim(context.Context, database.Store, uuid.UUID, string, uuid.UUID) (*uuid.UUID, error) {
14+
// Not entitled to claim prebuilds in AGPL version.
15+
return nil, nil
16+
}
17+
18+
func (c AGPLPrebuildClaimer) Initiator() uuid.UUID {
19+
return uuid.Nil
20+
}
21+
22+
var DefaultClaimer Claimer = AGPLPrebuildClaimer{}

‎coderd/prebuilds/reconcile.go

+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
package prebuilds
2+
3+
import (
4+
"context"
5+
)
6+
7+
type noopReconciler struct{}
8+
9+
func NewNoopReconciler() Reconciler {
10+
return &noopReconciler{}
11+
}
12+
13+
func (noopReconciler) RunLoop(context.Context) {}
14+
func (noopReconciler) Stop(context.Context, error) {}
15+
func (noopReconciler) ReconcileAll(context.Context) error { return nil }
16+
func (noopReconciler) ReconcileTemplate() error { return nil }
17+
18+
var _ Reconciler = noopReconciler{}

‎enterprise/coderd/coderd.go

+44-24
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package coderd
33
import (
44
"context"
55
"crypto/ed25519"
6+
"errors"
67
"fmt"
78
"math"
89
"net/http"
@@ -583,23 +584,6 @@ func New(ctx context.Context, options *Options) (_ *API, err error) {
583584
}
584585
go api.runEntitlementsLoop(ctx)
585586

586-
if api.AGPL.Experiments.Enabled(codersdk.ExperimentWorkspacePrebuilds) {
587-
// TODO: future enhancement, start this up without restarting coderd when entitlement is updated.
588-
if !api.Entitlements.Enabled(codersdk.FeatureWorkspacePrebuilds) {
589-
options.Logger.Warn(ctx, "prebuilds experiment enabled but not entitled to use")
590-
} else {
591-
api.prebuildsController = prebuilds.NewController(options.Database, options.Pubsub, options.DeploymentValues.Prebuilds, options.Logger.Named("prebuilds.controller"))
592-
go api.prebuildsController.Loop(ctx)
593-
594-
prebuildMetricsCollector := prebuilds.NewMetricsCollector(options.Database, options.Logger)
595-
// should this be api.prebuild...
596-
err = api.PrometheusRegistry.Register(prebuildMetricsCollector)
597-
if err != nil {
598-
return nil, xerrors.Errorf("unable to register prebuilds metrics collector: %w", err)
599-
}
600-
}
601-
}
602-
603587
return api, nil
604588
}
605589

@@ -654,7 +638,8 @@ type API struct {
654638
licenseMetricsCollector *license.MetricsCollector
655639
tailnetService *tailnet.ClientService
656640

657-
prebuildsController *prebuilds.Controller
641+
prebuildsReconciler agplprebuilds.Reconciler
642+
prebuildsMetricsCollector *prebuilds.MetricsCollector
658643
}
659644

660645
// writeEntitlementWarningsHeader writes the entitlement warnings to the response header
@@ -686,8 +671,10 @@ func (api *API) Close() error {
686671
api.Options.CheckInactiveUsersCancelFunc()
687672
}
688673

689-
if api.prebuildsController != nil {
690-
api.prebuildsController.Close(xerrors.New("api closed")) // TODO: determine root cause (requires changes up the stack, though).
674+
if api.prebuildsReconciler != nil {
675+
ctx, giveUp := context.WithTimeoutCause(context.Background(), time.Second*30, errors.New("gave up waiting for reconciler to stop"))
676+
defer giveUp()
677+
api.prebuildsReconciler.Stop(ctx, xerrors.New("api closed")) // TODO: determine root cause (requires changes up the stack, though).
691678
}
692679

693680
return api.AGPL.Close()
@@ -893,11 +880,22 @@ func (api *API) updateEntitlements(ctx context.Context) error {
893880
}
894881

895882
if initial, changed, enabled := featureChanged(codersdk.FeatureWorkspacePrebuilds); shouldUpdate(initial, changed, enabled) {
896-
c := agplprebuilds.DefaultClaimer
897-
if enabled {
898-
c = prebuilds.EnterpriseClaimer{}
883+
reconciler, claimer, metrics := api.setupPrebuilds(enabled)
884+
if api.prebuildsReconciler != nil {
885+
stopCtx, giveUp := context.WithTimeoutCause(context.Background(), time.Second*30, errors.New("gave up waiting for reconciler to stop"))
886+
defer giveUp()
887+
api.prebuildsReconciler.Stop(stopCtx, errors.New("entitlements change"))
888+
}
889+
890+
// Only register metrics once.
891+
if api.prebuildsMetricsCollector != nil {
892+
api.prebuildsMetricsCollector = metrics
899893
}
900-
api.AGPL.PrebuildsClaimer.Store(&c)
894+
895+
api.prebuildsReconciler = reconciler
896+
go reconciler.RunLoop(context.Background())
897+
898+
api.AGPL.PrebuildsClaimer.Store(&claimer)
901899
}
902900

903901
// External token encryption is soft-enforced
@@ -1168,3 +1166,25 @@ func (api *API) runEntitlementsLoop(ctx context.Context) {
11681166
func (api *API) Authorize(r *http.Request, action policy.Action, object rbac.Objecter) bool {
11691167
return api.AGPL.HTTPAuth.Authorize(r, action, object)
11701168
}
1169+
1170+
func (api *API) setupPrebuilds(entitled bool) (agplprebuilds.Reconciler, agplprebuilds.Claimer, *prebuilds.MetricsCollector) {
1171+
enabled := api.AGPL.Experiments.Enabled(codersdk.ExperimentWorkspacePrebuilds)
1172+
if !enabled || !entitled {
1173+
api.Logger.Debug(context.Background(), "prebuilds not enabled",
1174+
slog.F("experiment_enabled", enabled), slog.F("entitled", entitled))
1175+
1176+
return agplprebuilds.NewNoopReconciler(), agplprebuilds.DefaultClaimer, nil
1177+
}
1178+
1179+
logger := api.Logger.Named("prebuilds.metrics")
1180+
collector := prebuilds.NewMetricsCollector(api.Database, logger)
1181+
err := api.PrometheusRegistry.Register(collector)
1182+
if err != nil {
1183+
logger.Error(context.Background(), "failed to register prebuilds metrics collector", slog.F("error", err))
1184+
collector = nil
1185+
}
1186+
1187+
return prebuilds.NewStoreReconciler(api.Database, api.Pubsub, api.DeploymentValues.Prebuilds, api.Logger.Named("prebuilds")),
1188+
prebuilds.EnterpriseClaimer{},
1189+
collector
1190+
}

‎enterprise/coderd/prebuilds/claim_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ func TestClaimPrebuild(t *testing.T) {
8585
},
8686
})
8787

88-
controller := prebuilds.NewController(spy, pubsub, codersdk.PrebuildsConfig{}, testutil.Logger(t))
88+
controller := prebuilds.NewStoreReconciler(spy, pubsub, codersdk.PrebuildsConfig{}, testutil.Logger(t))
8989

9090
const (
9191
desiredInstances = 1

‎enterprise/coderd/prebuilds/controller_test.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ func TestNoReconciliationActionsIfNoPresets(t *testing.T) {
3333
ReconciliationInterval: serpent.Duration(testutil.WaitLong),
3434
}
3535
logger := testutil.Logger(t)
36-
controller := prebuilds.NewController(db, pubsub, cfg, logger)
36+
controller := prebuilds.NewStoreReconciler(db, pubsub, cfg, logger)
3737

3838
// given a template version with no presets
3939
org := dbgen.Organization(t, db, database.Organization{})
@@ -77,7 +77,7 @@ func TestNoReconciliationActionsIfNoPrebuilds(t *testing.T) {
7777
ReconciliationInterval: serpent.Duration(testutil.WaitLong),
7878
}
7979
logger := testutil.Logger(t)
80-
controller := prebuilds.NewController(db, pubsub, cfg, logger)
80+
controller := prebuilds.NewStoreReconciler(db, pubsub, cfg, logger)
8181

8282
// given there are presets, but no prebuilds
8383
org := dbgen.Organization(t, db, database.Organization{})
@@ -302,7 +302,7 @@ func TestActiveTemplateVersionPrebuilds(t *testing.T) {
302302
db, pubsub := dbtestutil.NewDB(t)
303303
cfg := codersdk.PrebuildsConfig{}
304304
logger := testutil.Logger(t)
305-
controller := prebuilds.NewController(db, pubsub, cfg, logger)
305+
controller := prebuilds.NewStoreReconciler(db, pubsub, cfg, logger)
306306

307307
orgID, userID, templateID := setupTestDBTemplate(t, db)
308308
_, _, prebuildID := setupTestDBPrebuild(
@@ -346,7 +346,7 @@ func TestInactiveTemplateVersionPrebuilds(t *testing.T) {
346346
db, pubsub := dbtestutil.NewDB(t)
347347
cfg := codersdk.PrebuildsConfig{}
348348
logger := testutil.Logger(t)
349-
controller := prebuilds.NewController(db, pubsub, cfg, logger)
349+
controller := prebuilds.NewStoreReconciler(db, pubsub, cfg, logger)
350350

351351
// when does a prebuild get deleted?
352352
// * when it is in some way permanently ineligible to be claimed

0 commit comments

Comments
 (0)