From f88c0f5c34e073380a27a83a6ba1601fda983401 Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Thu, 20 Oct 2022 12:08:44 +0000 Subject: [PATCH 1/7] Extend WorkspaceClusterFilter in gitpod-protocol This reverts commit bc74fb6998c39cce54bf7b91ed0ca7914ec013ab. --- components/gitpod-protocol/src/workspace-cluster.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/components/gitpod-protocol/src/workspace-cluster.ts b/components/gitpod-protocol/src/workspace-cluster.ts index 8725ed730dc052..30c047d038a3eb 100644 --- a/components/gitpod-protocol/src/workspace-cluster.ts +++ b/components/gitpod-protocol/src/workspace-cluster.ts @@ -104,6 +104,7 @@ export interface WorkspaceClusterDB { */ findFiltered(predicate: DeepPartial): Promise; } -export interface WorkspaceClusterFilter extends Pick { +export interface WorkspaceClusterFilter + extends Pick { minScore: number; } From f4a59ce145052cc00ea0fac3863ee65ba5d715ff Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Thu, 20 Oct 2022 10:20:58 +0000 Subject: [PATCH 2/7] Add tests for workspace-cluster-db implementation Check that it's possible to finder workspace clusters by name and application cluster. --- .../src/typeorm/workspace-cluster-db-impl.ts | 6 ++ .../src/workspace-cluster-db.spec.db.ts | 92 +++++++++++++++++++ 2 files changed, 98 insertions(+) create mode 100644 components/gitpod-db/src/workspace-cluster-db.spec.db.ts diff --git a/components/gitpod-db/src/typeorm/workspace-cluster-db-impl.ts b/components/gitpod-db/src/typeorm/workspace-cluster-db-impl.ts index ca0c378b9d17c5..2c117e97e03d8f 100644 --- a/components/gitpod-db/src/typeorm/workspace-cluster-db-impl.ts +++ b/components/gitpod-db/src/typeorm/workspace-cluster-db-impl.ts @@ -59,6 +59,12 @@ export class WorkspaceClusterDBImpl implements WorkspaceClusterDB { .createQueryBuilder("wsc") .select(Object.keys(prototype).map((k) => `wsc.${k}`)) .where("TRUE = TRUE"); // make sure andWhere works + if (predicate.name !== undefined) { + qb = qb.andWhere("wsc.name = :name", predicate); + } + if (predicate.applicationCluster !== undefined) { + qb = qb.andWhere("wsc.applicationCluster = :applicationCluster", predicate); + } if (predicate.state !== undefined) { qb = qb.andWhere("wsc.state = :state", predicate); } diff --git a/components/gitpod-db/src/workspace-cluster-db.spec.db.ts b/components/gitpod-db/src/workspace-cluster-db.spec.db.ts new file mode 100644 index 00000000000000..b3b9116dc7b767 --- /dev/null +++ b/components/gitpod-db/src/workspace-cluster-db.spec.db.ts @@ -0,0 +1,92 @@ +/** + * 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 * as chai from "chai"; +import { suite, test, timeout } from "mocha-typescript"; +import { testContainer } from "./test-container"; +import { TypeORM } from "./typeorm/typeorm"; +import { WorkspaceClusterDB } from "@gitpod/gitpod-protocol/lib/workspace-cluster"; +import { DBWorkspaceCluster } from "./typeorm/entity/db-workspace-cluster"; +const expect = chai.expect; + +@suite +@timeout(5000) +export class WorkspaceClusterDBSpec { + typeORM = testContainer.get(TypeORM); + db = testContainer.get(WorkspaceClusterDB); + + async before() { + await this.clear(); + } + + async after() { + await this.clear(); + } + + protected async clear() { + const connection = await this.typeORM.getConnection(); + const manager = connection.manager; + await manager.clear(DBWorkspaceCluster); + } + + @test public async testFindFilteredByName() { + const wsc1: DBWorkspaceCluster = { + name: "eu71", + applicationCluster: "eu02", + url: "some-url", + state: "available", + score: 100, + maxScore: 100, + govern: true, + }; + const wsc2: DBWorkspaceCluster = { + name: "us71", + applicationCluster: "eu02", + url: "some-url", + state: "cordoned", + score: 0, + maxScore: 0, + govern: false, + }; + + await this.db.save(wsc1); + await this.db.save(wsc2); + + const wscs = await this.db.findFiltered({ name: "eu71" }); + expect(wscs.length).to.equal(1); + expect(wscs[0].name).to.equal("eu71"); + } + + @test public async testFindFilteredByApplicationCluster() { + const wsc1: DBWorkspaceCluster = { + name: "eu71", + applicationCluster: "eu02", + url: "some-url", + state: "available", + score: 100, + maxScore: 100, + govern: true, + }; + const wsc2: DBWorkspaceCluster = { + name: "us71", + applicationCluster: "us02", + url: "some-url", + state: "available", + score: 100, + maxScore: 100, + govern: true, + }; + + await this.db.save(wsc1); + await this.db.save(wsc2); + + const wscs = await this.db.findFiltered({ applicationCluster: "eu02" }); + expect(wscs.length).to.equal(1); + expect(wscs[0].name).to.equal("eu71"); + } +} + +module.exports = WorkspaceClusterDBSpec; From 5cab1d63d0765018f25064eb606e549e9aacce7c Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Fri, 21 Oct 2022 13:45:38 +0000 Subject: [PATCH 3/7] Make client providers filter by app cluster This reverts commit 17531ae098f1098084e58ce1284a635ee7704483. --- .../typescript/src/client-provider-source.ts | 54 ++++++++++--------- 1 file changed, 30 insertions(+), 24 deletions(-) diff --git a/components/ws-manager-api/typescript/src/client-provider-source.ts b/components/ws-manager-api/typescript/src/client-provider-source.ts index 416bf85c4d4096..453b7b92e8540b 100644 --- a/components/ws-manager-api/typescript/src/client-provider-source.ts +++ b/components/ws-manager-api/typescript/src/client-provider-source.ts @@ -3,28 +3,32 @@ * Licensed under the GNU Affero General Public License (AGPL). * See License-AGPL.txt in the project root for license information. */ -import { injectable, inject, multiInject } from 'inversify'; -import { TLSConfig, WorkspaceCluster, WorkspaceClusterDB, WorkspaceClusterWoTLS } from '@gitpod/gitpod-protocol/lib/workspace-cluster'; -import { log } from '@gitpod/gitpod-protocol/lib/util/logging'; +import { injectable, inject, multiInject } from "inversify"; +import { + TLSConfig, + WorkspaceCluster, + WorkspaceClusterDB, + WorkspaceClusterWoTLS, +} from "@gitpod/gitpod-protocol/lib/workspace-cluster"; +import { log } from "@gitpod/gitpod-protocol/lib/util/logging"; export const WorkspaceManagerClientProviderSource = Symbol("WorkspaceManagerClientProviderSource"); export interface WorkspaceManagerClientProviderSource { - getWorkspaceCluster(name: string): Promise; - getAllWorkspaceClusters(): Promise; + getWorkspaceCluster(name: string, applicationCluster: string): Promise; + getAllWorkspaceClusters(applicationCluster: string): Promise; } - @injectable() export class WorkspaceManagerClientProviderEnvSource implements WorkspaceManagerClientProviderSource { protected _clusters: WorkspaceCluster[] | undefined = undefined; - public async getWorkspaceCluster(name: string): Promise { - return this.clusters.find(m => m.name === name); + public async getWorkspaceCluster(name: string, applicationCluster: string): Promise { + return this.clusters.find((m) => m.name === name && m.applicationCluster === applicationCluster); } - public async getAllWorkspaceClusters(): Promise { - return this.clusters; + public async getAllWorkspaceClusters(applicationCluster: string): Promise { + return this.clusters.filter((m) => m.applicationCluster === applicationCluster) ?? []; } protected get clusters(): WorkspaceCluster[] { @@ -40,9 +44,9 @@ export class WorkspaceManagerClientProviderEnvSource implements WorkspaceManager throw new Error("WSMAN_CFG_MANAGERS not set!"); } - const decoded = Buffer.from(configEncoded, 'base64').toString(); + const decoded = Buffer.from(configEncoded, "base64").toString(); const clusters = JSON.parse(decoded) as WorkspaceCluster[]; - return clusters.map(c => { + return clusters.map((c) => { if (!c.tls) { return c; } @@ -53,8 +57,8 @@ export class WorkspaceManagerClientProviderEnvSource implements WorkspaceManager ca: TLSConfig.loadFromBase64File(c.tls.ca), crt: TLSConfig.loadFromBase64File(c.tls.crt), key: TLSConfig.loadFromBase64File(c.tls.key), - } - } + }, + }; }); } } @@ -64,12 +68,12 @@ export class WorkspaceManagerClientProviderDBSource implements WorkspaceManagerC @inject(WorkspaceClusterDB) protected readonly db: WorkspaceClusterDB; - public async getWorkspaceCluster(name: string): Promise { - return await this.db.findByName(name); + public async getWorkspaceCluster(name: string, applicationCluster: string): Promise { + return this.db.findByName(name, applicationCluster); } - public async getAllWorkspaceClusters(): Promise { - return await this.db.findFiltered({}); + public async getAllWorkspaceClusters(applicationCluster: string): Promise { + return await this.db.findFiltered({ applicationCluster }); } } @@ -78,9 +82,9 @@ export class WorkspaceManagerClientProviderCompositeSource implements WorkspaceM @multiInject(WorkspaceManagerClientProviderSource) protected readonly sources: WorkspaceManagerClientProviderSource[]; - async getWorkspaceCluster(name: string): Promise { + async getWorkspaceCluster(name: string, applicationCluster: string): Promise { for (const source of this.sources) { - const info = await source.getWorkspaceCluster(name); + const info = await source.getWorkspaceCluster(name, applicationCluster); if (info !== undefined) { return info; } @@ -88,13 +92,15 @@ export class WorkspaceManagerClientProviderCompositeSource implements WorkspaceM return undefined; } - async getAllWorkspaceClusters(): Promise { + async getAllWorkspaceClusters(applicationCluster: string): Promise { const allClusters: Map = new Map(); for (const source of this.sources) { - const clusters = await source.getAllWorkspaceClusters(); + const clusters = await source.getAllWorkspaceClusters(applicationCluster); for (const cluster of clusters) { if (allClusters.has(cluster.name)) { - log.warn(`${cluster.name} is specified multiple times, overriding with: \n${JSON.stringify(cluster)}`); + log.warn( + `${cluster.name} is specified multiple times, overriding with: \n${JSON.stringify(cluster)}`, + ); } allClusters.set(cluster.name, cluster); } @@ -105,4 +111,4 @@ export class WorkspaceManagerClientProviderCompositeSource implements WorkspaceM } return result; } -} \ No newline at end of file +} From 5ec64298917b0a943818a2e0bf34ffca119410a7 Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Fri, 21 Oct 2022 13:46:21 +0000 Subject: [PATCH 4/7] Pass applicationCluster everywhere Connect the configured installationShortname aka applicationCluster from server, ws-manager-bridge, and the image-builder-api to workspace cluster provider. --- .../image-builder-api/typescript/src/sugar.ts | 9 +++-- .../ee/src/workspace/gitpod-server-impl.ts | 25 ++++++++++--- .../server/src/user/user-deletion-service.ts | 5 ++- .../src/workspace/gitpod-server-impl.ts | 36 +++++++++++++++---- ...ce-cluster-imagebuilder-client-provider.ts | 5 +-- .../workspace/workspace-deletion-service.ts | 9 +++-- .../server/src/workspace/workspace-starter.ts | 23 +++++++++--- .../typescript/src/client-provider.spec.ts | 12 +++++-- .../typescript/src/client-provider.ts | 20 +++++++---- .../src/bridge-controller.ts | 4 +-- .../src/cluster-service-server.ts | 9 +++-- .../src/cluster-sync-service.ts | 10 ++++-- 12 files changed, 128 insertions(+), 39 deletions(-) diff --git a/components/image-builder-api/typescript/src/sugar.ts b/components/image-builder-api/typescript/src/sugar.ts index 848bc57623c2a4..256f3517e54e99 100644 --- a/components/image-builder-api/typescript/src/sugar.ts +++ b/components/image-builder-api/typescript/src/sugar.ts @@ -31,7 +31,12 @@ export const ImageBuilderClientProvider = Symbol("ImageBuilderClientProvider"); // ImageBuilderClientProvider caches image builder connections export interface ImageBuilderClientProvider { - getClient(user: User, workspace: Workspace, instance?: WorkspaceInstance): Promise; + getClient( + applicationCluster: string, + user: User, + workspace: Workspace, + instance?: WorkspaceInstance, + ): Promise; } function withTracing(ctx: TraceContext) { @@ -91,7 +96,7 @@ export class CachingImageBuilderClientProvider implements ImageBuilderClientProv return connection; } - async getClient(user: User, workspace: Workspace, instance?: WorkspaceInstance) { + async getClient(applicationCluster: string, user: User, workspace: Workspace, instance?: WorkspaceInstance) { return this.getDefault(); } diff --git a/components/server/ee/src/workspace/gitpod-server-impl.ts b/components/server/ee/src/workspace/gitpod-server-impl.ts index 196646fa175a82..96f93aec633709 100644 --- a/components/server/ee/src/workspace/gitpod-server-impl.ts +++ b/components/server/ee/src/workspace/gitpod-server-impl.ts @@ -387,7 +387,10 @@ export class GitpodServerEEImpl extends GitpodServerImpl { await this.guardAccess({ kind: "workspaceInstance", subject: runningInstance, workspace: workspace }, "update"); // if any other running instance has a custom timeout other than the user's default, we'll reset that timeout - const client = await this.workspaceManagerClientProvider.get(runningInstance.region); + const client = await this.workspaceManagerClientProvider.get( + runningInstance.region, + this.config.installationShortname, + ); const defaultTimeout = await this.entitlementService.getDefaultWorkspaceTimeout(user, new Date()); const instancesWithReset = runningInstances.filter( (i) => i.workspaceId !== workspaceId && i.status.timeout !== defaultTimeout && i.status.phase === "running", @@ -398,7 +401,10 @@ export class GitpodServerEEImpl extends GitpodServerImpl { req.setId(i.id); req.setDuration(this.userService.workspaceTimeoutToDuration(defaultTimeout)); - const client = await this.workspaceManagerClientProvider.get(i.region); + const client = await this.workspaceManagerClientProvider.get( + i.region, + this.config.installationShortname, + ); return client.setTimeout(ctx, req); }), ); @@ -436,7 +442,10 @@ export class GitpodServerEEImpl extends GitpodServerImpl { const req = new DescribeWorkspaceRequest(); req.setId(runningInstance.id); - const client = await this.workspaceManagerClientProvider.get(runningInstance.region); + const client = await this.workspaceManagerClientProvider.get( + runningInstance.region, + this.config.installationShortname, + ); const desc = await client.describeWorkspace(ctx, req); const duration = this.userService.durationToWorkspaceTimeout(desc.getStatus()!.getSpec()!.getTimeout()); const durationRaw = this.userService.workspaceTimeoutToDuration(duration); @@ -491,7 +500,10 @@ export class GitpodServerEEImpl extends GitpodServerImpl { req.setId(instance.id); req.setLevel(lvlmap.get(level)!); - const client = await this.workspaceManagerClientProvider.get(instance.region); + const client = await this.workspaceManagerClientProvider.get( + instance.region, + this.config.installationShortname, + ); await client.controlAdmission(ctx, req); } @@ -517,7 +529,10 @@ export class GitpodServerEEImpl extends GitpodServerImpl { } await this.guardAccess({ kind: "workspaceInstance", subject: instance, workspace }, "get"); - const client = await this.workspaceManagerClientProvider.get(instance.region); + const client = await this.workspaceManagerClientProvider.get( + instance.region, + this.config.installationShortname, + ); const request = new TakeSnapshotRequest(); request.setId(instance.id); request.setReturnImmediately(true); diff --git a/components/server/src/user/user-deletion-service.ts b/components/server/src/user/user-deletion-service.ts index 9645bde4e0ecab..64fe0c3a465168 100644 --- a/components/server/src/user/user-deletion-service.ts +++ b/components/server/src/user/user-deletion-service.ts @@ -122,7 +122,10 @@ export class UserDeletionService { req.setPolicy(StopWorkspacePolicy.NORMALLY); try { - const manager = await this.workspaceManagerClientProvider.get(wsi.region); + const manager = await this.workspaceManagerClientProvider.get( + wsi.region, + this.config.installationShortname, + ); await manager.stopWorkspace({}, req); } catch (err) { log.debug( diff --git a/components/server/src/workspace/gitpod-server-impl.ts b/components/server/src/workspace/gitpod-server-impl.ts index 9d34e1092fbef5..82d058c719e8c3 100644 --- a/components/server/src/workspace/gitpod-server-impl.ts +++ b/components/server/src/workspace/gitpod-server-impl.ts @@ -931,7 +931,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { req.setId(instanceId); req.setClosed(wasClosed); - const client = await this.workspaceManagerClientProvider.get(wsi.region); + const client = await this.workspaceManagerClientProvider.get(wsi.region, this.config.installationShortname); await client.markActive(ctx, req); if (options && options.roundTripTime && Number.isFinite(options.roundTripTime)) { @@ -1473,7 +1473,10 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { const req = new DescribeWorkspaceRequest(); req.setId(instance.id); - const client = await this.workspaceManagerClientProvider.get(instance.region); + const client = await this.workspaceManagerClientProvider.get( + instance.region, + this.config.installationShortname, + ); const desc = await client.describeWorkspace(ctx, req); if (!desc.hasStatus()) { @@ -1526,7 +1529,10 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { req.setExpose(true); try { - const client = await this.workspaceManagerClientProvider.get(runningInstance.region); + const client = await this.workspaceManagerClientProvider.get( + runningInstance.region, + this.config.installationShortname, + ); await client.controlPort(ctx, req); } catch (e) { throw this.mapGrpcError(e); @@ -1578,7 +1584,10 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { req.setSpec(spec); req.setExpose(false); - const client = await this.workspaceManagerClientProvider.get(instance.region); + const client = await this.workspaceManagerClientProvider.get( + instance.region, + this.config.installationShortname, + ); await client.controlPort(ctx, req); } @@ -1985,7 +1994,10 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { const req = new UpdateSSHKeyRequest(); req.setId(instance.id); req.setKeysList(keys); - const cli = await this.workspaceManagerClientProvider.get(instance.region); + const cli = await this.workspaceManagerClientProvider.get( + instance.region, + this.config.installationShortname, + ); await cli.updateSSHPublicKey(ctx, req); } catch (err) { const logCtx = { userId, instanceId: instance.id }; @@ -3184,9 +3196,19 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { }, ); if (isMovedImageBuilder) { - return this.wsClusterImageBuilderClientProvider.getClient(user, workspace, instance); + return this.wsClusterImageBuilderClientProvider.getClient( + this.config.installationShortname, + user, + workspace, + instance, + ); } else { - return this.imagebuilderClientProvider.getClient(user, workspace, instance); + return this.imagebuilderClientProvider.getClient( + this.config.installationShortname, + user, + workspace, + instance, + ); } } diff --git a/components/server/src/workspace/workspace-cluster-imagebuilder-client-provider.ts b/components/server/src/workspace/workspace-cluster-imagebuilder-client-provider.ts index 4cb2c40ec51073..3961e83965b907 100644 --- a/components/server/src/workspace/workspace-cluster-imagebuilder-client-provider.ts +++ b/components/server/src/workspace/workspace-cluster-imagebuilder-client-provider.ts @@ -32,13 +32,14 @@ export class WorkspaceClusterImagebuilderClientProvider implements ImageBuilderC protected readonly connectionCache = new Map(); async getClient( + applicationCluster: string, user: ExtendedUser, workspace: Workspace, instance: WorkspaceInstance, ): Promise { - const clusters = await this.clientProvider.getStartClusterSets(user, workspace, instance); + const clusters = await this.clientProvider.getStartClusterSets(applicationCluster, user, workspace, instance); for await (let cluster of clusters) { - const info = await this.source.getWorkspaceCluster(cluster.installation); + const info = await this.source.getWorkspaceCluster(cluster.installation, applicationCluster); if (!info) { continue; } diff --git a/components/server/src/workspace/workspace-deletion-service.ts b/components/server/src/workspace/workspace-deletion-service.ts index e4af8f9398464d..e6186d624459fe 100644 --- a/components/server/src/workspace/workspace-deletion-service.ts +++ b/components/server/src/workspace/workspace-deletion-service.ts @@ -145,7 +145,9 @@ export class WorkspaceDeletionService { const span = TraceContext.startSpan("garbageCollectVolumeSnapshot", ctx); try { - const allClusters = await this.workspaceManagerClientProvider.getAllWorkspaceClusters(); + const allClusters = await this.workspaceManagerClientProvider.getAllWorkspaceClusters( + this.config.installationShortname, + ); // we need to do two things here: // 1. we want to delete volume snapshot object from all workspace clusters // 2. we want to delete cloud provider source snapshot @@ -154,7 +156,10 @@ export class WorkspaceDeletionService { let availableClusters = allClusters.filter((c) => c.state === "available"); for (let cluster of availableClusters) { - const client = await this.workspaceManagerClientProvider.get(cluster.name); + const client = await this.workspaceManagerClientProvider.get( + cluster.name, + this.config.installationShortname, + ); const req = new DeleteVolumeSnapshotRequest(); req.setId(vs.id); req.setVolumeHandle(vs.volumeHandle); diff --git a/components/server/src/workspace/workspace-starter.ts b/components/server/src/workspace/workspace-starter.ts index fe058aab3f09a5..2a334fc2ed6dca 100644 --- a/components/server/src/workspace/workspace-starter.ts +++ b/components/server/src/workspace/workspace-starter.ts @@ -441,7 +441,7 @@ export class WorkspaceStarter { req.setId(instanceId); req.setPolicy(policy || StopWorkspacePolicy.NORMALLY); - const client = await this.clientProvider.get(instanceRegion); + const client = await this.clientProvider.get(instanceRegion, this.config.installationShortname); await client.stopWorkspace(ctx, req); } @@ -617,7 +617,12 @@ export class WorkspaceStarter { instance: WorkspaceInstance, ): Promise { let lastInstallation = ""; - const clusters = await this.clientProvider.getStartClusterSets(euser, workspace, instance); + const clusters = await this.clientProvider.getStartClusterSets( + this.config.installationShortname, + euser, + workspace, + instance, + ); for await (let cluster of clusters) { try { // getStartManager will throw an exception if there's no cluster available and hence exit the loop @@ -1974,9 +1979,19 @@ export class WorkspaceStarter { }, ); if (isMovedImageBuilder) { - return this.wsClusterImageBuilderClientProvider.getClient(user, workspace, instance); + return this.wsClusterImageBuilderClientProvider.getClient( + this.config.installationShortname, + user, + workspace, + instance, + ); } else { - return this.imagebuilderClientProvider.getClient(user, workspace, instance); + return this.imagebuilderClientProvider.getClient( + this.config.installationShortname, + user, + workspace, + instance, + ); } } } diff --git a/components/ws-manager-api/typescript/src/client-provider.spec.ts b/components/ws-manager-api/typescript/src/client-provider.spec.ts index 845b8573281e63..edada5b0d01056 100644 --- a/components/ws-manager-api/typescript/src/client-provider.spec.ts +++ b/components/ws-manager-api/typescript/src/client-provider.spec.ts @@ -127,7 +127,11 @@ class TestClientProvider { this.provider = c.get(WorkspaceManagerClientProvider); // we don't actually want to try and connect here - this.provider.get = async (name: string, grpcOptions?: object): Promise => { + this.provider.get = async ( + name: string, + applicationCluster: string, + grpcOptions?: object, + ): Promise => { return {} as PromisifiedWorkspaceManagerClient; }; } @@ -136,12 +140,13 @@ class TestClientProvider { public async getStartClusterSets() { await this.expectInstallations( [["a2", "a3"]], - await this.provider.getStartClusterSets({} as User, {} as Workspace, {} as WorkspaceInstance), + await this.provider.getStartClusterSets("xx01", {} as User, {} as Workspace, {} as WorkspaceInstance), "default case", ); await this.expectInstallations( [["con1"], ["a2", "a3", "con1"]], await this.provider.getStartClusterSets( + "xx01", { rolesOrPermissions: ["new-workspace-cluster"] } as User, {} as Workspace, {} as WorkspaceInstance, @@ -151,6 +156,7 @@ class TestClientProvider { await this.expectInstallations( [["a2", "a3", "con2"]], await this.provider.getStartClusterSets( + "xx01", { rolesOrPermissions: ["monitor"] } as User, {} as Workspace, {} as WorkspaceInstance, @@ -159,7 +165,7 @@ class TestClientProvider { ); await this.expectInstallations( [["a2", "a3"]], - await this.provider.getStartClusterSets({} as User, {} as Workspace, {} as WorkspaceInstance), + await this.provider.getStartClusterSets("xx01", {} as User, {} as Workspace, {} as WorkspaceInstance), "cluster has permission w/o precedence, user NOT", ); } diff --git a/components/ws-manager-api/typescript/src/client-provider.ts b/components/ws-manager-api/typescript/src/client-provider.ts index f0c5ab71c9625f..eb4d12275611f9 100644 --- a/components/ws-manager-api/typescript/src/client-provider.ts +++ b/components/ws-manager-api/typescript/src/client-provider.ts @@ -46,11 +46,12 @@ export class WorkspaceManagerClientProvider implements Disposable { * @returns a set of workspace clusters we can start the workspace in */ public async getStartClusterSets( + applicationCluster: string, user: ExtendedUser, workspace: Workspace, instance: WorkspaceInstance, ): Promise { - const allClusters = await this.source.getAllWorkspaceClusters(); + const allClusters = await this.source.getAllWorkspaceClusters(applicationCluster); const availableClusters = allClusters.filter((c) => c.score > 0 && c.state === "available"); const sets = workspaceClusterSetsAuthorized @@ -59,7 +60,7 @@ export class WorkspaceManagerClientProvider implements Disposable { if (!r) { return; } - return new ClusterSet(this, r); + return new ClusterSet(this, r, applicationCluster); }) .filter((s) => s !== undefined) as ClusterSet[]; @@ -90,9 +91,13 @@ export class WorkspaceManagerClientProvider implements Disposable { * @param name * @returns The WorkspaceManagerClient identified by the name. Throws an error if there is none. */ - public async get(name: string, grpcOptions?: object): Promise { + public async get( + name: string, + applicationCluster: string, + grpcOptions?: object, + ): Promise { const getConnectionInfo = async () => { - const cluster = await this.source.getWorkspaceCluster(name); + const cluster = await this.source.getWorkspaceCluster(name, applicationCluster); if (!cluster) { throw new Error(`Unknown workspace manager \"${name}\"`); } @@ -130,8 +135,8 @@ export class WorkspaceManagerClientProvider implements Disposable { /** * @returns All WorkspaceClusters (without TLS config) */ - public async getAllWorkspaceClusters(): Promise { - return this.source.getAllWorkspaceClusters(); + public async getAllWorkspaceClusters(applicationCluster: string): Promise { + return this.source.getAllWorkspaceClusters(applicationCluster); } public createConnection( @@ -178,6 +183,7 @@ class ClusterSet implements AsyncIterator { constructor( protected readonly provider: WorkspaceManagerClientProvider, protected readonly cluster: WorkspaceClusterWoTLS[], + protected readonly applicationCluster: string, ) {} public async next(): Promise> { @@ -192,7 +198,7 @@ class ClusterSet implements AsyncIterator { const grpcOptions: grpc.ClientOptions = { ...defaultGRPCOptions, }; - const client = await this.provider.get(chosenCluster.name, grpcOptions); + const client = await this.provider.get(chosenCluster.name, this.applicationCluster, grpcOptions); return { done: false, value: { diff --git a/components/ws-manager-bridge/src/bridge-controller.ts b/components/ws-manager-bridge/src/bridge-controller.ts index 1cd1706dac1ead..12a823ad7a5a16 100644 --- a/components/ws-manager-bridge/src/bridge-controller.ts +++ b/components/ws-manager-bridge/src/bridge-controller.ts @@ -100,14 +100,14 @@ export class BridgeController { ...defaultGRPCOptions, }; const clientProvider = async () => { - return this.clientProvider.get(cluster.name, grpcOptions); + return this.clientProvider.get(cluster.name, this.config.installation, grpcOptions); }; bridge.start(cluster, clientProvider); return bridge; } protected async getAllWorkspaceClusters(): Promise> { - const allInfos = await this.clientProvider.getAllWorkspaceClusters(); + const allInfos = await this.clientProvider.getAllWorkspaceClusters(this.config.installation); const result: Map = new Map(); for (const cluster of allInfos) { result.set(cluster.name, cluster); diff --git a/components/ws-manager-bridge/src/cluster-service-server.ts b/components/ws-manager-bridge/src/cluster-service-server.ts index ca70fd999ecce6..03a6d4b4f6c7a2 100644 --- a/components/ws-manager-bridge/src/cluster-service-server.ts +++ b/components/ws-manager-bridge/src/cluster-service-server.ts @@ -163,7 +163,12 @@ export class ClusterService implements IClusterServiceServer { {}, ); if (enabled) { - let classConstraints = await getSupportedWorkspaceClasses(this.clientProvider, newCluster, false); + let classConstraints = await getSupportedWorkspaceClasses( + this.clientProvider, + newCluster, + this.config.installation, + false, + ); newCluster.admissionConstraints = admissionConstraints.concat(classConstraints); } else { // try to connect to validate the config. Throws an exception if it fails. @@ -305,7 +310,7 @@ export class ClusterService implements IClusterServiceServer { dbClusterIdx.set(cluster.name, true); } - const allCluster = await this.allClientProvider.getAllWorkspaceClusters(); + const allCluster = await this.allClientProvider.getAllWorkspaceClusters(this.config.installation); for (const cluster of allCluster) { if (dbClusterIdx.get(cluster.name)) { continue; diff --git a/components/ws-manager-bridge/src/cluster-sync-service.ts b/components/ws-manager-bridge/src/cluster-sync-service.ts index 0c40de943ec9b1..db2ce5d802272d 100644 --- a/components/ws-manager-bridge/src/cluster-sync-service.ts +++ b/components/ws-manager-bridge/src/cluster-sync-service.ts @@ -51,7 +51,12 @@ export class ClusterSyncService { let allClusters = await this.clusterDB.findFiltered({}); for (const cluster of allClusters) { try { - let supportedClasses = await getSupportedWorkspaceClasses(this.clientProvider, cluster, true); + let supportedClasses = await getSupportedWorkspaceClasses( + this.clientProvider, + cluster, + this.config.installation, + true, + ); let existingOtherConstraints = cluster.admissionConstraints?.filter((c) => c.type !== "has-class"); cluster.admissionConstraints = existingOtherConstraints?.concat(supportedClasses); await this.clusterDB.save(cluster); @@ -70,6 +75,7 @@ export class ClusterSyncService { export async function getSupportedWorkspaceClasses( clientProvider: WorkspaceManagerClientProvider, cluster: WorkspaceCluster, + applicationCluster: string, useCache: boolean, ) { let constraints = await new Promise(async (resolve, reject) => { @@ -78,7 +84,7 @@ export async function getSupportedWorkspaceClasses( }; let client = useCache ? await ( - await clientProvider.get(cluster.name, grpcOptions) + await clientProvider.get(cluster.name, applicationCluster, grpcOptions) ).client : clientProvider.createConnection(WorkspaceManagerClient, cluster, grpcOptions); From a468b60dc83ec26a60fbdc07cc0df51361e9aa5f Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Fri, 21 Oct 2022 15:05:21 +0000 Subject: [PATCH 5/7] Make applicationCluster mandatory when filtering --- .../gitpod-db/src/typeorm/workspace-cluster-db-impl.ts | 4 ++-- .../gitpod-db/src/workspace-cluster-db.spec.db.ts | 2 +- components/gitpod-protocol/src/workspace-cluster.ts | 10 +++++----- .../ws-manager-bridge/src/cluster-service-server.ts | 9 +++++++-- .../ws-manager-bridge/src/cluster-sync-service.ts | 2 +- 5 files changed, 16 insertions(+), 11 deletions(-) diff --git a/components/gitpod-db/src/typeorm/workspace-cluster-db-impl.ts b/components/gitpod-db/src/typeorm/workspace-cluster-db-impl.ts index 2c117e97e03d8f..173a3cd544ceb9 100644 --- a/components/gitpod-db/src/typeorm/workspace-cluster-db-impl.ts +++ b/components/gitpod-db/src/typeorm/workspace-cluster-db-impl.ts @@ -4,7 +4,7 @@ * See License-AGPL.txt in the project root for license information. */ -import { Repository, EntityManager, DeepPartial } from "typeorm"; +import { Repository, EntityManager } from "typeorm"; import { injectable, inject } from "inversify"; import { TypeORM } from "./typeorm"; import { WorkspaceClusterDB } from "../workspace-cluster-db"; @@ -42,7 +42,7 @@ export class WorkspaceClusterDBImpl implements WorkspaceClusterDB { return repo.findOne(name); } - async findFiltered(predicate: DeepPartial): Promise { + async findFiltered(predicate: WorkspaceClusterFilter): Promise { const prototype: WorkspaceClusterWoTLS = { name: "", url: "", diff --git a/components/gitpod-db/src/workspace-cluster-db.spec.db.ts b/components/gitpod-db/src/workspace-cluster-db.spec.db.ts index b3b9116dc7b767..bb4c8a529fc39f 100644 --- a/components/gitpod-db/src/workspace-cluster-db.spec.db.ts +++ b/components/gitpod-db/src/workspace-cluster-db.spec.db.ts @@ -55,7 +55,7 @@ export class WorkspaceClusterDBSpec { await this.db.save(wsc1); await this.db.save(wsc2); - const wscs = await this.db.findFiltered({ name: "eu71" }); + const wscs = await this.db.findFiltered({ name: "eu71", applicationCluster: "eu02" }); expect(wscs.length).to.equal(1); expect(wscs[0].name).to.equal("eu71"); } diff --git a/components/gitpod-protocol/src/workspace-cluster.ts b/components/gitpod-protocol/src/workspace-cluster.ts index 30c047d038a3eb..ca6e07fc360aaf 100644 --- a/components/gitpod-protocol/src/workspace-cluster.ts +++ b/components/gitpod-protocol/src/workspace-cluster.ts @@ -102,9 +102,9 @@ export interface WorkspaceClusterDB { * Lists all WorkspaceClusterWoTls for which the given predicate is true (does not return TLS for size/speed concerns) * @param predicate */ - findFiltered(predicate: DeepPartial): Promise; -} -export interface WorkspaceClusterFilter - extends Pick { - minScore: number; + findFiltered(predicate: WorkspaceClusterFilter): Promise; } + +export type WorkspaceClusterFilter = Pick & + DeepPartial> & + Partial<{ minScore: number }>; diff --git a/components/ws-manager-bridge/src/cluster-service-server.ts b/components/ws-manager-bridge/src/cluster-service-server.ts index 03a6d4b4f6c7a2..cfaf30f2ee3375 100644 --- a/components/ws-manager-bridge/src/cluster-service-server.ts +++ b/components/ws-manager-bridge/src/cluster-service-server.ts @@ -88,7 +88,10 @@ export class ClusterService implements IClusterServiceServer { const req = call.request.toObject(); const clusterByNamePromise = this.clusterDB.findByName(req.name); - const clusterByUrlPromise = this.clusterDB.findFiltered({ url: req.url }); + const clusterByUrlPromise = this.clusterDB.findFiltered({ + url: req.url, + applicationCluster: this.config.installation, + }); const [clusterByName, clusterByUrl] = await Promise.all([clusterByNamePromise, clusterByUrlPromise]); @@ -303,7 +306,9 @@ export class ClusterService implements IClusterServiceServer { const response = new ListResponse(); const dbClusterIdx = new Map(); - const allDBClusters = await this.clusterDB.findFiltered({}); + const allDBClusters = await this.clusterDB.findFiltered({ + applicationCluster: this.config.installation, + }); for (const cluster of allDBClusters) { const clusterStatus = convertToGRPC(cluster); response.addStatus(clusterStatus); diff --git a/components/ws-manager-bridge/src/cluster-sync-service.ts b/components/ws-manager-bridge/src/cluster-sync-service.ts index db2ce5d802272d..41c08a86aab56e 100644 --- a/components/ws-manager-bridge/src/cluster-sync-service.ts +++ b/components/ws-manager-bridge/src/cluster-sync-service.ts @@ -48,7 +48,7 @@ export class ClusterSyncService { } log.debug("reconciling workspace classes..."); - let allClusters = await this.clusterDB.findFiltered({}); + let allClusters = await this.clusterDB.findFiltered({ applicationCluster: this.config.installation }); for (const cluster of allClusters) { try { let supportedClasses = await getSupportedWorkspaceClasses( From 062d201df0e33b3fc20f3ee5c0d7d43748051982 Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Mon, 24 Oct 2022 06:59:25 +0000 Subject: [PATCH 6/7] Make `findByName` take an applicationCluster Finding a workspace cluster by name only makes sense in the context of a particular workspace cluster. --- .../src/typeorm/workspace-cluster-db-impl.ts | 4 +-- .../src/workspace-cluster-db.spec.db.ts | 30 ++++++++++++++++++- .../gitpod-protocol/src/workspace-cluster.ts | 2 +- .../src/cluster-service-server.ts | 4 +-- 4 files changed, 34 insertions(+), 6 deletions(-) diff --git a/components/gitpod-db/src/typeorm/workspace-cluster-db-impl.ts b/components/gitpod-db/src/typeorm/workspace-cluster-db-impl.ts index 173a3cd544ceb9..85519a605fc0d2 100644 --- a/components/gitpod-db/src/typeorm/workspace-cluster-db-impl.ts +++ b/components/gitpod-db/src/typeorm/workspace-cluster-db-impl.ts @@ -37,9 +37,9 @@ export class WorkspaceClusterDBImpl implements WorkspaceClusterDB { await repo.delete(name); } - async findByName(name: string): Promise { + async findByName(name: string, applicationCluster: string): Promise { const repo = await this.getRepo(); - return repo.findOne(name); + return repo.findOne({ name, applicationCluster }); } async findFiltered(predicate: WorkspaceClusterFilter): Promise { diff --git a/components/gitpod-db/src/workspace-cluster-db.spec.db.ts b/components/gitpod-db/src/workspace-cluster-db.spec.db.ts index bb4c8a529fc39f..06b6fa3e589f75 100644 --- a/components/gitpod-db/src/workspace-cluster-db.spec.db.ts +++ b/components/gitpod-db/src/workspace-cluster-db.spec.db.ts @@ -8,7 +8,7 @@ import * as chai from "chai"; import { suite, test, timeout } from "mocha-typescript"; import { testContainer } from "./test-container"; import { TypeORM } from "./typeorm/typeorm"; -import { WorkspaceClusterDB } from "@gitpod/gitpod-protocol/lib/workspace-cluster"; +import { WorkspaceCluster, WorkspaceClusterDB } from "@gitpod/gitpod-protocol/lib/workspace-cluster"; import { DBWorkspaceCluster } from "./typeorm/entity/db-workspace-cluster"; const expect = chai.expect; @@ -32,6 +32,34 @@ export class WorkspaceClusterDBSpec { await manager.clear(DBWorkspaceCluster); } + @test public async findByName() { + const wsc1: DBWorkspaceCluster = { + name: "eu71", + applicationCluster: "eu02", + url: "some-url", + state: "available", + score: 100, + maxScore: 100, + govern: true, + }; + const wsc2: DBWorkspaceCluster = { + name: "us71", + applicationCluster: "eu02", + url: "some-url", + state: "cordoned", + score: 0, + maxScore: 0, + govern: false, + }; + + await this.db.save(wsc1); + await this.db.save(wsc2); + + const wsc = await this.db.findByName("eu71", "eu02"); + expect(wsc).not.to.be.undefined; + expect((wsc as WorkspaceCluster).name).to.equal("eu71"); + } + @test public async testFindFilteredByName() { const wsc1: DBWorkspaceCluster = { name: "eu71", diff --git a/components/gitpod-protocol/src/workspace-cluster.ts b/components/gitpod-protocol/src/workspace-cluster.ts index ca6e07fc360aaf..83190c3eee3c34 100644 --- a/components/gitpod-protocol/src/workspace-cluster.ts +++ b/components/gitpod-protocol/src/workspace-cluster.ts @@ -96,7 +96,7 @@ export interface WorkspaceClusterDB { * Finds a WorkspaceCluster with the given name. If there is none, `undefined` is returned. * @param name */ - findByName(name: string): Promise; + findByName(name: string, applicationCluster: string): Promise; /** * Lists all WorkspaceClusterWoTls for which the given predicate is true (does not return TLS for size/speed concerns) diff --git a/components/ws-manager-bridge/src/cluster-service-server.ts b/components/ws-manager-bridge/src/cluster-service-server.ts index cfaf30f2ee3375..d62800f7dbac29 100644 --- a/components/ws-manager-bridge/src/cluster-service-server.ts +++ b/components/ws-manager-bridge/src/cluster-service-server.ts @@ -87,7 +87,7 @@ export class ClusterService implements IClusterServiceServer { // check if the name or URL are already registered/in use const req = call.request.toObject(); - const clusterByNamePromise = this.clusterDB.findByName(req.name); + const clusterByNamePromise = this.clusterDB.findByName(req.name, this.config.installation); const clusterByUrlPromise = this.clusterDB.findFiltered({ url: req.url, applicationCluster: this.config.installation, @@ -211,7 +211,7 @@ export class ClusterService implements IClusterServiceServer { this.queue.enqueue(async () => { try { const req = call.request.toObject(); - const cluster = await this.clusterDB.findByName(req.name); + const cluster = await this.clusterDB.findByName(req.name, this.config.installation); if (!cluster) { throw new GRPCError( grpc.status.NOT_FOUND, From 3cdaa1d6637204f58216b4648b63ffd1727cbbe2 Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Mon, 24 Oct 2022 07:01:42 +0000 Subject: [PATCH 7/7] Make `deleteByName` take an applicationCluster Deleting a workspace cluster by name only makes sense in the context of a particular workspace cluster. --- .../src/typeorm/workspace-cluster-db-impl.ts | 4 +- .../src/workspace-cluster-db.spec.db.ts | 46 +++++++++++++++++-- .../gitpod-protocol/src/workspace-cluster.ts | 2 +- .../src/cluster-service-server.ts | 2 +- 4 files changed, 47 insertions(+), 7 deletions(-) diff --git a/components/gitpod-db/src/typeorm/workspace-cluster-db-impl.ts b/components/gitpod-db/src/typeorm/workspace-cluster-db-impl.ts index 85519a605fc0d2..40df731a32dfdb 100644 --- a/components/gitpod-db/src/typeorm/workspace-cluster-db-impl.ts +++ b/components/gitpod-db/src/typeorm/workspace-cluster-db-impl.ts @@ -32,9 +32,9 @@ export class WorkspaceClusterDBImpl implements WorkspaceClusterDB { await repo.save(cluster); } - async deleteByName(name: string): Promise { + async deleteByName(name: string, applicationCluster: string): Promise { const repo = await this.getRepo(); - await repo.delete(name); + await repo.delete({ name, applicationCluster }); } async findByName(name: string, applicationCluster: string): Promise { diff --git a/components/gitpod-db/src/workspace-cluster-db.spec.db.ts b/components/gitpod-db/src/workspace-cluster-db.spec.db.ts index 06b6fa3e589f75..33c9bc6196c950 100644 --- a/components/gitpod-db/src/workspace-cluster-db.spec.db.ts +++ b/components/gitpod-db/src/workspace-cluster-db.spec.db.ts @@ -55,9 +55,49 @@ export class WorkspaceClusterDBSpec { await this.db.save(wsc1); await this.db.save(wsc2); - const wsc = await this.db.findByName("eu71", "eu02"); - expect(wsc).not.to.be.undefined; - expect((wsc as WorkspaceCluster).name).to.equal("eu71"); + // Can find the eu71 cluster as seen by the eu02 application cluster. + const result = await this.db.findByName("eu71", "eu02"); + expect(result).not.to.be.undefined; + expect((result as WorkspaceCluster).name).to.equal("eu71"); + + // Can't find the eu71 cluster as seen by the us02 application cluster. + // (no record in the db for that (ws-cluster, app-cluster) combination). + const result2 = await this.db.findByName("eu71", "us02"); + expect(result2).to.be.undefined; + + // Can find the us71 cluster as seen by the eu02 application cluster. + const result3 = await this.db.findByName("us71", "eu02"); + expect(result3).not.to.be.undefined; + expect((result3 as WorkspaceCluster).name).to.equal("us71"); + } + + @test public async deleteByName() { + const wsc1: DBWorkspaceCluster = { + name: "eu71", + applicationCluster: "eu02", + url: "some-url", + state: "available", + score: 100, + maxScore: 100, + govern: true, + }; + const wsc2: DBWorkspaceCluster = { + name: "us71", + applicationCluster: "eu02", + url: "some-url", + state: "cordoned", + score: 0, + maxScore: 0, + govern: false, + }; + + await this.db.save(wsc1); + await this.db.save(wsc2); + + // Can delete the eu71 cluster as seen by the eu02 application cluster. + await this.db.deleteByName("eu71", "eu02"); + expect(await this.db.findByName("eu71", "eu02")).to.be.undefined; + expect(await this.db.findByName("us71", "eu02")).not.to.be.undefined; } @test public async testFindFilteredByName() { diff --git a/components/gitpod-protocol/src/workspace-cluster.ts b/components/gitpod-protocol/src/workspace-cluster.ts index 83190c3eee3c34..0c0331f026787a 100644 --- a/components/gitpod-protocol/src/workspace-cluster.ts +++ b/components/gitpod-protocol/src/workspace-cluster.ts @@ -90,7 +90,7 @@ export interface WorkspaceClusterDB { * Deletes the cluster identified by this name, if any. * @param name */ - deleteByName(name: string): Promise; + deleteByName(name: string, applicationCluster: string): Promise; /** * Finds a WorkspaceCluster with the given name. If there is none, `undefined` is returned. diff --git a/components/ws-manager-bridge/src/cluster-service-server.ts b/components/ws-manager-bridge/src/cluster-service-server.ts index d62800f7dbac29..b0816f79997a4d 100644 --- a/components/ws-manager-bridge/src/cluster-service-server.ts +++ b/components/ws-manager-bridge/src/cluster-service-server.ts @@ -288,7 +288,7 @@ export class ClusterService implements IClusterServiceServer { ); } - await this.clusterDB.deleteByName(req.name); + await this.clusterDB.deleteByName(req.name, this.config.installation); log.info({}, "cluster deregistered", { cluster: req.name }); this.triggerReconcile("deregister", req.name);