feat: add migrations and queries to support prebuilds#16891
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
We need two separate locks because we'll have a wider reconciliation loop which must hold a global lock, and an inner lock which is used to protect the piece where we determine the state. The latter can be called outside of a reconciliation loop (as in the metrics collection).
e1b1e36 to
81ba8d2
Compare
2024110 to
390a1fd
Compare
|
Do we want to add unit-tests on DB level in this PR to make sure VIEWs and SQL queries are correct? |
Feels like overkill to me, since we have tests which rely on the views indirectly, but if you feel it'd add value then go ahead 👍 |
70ecdcc to
7e062e2
Compare
Signed-off-by: Danny Kopping <dannykopping@gmail.com>
Signed-off-by: Danny Kopping <dannykopping@gmail.com>
Signed-off-by: Danny Kopping <dannykopping@gmail.com>
Signed-off-by: Danny Kopping <dannykopping@gmail.com>
appeasing linter Signed-off-by: Danny Kopping <dannykopping@gmail.com>
Signed-off-by: Danny Kopping <dannykopping@gmail.com>
Signed-off-by: Danny Kopping <dannykopping@gmail.com>
c92fb1b to
e7e9c27
Compare
Signed-off-by: Danny Kopping <dannykopping@gmail.com>
Emyrk
left a comment
There was a problem hiding this comment.
👍 on the RBAC checks. @spikecurtis no ResourceSystem is now used.
Resource<Type>.All() is used in almost all of these because there is no filter for orgs or a user. The reconciliation loop queries across all orgs.
The queries should succeed or fail as an entire set, rather then be filtered. Based on how they are used. In the future, if we add more views for prebuilds, we can update these queries to filter and apply the appropriate checks. Similar to how we filter workspaces & templates today.
| workspaceObject := rbac.ResourceWorkspace.WithOwner(arg.NewUserID.String()).InOrg(preset.OrganizationID) | ||
| err = q.authorizeContext(ctx, policy.ActionCreate, workspaceObject.RBACObject()) | ||
| if err != nil { | ||
| return empty, err | ||
| } | ||
|
|
||
| tpl, err := q.GetTemplateByID(ctx, preset.TemplateID.UUID) | ||
| if err != nil { | ||
| return empty, xerrors.Errorf("verify template by id: %w", err) | ||
| } | ||
| if err := q.authorizeContext(ctx, policy.ActionUse, tpl); err != nil { | ||
| return empty, xerrors.Errorf("use template for workspace: %w", err) | ||
| } |
There was a problem hiding this comment.
Just leave a comment this should match InsertWorkspace
| func (q *querier) CountInProgressPrebuilds(ctx context.Context) ([]database.CountInProgressPrebuildsRow, error) { | ||
| if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceWorkspace.All()); err != nil { | ||
| return nil, err | ||
| } | ||
| return q.db.CountInProgressPrebuilds(ctx) | ||
| } |
There was a problem hiding this comment.
This query spans all organizations, and prebuilds are just workspaces 👍
| func (q *querier) GetPresetsBackoff(ctx context.Context, lookback time.Time) ([]database.GetPresetsBackoffRow, error) { | ||
| // GetPresetsBackoff returns a list of template version presets along with metadata such as the number of failed prebuilds. | ||
| if err := q.authorizeContext(ctx, policy.ActionViewInsights, rbac.ResourceTemplate.All()); err != nil { | ||
| return nil, err | ||
| } | ||
| return q.db.GetPresetsBackoff(ctx, lookback) | ||
| } |
There was a problem hiding this comment.
@spikecurtis we decided to just lump in the preset metrics with template insights. Since template insights are metrics related to a template, prebuild metrics metadata is essentially the same imo.
Depends on #16916 (change base to
mainonce it is merged)Closes coder/internal#514
This is one of several PRs to decompose the
dk/prebuildsfeature branch into separate PRs to merge intomain.