Skip to content

Pluggable Workspace Cluster: Admission Constraints #4158

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 5 commits into from
May 7, 2021
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
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import { PrimaryColumn, Column, Entity, Index } from "typeorm";
import { TLSConfig, WorkspaceCluster, WorkspaceClusterState } from "@gitpod/gitpod-protocol/lib/workspace-cluster";
import { AdmissionConstraint, TLSConfig, WorkspaceCluster, WorkspaceClusterState } from "@gitpod/gitpod-protocol/lib/workspace-cluster";
import { ValueTransformer } from "typeorm/decorator/options/ValueTransformer";

@Entity()
Expand Down Expand Up @@ -59,4 +59,27 @@ export class DBWorkspaceCluster implements WorkspaceCluster {

@Column()
govern: boolean;

@Column({
type: "simple-json",
transformer: (() => {
const defaultValue: AdmissionConstraint[] = [];
const jsonifiedDefault = JSON.stringify(defaultValue);
return <ValueTransformer> {
to(value: any): any {
if (!value) {
return jsonifiedDefault;
}
return JSON.stringify(value);
},
from(value: any): any {
if (value === jsonifiedDefault) {
return undefined;
}
return JSON.parse(value);
}
};
})()
})
admissionConstraints?: AdmissionConstraint[];
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/**
* Copyright (c) 2021 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 {MigrationInterface, QueryRunner} from "typeorm";
import { columnExists, tableExists } from "./helper/helper";

export class AdmissionConstraints1620209434733 implements MigrationInterface {

public async up(queryRunner: QueryRunner): Promise<any> {
if (await tableExists(queryRunner, "d_b_workspace_cluster")) {
if (!(await columnExists(queryRunner, "d_b_workspace_cluster", "admissionConstraints"))) {
await queryRunner.query("ALTER TABLE d_b_workspace_cluster ADD COLUMN admissionConstraints TEXT NOT NULL");
}
}
}

public async down(queryRunner: QueryRunner): Promise<any> {
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import { TypeORM } from "./typeorm";
import { WorkspaceClusterDB } from "../workspace-cluster-db";
import { DBWorkspaceCluster } from "./entity/db-workspace-cluster";
import { WorkspaceCluster, WorkspaceClusterFilter, WorkspaceClusterWoTls } from "@gitpod/gitpod-protocol/lib/workspace-cluster";
import { WorkspaceCluster, WorkspaceClusterFilter, WorkspaceClusterWoTLS } from "@gitpod/gitpod-protocol/lib/workspace-cluster";

@injectable()
export class WorkspaceClusterDBImpl implements WorkspaceClusterDB {
Expand Down Expand Up @@ -40,14 +40,15 @@ import { WorkspaceCluster, WorkspaceClusterFilter, WorkspaceClusterWoTls } from
}


async findFiltered(predicate: DeepPartial<WorkspaceClusterFilter>): Promise<WorkspaceClusterWoTls[]> {
const prototype: WorkspaceClusterWoTls = {
async findFiltered(predicate: DeepPartial<WorkspaceClusterFilter>): Promise<WorkspaceClusterWoTLS[]> {
const prototype: WorkspaceClusterWoTLS = {
name: "",
url: "",
score: 0,
maxScore: 0,
state: "available",
govern: false,
admissionConstraints: [],
};

const repo = await this.getRepo();
Expand Down
3 changes: 2 additions & 1 deletion components/gitpod-protocol/src/permission.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ export const Permissions = {
"admin-users": undefined,
"admin-workspaces": undefined,
"admin-api": undefined,
"ide-settings": undefined
"ide-settings": undefined,
"new-workspace-cluster": undefined,
};
export type PermissionName = keyof (typeof Permissions);
export const Roles = {"devops": undefined, "viewer": undefined, "admin": undefined };
Expand Down
14 changes: 12 additions & 2 deletions components/gitpod-protocol/src/workspace-cluster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import * as fs from 'fs';
import { filePathTelepresenceAware } from './env';
import { DeepPartial } from "./util/deep-partial";
import { Without } from './util/without';
import { PermissionName } from './permission';

export interface WorkspaceCluster {
// Name of the workspace cluster.
Expand All @@ -32,7 +33,11 @@ export interface WorkspaceCluster {

// True if this bridge should control this cluster
govern: boolean;

// An optional set of constraints that limit who can start workspaces on the cluster
admissionConstraints?: AdmissionConstraint[];
}

export type WorkspaceClusterState = "available" | "cordoned" | "draining";
export interface TLSConfig {
// the CA shared between client and server (base64 encoded)
Expand All @@ -45,9 +50,14 @@ export interface TLSConfig {
export namespace TLSConfig {
export const loadFromBase64File = (path: string): string => fs.readFileSync(filePathTelepresenceAware(path)).toString("base64");
}
export type WorkspaceClusterWoTls = Without<WorkspaceCluster, "tls">;
export type WorkspaceClusterWoTLS = Without<WorkspaceCluster, "tls">;
export type WorkspaceManagerConnectionInfo = Pick<WorkspaceCluster, "name" | "url" | "tls">;

export type AdmissionConstraint = AdmissionConstraintFeaturePreview | AdmissionConstraintHasRole;
export type AdmissionConstraintFeaturePreview = { type: "has-feature-preview" };
export type AdmissionConstraintHasRole = { type: "has-permission", permission: PermissionName };


export const WorkspaceClusterDB = Symbol("WorkspaceClusterDB");
export interface WorkspaceClusterDB {
/**
Expand All @@ -73,7 +83,7 @@ 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<WorkspaceClusterFilter>): Promise<WorkspaceClusterWoTls[]>;
findFiltered(predicate: DeepPartial<WorkspaceClusterFilter>): Promise<WorkspaceClusterWoTLS[]>;
}
export interface WorkspaceClusterFilter extends Pick<WorkspaceCluster, "state" | "govern" | "url"> {
minScore: number;
Expand Down
2 changes: 1 addition & 1 deletion components/server/src/workspace/workspace-starter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ export class WorkspaceStarter {
startRequest.setServicePrefix(workspace.id);

// tell the world we're starting this instance
const { manager, installation } = await this.clientProvider.getStartManager();
const { manager, installation } = await this.clientProvider.getStartManager(user, workspace, instance);
instance.status.phase = "pending";
instance.region = installation;
await this.workspaceDb.trace({ span }).storeInstance(instance);
Expand Down
6 changes: 6 additions & 0 deletions components/ws-manager-api/typescript/mocha.opts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
--require ts-node/register
--require reflect-metadata/Reflect
--require source-map-support/register
--reporter spec
--watch-extensions ts
--exit
7 changes: 5 additions & 2 deletions components/ws-manager-api/typescript/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
],
"scripts": {
"build": "tsc && cp -f src/*.js src/*d.ts lib",
"watch": "leeway exec --package .:lib --transitive-dependencies --filter-type yarn --components --parallel -- tsc -w --preserveWatchOutput"
"watch": "leeway exec --package .:lib --transitive-dependencies --filter-type yarn --components --parallel -- tsc -w --preserveWatchOutput",
"test": "mocha --opts mocha.opts 'src/**/*.spec.ts'"
},
"dependencies": {
"@gitpod/content-service": "0.1.5",
Expand All @@ -24,6 +25,8 @@
"@types/node": "^10",
"grpc-tools": "^1.11.1",
"grpc_tools_node_protoc_ts": "^5.2.1",
"typescript-formatter": "^7.2.2"
"typescript-formatter": "^7.2.2",
"@testdeck/mocha": "0.1.2",
"@types/chai": "^4.1.2"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@
* 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 { 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<WorkspaceCluster | undefined>;
getAllWorkspaceClusters(): Promise<WorkspaceClusterWoTls[]>;
getAllWorkspaceClusters(): Promise<WorkspaceClusterWoTLS[]>;
}


Expand All @@ -23,7 +23,7 @@ export class WorkspaceManagerClientProviderEnvSource implements WorkspaceManager
return this.clusters.find(m => m.name === name);
}

public async getAllWorkspaceClusters(): Promise<WorkspaceClusterWoTls[]> {
public async getAllWorkspaceClusters(): Promise<WorkspaceClusterWoTLS[]> {
return this.clusters;
}

Expand Down Expand Up @@ -68,7 +68,7 @@ export class WorkspaceManagerClientProviderDBSource implements WorkspaceManagerC
return await this.db.findByName(name);
}

public async getAllWorkspaceClusters(): Promise<WorkspaceClusterWoTls[]> {
public async getAllWorkspaceClusters(): Promise<WorkspaceClusterWoTLS[]> {
return await this.db.findFiltered({});
}
}
Expand All @@ -88,8 +88,8 @@ export class WorkspaceManagerClientProviderCompositeSource implements WorkspaceM
return undefined;
}

async getAllWorkspaceClusters(): Promise<WorkspaceClusterWoTls[]> {
const allClusters: Map<string, WorkspaceClusterWoTls> = new Map();
async getAllWorkspaceClusters(): Promise<WorkspaceClusterWoTLS[]> {
const allClusters: Map<string, WorkspaceClusterWoTLS> = new Map();
for (const source of this.sources) {
const clusters = await source.getAllWorkspaceClusters();
for (const cluster of clusters) {
Expand All @@ -99,7 +99,7 @@ export class WorkspaceManagerClientProviderCompositeSource implements WorkspaceM
allClusters.set(cluster.name, cluster);
}
}
const result: WorkspaceClusterWoTls[] = [];
const result: WorkspaceClusterWoTLS[] = [];
for (const [_, cluster] of allClusters) {
result.push(cluster);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/**
* Copyright (c) 2021 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 "reflect-metadata";

import { Container } from 'inversify';
import { suite, test } from "@testdeck/mocha";
import * as chai from 'chai';
import { 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";
const expect = chai.expect;

@suite
class TestClientProvider {
protected provider: WorkspaceManagerClientProvider;

public before() {
const c = new Container();
c.bind(WorkspaceManagerClientProvider).toSelf().inSingletonScope();
c.bind(WorkspaceManagerClientProviderCompositeSource).toSelf().inSingletonScope();
c.bind(WorkspaceManagerClientProviderSource).toDynamicValue((): WorkspaceManagerClientProviderSource => {
const cluster: WorkspaceCluster[] = [
{ name: "c1", govern: true, maxScore: 100, score: 0, state: "cordoned", url: "", admissionConstraints: [] },
{ name: "c2", govern: true, maxScore: 100, score: 50, state: "cordoned", url: "", admissionConstraints: [] },
{ name: "c3", govern: false, maxScore: 100, score: 50, state: "cordoned", url: "", admissionConstraints: [] },
{ name: "a1", govern: true, maxScore: 100, score: 0, state: "available", url: "", admissionConstraints: [] },
{ 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: [
{ type: "has-permission", permission: "new-workspace-cluster" },
]
},
];
return <WorkspaceManagerClientProviderSource>{
getAllWorkspaceClusters: async () => { return cluster as WorkspaceClusterWoTLS[] },
getWorkspaceCluster: async (name: string) => {
return cluster.find(c => c.name === name);
}
};
}).inSingletonScope();
this.provider = c.get(WorkspaceManagerClientProvider);
}

@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({
rolesOrPermissions: ["admin"]
} as User, {} as Workspace, {} as WorkspaceInstance));
}

private expectInstallations(expected: string[], actual: WorkspaceClusterWoTLS[]) {
expect(actual.map(e => e.name).sort()).to.be.eql(expected);
}

}

module.exports = new TestClientProvider()
40 changes: 31 additions & 9 deletions components/ws-manager-api/typescript/src/client-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import * as grpc from "@grpc/grpc-js";
import { injectable, inject } from 'inversify';
import { WorkspaceManagerClient } from './core_grpc_pb';
import { PromisifiedWorkspaceManagerClient, linearBackoffStrategy } from "./promisified-client";
import { Disposable } from "@gitpod/gitpod-protocol";
import { WorkspaceClusterWoTls, WorkspaceManagerConnectionInfo } from '@gitpod/gitpod-protocol/lib/workspace-cluster';
import { Disposable, User, Workspace, WorkspaceInstance } from "@gitpod/gitpod-protocol";
import { WorkspaceClusterWoTLS, WorkspaceManagerConnectionInfo } from '@gitpod/gitpod-protocol/lib/workspace-cluster';
import { WorkspaceManagerClientProviderCompositeSource, WorkspaceManagerClientProviderSource } from "./client-provider-source";
import { log } from '@gitpod/gitpod-protocol/lib/util/logging';

Expand All @@ -28,17 +28,22 @@ export class WorkspaceManagerClientProvider implements Disposable {
*
* @returns The WorkspaceManagerClient that was chosen to start the next workspace with.
*/
public async getStartManager(): Promise<{ manager: PromisifiedWorkspaceManagerClient, installation: string}> {
const allClusters = await this.source.getAllWorkspaceClusters();
const availableClusters = allClusters.filter((c) => c.score >= 0 && c.govern && c.state === "available");
const chosenCluster = chooseCluster(availableClusters);
public async getStartManager(user: User, workspace: Workspace, instance: WorkspaceInstance): Promise<{ manager: PromisifiedWorkspaceManagerClient, installation: string}> {
const availableCluster = await this.getAvailableStartCluster(user, workspace, instance);
const chosenCluster = chooseCluster(availableCluster);
const client = await this.get(chosenCluster.name);
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));
Copy link
Member

Choose a reason for hiding this comment

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

  • c.govern

👍

return availableClusters;
}

/**
* @param name
* @returns The WorkspaceManagerClient identified by the name. Throws an error if there is none.
Expand Down Expand Up @@ -73,7 +78,7 @@ export class WorkspaceManagerClientProvider implements Disposable {
/**
* @returns All WorkspaceClusters (without TLS config)
*/
public async getAllWorkspaceClusters(): Promise<WorkspaceClusterWoTls[]> {
public async getAllWorkspaceClusters(): Promise<WorkspaceClusterWoTLS[]> {
return this.source.getAllWorkspaceClusters();
}

Expand Down Expand Up @@ -106,12 +111,12 @@ export class WorkspaceManagerClientProvider implements Disposable {
* @param clusters
* @returns The chosen cluster. Throws an error if there are 0 WorkspaceClusters to choose from.
*/
function chooseCluster(availableCluster: WorkspaceClusterWoTls[]): WorkspaceClusterWoTls {
function chooseCluster(availableCluster: WorkspaceClusterWoTLS[]): WorkspaceClusterWoTLS {
if (availableCluster.length === 0) {
throw new Error("No cluster to choose from!");
}

const scoreFunc = (c: WorkspaceClusterWoTls): number => {
const scoreFunc = (c: WorkspaceClusterWoTLS): number => {
let score = c.score; // here is the point where we may want to implement non-static approaches

// clamp to maxScore
Expand All @@ -134,4 +139,21 @@ 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);
}
});
};
}
Loading