Skip to content

Commit 6b4e6f5

Browse files
committed
[WorkspaceStarter] Don't call imageBuilder.needsImageBuild as we don't need it anymore
Before, we used it to decide between handling the startWorkspace request sync or async. Now we are always async. And the check whether we actually need an image build is done in "build" anyway.
1 parent 952d922 commit 6b4e6f5

File tree

1 file changed

+17
-83
lines changed

1 file changed

+17
-83
lines changed

components/server/src/workspace/workspace-starter.ts

Lines changed: 17 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,6 @@ import {
7979
BuildStatus,
8080
ImageBuilderClientProvider,
8181
ResolveBaseImageRequest,
82-
ResolveWorkspaceImageRequest,
8382
} from "@gitpod/image-builder/lib";
8483
import {
8584
IDEImage,
@@ -368,31 +367,13 @@ export class WorkspaceStarter {
368367
workspace.context,
369368
);
370369

371-
const forceRebuild = !!workspace.context.forceImageBuild;
372-
try {
373-
// if we need to build the workspace image we must not wait for actuallyStartWorkspace to return as that would block the
374-
// frontend until the image is built.
375-
const additionalAuth = await this.getAdditionalImageAuth(envVars);
376-
const needsImageBuild =
377-
forceRebuild || (await this.needsImageBuild(ctx, user, workspace, instance, additionalAuth));
378-
if (needsImageBuild) {
379-
instance.status.conditions = {
380-
neededImageBuild: true,
381-
};
382-
}
383-
ctx.span?.setTag("needsImageBuild", needsImageBuild);
384-
} catch (err) {
385-
// if we fail to check if the workspace needs an image build (e.g. becuase the image builder is unavailable),
386-
// we must properly fail the workspace instance, i.e. set its status to stopped, deal with prebuilds etc.
387-
//
388-
// Once we've reached actuallyStartWorkspace that function will take care of failing the instance.
389-
await this.failInstanceStart(ctx, err, workspace, instance, abortSignal);
390-
throw err;
391-
}
392-
393-
await this.actuallyStartWorkspace(ctx, instance, workspace, user, envVars, abortSignal, forceRebuild);
370+
await this.actuallyStartWorkspace(ctx, instance, workspace, user, envVars, abortSignal);
394371
} catch (err) {
395-
log.error({ instanceId }, "error in reconcileWorkspaceStart", err);
372+
this.logAndTraceStartWorkspaceError(
373+
ctx,
374+
{ userId: user.id, workspaceId: workspace.id, instanceId },
375+
err,
376+
);
396377
} finally {
397378
ctx.span.finish();
398379
}
@@ -532,7 +513,6 @@ export class WorkspaceStarter {
532513
user: User,
533514
envVars: ResolvedEnvVars,
534515
abortSignal: RedlockAbortSignal,
535-
forceRebuild?: boolean,
536516
): Promise<void> {
537517
const span = TraceContext.startSpan("actuallyStartWorkspace", ctx);
538518
const region = instance.configuration.regionPreference;
@@ -543,6 +523,7 @@ export class WorkspaceStarter {
543523
organizationId: workspace.organizationId,
544524
workspaceId: workspace.id,
545525
};
526+
const forceRebuild = !!workspace.context.forceImageBuild;
546527
log.info(logCtx, "Attempting to start workspace", {
547528
forceRebuild: forceRebuild,
548529
});
@@ -1146,62 +1127,6 @@ export class WorkspaceStarter {
11461127
}
11471128
}
11481129

1149-
private async needsImageBuild(
1150-
ctx: TraceContext,
1151-
user: User,
1152-
workspace: Workspace,
1153-
instance: WorkspaceInstance,
1154-
additionalAuth: Map<string, string>,
1155-
): Promise<boolean> {
1156-
const span = TraceContext.startSpan("needsImageBuild", ctx);
1157-
try {
1158-
const client = await this.getImageBuilderClient(
1159-
user,
1160-
workspace,
1161-
instance,
1162-
instance.configuration.regionPreference,
1163-
);
1164-
const { src, auth, disposable } = await this.prepareBuildRequest(
1165-
{ span },
1166-
workspace,
1167-
workspace.imageSource!,
1168-
user,
1169-
additionalAuth,
1170-
);
1171-
1172-
const req = new ResolveWorkspaceImageRequest();
1173-
req.setSource(src);
1174-
req.setAuth(auth);
1175-
const result = await client.resolveWorkspaceImage({ span }, req);
1176-
1177-
if (!!disposable) {
1178-
disposable.dispose();
1179-
}
1180-
1181-
if (result.getStatus() != BuildStatus.DONE_SUCCESS) {
1182-
log.info(
1183-
{
1184-
instanceId: instance.id,
1185-
userId: workspace.ownerId,
1186-
workspaceId: workspace.id,
1187-
organizationId: workspace.organizationId,
1188-
},
1189-
"Resolve workspace was unsuccessful",
1190-
{
1191-
buildStatus: result.getStatus(),
1192-
},
1193-
);
1194-
}
1195-
1196-
return result.getStatus() != BuildStatus.DONE_SUCCESS;
1197-
} catch (err) {
1198-
TraceContext.setError({ span }, err);
1199-
throw err;
1200-
} finally {
1201-
span.finish();
1202-
}
1203-
}
1204-
12051130
private async buildWorkspaceImage(
12061131
ctx: TraceContext,
12071132
user: User,
@@ -1266,6 +1191,11 @@ export class WorkspaceStarter {
12661191
const result = await client.build({ span }, req, imageBuildLogInfo);
12671192

12681193
if (result.actuallyNeedsBuild) {
1194+
instance.status.conditions = {
1195+
...instance.status.conditions,
1196+
neededImageBuild: true,
1197+
}; // Stored below
1198+
ctx.span?.setTag("needsImageBuild", true);
12691199
increaseImageBuildsStartedTotal();
12701200
}
12711201

@@ -1352,7 +1282,11 @@ export class WorkspaceStarter {
13521282
await this.failInstanceStart({ span }, err, workspace, instance, abortSignal);
13531283

13541284
const looksLikeUserError = (msg: string): boolean => {
1355-
return msg.startsWith("build failed:") || msg.includes("headless task failed:");
1285+
return (
1286+
msg.startsWith("build failed:") ||
1287+
msg.includes("headless task failed:") ||
1288+
msg.includes("cannot resolve image")
1289+
);
13561290
};
13571291
if (looksLikeUserError(message)) {
13581292
log.info(

0 commit comments

Comments
 (0)