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~~

EDIT: prior art exists with codersdk.PrebuildsConfig and in numerous other places in the codebase. I don't have any better naming suggestion, so leave it as coder prebuilds

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?

coderd/audit/request.go

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},

Yes, good catch! 👀 So it means that only the owner of the ResourceDeploymentConfig can update the settings, right? Should we maybe extend it to the Admins? 🤔

I don't think there exists any other suitable role that should have permission to edit site-wide deployment config? Our choices are site-wide template admin or site-wide user admin, neither of which feel right to me. Site-wide auditor should only have read permissions, and all of the org-scoped admins are out of site-wide scope.

This might be something to explore in a separate PR.

Yes, this could definitely be addressed in a separate PR. You might be right that adding the permission to admin roles isn’t the best approach. My main concern was that currently only users with the Owner role can update this setting. In our dogfood environment, this works fine since many users have the Owner role, but this is not the typical case.

On second thought, limiting this permission to the Owner role does make sense, since it’s an important command and users should be fully aware of the implications when using it.

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.")

My reasoning was that this command doesn't actually produce any output of its own.
It reports its own status, but that's more logging than output.

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? 🤔

There's a rule to check that we test with non-owner roles as they're usually more appropriate. In this case, owner is the appropriate role.

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

deduplicate prebuilds pause business logic

cdd55fe

SasSwart enabled auto-merge (squash) July 2, 2025 14:55

SasSwart merged commit 01163ea into main Jul 2, 2025
39 checks passed

SasSwart deleted the jjs/internal-368 branch July 2, 2025 15:05

github-actions bot locked and limited conversation to collaborators Jul 2, 2025

	_, _ = 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

feat: allow users to pause prebuilt workspace reconciliation #18700

Uh oh!

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 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

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!

johnstcn Jul 2, 2025 • 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!

Uh oh!

Uh oh!

SasSwart commented Jul 1, 2025 •

edited

Loading

johnstcn commented Jul 2, 2025 •

edited

Loading

ssncferreira left a comment •

edited

Loading

johnstcn Jul 2, 2025 •

edited

Loading