From 6b4e6f57fa6b5f724f2336a2e92f3aef1930be68 Mon Sep 17 00:00:00 2001
From: Gero Posmyk-Leinemann <gero@gitpod.io>
Date: Fri, 22 Sep 2023 08:48:12 +0000
Subject: [PATCH] [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.
---
 .../server/src/workspace/workspace-starter.ts | 100 +++---------------
 1 file changed, 17 insertions(+), 83 deletions(-)

diff --git a/components/server/src/workspace/workspace-starter.ts b/components/server/src/workspace/workspace-starter.ts
index 4c0cd0537b1635..543d0bd0e8f43d 100644
--- a/components/server/src/workspace/workspace-starter.ts
+++ b/components/server/src/workspace/workspace-starter.ts
@@ -79,7 +79,6 @@ import {
     BuildStatus,
     ImageBuilderClientProvider,
     ResolveBaseImageRequest,
-    ResolveWorkspaceImageRequest,
 } from "@gitpod/image-builder/lib";
 import {
     IDEImage,
@@ -368,31 +367,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();
             }
@@ -532,7 +513,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;
@@ -543,6 +523,7 @@ export class WorkspaceStarter {
             organizationId: workspace.organizationId,
             workspaceId: workspace.id,
         };
+        const forceRebuild = !!workspace.context.forceImageBuild;
         log.info(logCtx, "Attempting to start workspace", {
             forceRebuild: forceRebuild,
         });
@@ -1146,62 +1127,6 @@ export class WorkspaceStarter {
         }
     }
 
-    private async needsImageBuild(
-        ctx: TraceContext,
-        user: User,
-        workspace: Workspace,
-        instance: WorkspaceInstance,
-        additionalAuth: Map<string, string>,
-    ): Promise<boolean> {
-        const span = TraceContext.startSpan("needsImageBuild", ctx);
-        try {
-            const client = await this.getImageBuilderClient(
-                user,
-                workspace,
-                instance,
-                instance.configuration.regionPreference,
-            );
-            const { src, auth, disposable } = await this.prepareBuildRequest(
-                { span },
-                workspace,
-                workspace.imageSource!,
-                user,
-                additionalAuth,
-            );
-
-            const req = new ResolveWorkspaceImageRequest();
-            req.setSource(src);
-            req.setAuth(auth);
-            const result = await client.resolveWorkspaceImage({ span }, req);
-
-            if (!!disposable) {
-                disposable.dispose();
-            }
-
-            if (result.getStatus() != BuildStatus.DONE_SUCCESS) {
-                log.info(
-                    {
-                        instanceId: instance.id,
-                        userId: workspace.ownerId,
-                        workspaceId: workspace.id,
-                        organizationId: workspace.organizationId,
-                    },
-                    "Resolve workspace was unsuccessful",
-                    {
-                        buildStatus: result.getStatus(),
-                    },
-                );
-            }
-
-            return result.getStatus() != BuildStatus.DONE_SUCCESS;
-        } catch (err) {
-            TraceContext.setError({ span }, err);
-            throw err;
-        } finally {
-            span.finish();
-        }
-    }
-
     private async buildWorkspaceImage(
         ctx: TraceContext,
         user: User,
@@ -1266,6 +1191,11 @@ export class WorkspaceStarter {
             const result = await client.build({ span }, req, imageBuildLogInfo);
 
             if (result.actuallyNeedsBuild) {
+                instance.status.conditions = {
+                    ...instance.status.conditions,
+                    neededImageBuild: true,
+                }; // Stored below
+                ctx.span?.setTag("needsImageBuild", true);
                 increaseImageBuildsStartedTotal();
             }
 
@@ -1352,7 +1282,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(