feat: allow users to pause prebuilt workspace reconciliation #18700

SasSwart · 2025-07-01T18:57:38Z

This PR provides two commands:

coder prebuilds pause
coder prebuilds resume

These allow the suspension of all prebuilds activity, intended for use if prebuilds are misbehaving.

…liation

johnstcn

LGTM so far, just missing the final piece of plumbing

…whether prebuilds are currently paused

johnstcn · 2025-07-02T08:57:22Z

Might be good to also avoid the term prebuilds in the CLI command unless there is prior art

dannykopping · 2025-07-02T08:57:29Z

enterprise/coderd/prebuilds/metricscollector.go

@@ -29,6 +29,7 @@ const (
 	MetricEligibleGauge             = namespace + "eligible"
 	MetricPresetHardLimitedGauge    = namespace + "preset_hard_limited"
 	MetricLastUpdatedGauge          = namespace + "metrics_last_updated"
+	MetricReconciliationPausedGauge = namespace + "reconciliation_paused"


Paused is difficult to reason about; I'd suggest inverting and make it _running so that a good value is 1 and a bad value is 0.

paused in this case has a technical meaning relating to a specific setting. If we rename this to running then I think we need to change the setting to be {"reconciliation_running": true}, which doesn't communicate the intent of the API clearly.

I'd like to keep it how it currently is, but as compensatory measures I will:

update the metric documentation to be clear about its meaning

add a second metric to represent a general "check engine light" for prebuilds

dannykopping

This will be pretty handy, LGTM!

dannykopping · 2025-07-02T09:07:52Z

enterprise/coderd/prebuilds.go

+		return
+	}
+
+	if bytes.Equal(settingsJSON, []byte(currentSettingsJSON)) {


Nit: not a problem now but when we add more settings, map ordering may cause this check to fail.

good catch.

enterprise/coderd/prebuilds/metricscollector.go

enterprise/coderd/prebuilds/reconcile.go

dannykopping · 2025-07-02T09:12:33Z

enterprise/coderd/prebuilds/reconcile_test.go

+	require.Len(t, workspaces, 2, "should have created 2 prebuilds")
+
+	// Now pause prebuilds reconciliation
+	err = db.UpsertPrebuildsSettings(ctx, `{"reconciliation_paused": true}`)


I'd suggest going via the API here to avoid duplicating this business logic.

ssncferreira

LGTM 🎉 Thanks for creating this.
There is just a question about rbac: with this change we require the permission rbac.ResourceDeploymentConfig, so that means that only the owner of the resouce or a user with role auditorRole can upsert this setting. Should we extend it to Admins?

ssncferreira · 2025-07-02T11:09:33Z

coderd/audit/request.go

@@ -113,6 +113,8 @@ func ResourceTarget[T Auditable](tgt T) string {
 		return "" // no target?
 	case database.NotificationsSettings:
 		return "" // no target?
+	case database.PrebuildsSettings:


What is the reason of this file update? 🤔

We need to be able to audit who pauses prebuilds.

ssncferreira · 2025-07-02T11:21:24Z

coderd/database/dbauthz/dbauthz.go

@@ -5101,6 +5105,13 @@ func (q *querier) UpsertOAuthSigningKey(ctx context.Context, value string) error
 	return q.db.UpsertOAuthSigningKey(ctx, value)
 }

+func (q *querier) UpsertPrebuildsSettings(ctx context.Context, value string) error {
+	if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceDeploymentConfig); err != nil {


Just to clarify here on the rbac side, this means that only a user with this permission can update the setting. From https://github.com/coder/coder/blob/main/coderd/rbac/roles.go currently, only the auditorRole has this permission. Is that intended?

Auditor only has read though?

coder/coderd/rbac/roles.go

Line 327 in 8a69f6a

ResourceDeploymentConfig.Type: {policy.ActionRead},

ssncferreira · 2025-07-02T11:22:43Z

coderd/database/types.go

@@ -35,6 +35,11 @@ type NotificationsSettings struct {
 	NotifierPaused bool      `db:"notifier_paused" json:"notifier_paused"`
 }

+type PrebuildsSettings struct {
+	ID                   uuid.UUID `db:"id" json:"id"`
+	ReconciliationPaused bool      `db:"reconciliation_paused" json:"reconciliation_paused"`


Do you think it would also be useful to add a timestamp?

ssncferreira · 2025-07-02T11:30:32Z

enterprise/coderd/prebuilds.go

+
+	if bytes.Equal(settingsJSON, []byte(currentSettingsJSON)) {
+		// See: https://www.rfc-editor.org/rfc/rfc7232#section-4.1
+		httpapi.Write(ctx, rw, http.StatusNotModified, nil)


nit: according to the RFC the 304 Not Modified is normally a response to a GET request.
Maybe a 204 No Content makes more sense in this case, wdyt? https://datatracker.ietf.org/doc/html/rfc7231#section-6.3.5

ssncferreira · 2025-07-02T11:32:24Z

enterprise/cli/prebuilds.go

+				return xerrors.Errorf("unable to pause prebuilds: %w", err)
+			}
+
+			_, _ = fmt.Fprintln(inv.Stderr, "Prebuilds are now paused.")


Why not?

Suggested change

_, _ = fmt.Fprintln(inv.Stderr, "Prebuilds are now paused.")

_, _ = fmt.Fprintln(inv.Stdout, "Prebuilds are now paused.")

ssncferreira · 2025-07-02T11:35:09Z

enterprise/cli/prebuilds_test.go

+		assert.Contains(t, buf.String(), "Prebuilds are now paused.")
+
+		// Verify the settings were actually updated
+		//nolint:gocritic // Only owners can change deployment settings


Why the //nolint:gocritic? 🤔

ssncferreira · 2025-07-02T11:43:19Z

enterprise/coderd/prebuilds/reconcile_test.go

+	err = db.UpsertPrebuildsSettings(ctx, `{"reconciliation_paused": false}`)
+	require.NoError(t, err)
+
+	// Run reconciliation again - it should now recreate the prebuilds


Nice test 🤩

feat: add cli commands to pause and resume prebuilt workspace reconci…

4751936

…liation

github-actions bot assigned SasSwart Jul 1, 2025

SasSwart changed the title ~~feat: add cli commands to pause and resume prebuilt workspace reconci…~~ feat: provide a mechanism to pause prebuilt workspace reconciliation Jul 1, 2025

renumber migrations

0b4d669

SasSwart changed the title ~~feat: provide a mechanism to pause prebuilt workspace reconciliation~~ feat: allow users to pause prebuilt workspace reconciliation Jul 1, 2025

SasSwart requested review from johnstcn and ssncferreira July 1, 2025 19:02

johnstcn reviewed Jul 1, 2025

View reviewed changes

SasSwart added 2 commits July 1, 2025 19:42

make gen

caa9224

get prebuilds to honor the pause setting and expose a metric to show …

140d820

…whether prebuilds are currently paused

SasSwart requested a review from johnstcn July 2, 2025 08:26

SasSwart marked this pull request as ready for review July 2, 2025 08:29

johnstcn approved these changes Jul 2, 2025

View reviewed changes

dannykopping reviewed Jul 2, 2025

View reviewed changes

dannykopping approved these changes Jul 2, 2025

View reviewed changes

ssncferreira reviewed Jul 2, 2025

View reviewed changes

	_, _ = fmt.Fprintln(inv.Stderr, "Prebuilds are now paused.")
	_, _ = fmt.Fprintln(inv.Stdout, "Prebuilds are now paused.")

feat: allow users to pause prebuilt workspace reconciliation #18700

Are you sure you want to change the base?

feat: allow users to pause prebuilt workspace reconciliation #18700

Conversation

SasSwart commented Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

johnstcn left a comment

Choose a reason for hiding this comment

Uh oh!

johnstcn commented Jul 2, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dannykopping left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ssncferreira left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

SasSwart commented Jul 1, 2025 •

edited

Loading

ssncferreira left a comment •

edited

Loading