Skip to content

Commit 41b43fc

Browse files
committed
[server] Simplify class selection
1 parent 7d8781a commit 41b43fc

File tree

4 files changed

+41
-101
lines changed

4 files changed

+41
-101
lines changed

components/server/src/workspace/workspace-classes.spec.ts

Lines changed: 25 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,6 @@ let config: WorkspaceClassesConfig = [
1717
description: "Up to 4 vCPU, 8 GB memory, 30GB storage",
1818
powerups: 1,
1919
deprecated: false,
20-
resources: {
21-
cpu: 4,
22-
memory: 8,
23-
storage: 30,
24-
},
2520
},
2621
];
2722

@@ -36,11 +31,6 @@ config.push({
3631
marker: {
3732
moreResources: true,
3833
},
39-
resources: {
40-
cpu: 8,
41-
memory: 16,
42-
storage: 50,
43-
},
4434
});
4535

4636
config.push({
@@ -51,37 +41,50 @@ config.push({
5141
description: "Up to 8 vCPU, 16 GB memory, 50GB storage",
5242
powerups: 2,
5343
deprecated: true,
54-
resources: {
55-
cpu: 8,
56-
memory: 16,
57-
storage: 50,
44+
marker: {
45+
moreResources: true,
5846
},
5947
});
6048

6149
describe("workspace-classes", function () {
6250
describe("can substitute", function () {
6351
it("classes are the same", function () {
64-
const classId = WorkspaceClasses.canSubstitute("g1-large", "g1-large", config);
52+
const classId = WorkspaceClasses.selectClassForRegular("g1-large", "g1-large", config);
53+
expect(classId).to.be.equal("g1-large");
54+
});
55+
56+
it("prebuild has more resources, substitute has not", function () {
57+
const classId = WorkspaceClasses.selectClassForRegular("g1-large", "g1-standard", config);
6558
expect(classId).to.be.equal("g1-large");
6659
});
6760

68-
it("substitute is acceptable", function () {
69-
const classId = WorkspaceClasses.canSubstitute("g1-standard", "g1-large", config);
61+
it("prebuild has more resources, substitute also has more resources", function () {
62+
const classId = WorkspaceClasses.selectClassForRegular("g1-large", "g1-large", config);
7063
expect(classId).to.be.equal("g1-large");
7164
});
7265

73-
it("substitute is deprecated", function () {
74-
const classId = WorkspaceClasses.canSubstitute("g1-standard", "g1-deprecated", config);
66+
it("prebuild has more resources, substitute has not, prebuild is deprecated", function () {
67+
const classId = WorkspaceClasses.selectClassForRegular("g1-deprecated", "g1-standard", config);
7568
expect(classId).to.be.equal("g1-large");
7669
});
7770

78-
it("current is deprecated, substitute not acceptable", function () {
79-
const classId = WorkspaceClasses.canSubstitute("g1-deprecated", "g1-standard", config);
71+
it("prebuild has more resources, substitute has not, prebuild not deprecated", function () {
72+
const classId = WorkspaceClasses.selectClassForRegular("g1-large", "g1-standard", config);
8073
expect(classId).to.be.equal("g1-large");
8174
});
8275

76+
it("prebuild does not have more resources, return substitute", function () {
77+
const classId = WorkspaceClasses.selectClassForRegular("g1-standard", "g1-large", config);
78+
expect(classId).to.be.equal("g1-large");
79+
});
80+
81+
it("prebuild does not have more resources, substitute unknown", function () {
82+
const classId = WorkspaceClasses.selectClassForRegular("g1-standard", "g1-unknown", config);
83+
expect(classId).to.be.equal("g1-standard");
84+
});
85+
8386
it("substitute is not acceptable", function () {
84-
const classId = WorkspaceClasses.canSubstitute("g1-large", "g1-standard", config);
87+
const classId = WorkspaceClasses.selectClassForRegular("g1-large", "g1-standard", config);
8588
expect(classId).to.be.equal("g1-large");
8689
});
8790
});

components/server/src/workspace/workspace-classes.ts

Lines changed: 15 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -41,18 +41,6 @@ export interface WorkspaceClassConfig {
4141
// Marks this class as the one that users marked with "GetMoreResources" receive
4242
moreResources: boolean;
4343
};
44-
45-
// The resources that this class provides
46-
resources: WorkspaceClassResources;
47-
}
48-
49-
export interface WorkspaceClassResources {
50-
// Storage in gigabyte
51-
storage: number;
52-
// Number of cpus
53-
cpu: number;
54-
// Memory in gigabyte
55-
memory: number;
5644
}
5745

5846
export namespace WorkspaceClasses {
@@ -182,19 +170,11 @@ export namespace WorkspaceClasses {
182170
}
183171

184172
/**
185-
* Checks if the current class can be replaced by another class
186-
* - If both classes are the same the current class will be returned
187-
* - If the proposed substitute class has at least as much resources as the current class replace it
188-
* - If the substitute does not provide sufficient resources
189-
* - If current class is deprecated
190-
* - Try to find another class that provides at least as much resources as the deprecated one
191-
* - If this also fails, return default class
192-
* - If current class is not deprecated return current class
193173
* @param currentClassId
194174
* @param substituteClassId
195175
* @param classes
196176
*/
197-
export function canSubstitute(
177+
export function selectClassForRegular(
198178
currentClassId: string,
199179
substituteClassId: string | undefined,
200180
classes: WorkspaceClassesConfig,
@@ -206,50 +186,22 @@ export namespace WorkspaceClasses {
206186
const current = classes.find((c) => c.id === currentClassId);
207187
let substitute = classes.find((c) => c.id === substituteClassId);
208188

209-
if (!current) {
210-
throw new Error("class not defined: " + currentClassId);
211-
}
212-
213-
if (!substitute) {
214-
throw new Error("class not defined: " + substituteClassId);
215-
}
216-
217-
if (substitute.deprecated) {
218-
substitute = findClosestAlternative(substitute, classes);
219-
}
220-
221-
if (substitute && providesMinimalResources(substitute, current)) {
222-
return substitute.id;
223-
}
224-
225-
if (current.deprecated) {
226-
const alternative = findClosestAlternative(current, classes);
227-
if (!alternative) {
228-
return getDefaultId(classes);
189+
if (current?.marker?.moreResources) {
190+
if (substitute?.marker?.moreResources) {
191+
return substitute?.id;
229192
} else {
230-
return alternative.id;
193+
if (current.deprecated) {
194+
return getMoreResourcesIdOrDefault(classes);
195+
} else {
196+
return current.id;
197+
}
198+
}
199+
} else {
200+
if (substitute?.id) {
201+
return substitute.id;
202+
} else {
203+
return getDefaultId(classes);
231204
}
232205
}
233-
234-
return current.id;
235-
}
236-
237-
function providesMinimalResources(class1: WorkspaceClassConfig, class2: WorkspaceClassConfig): boolean {
238-
return (
239-
class1.category === class2.category &&
240-
class1.resources.cpu >= class2.resources.cpu &&
241-
class1.resources.memory >= class2.resources.memory &&
242-
class1.resources.storage >= class2.resources.storage
243-
);
244-
}
245-
246-
function findClosestAlternative(
247-
current: WorkspaceClassConfig,
248-
classes: WorkspaceClassesConfig,
249-
): WorkspaceClassConfig | undefined {
250-
return classes
251-
.filter((cl) => !cl.deprecated)
252-
.sort((a, b) => a.resources.storage - b.resources.storage)
253-
.find((cl) => providesMinimalResources(cl, current));
254206
}
255207
}

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

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -428,11 +428,6 @@ class WorkspaceClassTestBuilder {
428428
description: "Up to 4 vCPU, 8 GB memory, 30GB storage",
429429
powerups: 1,
430430
deprecated: false,
431-
resources: {
432-
cpu: 4,
433-
memory: 8,
434-
storage: 30,
435-
},
436431
},
437432
];
438433

@@ -447,11 +442,6 @@ class WorkspaceClassTestBuilder {
447442
marker: {
448443
moreResources: this.hasMoreResources,
449444
},
450-
resources: {
451-
cpu: 8,
452-
memory: 16,
453-
storage: 50,
454-
},
455445
});
456446

457447
config.push({
@@ -462,11 +452,6 @@ class WorkspaceClassTestBuilder {
462452
description: "Up to 8 vCPU, 16 GB memory, 50GB storage",
463453
powerups: 2,
464454
deprecated: true,
465-
resources: {
466-
cpu: 8,
467-
memory: 16,
468-
storage: 50,
469-
},
470455
});
471456

472457
const workspaceDb = new MockWorkspaceDb(!!this.basedOnPrebuild);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ export async function getWorkspaceClassForInstance(
210210
config,
211211
entitlementService,
212212
);
213-
workspaceClass = WorkspaceClasses.canSubstitute(prebuildClass, userClass, config);
213+
workspaceClass = WorkspaceClasses.selectClassForRegular(prebuildClass, userClass, config);
214214
} else if (user.additionalData?.workspaceClasses?.regular) {
215215
workspaceClass = user.additionalData?.workspaceClasses?.regular;
216216
}

0 commit comments

Comments
 (0)