Skip to content

[bridge] Extracting updating of prebuilds into PrebuildUpdater (2/3) #10425

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions components/ws-manager-bridge/ee/src/container-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
*/

import { ContainerModule } from "inversify";
import { WorkspaceManagerBridgeEE } from "./bridge";
import { WorkspaceManagerBridge } from "../../src/bridge";
import { PrebuildUpdater } from "../../src/prebuild-updater";
import { PrebuildUpdaterDB } from "./prebuild-updater-db";

export const containerModuleEE = new ContainerModule((bind, unbind, isBound, rebind) => {
rebind(WorkspaceManagerBridge).to(WorkspaceManagerBridgeEE).inRequestScope();
rebind(PrebuildUpdater).to(PrebuildUpdaterDB).inSingletonScope();
});
Original file line number Diff line number Diff line change
@@ -1,24 +1,37 @@
/**
* Copyright (c) 2020 Gitpod GmbH. All rights reserved.
* Copyright (c) 2022 Gitpod GmbH. All rights reserved.
* Licensed under the Gitpod Enterprise Source Code License,
* See License.enterprise.txt in the project root folder.
*/

import { WorkspaceManagerBridge } from "../../src/bridge";
import { inject, injectable } from "inversify";
import { TraceContext } from "@gitpod/gitpod-protocol/lib/util/tracing";
import { WorkspaceStatus, WorkspaceType } from "@gitpod/ws-manager/lib";
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";
import { PrebuildUpdater } from "../../src/prebuild-updater";
import { DBWithTracing, TracedWorkspaceDB } from "@gitpod/gitpod-db/lib/traced-db";
import { WorkspaceDB } from "@gitpod/gitpod-db/lib/workspace-db";
import { MessageBusIntegration } from "../../src/messagebus-integration";
import { PrometheusMetricsExporter } from "../../src/prometheus-metrics-exporter";

@injectable()
export class WorkspaceManagerBridgeEE extends WorkspaceManagerBridge {
export class PrebuildUpdaterDB implements PrebuildUpdater {
@inject(PrebuildStateMapper)
protected readonly prebuildStateMapper: PrebuildStateMapper;

protected async updatePrebuiltWorkspace(
@inject(TracedWorkspaceDB)
protected readonly workspaceDB: DBWithTracing<WorkspaceDB>;

@inject(MessageBusIntegration)
protected readonly messagebus: MessageBusIntegration;

@inject(PrometheusMetricsExporter)
protected readonly prometheusExporter: PrometheusMetricsExporter;

async updatePrebuiltWorkspace(
ctx: TraceContext,
userId: string,
status: WorkspaceStatus.AsObject,
Expand Down Expand Up @@ -91,7 +104,7 @@ export class WorkspaceManagerBridgeEE extends WorkspaceManagerBridge {
}
}

protected async stopPrebuildInstance(ctx: TraceContext, instance: WorkspaceInstance): Promise<void> {
async stopPrebuildInstance(ctx: TraceContext, instance: WorkspaceInstance): Promise<void> {
const span = TraceContext.startSpan("stopPrebuildInstance", ctx);

const prebuild = await this.workspaceDB.trace({}).findPrebuildByWorkspaceID(instance.workspaceId);
Expand Down
21 changes: 6 additions & 15 deletions components/ws-manager-bridge/src/bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import { WorkspaceCluster } from "@gitpod/gitpod-protocol/lib/workspace-cluster"
import { repeat } from "@gitpod/gitpod-protocol/lib/util/repeat";
import { PreparingUpdateEmulator, PreparingUpdateEmulatorFactory } from "./preparing-update-emulator";
import { performance } from "perf_hooks";
import { PrebuildUpdater } from "./prebuild-updater";

export const WorkspaceManagerBridgeFactory = Symbol("WorkspaceManagerBridgeFactory");

Expand Down Expand Up @@ -73,6 +74,9 @@ export class WorkspaceManagerBridge implements Disposable {
@inject(IAnalyticsWriter)
protected readonly analytics: IAnalyticsWriter;

@inject(PrebuildUpdater)
protected readonly prebuildUpdater: PrebuildUpdater;

protected readonly disposables = new DisposableCollection();
protected readonly queues = new Map<string, Queue>();

Expand Down Expand Up @@ -348,7 +352,7 @@ export class WorkspaceManagerBridge implements Disposable {
span.setTag("after", JSON.stringify(instance));

// now notify all prebuild listeners about updates - and update DB if needed
await this.updatePrebuiltWorkspace({ span }, userId, status, writeToDB);
await this.prebuildUpdater.updatePrebuiltWorkspace({ span }, userId, status, writeToDB);

// update volume snapshot information
if (
Expand Down Expand Up @@ -462,24 +466,11 @@ export class WorkspaceManagerBridge implements Disposable {
instance.stoppedTime = new Date().toISOString();
promises.push(this.workspaceDB.trace({}).storeInstance(instance));
promises.push(this.onInstanceStopped({}, ri.workspace.ownerId, instance));
promises.push(this.stopPrebuildInstance(ctx, instance));
promises.push(this.prebuildUpdater.stopPrebuildInstance(ctx, instance));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@geropl I don't think this does anything useful. Looks to me like it would update the workspace instance that got set to 'stopped' a few lines above now to 'aborted'. Am I missing something?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks to me like it would update the workspace instance that got set to 'stopped' a few lines above now to 'aborted'. Am I missing something?

It updates the PrebuiltWorkspace that's corresponding that WorkspaceInstance to aborted, which would otherwise not bet updated.

In what context are you looking into this? Can I help?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It updates the PrebuiltWorkspace that's corresponding that WorkspaceInstance to aborted, which would otherwise not bet updated.

Why does it do that? And how can a prebuild not be finished when there is a workspace based on it?

In what context are you looking into this? Can I help?

Just reading the code and trying to make sense of it 😇

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, got it now. Forget it 😁

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What confused me is that the method is called stopPrebuildInstance while it only aborts the PrebuildWorkspace in the database.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good 😉

}
await Promise.all(promises);
}

protected async updatePrebuiltWorkspace(
ctx: TraceContext,
userId: string,
status: WorkspaceStatus.AsObject,
writeToDB: boolean,
) {
// prebuilds are an EE feature - we just need the hook here
}

protected async stopPrebuildInstance(ctx: TraceContext, instance: WorkspaceInstance): Promise<void> {
// prebuilds are an EE feature - we just need the hook here
}

protected async onInstanceStopped(
ctx: TraceContext,
ownerUserID: string,
Expand Down
2 changes: 2 additions & 0 deletions components/ws-manager-bridge/src/container-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import { IClientCallMetrics } from "@gitpod/content-service/lib/client-call-metr
import { PrometheusClientCallMetrics } from "@gitpod/gitpod-protocol/lib/messaging/client-call-metrics";
import { PreparingUpdateEmulator, PreparingUpdateEmulatorFactory } from "./preparing-update-emulator";
import { PrebuildStateMapper } from "./prebuild-state-mapper";
import { PrebuildUpdater, PrebuildUpdaterNoOp } from "./prebuild-updater";

export const containerModule = new ContainerModule((bind) => {
bind(MessagebusConfiguration).toSelf().inSingletonScope();
Expand Down Expand Up @@ -83,4 +84,5 @@ export const containerModule = new ContainerModule((bind) => {
bind(PreparingUpdateEmulatorFactory).toAutoFactory(PreparingUpdateEmulator);

bind(PrebuildStateMapper).toSelf().inSingletonScope();
bind(PrebuildUpdater).to(PrebuildUpdaterNoOp).inSingletonScope();
});
35 changes: 35 additions & 0 deletions components/ws-manager-bridge/src/prebuild-updater.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/**
* Copyright (c) 2022 Gitpod GmbH. All rights reserved.
* Licensed under the Gitpod Enterprise Source Code License,
* See License.enterprise.txt in the project root folder.
*/

import { TraceContext } from "@gitpod/gitpod-protocol/lib/util/tracing";
import { WorkspaceStatus } from "@gitpod/ws-manager/lib";
import { WorkspaceInstance } from "@gitpod/gitpod-protocol";
import { injectable } from "inversify";

export const PrebuildUpdater = Symbol("PrebuildUpdater");

export interface PrebuildUpdater {
updatePrebuiltWorkspace(
ctx: TraceContext,
userId: string,
status: WorkspaceStatus.AsObject,
writeToDB: boolean,
): Promise<void>;

stopPrebuildInstance(ctx: TraceContext, instance: WorkspaceInstance): Promise<void>;
}

@injectable()
export class PrebuildUpdaterNoOp implements PrebuildUpdater {
async updatePrebuiltWorkspace(
ctx: TraceContext,
userId: string,
status: WorkspaceStatus.AsObject,
writeToDB: boolean,
): Promise<void> {}

async stopPrebuildInstance(ctx: TraceContext, instance: WorkspaceInstance): Promise<void> {}
}