fix(coderd): wake dormant workspace when attempting to start it #21306
Conversation
johnstcn
changed the title
sreya
left a comment
sreya
left a comment
There was a problem hiding this comment.
I think this is fine...the intent originally was for a user to have be deliberate about "activating" a dormant workspace to continue using it. I think back then the idea was that I did not want dormant workspaces to be activated unintentionally as a result of scripts or bulk actions.
coderd/workspacebuilds.go
Outdated
| // #20925: if the workspace is dormant and we are starting the workspace, | ||
| // we need to unset that status before inserting a new build. | ||
| if workspace.DormantAt.Valid && transition == database.WorkspaceTransitionStart { | ||
| if _, err := api.Database.UpdateWorkspaceDormantDeletingAt(ctx, database.UpdateWorkspaceDormantDeletingAtParams{ | ||
| ID: workspace.ID, | ||
| DormantAt: sql.NullTime{Valid: false}, | ||
| }); err != nil { | ||
| return codersdk.WorkspaceBuild{}, httperror.NewResponseError(http.StatusInternalServerError, codersdk.Response{ | ||
| Message: "Internal error unsetting workspace dormant status", | ||
| Detail: err.Error(), | ||
| }) | ||
| } | ||
| // We need to audit this change separately. | ||
| updatedWorkspace := workspace.WorkspaceTable() | ||
| updatedWorkspace.DormantAt = sql.NullTime{Valid: false} | ||
| auditor := api.Auditor.Load() | ||
| bag := audit.BaggageFromContext(ctx) | ||
| audit.BackgroundAudit(ctx, &audit.BackgroundAuditParams[database.WorkspaceTable]{ | ||
| Audit: *auditor, | ||
| Old: workspace.WorkspaceTable(), | ||
| New: updatedWorkspace, | ||
| Log: api.Logger, | ||
| UserID: apiKey.UserID, | ||
| OrganizationID: workspace.OrganizationID, | ||
| RequestID: workspace.ID, | ||
| IP: bag.IP, | ||
| Action: database.AuditActionWrite, | ||
| Status: http.StatusOK, | ||
| }) | ||
| } |
There was a problem hiding this comment.
Should this happen inside the transaction below? So if the workspace build cannot be created, the dormancy remains?
I can also see why the dormancy should be toggled regardless.
There was a problem hiding this comment.
I can see an argument either way; I'm choosing to interpret an attempt to start as enough to reactivate. I also considered doing it in wsbuilder but that didn't "feel" right.
Edit: I went ahead and updated it to unset it inside the tx. We can move it out later if needed.
Relates to #20925
This PR modifies the
postWorkspaceBuildhandler to automatically unset dormancy on a workspace when a start transition is requested. Previously, the client was responsible for unsetting the dormancy on the workspace prior to posting a workspace build.