From ccddd3d9cf0d926b818d9a4e6e3c3018ffba6a8e Mon Sep 17 00:00:00 2001 From: Gero Posmyk-Leinemann Date: Wed, 11 May 2022 08:42:06 +0000 Subject: [PATCH] [bridge] prebuild: map "stopping && no snapshot" to "failed" state & test --- components/ws-manager-bridge/BUILD.yaml | 4 +- components/ws-manager-bridge/ee/src/bridge.ts | 66 +++------ components/ws-manager-bridge/mocha.opts | 6 + components/ws-manager-bridge/package.json | 8 ++ .../ws-manager-bridge/src/container-module.ts | 3 + .../src/prebuild-state-mapper.spec.ts | 126 ++++++++++++++++++ .../src/prebuild-state-mapper.ts | 94 +++++++++++++ 7 files changed, 258 insertions(+), 49 deletions(-) create mode 100644 components/ws-manager-bridge/mocha.opts create mode 100644 components/ws-manager-bridge/src/prebuild-state-mapper.spec.ts create mode 100644 components/ws-manager-bridge/src/prebuild-state-mapper.ts diff --git a/components/ws-manager-bridge/BUILD.yaml b/components/ws-manager-bridge/BUILD.yaml index f35d9f6c64a755..7f5cbf1a640096 100644 --- a/components/ws-manager-bridge/BUILD.yaml +++ b/components/ws-manager-bridge/BUILD.yaml @@ -4,6 +4,7 @@ packages: srcs: - "**/*.ts" - package.json + - mocha.opts deps: - components/content-service-api/typescript:lib - components/gitpod-db:lib @@ -16,7 +17,8 @@ packages: packaging: offline-mirror yarnLock: ${coreYarnLockBase}/yarn.lock tsconfig: tsconfig.json - dontTest: true + commands: + test: ["yarn", "test"] - name: docker type: docker deps: diff --git a/components/ws-manager-bridge/ee/src/bridge.ts b/components/ws-manager-bridge/ee/src/bridge.ts index a8bb390a9d71e0..1754048b35db7d 100644 --- a/components/ws-manager-bridge/ee/src/bridge.ts +++ b/components/ws-manager-bridge/ee/src/bridge.ts @@ -5,15 +5,19 @@ */ import { WorkspaceManagerBridge } from "../../src/bridge"; -import { injectable } from "inversify"; +import { inject, injectable } from "inversify"; import { TraceContext } from "@gitpod/gitpod-protocol/lib/util/tracing"; import { WorkspaceStatus, WorkspaceType, WorkspacePhase } from "@gitpod/ws-manager/lib"; -import { HeadlessWorkspaceEvent, HeadlessWorkspaceEventType } from "@gitpod/gitpod-protocol/lib/headless-workspace-log"; +import { HeadlessWorkspaceEvent } from "@gitpod/gitpod-protocol/lib/headless-workspace-log"; import { WorkspaceInstance } from "@gitpod/gitpod-protocol"; import { log, LogContext } from "@gitpod/gitpod-protocol/lib/util/logging"; +import { PrebuildStateMapper } from "../../src/prebuild-state-mapper"; @injectable() export class WorkspaceManagerBridgeEE extends WorkspaceManagerBridge { + @inject(PrebuildStateMapper) + protected readonly prebuildStateMapper: PrebuildStateMapper; + protected async cleanupProbeWorkspace(ctx: TraceContext, status: WorkspaceStatus.AsObject | undefined) { if (!status) { return; @@ -75,65 +79,31 @@ export class WorkspaceManagerBridgeEE extends WorkspaceManagerBridge { } prebuild.statusVersion = status.statusVersion; - if (prebuild.state === "queued") { - // We've received an update from ws-man for this workspace, hence it must be running. - prebuild.state = "building"; - - if (writeToDB) { - await this.workspaceDB.trace({ span }).storePrebuiltWorkspace(prebuild); - } - await this.messagebus.notifyHeadlessUpdate({ span }, userId, workspaceId, { - type: HeadlessWorkspaceEventType.Started, - workspaceID: workspaceId, - }); - } - - if (status.phase === WorkspacePhase.STOPPING) { - let headlessUpdateType: HeadlessWorkspaceEventType = HeadlessWorkspaceEventType.Aborted; - if (!!status.conditions!.timeout) { - prebuild.state = "timeout"; - prebuild.error = status.conditions!.timeout; - headlessUpdateType = HeadlessWorkspaceEventType.AbortedTimedOut; - } else if (!!status.conditions!.failed) { - prebuild.state = "failed"; - prebuild.error = status.conditions!.failed; - headlessUpdateType = HeadlessWorkspaceEventType.Failed; - } else if (!!status.conditions!.stoppedByRequest) { - prebuild.state = "aborted"; - prebuild.error = "Cancelled"; - headlessUpdateType = HeadlessWorkspaceEventType.Aborted; - } else if (!!status.conditions!.headlessTaskFailed) { - prebuild.state = "available"; - if (status.conditions!.headlessTaskFailed) prebuild.error = status.conditions!.headlessTaskFailed; - prebuild.snapshot = status.conditions!.snapshot; - headlessUpdateType = HeadlessWorkspaceEventType.FinishedButFailed; - } else if (!!status.conditions!.snapshot) { - prebuild.state = "available"; - prebuild.snapshot = status.conditions!.snapshot; - headlessUpdateType = HeadlessWorkspaceEventType.FinishedSuccessfully; - } else { - // stopping event with no clear outcome (i.e. no snapshot yet) - return; - } + const update = this.prebuildStateMapper.mapWorkspaceStatusToPrebuild(status); + if (update) { + const updatedPrebuild = { + ...prebuild, + ...update, + }; - span.setTag("updatePrebuildWorkspace.prebuild.state", prebuild.state); - span.setTag("updatePrebuildWorkspace.prebuild.error", prebuild.error); + span.setTag("updatePrebuildWorkspace.prebuild.state", updatedPrebuild.state); + span.setTag("updatePrebuildWorkspace.prebuild.error", updatedPrebuild.error); if (writeToDB) { - await this.workspaceDB.trace({ span }).storePrebuiltWorkspace(prebuild); + await this.workspaceDB.trace({ span }).storePrebuiltWorkspace(updatedPrebuild); } // notify updates // headless update await this.messagebus.notifyHeadlessUpdate({ span }, userId, workspaceId, { - type: headlessUpdateType, + type: update.type, workspaceID: workspaceId, }); // prebuild info - const info = (await this.workspaceDB.trace({ span }).findPrebuildInfos([prebuild.id]))[0]; + const info = (await this.workspaceDB.trace({ span }).findPrebuildInfos([updatedPrebuild.id]))[0]; if (info) { - this.messagebus.notifyOnPrebuildUpdate({ info, status: prebuild.state }); + this.messagebus.notifyOnPrebuildUpdate({ info, status: updatedPrebuild.state }); } } } catch (e) { diff --git a/components/ws-manager-bridge/mocha.opts b/components/ws-manager-bridge/mocha.opts new file mode 100644 index 00000000000000..972579ee21ef81 --- /dev/null +++ b/components/ws-manager-bridge/mocha.opts @@ -0,0 +1,6 @@ +--require ts-node/register +--require reflect-metadata/Reflect +--require source-map-support/register +--reporter spec +--watch-extensions ts +--exit diff --git a/components/ws-manager-bridge/package.json b/components/ws-manager-bridge/package.json index a1ba86a0f13c38..679af8e998524e 100644 --- a/components/ws-manager-bridge/package.json +++ b/components/ws-manager-bridge/package.json @@ -6,6 +6,7 @@ "scripts": { "start": "node ./dist/src/index.js", "start-ee": "node ./dist/ee/src/index.js", + "test": "mocha --opts mocha.opts './**/*.spec.ts' --exclude './node_modules/**'", "prepare": "yarn clean && yarn build", "build": "npx tsc", "build:clean": "yarn clean && yarn build", @@ -36,8 +37,15 @@ }, "devDependencies": { "@types/amqplib": "^0.8.2", + "@types/chai": "^4.2.21", "@types/express": "^4.17.13", "@types/google-protobuf": "^3.7.4", + "@types/mocha": "^2.2.45", + "chai": "^4.3.4", + "expect": "^1.20.2", + "mocha": "^5.0.0", + "mocha-typescript": "^1.1.11", + "ts-node": "<7.0.0", "tslint": "^5.9.1", "typescript": "~4.4.2", "watch": "^1.0.2" diff --git a/components/ws-manager-bridge/src/container-module.ts b/components/ws-manager-bridge/src/container-module.ts index 00a54077f9470d..bd0f8c09b57823 100644 --- a/components/ws-manager-bridge/src/container-module.ts +++ b/components/ws-manager-bridge/src/container-module.ts @@ -33,6 +33,7 @@ import { MetaInstanceController } from "./meta-instance-controller"; import { IClientCallMetrics } from "@gitpod/content-service/lib/client-call-metrics"; import { PrometheusClientCallMetrics } from "@gitpod/gitpod-protocol/lib/messaging/client-call-metrics"; import { PreparingUpdateEmulator, PreparingUpdateEmulatorFactory } from "./preparing-update-emulator"; +import { PrebuildStateMapper } from "./prebuild-state-mapper"; export const containerModule = new ContainerModule((bind) => { bind(MessagebusConfiguration).toSelf().inSingletonScope(); @@ -80,4 +81,6 @@ export const containerModule = new ContainerModule((bind) => { bind(PreparingUpdateEmulator).toSelf().inRequestScope(); bind(PreparingUpdateEmulatorFactory).toAutoFactory(PreparingUpdateEmulator); + + bind(PrebuildStateMapper).toSelf().inSingletonScope(); }); diff --git a/components/ws-manager-bridge/src/prebuild-state-mapper.spec.ts b/components/ws-manager-bridge/src/prebuild-state-mapper.spec.ts new file mode 100644 index 00000000000000..36b3289c89e411 --- /dev/null +++ b/components/ws-manager-bridge/src/prebuild-state-mapper.spec.ts @@ -0,0 +1,126 @@ +/** + * Copyright (c) 2022 Gitpod GmbH. All rights reserved. + * Licensed under the GNU Affero General Public License (AGPL). + * See License-AGPL.txt in the project root for license information. + */ + +import { suite, test } from "mocha-typescript"; +import * as chai from "chai"; +import { PrebuildStateMapper } from "./prebuild-state-mapper"; +import { WorkspaceConditionBool, WorkspacePhase, WorkspaceStatus } from "@gitpod/ws-manager/lib"; +import { PrebuiltWorkspace } from "@gitpod/gitpod-protocol"; + +const expect = chai.expect; + +@suite +class TestPrebuildStateMapper { + @test public testAll() { + const id = "12345"; + const snapshot = "some-valid-snapshot"; + const failed = "some system error"; + const headlessTaskFailed = "some user/content error"; + + const table: { + name: string; + status: Pick & { + conditions: Partial; + }; + expected: (Omit, "error"> & { hasError?: boolean }) | undefined; + }[] = [ + { + name: "STOPPED", + expected: undefined, + status: { + id, + phase: WorkspacePhase.STOPPED, + conditions: { + snapshot, + }, + }, + }, + { + name: "failed", + expected: { + state: "failed", + hasError: true, + }, + status: { + id, + phase: WorkspacePhase.STOPPING, + conditions: { + failed, + }, + }, + }, + { + name: "failed - no snapshot", + expected: { + state: "failed", + hasError: true, + }, + status: { + id, + phase: WorkspacePhase.STOPPING, + conditions: { + snapshot: "", + }, + }, + }, + { + name: "aborted", + expected: { + state: "aborted", + hasError: true, + }, + status: { + id, + phase: WorkspacePhase.STOPPING, + conditions: { + stoppedByRequest: WorkspaceConditionBool.TRUE, + }, + }, + }, + { + name: "user/content error", + expected: { + state: "available", + hasError: true, + snapshot, + }, + status: { + id, + phase: WorkspacePhase.STOPPING, + conditions: { + headlessTaskFailed, + snapshot, + }, + }, + }, + { + name: "available and no error", + expected: { + state: "available", + hasError: false, + snapshot, + }, + status: { + id, + phase: WorkspacePhase.STOPPING, + conditions: { + snapshot, + }, + }, + }, + ]; + + for (const test of table) { + const cut = new PrebuildStateMapper(); + const actual = cut.mapWorkspaceStatusToPrebuild(test.status as WorkspaceStatus.AsObject); + expect(!!actual?.update.error, test.name + ": hasError").to.be.equal(!!test.expected?.hasError); + delete actual?.update.error; + delete test.expected?.hasError; + expect(actual?.update, test.name).to.deep.equal(test.expected); + } + } +} +module.exports = new TestPrebuildStateMapper(); // Only to circumvent no usage warning :-/ diff --git a/components/ws-manager-bridge/src/prebuild-state-mapper.ts b/components/ws-manager-bridge/src/prebuild-state-mapper.ts new file mode 100644 index 00000000000000..2d4cb308f7d81c --- /dev/null +++ b/components/ws-manager-bridge/src/prebuild-state-mapper.ts @@ -0,0 +1,94 @@ +/** + * Copyright (c) 2022 Gitpod GmbH. All rights reserved. + * Licensed under the GNU Affero General Public License (AGPL). + * See License-AGPL.txt in the project root for license information. + */ + +import { HeadlessWorkspaceEventType, PrebuiltWorkspace } from "@gitpod/gitpod-protocol"; +import { log } from "@gitpod/gitpod-protocol/lib/util/logging"; +import { WorkspacePhase, WorkspaceStatus } from "@gitpod/ws-manager/lib"; +import { injectable } from "inversify"; + +export interface PrebuildUpdate { + type: HeadlessWorkspaceEventType; + update: Partial; +} + +@injectable() +export class PrebuildStateMapper { + mapWorkspaceStatusToPrebuild(status: WorkspaceStatus.AsObject): PrebuildUpdate | undefined { + if (status.phase === WorkspacePhase.STOPPED) { + // Ideally, we'd love to hande STOPPED identical to STOPPING, because we want to assume that all conditions are stable. + // For reliabilies sake we don't do it because experiences shows that unstable conditions are one of the most common sources of errors. + return undefined; + } + + if (status.phase === WorkspacePhase.STOPPING) { + if (!!status.conditions!.timeout) { + return { + type: HeadlessWorkspaceEventType.AbortedTimedOut, + update: { + state: "timeout", + error: status.conditions!.timeout, + }, + }; + } else if (!!status.conditions!.failed) { + return { + type: HeadlessWorkspaceEventType.Failed, + update: { + state: "failed", + error: status.conditions!.failed, + }, + }; + } else if (!!status.conditions!.stoppedByRequest) { + return { + type: HeadlessWorkspaceEventType.Aborted, + update: { + state: "aborted", + error: "Cancelled", + }, + }; + } else if (!!status.conditions!.headlessTaskFailed) { + const result: PrebuildUpdate = { + type: HeadlessWorkspaceEventType.FinishedButFailed, + update: { + state: "available", + snapshot: status.conditions!.snapshot, + error: status.conditions!.headlessTaskFailed, + }, + }; + return result; + } else if (!!status.conditions!.snapshot) { + return { + type: HeadlessWorkspaceEventType.FinishedSuccessfully, + update: { + state: "available", + snapshot: status.conditions!.snapshot, + }, + }; + } else if (!status.conditions!.snapshot) { + // STOPPING && no snapshot? Definitely an error case + return { + type: HeadlessWorkspaceEventType.Failed, + update: { + state: "failed", + error: "error while taking snapshot", + }, + }; + } else { + log.error({ instanceId: status.id }, "unhandled prebuild status update", { + phase: status.phase, + conditions: status.phase, + }); + return undefined; + } + } + + return { + type: HeadlessWorkspaceEventType.Started, + update: { + state: "building", + }, + }; + } +}