Skip to content

Commit

Permalink
[WorkspaceStarter] Fail instance on image resolution errors
Browse files Browse the repository at this point in the history
  • Loading branch information
geropl committed Sep 20, 2023
1 parent 952d922 commit 97a61b7
Showing 1 changed file with 24 additions and 26 deletions.
50 changes: 24 additions & 26 deletions components/server/src/workspace/workspace-starter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -368,31 +368,13 @@ export class WorkspaceStarter {
workspace.context,
);

const forceRebuild = !!workspace.context.forceImageBuild;
try {
// if we need to build the workspace image we must not wait for actuallyStartWorkspace to return as that would block the
// frontend until the image is built.
const additionalAuth = await this.getAdditionalImageAuth(envVars);
const needsImageBuild =
forceRebuild || (await this.needsImageBuild(ctx, user, workspace, instance, additionalAuth));
if (needsImageBuild) {
instance.status.conditions = {
neededImageBuild: true,
};
}
ctx.span?.setTag("needsImageBuild", needsImageBuild);
} catch (err) {
// if we fail to check if the workspace needs an image build (e.g. becuase the image builder is unavailable),
// we must properly fail the workspace instance, i.e. set its status to stopped, deal with prebuilds etc.
//
// Once we've reached actuallyStartWorkspace that function will take care of failing the instance.
await this.failInstanceStart(ctx, err, workspace, instance, abortSignal);
throw err;
}

await this.actuallyStartWorkspace(ctx, instance, workspace, user, envVars, abortSignal, forceRebuild);
await this.actuallyStartWorkspace(ctx, instance, workspace, user, envVars, abortSignal);
} catch (err) {
log.error({ instanceId }, "error in reconcileWorkspaceStart", err);
this.logAndTraceStartWorkspaceError(
ctx,
{ userId: user.id, workspaceId: workspace.id, instanceId },
err,
);
} finally {
ctx.span.finish();
}
Expand Down Expand Up @@ -532,7 +514,6 @@ export class WorkspaceStarter {
user: User,
envVars: ResolvedEnvVars,
abortSignal: RedlockAbortSignal,
forceRebuild?: boolean,
): Promise<void> {
const span = TraceContext.startSpan("actuallyStartWorkspace", ctx);
const region = instance.configuration.regionPreference;
Expand All @@ -543,6 +524,7 @@ export class WorkspaceStarter {
organizationId: workspace.organizationId,
workspaceId: workspace.id,
};
const forceRebuild = !!workspace.context.forceImageBuild;
log.info(logCtx, "Attempting to start workspace", {
forceRebuild: forceRebuild,
});
Expand Down Expand Up @@ -1216,6 +1198,18 @@ export class WorkspaceStarter {
const span = TraceContext.startSpan("buildWorkspaceImage", ctx);

try {
// TODO(gpl) I looks like we really don't need the whole "needsImageBuild" anymore:
// Before we used it to distinguish between sync/async, now we are always async.
// Still keeping it here for now to understand whether there are any side effects in image-builder-mk3
const needsImageBuild =
forceRebuild || (await this.needsImageBuild(ctx, user, workspace, instance, additionalAuth));
if (needsImageBuild) {
instance.status.conditions = {
neededImageBuild: true,
};
}
ctx.span?.setTag("needsImageBuild", needsImageBuild);

// Start build...
const client = await this.getImageBuilderClient(user, workspace, instance, region);
const { src, auth, disposable } = await this.prepareBuildRequest(
Expand Down Expand Up @@ -1352,7 +1346,11 @@ export class WorkspaceStarter {
await this.failInstanceStart({ span }, err, workspace, instance, abortSignal);

const looksLikeUserError = (msg: string): boolean => {
return msg.startsWith("build failed:") || msg.includes("headless task failed:");
return (
msg.startsWith("build failed:") ||
msg.includes("headless task failed:") ||
msg.includes("cannot resolve image")
);
};
if (looksLikeUserError(message)) {
log.info(
Expand Down

0 comments on commit 97a61b7

Please sign in to comment.