[ws-manager-api] Introduce start cluster preference sets

csweichel · roboquat · commit ecf598ac0e36 · 2022-02-02T18:46:24.000+01:00
diff --git a/components/server/src/workspace/workspace-starter.ts b/components/server/src/workspace/workspace-starter.ts
@@ -176,12 +176,12 @@ export class WorkspaceStarter {
 
             // tell the world we're starting this instance
             let resp: StartWorkspaceResponse.AsObject | undefined;
-            let exceptInstallation: string[] = [];
             let lastInstallation = "";
-            for (let i = 0; i < 5; i++) {
+            const clusters = await this.clientProvider.getStartClusterSets(user, 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
-                    const { manager, installation } = await this.clientProvider.getStartManager(user, workspace, instance, exceptInstallation);
+                    const { manager, installation } = cluster;
                     lastInstallation = installation;
 
                     instance.status.phase = "pending";
@@ -203,7 +203,6 @@ export class WorkspaceStarter {
                 } catch (err: any) {
                     if ('code' in err && err.code !== grpc.status.OK && lastInstallation !== "") {
                         log.error({ instanceId: instance.id }, "cannot start workspace on cluster, might retry", err, { cluster: lastInstallation });
-                        exceptInstallation.push(lastInstallation);
                     } else {
                         throw err;
                     }
diff --git a/components/ws-manager-api/typescript/src/client-provider.spec.ts b/components/ws-manager-api/typescript/src/client-provider.spec.ts
@@ -9,10 +9,12 @@ import "reflect-metadata";
 import { Container } from 'inversify';
 import { suite, test } from "@testdeck/mocha";
 import * as chai from 'chai';
-import { WorkspaceManagerClientProvider } from "./client-provider";
+import { IWorkspaceClusterStartSet, WorkspaceManagerClientProvider } from "./client-provider";
 import { WorkspaceManagerClientProviderCompositeSource, WorkspaceManagerClientProviderSource } from "./client-provider-source";
 import { WorkspaceCluster, WorkspaceClusterWoTLS } from "@gitpod/gitpod-protocol/lib/workspace-cluster";
 import { User, Workspace, WorkspaceInstance } from "@gitpod/gitpod-protocol";
+import { PromisifiedWorkspaceManagerClient } from ".";
+import { Constraint, constraintNewWorkspaceCluster, ExtendedUser, intersect, invert } from "./constraints";
 const expect = chai.expect;
 
 @suite
@@ -32,12 +34,7 @@ class TestClientProvider {
                 { name: "a2", govern: true, maxScore: 100, score: 50, state: "available", url: "", admissionConstraints: [] },
                 { name: "a3", govern: false, maxScore: 100, score: 50, state: "available", url: "", admissionConstraints: [] },
                 {
-                    name: "con1", govern: true, maxScore: 100, score: 0, state: "available", url: "", admissionConstraints: [
-                        { type: "has-feature-preview" },
-                    ]
-                },
-                {
-                    name: "con2", govern: true, maxScore: 100, score: 50, state: "available", url: "", admissionConstraints: [
+                    name: "con1", govern: true, maxScore: 100, score: 50, state: "available", url: "", admissionConstraints: [
                         { type: "has-permission", permission: "new-workspace-cluster" },
                     ]
                 },
@@ -50,28 +47,72 @@ class TestClientProvider {
             };
         }).inSingletonScope();
         this.provider = c.get(WorkspaceManagerClientProvider);
+
+        // we don't actually want to try and connect here
+        this.provider.get = async (name: string, grpcOptions?: object): Promise<PromisifiedWorkspaceManagerClient> => { return {} as PromisifiedWorkspaceManagerClient };
     }
 
     @test
-    public async testGetStarterWorkspaceCluster() {
-        this.expectInstallations(["a1", "a2", "a3"], await this.provider.getAvailableStartCluster({} as User, {} as Workspace, {} as WorkspaceInstance));
-        this.expectInstallations(["a1", "a2", "a3", "con1"], await this.provider.getAvailableStartCluster({
-            additionalData: {featurePreview: true}
-        } as User, {} as Workspace, {} as WorkspaceInstance));
-        this.expectInstallations(["a1", "a2", "a3", "con2"], await this.provider.getAvailableStartCluster({
+    public async getStartClusterSets() {
+        await this.expectInstallations(["a1", "a2", "a3"], await this.provider.getStartClusterSets({} as User, {} as Workspace, {} as WorkspaceInstance), "default case");
+        this.expectInstallations(["a1", "a2", "a3", "con1"], await this.provider.getStartClusterSets({
             rolesOrPermissions: ["new-workspace-cluster"]
-        } as User, {} as Workspace, {} as WorkspaceInstance));
+        } as User, {} as Workspace, {} as WorkspaceInstance), "new workspace cluster");
+    }
+
+    @test
+    public async testInvert() {
+        expect(materializeConstraint(invert(everything))).to.be.empty;
+        expect(materializeConstraint(invert(nothing))).to.be.eql(materializeConstraint(everything));
+    }
+
+    @test
+    public testIntersect() {
+        expect(materializeConstraint(intersect(everything, nothing)), "eveything U nothing == nothing").to.be.empty;
+        expect(materializeConstraint(intersect(everything, everything, nothing)), "eveything U everything U nothing == nothing").to.be.empty;
+        expect(materializeConstraint(intersect(everything, everything)), "eveything U everything == everything").to.be.eql(materializeConstraint(everything));
+
+        const something = (all: WorkspaceClusterWoTLS[], user: ExtendedUser, workspace: Workspace, instance: WorkspaceInstance) => all.filter(c => c.name === "a1");
+        expect(materializeConstraint(intersect(something, nothing)), "something U nothing == nothing").to.be.empty;
+        expect(materializeConstraint(intersect(everything, something)), "eveything U something == something").to.be.eql(materializeConstraint(something));
+        expect(materializeConstraint(intersect(everything, something, nothing)), "everything U something U nothing == nothing").to.be.empty;
+
+        expect(materializeConstraint(intersect(everything, invert(nothing))), "everything U invert(nothing) == everything").to.be.eql(materializeConstraint(everything));
     }
 
-    public async getStartManager() {
-        const { installation } = await this.provider.getStartManager({} as User, {} as Workspace, {} as WorkspaceInstance, ["a2", "a3"]);
-        expect(installation).to.be.eql("a1");
+    @test
+    public testConstraintNewWorkspaceCluster() {
+        const clusters: WorkspaceClusterWoTLS[] = [
+            {name: "a1", admissionConstraints: [{ type: "has-permission", permission: "new-workspace-cluster" }]} as WorkspaceClusterWoTLS,
+            {name: "b1" } as WorkspaceClusterWoTLS,
+        ]
+        expect(constraintNewWorkspaceCluster(clusters, {} as ExtendedUser, {} as Workspace, {} as WorkspaceInstance).map(c => c.name)).to.be.empty;
+        expect(constraintNewWorkspaceCluster(clusters, {rolesOrPermissions:["new-workspace-cluster"]} as ExtendedUser, {} as Workspace, {} as WorkspaceInstance).map(c => c.name)).to.be.eql(["a1"]);
     }
 
-    private expectInstallations(expected: string[], actual: WorkspaceClusterWoTLS[]) {
-        expect(actual.map(e => e.name).sort()).to.be.eql(expected);
+    private async expectInstallations(expected: string[], actual: IWorkspaceClusterStartSet, msg: string) {
+        const a: string[] = [];
+        for await (const c of actual) {
+            a.push(c.installation);
+        }
+
+        expect(a.sort(), msg).to.be.eql(expected);
     }
 
 }
 
+const everything = (all: WorkspaceClusterWoTLS[], user: ExtendedUser, workspace: Workspace, instance: WorkspaceInstance) => all;
+const nothing = (all: WorkspaceClusterWoTLS[], user: ExtendedUser, workspace: Workspace, instance: WorkspaceInstance) => [];
+
+function materializeConstraint(c: Constraint): string[] {
+    const cluster: WorkspaceClusterWoTLS[] = [
+        {name: "a1"} as WorkspaceClusterWoTLS,
+        {name: "a2"} as WorkspaceClusterWoTLS,
+        {name: "a3"} as WorkspaceClusterWoTLS,
+        {name: "a4"} as WorkspaceClusterWoTLS,
+    ];
+
+    return c(cluster, {} as ExtendedUser, {} as Workspace, {} as WorkspaceInstance).map(cluster => cluster.name);
+}
+
 module.exports = new TestClientProvider()
diff --git a/components/ws-manager-api/typescript/src/client-provider.ts b/components/ws-manager-api/typescript/src/client-provider.ts
@@ -5,26 +5,19 @@
  */
 
 import { createClientCallMetricsInterceptor, IClientCallMetrics } from "@gitpod/content-service/lib/client-call-metrics";
-import { Disposable, User, Workspace, WorkspaceInstance } from "@gitpod/gitpod-protocol";
+import { Disposable, Workspace, WorkspaceInstance } from "@gitpod/gitpod-protocol";
 import { defaultGRPCOptions } from '@gitpod/gitpod-protocol/lib/util/grpc';
 import { log } from '@gitpod/gitpod-protocol/lib/util/logging';
 import { WorkspaceClusterWoTLS, WorkspaceManagerConnectionInfo } from '@gitpod/gitpod-protocol/lib/workspace-cluster';
 import * as grpc from "@grpc/grpc-js";
 import { inject, injectable, optional } from 'inversify';
 import { WorkspaceManagerClientProviderCompositeSource, WorkspaceManagerClientProviderSource } from "./client-provider-source";
+import { ExtendedUser, workspaceClusterSets } from "./constraints";
 import { WorkspaceManagerClient } from './core_grpc_pb';
 import { linearBackoffStrategy, PromisifiedWorkspaceManagerClient } from "./promisified-client";
 
 export const IWorkspaceManagerClientCallMetrics = Symbol('IWorkspaceManagerClientCallMetrics')
 
-/**
- * ExtendedUser adds additional attributes to a user which are helpful
- * during cluster selection.
- */
-export interface ExtendedUser extends User  {
-    level?: string;
-}
-
 @injectable()
 export class WorkspaceManagerClientProvider implements Disposable {
     @inject(WorkspaceManagerClientProviderCompositeSource)
@@ -39,41 +32,48 @@ export class WorkspaceManagerClientProvider implements Disposable {
     protected readonly connectionCache = new Map<string, WorkspaceManagerClient>();
 
     /**
-     * Throws an error if there is not WorkspaceManagerClient available.
+     * getStartClusterSets produces a set of workspace clusters we can try to start a workspace in.
+     * If starting a workspace fails in one cluster, the caller is expected to "pop" another cluster
+     * of the list until the workspace start has succeeded or pop returns undefined.
      *
-     * @returns The WorkspaceManagerClient that was chosen to start the next workspace with.
+     * @param user user who wants to starts a workspace manager
+     * @param workspace the workspace we want to start
+     * @param instance the instance we want to start
+     * @returns a set of workspace clusters we can start the workspace in
      */
-    public async getStartManager(user: ExtendedUser, workspace: Workspace, instance: WorkspaceInstance, exceptInstallations?: string[]): Promise<{ manager: PromisifiedWorkspaceManagerClient, installation: string }> {
-        let availableCluster = await this.getAvailableStartCluster(user, workspace, instance);
-        if (!!exceptInstallations) {
-            availableCluster = availableCluster.filter(c => !exceptInstallations?.includes(c.name));
-        }
-
-        let chosenCluster = chooseCluster(availableCluster.filter(admissionPreferencesFilter(user)))
-        if (!chosenCluster) {
-            // We did not choose a prefered cluster. That's ok, we'll try with the remaining set.
-            chosenCluster = chooseCluster(availableCluster);
-        }
-        if (!chosenCluster) {
-            // We still haven't choosen a cluster. That's a problem.
-            throw new Error("no available workspace cluster to choose from!");
-        }
+    public async getStartClusterSets(user: ExtendedUser, workspace: Workspace, instance: WorkspaceInstance): Promise<IWorkspaceClusterStartSet> {
+        const allClusters = await this.source.getAllWorkspaceClusters();
+        const availableClusters = allClusters.filter(c => c.score >= 0 && c.state === "available");
 
+        const sets = workspaceClusterSets.map(constraints => {
+            const r = constraints.constraint(availableClusters, user, workspace, instance);
+            if (!r) {
+                return;
+            }
+            return new ClusterSet(this, r);
+        }).filter(s => s !== undefined) as ClusterSet[];
 
-        const grpcOptions: grpc.ClientOptions = {
-            ...defaultGRPCOptions,
-        };
-        const client = await this.get(chosenCluster.name, grpcOptions);
         return {
-            manager: client,
-            installation: chosenCluster.name,
-        };
-    }
-
-    public async getAvailableStartCluster(user: User, workspace: Workspace, instance: WorkspaceInstance): Promise<WorkspaceClusterWoTLS[]> {
-        const allClusters = await this.source.getAllWorkspaceClusters();
-        const availableClusters = allClusters.filter(c => c.score >= 0 && c.state === "available").filter(admissionConstraintsFilter(user, workspace, instance));
-        return availableClusters;
+            [Symbol.asyncIterator]: (): AsyncIterator<ClusterClientEntry> => {
+                return {
+                    next: async (): Promise<IteratorResult<ClusterClientEntry>> => {
+                        while (true) {
+                            if (sets.length === 0) {
+                                return {done: true, value: undefined};
+                            }
+
+                            let res = await sets[sets.length - 1].next();
+                            if (!!res.done) {
+                                sets.pop();
+                                continue;
+                            }
+
+                            return res;
+                        }
+                    }
+                }
+            }
+        }
     }
 
     /**
@@ -143,7 +143,39 @@ export class WorkspaceManagerClientProvider implements Disposable {
     }
 }
 
+export interface IWorkspaceClusterStartSet extends AsyncIterable<ClusterClientEntry> {}
 
+export interface ClusterClientEntry { manager: PromisifiedWorkspaceManagerClient, installation: string }
+
+/**
+ * ClusterSet is an iterator
+ */
+class ClusterSet implements AsyncIterator<ClusterClientEntry> {
+    protected usedCluster: string[] = [];
+    constructor(protected readonly provider: WorkspaceManagerClientProvider, protected readonly cluster: WorkspaceClusterWoTLS[]) {}
+
+    public async next(): Promise<IteratorResult<ClusterClientEntry>> {
+        const available = this.cluster.filter(c => !this.usedCluster.includes(c.name));
+        const chosenCluster = chooseCluster(available);
+        if (!chosenCluster) {
+            // empty set
+            return {done: true, value: undefined };
+        }
+        this.usedCluster.push(chosenCluster.name);
+
+        const grpcOptions: grpc.ClientOptions = {
+            ...defaultGRPCOptions,
+        };
+        const client = await this.provider.get(chosenCluster.name, grpcOptions);
+        return {
+            done: false,
+            value: {
+                manager: client,
+                installation: chosenCluster.name,
+            }
+        };
+    }
+}
 
 /**
  *
@@ -175,33 +207,3 @@ function chooseCluster(availableCluster: WorkspaceClusterWoTLS[]): WorkspaceClus
     }
     return availableCluster[availableCluster.length - 1];
 }
-
-function admissionConstraintsFilter(user: User, workspace: Workspace, instance: WorkspaceInstance): (c: WorkspaceClusterWoTLS) => boolean {
-    return (c: WorkspaceClusterWoTLS) => {
-        if (!c.admissionConstraints) {
-            return true;
-        }
-
-        return (c.admissionConstraints || []).every(con => {
-            switch (con.type) {
-                case "has-feature-preview":
-                    return !!user.additionalData && !!user.additionalData.featurePreview;
-                case "has-permission":
-                    return user.rolesOrPermissions?.includes(con.permission);
-            }
-        });
-    };
-}
-
-function admissionPreferencesFilter(user: ExtendedUser): (c: WorkspaceClusterWoTLS) => boolean {
-    return (c: WorkspaceClusterWoTLS) => {
-        return (c.admissionPreferences || []).every(pref => {
-            switch (pref.type) {
-                case "user-level":
-                    return pref.level === user.level;
-                default:
-                    return false;
-            }
-        })
-    };
-}
diff --git a/components/ws-manager-api/typescript/src/constraints.ts b/components/ws-manager-api/typescript/src/constraints.ts
