Skip to content

Commit 2e777e2

Browse files
FuristoFuristo
committed
Incorporate review feedback
- Use latest workspace instance to set workspace class - Add more detailed configuration for workspace classes
1 parent 0c74326 commit 2e777e2

File tree

10 files changed

+84
-118
lines changed

10 files changed

+84
-118
lines changed

‎components/gitpod-db/src/typeorm/entity/db-workspace.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -121,10 +121,4 @@ export class DBWorkspace implements Workspace {
121121
})
122122
@Index("ind_basedOnSnapshotId")
123123
basedOnSnapshotId?: string;
124-
125-
@Column({
126-
default: "",
127-
transformer: Transformer.MAP_EMPTY_STR_TO_UNDEFINED,
128-
})
129-
workspaceClass?: string;
130124
}

‎components/gitpod-db/src/typeorm/migration/1657653450875-WorkspaceClass.ts

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

‎components/gitpod-protocol/src/protocol.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -690,8 +690,6 @@ export interface Workspace {
690690
basedOnPrebuildId?: string;
691691

692692
basedOnSnapshotId?: string;
693-
694-
workspaceClass?: string;
695693
}
696694

697695
export type WorkspaceSoftDeletion = "user" | "gc";

‎components/server/ee/src/workspace/workspace-factory.ts

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ import { UserDB } from "@gitpod/gitpod-db/lib";
3232
import { UserCounter } from "../user/user-counter";
3333
import { increasePrebuildsStartedCounter } from "../../../src/prometheus-metrics";
3434
import { DeepPartial } from "@gitpod/gitpod-protocol/lib/util/deep-partial";
35-
import { getExperimentsClientForBackend } from "@gitpod/gitpod-protocol/lib/experiments/configcat-server";
3635

3736
@injectable()
3837
export class WorkspaceFactoryEE extends WorkspaceFactory {
@@ -318,7 +317,6 @@ export class WorkspaceFactoryEE extends WorkspaceFactory {
318317
projectId = project.id;
319318
}
320319
}
321-
const workspaceClass = await this.getWorkspaceClass(user);
322320

323321
const id = await this.generateWorkspaceID(context);
324322
const newWs: Workspace = {
@@ -340,7 +338,6 @@ export class WorkspaceFactoryEE extends WorkspaceFactory {
340338
baseImageNameResolved: buildWorkspace.baseImageNameResolved,
341339
basedOnPrebuildId: context.prebuiltWorkspace.id,
342340
config,
343-
workspaceClass,
344341
};
345342
await this.db.trace({ span }).store(newWs);
346343
return newWs;
@@ -351,27 +348,6 @@ export class WorkspaceFactoryEE extends WorkspaceFactory {
351348
span.finish();
352349
}
353350
}
354-
355-
protected async getWorkspaceClass(user: User): Promise<string> {
356-
let workspaceClass = "";
357-
let classesEnabled = await getExperimentsClientForBackend().getValueAsync("workspace_classes", false, {
358-
user: user,
359-
});
360-
if (classesEnabled) {
361-
if (user.additionalData?.workspaceClasses?.prebuild) {
362-
workspaceClass = user.additionalData?.workspaceClasses?.prebuild;
363-
} else {
364-
// legacy support
365-
if (await this.userService.userGetsMoreResources(user)) {
366-
workspaceClass = this.config.workspaceClasses.defaultMoreResources;
367-
} else {
368-
workspaceClass = this.config.workspaceClasses.default;
369-
}
370-
}
371-
}
372-
373-
return workspaceClass;
374-
}
375351
}
376352

377353
function filterForLogging(context: StartPrebuildContext) {

‎components/server/ee/src/workspace/workspace-starter.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,21 @@ export class WorkspaceStarterEE extends WorkspaceStarter {
2929
protected async newInstance(
3030
ctx: TraceContext,
3131
workspace: Workspace,
32+
previousInstance: WorkspaceInstance | undefined,
3233
user: User,
3334
excludeFeatureFlags: NamedWorkspaceFeatureFlag[],
3435
ideConfig: IDEConfig,
3536
forcePVC: boolean,
3637
): Promise<WorkspaceInstance> {
37-
const instance = await super.newInstance(ctx, workspace, user, excludeFeatureFlags, ideConfig, forcePVC);
38+
const instance = await super.newInstance(
39+
ctx,
40+
workspace,
41+
previousInstance,
42+
user,
43+
excludeFeatureFlags,
44+
ideConfig,
45+
forcePVC,
46+
);
3847
if (await this.eligibilityService.hasFixedWorkspaceResources(user)) {
3948
const config: WorkspaceInstanceConfiguration = instance.configuration!;
4049
const ff = config.featureFlags || [];

‎components/server/src/config.ts

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,27 @@ export interface WorkspaceGarbageCollection {
5555
contentChunkLimit: number;
5656
}
5757

58+
type WorkspaceClassesConfig = [WorkspaceClassConfig];
59+
60+
interface WorkspaceClassConfig {
61+
// The technical string we use to identify the class with internally
62+
id: string;
63+
64+
// Is the "default" class. The config is validated to only every have exactly _one_ default class.
65+
isDefault: boolean;
66+
67+
// The string we display to users in the UI
68+
displayName: string;
69+
70+
// Whether or not to:
71+
// - offer users this Workspace class for selection
72+
// - use this class to start workspaces with. If a user has a class marked like this configured and starts
73+
deprecated: boolean;
74+
75+
// The price for this workspace class in "credits per minute"
76+
creditsPerMinute: number;
77+
}
78+
5879
/**
5980
* This is the config shape as found in the configuration file, e.g. server-configmap.yaml
6081
*/
@@ -186,12 +207,9 @@ export interface ConfigSerialized {
186207
inactivityPeriodForRepos?: number;
187208

188209
/**
189-
* Options related to workspace classes
210+
* Supported workspace classes
190211
*/
191-
workspaceClasses: {
192-
default: string;
193-
defaultMoreResources: string;
194-
};
212+
workspaceClasses: WorkspaceClassesConfig;
195213
}
196214

197215
export namespace ConfigFile {

‎components/server/src/workspace/workspace-factory.ts

Lines changed: 1 addition & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,13 @@ import {
1919
WorkspaceContext,
2020
WorkspaceProbeContext,
2121
} from "@gitpod/gitpod-protocol";
22-
import { getExperimentsClientForBackend } from "@gitpod/gitpod-protocol/lib/experiments/configcat-server";
2322
import { ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error";
2423
import { generateWorkspaceID } from "@gitpod/gitpod-protocol/lib/util/generate-workspace-id";
2524
import { log } from "@gitpod/gitpod-protocol/lib/util/logging";
2625
import { TraceContext } from "@gitpod/gitpod-protocol/lib/util/tracing";
2726
import { inject, injectable } from "inversify";
2827
import { ResponseError } from "vscode-jsonrpc";
29-
import { Config } from "../config";
3028
import { RepoURL } from "../repohost";
31-
import { UserService } from "../user/user-service";
3229
import { ConfigProvider } from "./config-provider";
3330
import { ImageSourceProvider } from "./image-source-provider";
3431

@@ -39,8 +36,6 @@ export class WorkspaceFactory {
3936
@inject(TeamDB) protected readonly teamDB: TeamDB;
4037
@inject(ConfigProvider) protected configProvider: ConfigProvider;
4138
@inject(ImageSourceProvider) protected imageSourceProvider: ImageSourceProvider;
42-
@inject(Config) protected config: Config;
43-
@inject(UserService) protected readonly userService: UserService;
4439

4540
public async createForContext(
4641
ctx: TraceContext,
@@ -129,7 +124,7 @@ export class WorkspaceFactory {
129124

130125
const id = await this.generateWorkspaceID(context);
131126
const date = new Date().toISOString();
132-
const workspaceClass = await this.getWorkspaceClass(user);
127+
133128
const newWs = <Workspace>{
134129
id,
135130
type: "regular",
@@ -148,7 +143,6 @@ export class WorkspaceFactory {
148143
baseImageNameResolved: workspace.baseImageNameResolved,
149144
basedOnSnapshotId: context.snapshotId,
150145
imageSource: workspace.imageSource,
151-
workspaceClass,
152146
};
153147
if (snapshot.layoutData) {
154148
// we don't need to await here, as the layoutdata will be requested earliest in a couple of seconds by the theia IDE
@@ -213,7 +207,6 @@ export class WorkspaceFactory {
213207
}
214208

215209
const id = await this.generateWorkspaceID(context);
216-
const workspaceClass = await this.getWorkspaceClass(user);
217210
const newWs: Workspace = {
218211
id,
219212
type: "regular",
@@ -225,7 +218,6 @@ export class WorkspaceFactory {
225218
context,
226219
imageSource,
227220
config,
228-
workspaceClass,
229221
};
230222
await this.db.trace({ span }).store(newWs);
231223
return newWs;
@@ -266,26 +258,4 @@ export class WorkspaceFactory {
266258
}
267259
return await generateWorkspaceID();
268260
}
269-
270-
protected async getWorkspaceClass(user: User): Promise<string> {
271-
let workspaceClass = "";
272-
let classesEnabled = await getExperimentsClientForBackend().getValueAsync("workspace_classes", false, {
273-
user: user,
274-
});
275-
if (classesEnabled) {
276-
workspaceClass = this.config.workspaceClasses.default;
277-
if (user.additionalData?.workspaceClasses?.regular) {
278-
workspaceClass = user.additionalData?.workspaceClasses?.regular;
279-
} else {
280-
// legacy support
281-
if (await this.userService.userGetsMoreResources(user)) {
282-
workspaceClass = this.config.workspaceClasses.defaultMoreResources;
283-
} else {
284-
workspaceClass = this.config.workspaceClasses.default;
285-
}
286-
}
287-
}
288-
289-
return workspaceClass;
290-
}
291261
}

‎components/server/src/workspace/workspace-starter.ts

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -274,11 +274,11 @@ export class WorkspaceStarter {
274274
const hasValidBackup = pastInstances.some(
275275
(i) => !!i.status && !!i.status.conditions && !i.status.conditions.failed,
276276
);
277-
let lastValidWorkspaceInstanceId = "";
277+
let lastValidWorkspaceInstance: WorkspaceInstance | undefined;
278278
if (hasValidBackup) {
279-
lastValidWorkspaceInstanceId = pastInstances.reduce((previousValue, currentValue) =>
279+
lastValidWorkspaceInstance = pastInstances.reduce((previousValue, currentValue) =>
280280
currentValue.creationTime > previousValue.creationTime ? currentValue : previousValue,
281-
).id;
281+
);
282282
}
283283

284284
const ideConfig = await this.ideConfigService.config;
@@ -290,6 +290,7 @@ export class WorkspaceStarter {
290290
await this.newInstance(
291291
ctx,
292292
workspace,
293+
lastValidWorkspaceInstance,
293294
user,
294295
options.excludeFeatureFlags || [],
295296
ideConfig,
@@ -330,7 +331,7 @@ export class WorkspaceStarter {
330331
instance,
331332
workspace,
332333
user,
333-
lastValidWorkspaceInstanceId,
334+
lastValidWorkspaceInstance?.id ?? "",
334335
ideConfig,
335336
userEnvVars,
336337
projectEnvVars,
@@ -345,7 +346,7 @@ export class WorkspaceStarter {
345346
instance,
346347
workspace,
347348
user,
348-
lastValidWorkspaceInstanceId,
349+
lastValidWorkspaceInstance?.id ?? "",
349350
ideConfig,
350351
userEnvVars,
351352
projectEnvVars,
@@ -684,6 +685,7 @@ export class WorkspaceStarter {
684685
protected async newInstance(
685686
ctx: TraceContext,
686687
workspace: Workspace,
688+
previousInstance: WorkspaceInstance | undefined,
687689
user: User,
688690
excludeFeatureFlags: NamedWorkspaceFeatureFlag[],
689691
ideConfig: IDEConfig,
@@ -782,20 +784,31 @@ export class WorkspaceStarter {
782784
let classesEnabled = await getExperimentsClientForBackend().getValueAsync("workspace_classes", false, {
783785
user: user,
784786
});
787+
785788
if (classesEnabled) {
786-
// this is a workspace that was started before workspace classes
787-
// set the workspace class based on if the user "has more resources"
788-
if (!workspace.workspaceClass) {
789-
if (await this.userService.userGetsMoreResources(user)) {
790-
workspaceClass = this.config.workspaceClasses.defaultMoreResources;
791-
} else {
792-
workspaceClass = this.config.workspaceClasses.default;
789+
// this is either the first time we start the workspace or the workspace was started
790+
// before workspace classes and does not have a class yet
791+
if (!previousInstance?.workspaceClass) {
792+
if (workspace.type == "regular") {
793+
if (user.additionalData?.workspaceClasses?.regular) {
794+
workspaceClass = user.additionalData?.workspaceClasses?.regular;
795+
}
793796
}
794797

795-
workspace.workspaceClass = workspaceClass;
796-
this.workspaceDb.trace({ span }).store(workspace);
798+
if (workspace.type == "prebuild") {
799+
if (user.additionalData?.workspaceClasses?.prebuild) {
800+
workspaceClass = user.additionalData?.workspaceClasses?.prebuild;
801+
}
802+
}
803+
804+
if (!workspaceClass) {
805+
workspaceClass = this.config.workspaceClasses.find((cl) => cl.isDefault)?.id ?? "";
806+
if (await this.userService.userGetsMoreResources(user)) {
807+
workspaceClass = this.config.workspaceClasses.find((cl) => !cl.isDefault)?.id ?? "";
808+
}
809+
}
797810
} else {
798-
workspaceClass = workspace.workspaceClass;
811+
workspaceClass = previousInstance.workspaceClass;
799812
}
800813
}
801814

‎install/installer/pkg/components/server/configmap.go

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -237,9 +237,21 @@ func configmap(ctx *common.RenderContext) ([]runtime.Object, error) {
237237
// default limit for all cloneURLs
238238
"*": 50,
239239
},
240-
WorkspaceClasses: WorkspaceClasses{
241-
Default: "g1-standard",
242-
DefaultMoreResources: "g1-large",
240+
WorkspaceClasses: []WorkspaceClass{
241+
{
242+
Id: "g1-standard",
243+
DisplayName: "Standard",
244+
IsDefault: true,
245+
Deprecated: false,
246+
CreditsPerMinute: 10,
247+
},
248+
{
249+
Id: "g1-large",
250+
DisplayName: "Large",
251+
IsDefault: false,
252+
Deprecated: false,
253+
CreditsPerMinute: 20,
254+
},
243255
},
244256
}
245257

‎install/installer/pkg/components/server/types.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ type ConfigSerialized struct {
5252
// PrebuildLimiter defines the number of prebuilds allowed for each cloneURL in a given 1 minute interval
5353
// Key of "*" defines the default limit, unless there exists a cloneURL in the map which overrides it.
5454
PrebuildLimiter map[string]int `json:"prebuildLimiter"`
55-
WorkspaceClasses WorkspaceClasses `json:"workspaceClasses"`
55+
WorkspaceClasses []WorkspaceClass `json:"workspaceClasses"`
5656
}
5757

5858
type BlockedRepository struct {
@@ -135,9 +135,12 @@ type WorkspaceDefaults struct {
135135
TimeoutExtended *util.Duration `json:"timeoutExtended,omitempty"`
136136
}
137137

138-
type WorkspaceClasses struct {
139-
Default string `json:"default"`
140-
DefaultMoreResources string `json:"defaultMoreResources"`
138+
type WorkspaceClass struct {
139+
Id string `json:"id"`
140+
DisplayName string `json:"displayName"`
141+
IsDefault bool `json:"isDefault"`
142+
Deprecated bool `json:"deprecated"`
143+
CreditsPerMinute int32 `json:"creditsPerMinute"`
141144
}
142145

143146
type NamedWorkspaceFeatureFlag string

0 commit comments

Comments
 (0)