[server] Separate instance creation from instance starting (again) #18642

geropl · 2023-09-01T09:02:46Z

Description

In preparation of making sure we don't break workspace starts during rollouts, this PR does some cleanup in WorkspaceStarter. Most importantly, we ensure that after a WorkspaceInstance is stored in the DB, we can start a workspace from it. 👍 (also, makes the code cleaner).

Short summary to make the changes easier to digest:

introduced buildImageAndStartWorkspace to "mark" that separation, but also for usage in upcoming PRs
remove options.rethrow: not used anymore
instead of piping lastValidWorkspaceInstanceId through multiple layers for a simple check, condense it into fromBackup right away
persist the parts of ideConfig that we need during workspace start into instance.configuration.ideSetup

Summary generated by Copilot

`🤖 Generated by Copilot at f4fa9fd`

This pull request adds a new interface for IDE setup, refactors the workspace-starter and ide-service modules, and makes the configuration field mandatory for workspace instances.

Related Issue(s)

Related to EXP-549
Depends on: #18641

How to test

Documentation

Preview status

Gitpod was successfully deployed to your preview environment.

🏷️ Name - gpl-ws-starter-2
🔗 URL - gpl-ws-starter-2.preview.gitpod-dev.com/workspaces.
📚 Documentation - See our internal documentation for information on how to interact with your preview environment.
📦 Version - gpl-ws-starter-2-gha.16325
🗒️ Logs - GCP Logs Explorer

Build Options

Build

/werft with-werft
Run the build with werft instead of GHA
leeway-no-cache
/werft no-test
Run Leeway with --dont-test

Publish

/werft publish-to-npm
/werft publish-to-jb-marketplace

Installer

analytics=segment
with-dedicated-emulation
workspace-feature-flags
Add desired feature flags to the end of the line above, space separated

Preview Environment / Integration Tests

/werft with-local-preview
If enabled this will build install/preview
/werft with-preview
/werft with-large-vm
/werft with-gce-vm
If enabled this will create the environment on GCE infra
with-integration-tests=all
Valid options are all, workspace, webapp, ide, jetbrains, vscode, ssh. If enabled, with-preview and with-large-vm will be enabled.
with-monitoring

/hold

geropl · 2023-09-01T12:42:56Z

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

@@ -101,7 +101,7 @@ export class DBWorkspaceInstance implements WorkspaceInstance {
        type: "simple-json",
        nullable: true,
    })
-    configuration?: WorkspaceInstanceConfiguration;
+    configuration: WorkspaceInstanceConfiguration;


I checked the DB: We have 0 instances where this field is NULL or "".

geropl · 2023-09-01T12:43:21Z

components/gitpod-protocol/src/workspace-instance.ts

-    // Beware: this field was added retroactively and not all instances have valid
-    //         values here.
-    configuration?: WorkspaceInstanceConfiguration;
+    configuration: WorkspaceInstanceConfiguration;


I checked the DB: We have 0 instances where this field is NULL or "".

geropl · 2023-09-01T12:44:18Z

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

@@ -129,7 +130,6 @@ import { SYSTEM_USER } from "../authorization/authorizer";
 import { ResolvedEnvVars } from "../user/env-var-service";

 export interface StartWorkspaceOptions extends GitpodServer.StartWorkspaceOptions {


rethrow was never anywhere anymore.

geropl · 2023-09-01T12:48:39Z

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

@@ -308,6 +308,7 @@ export class WorkspaceStarter {
                project,
                options.excludeFeatureFlags || [],
                ideConfig,
+                options.region,


With this change, after instance creation, there is no need for any further lookup: The instance shape contains everything that is needed to start a workspace.

Only exception that is up for discussion: envVars. Ideally we should store those as well, but I refrained because we might store unencrypted passwords or similar.
Open for comments and ideas here!

Is it because JSON stored unencrypted? i.e. how is it different to value in env var tables?

i.e. how is it different to value in env var tables?

There we use an encrypting transformer (source).

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

geropl · 2023-09-01T12:51:50Z

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

    ): Promise<{ initializer: WorkspaceInitializer; disposable: Disposable }> {
        let result = new WorkspaceInitializer();
        const disp = new DisposableCollection();

-        if (lastValidWorkspaceInstanceId != "") {
+        if (fromBackup) {


This check was the only reason we piped lastValidWorkspaceInstanceId: string through 4 layers... 🙈

geropl · 2023-09-01T13:09:27Z

@akosyakov Would be great to get your feedback for the changes around ideConfig. I assumed that ideConfig.tasks or .envvars never contain secrets or similar, but would be great if you could confirm. 🙏

akosyakov · 2023-09-01T13:29:30Z

@geropl You can check here:

gitpod/components/ide-service/pkg/server/server.go

Line 352 in 938f0ba

    
           func (s *IDEServiceServer) ResolveWorkspaceConfig(ctx context.Context, req *api.ResolveWorkspaceConfigRequest) (resp *api.ResolveWorkspaceConfigResponse, err error) {

I think we don't have any tasks anymore, we added them for JB prebuild, but they moved to supervisor by now. There are couple env vars but they are not secrets.

akosyakov · 2023-09-04T06:33:24Z

/gh run recreate-vm=true

Comment triggered a workflow run

Started workflow run: 6069970179

recreate_vm: true

akosyakov

I smoke tested that workspace with custom settings still can be started. Code changes also look reasonable.

akosyakov · 2023-09-04T07:27:03Z

@geropl Prebuilds are another path right? We will need to take care about them separately from PrebuildManager?

geropl · 2023-09-04T07:29:17Z

Prebuilds are another path right?

Prebuilds are using this path as well (source).

geropl · 2023-09-04T07:32:11Z

/unhold

roboquat added do-not-merge/hold do-not-merge/work-in-progress size/L labels Sep 1, 2023

geropl force-pushed the gpl/ws-starter branch 4 times, most recently from 15ab10f to bd3a5a7 Compare September 1, 2023 12:15

geropl force-pushed the gpl/ws-starter-2 branch from f4fa9fd to 349ec71 Compare September 1, 2023 12:34

Base automatically changed from gpl/ws-starter to main September 1, 2023 12:39

roboquat added size/XL and removed size/L labels Sep 1, 2023

[server] Separate instance creation from instance starting (again)

c7bc851

geropl force-pushed the gpl/ws-starter-2 branch from 349ec71 to c7bc851 Compare September 1, 2023 12:40

roboquat added size/L and removed size/XL labels Sep 1, 2023

geropl commented Sep 1, 2023

View reviewed changes

components/server/src/workspace/workspace-starter.ts Outdated Show resolved Hide resolved

geropl commented Sep 1, 2023

View reviewed changes

geropl marked this pull request as ready for review September 1, 2023 13:00

geropl requested a review from a team as a code owner September 1, 2023 13:00

roboquat removed the do-not-merge/work-in-progress label Sep 1, 2023

github-actions bot added the team: team-experience label Sep 1, 2023

[server] Set instance.configuration.fromBackup

1d40a2b

geropl requested a review from akosyakov September 1, 2023 13:08

akosyakov approved these changes Sep 4, 2023

View reviewed changes

roboquat removed the do-not-merge/hold label Sep 4, 2023

roboquat merged commit 86fa3ba into main Sep 4, 2023

roboquat deleted the gpl/ws-starter-2 branch September 4, 2023 07:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[server] Separate instance creation from instance starting (again) #18642

[server] Separate instance creation from instance starting (again) #18642

Uh oh!

geropl commented Sep 1, 2023 •

edited by github-actions bot

Loading

Uh oh!

geropl Sep 1, 2023

Uh oh!

geropl Sep 1, 2023

Uh oh!

geropl Sep 1, 2023

Uh oh!

geropl Sep 1, 2023

Uh oh!

akosyakov Sep 4, 2023

Uh oh!

geropl Sep 4, 2023

Uh oh!

Uh oh!

geropl Sep 1, 2023

Uh oh!

geropl commented Sep 1, 2023

Uh oh!

akosyakov commented Sep 1, 2023

Uh oh!

akosyakov commented Sep 4, 2023 •

edited by github-actions bot

Loading

Uh oh!

akosyakov left a comment

Uh oh!

akosyakov commented Sep 4, 2023

Uh oh!

geropl commented Sep 4, 2023

Uh oh!

geropl commented Sep 4, 2023

Uh oh!

Uh oh!

		@@ -129,7 +130,6 @@ import { SYSTEM_USER } from "../authorization/authorizer";
		import { ResolvedEnvVars } from "../user/env-var-service";

		export interface StartWorkspaceOptions extends GitpodServer.StartWorkspaceOptions {

[server] Separate instance creation from instance starting (again) #18642

[server] Separate instance creation from instance starting (again) #18642

Uh oh!

Conversation

geropl commented Sep 1, 2023 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

🤖 Generated by Copilot at f4fa9fd

Related Issue(s)

How to test

Documentation

Preview status

Build Options

Uh oh!

geropl Sep 1, 2023

Choose a reason for hiding this comment

Uh oh!

geropl Sep 1, 2023

Choose a reason for hiding this comment

Uh oh!

geropl Sep 1, 2023

Choose a reason for hiding this comment

Uh oh!

geropl Sep 1, 2023

Choose a reason for hiding this comment

Uh oh!

akosyakov Sep 4, 2023

Choose a reason for hiding this comment

Uh oh!

geropl Sep 4, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

geropl Sep 1, 2023

Choose a reason for hiding this comment

Uh oh!

geropl commented Sep 1, 2023

Uh oh!

akosyakov commented Sep 1, 2023

Uh oh!

akosyakov commented Sep 4, 2023 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Comment triggered a workflow run

Uh oh!

akosyakov left a comment

Choose a reason for hiding this comment

Uh oh!

akosyakov commented Sep 4, 2023

Uh oh!

geropl commented Sep 4, 2023

Uh oh!

geropl commented Sep 4, 2023

Uh oh!

Uh oh!

geropl commented Sep 1, 2023 •

edited by github-actions bot

Loading

`🤖 Generated by Copilot at f4fa9fd`

akosyakov commented Sep 4, 2023 •

edited by github-actions bot

Loading