Skip to content

Commit 79506aa

Browse files
easyCZeasyCZ
authored
[ws-man-bridge] Remove non-governing codepaths (#16637)
* [ws-man-bridge] Remove non-governing codepaths * Fix
1 parent e51ff3e commit 79506aa

File tree

7 files changed

+44
-194
lines changed

7 files changed

+44
-194
lines changed

‎components/ws-manager-bridge/ee/src/prebuild-updater-db.ts

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,7 @@ export class PrebuildUpdaterDB implements PrebuildUpdater {
3131
@inject(PrometheusMetricsExporter)
3232
protected readonly prometheusExporter: PrometheusMetricsExporter;
3333

34-
async updatePrebuiltWorkspace(
35-
ctx: TraceContext,
36-
userId: string,
37-
status: WorkspaceStatus.AsObject,
38-
writeToDB: boolean,
39-
) {
34+
async updatePrebuiltWorkspace(ctx: TraceContext, userId: string, status: WorkspaceStatus.AsObject) {
4035
if (status.spec && status.spec.type != WorkspaceType.PREBUILD) {
4136
return;
4237
}
@@ -81,20 +76,16 @@ export class PrebuildUpdaterDB implements PrebuildUpdater {
8176
span.setTag("updatePrebuildWorkspace.prebuild.error", updatedPrebuild.error);
8277

8378
// Here we make sure that we increment the counter only when:
84-
// 1. the instance is governing ("writeToDB"), so that we don't get metrics from multiple pods,
85-
// 2. the state changes (we can receive multiple events with the same state)
79+
// the state changes (we can receive multiple events with the same state)
8680
if (
87-
writeToDB &&
8881
updatedPrebuild.state &&
8982
terminatingStates.includes(updatedPrebuild.state) &&
9083
updatedPrebuild.state !== prebuild.state
9184
) {
9285
this.prometheusExporter.increasePrebuildsCompletedCounter(updatedPrebuild.state);
9386
}
9487

95-
if (writeToDB) {
96-
await this.workspaceDB.trace({ span }).storePrebuiltWorkspace(updatedPrebuild);
97-
}
88+
await this.workspaceDB.trace({ span }).storePrebuiltWorkspace(updatedPrebuild);
9889

9990
// notify updates
10091
// headless update

‎components/ws-manager-bridge/src/bridge.ts

Lines changed: 34 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* See License.AGPL.txt in the project root for license information.
55
*/
66

7-
import { inject, injectable, interfaces } from "inversify";
7+
import { inject, injectable } from "inversify";
88
import { MessageBusIntegration } from "./messagebus-integration";
99
import {
1010
Disposable,
@@ -29,7 +29,6 @@ import { ClientProvider, WsmanSubscriber } from "./wsman-subscriber";
2929
import { Timestamp } from "google-protobuf/google/protobuf/timestamp_pb";
3030
import { Configuration } from "./config";
3131
import { WorkspaceCluster } from "@gitpod/gitpod-protocol/lib/workspace-cluster";
32-
import { PreparingUpdateEmulator, PreparingUpdateEmulatorFactory } from "./preparing-update-emulator";
3332
import { performance } from "perf_hooks";
3433
import { PrebuildUpdater } from "./prebuild-updater";
3534
import { WorkspaceInstanceController } from "./workspace-instance-controller";
@@ -44,7 +43,7 @@ function toBool(b: WorkspaceConditionBool | undefined): boolean | undefined {
4443
return b === WorkspaceConditionBool.TRUE;
4544
}
4645

47-
export type WorkspaceClusterInfo = Pick<WorkspaceCluster, "name" | "url" | "govern">;
46+
export type WorkspaceClusterInfo = Pick<WorkspaceCluster, "name" | "url">;
4847

4948
@injectable()
5049
export class WorkspaceManagerBridge implements Disposable {
@@ -60,9 +59,6 @@ export class WorkspaceManagerBridge implements Disposable {
6059
@inject(Configuration)
6160
protected readonly config: Configuration;
6261

63-
@inject(PreparingUpdateEmulatorFactory)
64-
protected readonly preparingUpdateEmulatorFactory: interfaces.Factory<PreparingUpdateEmulator>;
65-
6662
@inject(IAnalyticsWriter)
6763
protected readonly analytics: IAnalyticsWriter;
6864

@@ -78,57 +74,43 @@ export class WorkspaceManagerBridge implements Disposable {
7874
protected cluster: WorkspaceClusterInfo;
7975

8076
public start(cluster: WorkspaceClusterInfo, clientProvider: ClientProvider) {
81-
const logPayload = { name: cluster.name, url: cluster.url, govern: cluster.govern };
77+
const logPayload = { name: cluster.name, url: cluster.url };
8278
log.info(`Starting bridge to cluster...`, logPayload);
8379
this.cluster = cluster;
8480

85-
const startStatusUpdateHandler = (writeToDB: boolean) => {
81+
const startStatusUpdateHandler = () => {
8682
log.debug(`Starting status update handler: ${cluster.name}`, logPayload);
87-
/* no await */ this.startStatusUpdateHandler(clientProvider, writeToDB, logPayload)
83+
/* no await */ this.startStatusUpdateHandler(clientProvider, logPayload)
8884
// this is a mere safe-guard: we do not expect the code inside to fail
8985
.catch((err) => log.error("Cannot start status update handler", err));
9086
};
9187

92-
if (cluster.govern) {
93-
// notify servers and _update the DB_
94-
startStatusUpdateHandler(true);
95-
96-
// the actual "governing" part
97-
const controllerIntervalSeconds = this.config.controllerIntervalSeconds;
98-
if (controllerIntervalSeconds <= 0) {
99-
throw new Error("controllerIntervalSeconds <= 0!");
100-
}
88+
// notify servers and _update the DB_
89+
startStatusUpdateHandler();
10190

102-
log.debug(`Starting controller: ${cluster.name}`, logPayload);
103-
// Control all workspace instances, either against ws-manager or configured timeouts
104-
this.workspaceInstanceController.start(
105-
cluster.name,
106-
clientProvider,
107-
controllerIntervalSeconds,
108-
this.config.controllerMaxDisconnectSeconds,
109-
);
110-
} else {
111-
// _DO NOT_ update the DB (another bridge is responsible for that)
112-
// Still, listen to all updates, generate/derive new state and distribute it locally!
113-
startStatusUpdateHandler(false);
114-
115-
// emulate WorkspaceInstance updates for all Workspaces in the "preparing" or "building" phase in this cluster
116-
const updateEmulator = this.preparingUpdateEmulatorFactory() as PreparingUpdateEmulator;
117-
this.disposables.push(updateEmulator);
118-
updateEmulator.start(cluster.name);
91+
// the actual "governing" part
92+
const controllerIntervalSeconds = this.config.controllerIntervalSeconds;
93+
if (controllerIntervalSeconds <= 0) {
94+
throw new Error("controllerIntervalSeconds <= 0!");
11995
}
96+
97+
log.debug(`Starting controller: ${cluster.name}`, logPayload);
98+
// Control all workspace instances, either against ws-manager or configured timeouts
99+
this.workspaceInstanceController.start(
100+
cluster.name,
101+
clientProvider,
102+
controllerIntervalSeconds,
103+
this.config.controllerMaxDisconnectSeconds,
104+
);
105+
120106
log.info(`Started bridge to cluster.`, logPayload);
121107
}
122108

123109
public stop() {
124110
this.dispose();
125111
}
126112

127-
protected async startStatusUpdateHandler(
128-
clientProvider: ClientProvider,
129-
writeToDB: boolean,
130-
logPayload: {},
131-
): Promise<void> {
113+
protected async startStatusUpdateHandler(clientProvider: ClientProvider, logPayload: {}): Promise<void> {
132114
const subscriber = new WsmanSubscriber(clientProvider);
133115
this.disposables.push(subscriber);
134116

@@ -138,7 +120,7 @@ export class WorkspaceManagerBridge implements Disposable {
138120
ctx,
139121
sx,
140122
(m) => m.getId(),
141-
(ctx, msg) => this.handleStatusUpdate(ctx, msg, writeToDB),
123+
(ctx, msg) => this.handleStatusUpdate(ctx, msg),
142124
),
143125
);
144126
};
@@ -147,7 +129,7 @@ export class WorkspaceManagerBridge implements Disposable {
147129
ctx,
148130
s,
149131
(msg) => msg.getId(),
150-
(ctx, s) => this.handleStatusUpdate(ctx, s, writeToDB),
132+
(ctx, s) => this.handleStatusUpdate(ctx, s),
151133
);
152134
};
153135
await subscriber.subscribe({ onReconnect, onStatusUpdate }, logPayload);
@@ -172,7 +154,7 @@ export class WorkspaceManagerBridge implements Disposable {
172154
this.queues.set(instanceId, q);
173155
}
174156

175-
protected async handleStatusUpdate(ctx: TraceContext, rawStatus: WorkspaceStatus, writeToDB: boolean) {
157+
protected async handleStatusUpdate(ctx: TraceContext, rawStatus: WorkspaceStatus) {
176158
const start = performance.now();
177159
const status = rawStatus.toObject();
178160
log.info("Handling WorkspaceStatus update", filterStatus(status));
@@ -189,17 +171,12 @@ export class WorkspaceManagerBridge implements Disposable {
189171
};
190172

191173
try {
192-
this.prometheusExporter.reportWorkspaceInstanceUpdateStarted(
193-
writeToDB,
194-
this.cluster.name,
195-
status.spec.type,
196-
);
197-
await this.statusUpdate(ctx, rawStatus, writeToDB);
174+
this.prometheusExporter.reportWorkspaceInstanceUpdateStarted(this.cluster.name, status.spec.type);
175+
await this.statusUpdate(ctx, rawStatus);
198176
} catch (e) {
199177
const durationMs = performance.now() - start;
200178
this.prometheusExporter.reportWorkspaceInstanceUpdateCompleted(
201179
durationMs / 1000,
202-
writeToDB,
203180
this.cluster.name,
204181
status.spec.type,
205182
e,
@@ -210,14 +187,13 @@ export class WorkspaceManagerBridge implements Disposable {
210187
const durationMs = performance.now() - start;
211188
this.prometheusExporter.reportWorkspaceInstanceUpdateCompleted(
212189
durationMs / 1000,
213-
writeToDB,
214190
this.cluster.name,
215191
status.spec.type,
216192
);
217193
log.info(logCtx, "Successfully completed WorkspaceInstance status update");
218194
}
219195

220-
private async statusUpdate(ctx: TraceContext, rawStatus: WorkspaceStatus, writeToDB: boolean) {
196+
private async statusUpdate(ctx: TraceContext, rawStatus: WorkspaceStatus) {
221197
const status = rawStatus.toObject();
222198

223199
if (!status.spec || !status.metadata || !status.conditions) {
@@ -226,7 +202,6 @@ export class WorkspaceManagerBridge implements Disposable {
226202

227203
const span = TraceContext.startSpan("handleStatusUpdate", ctx);
228204
span.setTag("status", JSON.stringify(filterStatus(status)));
229-
span.setTag("writeToDB", writeToDB);
230205
span.setTag("statusVersion", status.statusVersion);
231206
try {
232207
// Beware of the ID mapping here: What's a workspace to the ws-manager is a workspace instance to the rest of the system.
@@ -388,16 +363,14 @@ export class WorkspaceManagerBridge implements Disposable {
388363
span.setTag("after", JSON.stringify(instance));
389364

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

393-
if (writeToDB) {
394-
await this.workspaceDB.trace(ctx).storeInstance(instance);
368+
await this.workspaceDB.trace(ctx).storeInstance(instance);
395369

396-
// cleanup
397-
// important: call this after the DB update
398-
if (!!lifecycleHandler) {
399-
await lifecycleHandler();
400-
}
370+
// cleanup
371+
// important: call this after the DB update
372+
if (!!lifecycleHandler) {
373+
await lifecycleHandler();
401374
}
402375
await this.messagebus.notifyOnInstanceUpdate(ctx, userId, instance);
403376
} catch (e) {

‎components/ws-manager-bridge/src/config.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,6 @@ export interface Configuration {
3535
stoppingPhaseSeconds: number;
3636
};
3737

38-
// emulatePreparingIntervalSeconds configures how often we check for Workspaces in phase "preparing" for clusters we do not govern
39-
emulatePreparingIntervalSeconds: number;
40-
4138
// clusterSyncIntervalSeconds configures how often we sync workspace cluster information
4239
clusterSyncIntervalSeconds: number;
4340
}

‎components/ws-manager-bridge/src/container-module.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ import { IAnalyticsWriter } from "@gitpod/gitpod-protocol/lib/analytics";
3131
import { newAnalyticsWriterFromEnv } from "@gitpod/gitpod-protocol/lib/util/analytics";
3232
import { IClientCallMetrics } from "@gitpod/gitpod-protocol/lib/util/grpc";
3333
import { PrometheusClientCallMetrics } from "@gitpod/gitpod-protocol/lib/messaging/client-call-metrics";
34-
import { PreparingUpdateEmulator, PreparingUpdateEmulatorFactory } from "./preparing-update-emulator";
3534
import { PrebuildStateMapper } from "./prebuild-state-mapper";
3635
import { PrebuildUpdater, PrebuildUpdaterNoOp } from "./prebuild-updater";
3736
import { DebugApp } from "@gitpod/gitpod-protocol/lib/util/debug-app";
@@ -84,9 +83,6 @@ export const containerModule = new ContainerModule((bind) => {
8483

8584
bind(IAnalyticsWriter).toDynamicValue(newAnalyticsWriterFromEnv).inSingletonScope();
8685

87-
bind(PreparingUpdateEmulator).toSelf().inRequestScope();
88-
bind(PreparingUpdateEmulatorFactory).toAutoFactory(PreparingUpdateEmulator);
89-
9086
bind(PrebuildStateMapper).toSelf().inSingletonScope();
9187
bind(PrebuildUpdater).to(PrebuildUpdaterNoOp).inSingletonScope();
9288

‎components/ws-manager-bridge/src/prebuild-updater.ts

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,24 +12,14 @@ import { injectable } from "inversify";
1212
export const PrebuildUpdater = Symbol("PrebuildUpdater");
1313

1414
export interface PrebuildUpdater {
15-
updatePrebuiltWorkspace(
16-
ctx: TraceContext,
17-
userId: string,
18-
status: WorkspaceStatus.AsObject,
19-
writeToDB: boolean,
20-
): Promise<void>;
15+
updatePrebuiltWorkspace(ctx: TraceContext, userId: string, status: WorkspaceStatus.AsObject): Promise<void>;
2116

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

2520
@injectable()
2621
export class PrebuildUpdaterNoOp implements PrebuildUpdater {
27-
async updatePrebuiltWorkspace(
28-
ctx: TraceContext,
29-
userId: string,
30-
status: WorkspaceStatus.AsObject,
31-
writeToDB: boolean,
32-
): Promise<void> {}
22+
async updatePrebuiltWorkspace(ctx: TraceContext, userId: string, status: WorkspaceStatus.AsObject): Promise<void> {}
3323

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

‎components/ws-manager-bridge/src/preparing-update-emulator.ts

Lines changed: 0 additions & 96 deletions
This file was deleted.

0 commit comments

Comments
 (0)