[installer] Allow to set default workspace timeout #8576

corneliusludmann · 2022-03-03T16:34:08Z

Description

This is a proposal that allows to set custom timeouts for the workspaces. It adds the following config values:

workspace:
  timeoutDefault: 30m
  timeoutExtended: 60m
  maxLifetime: 36h
  timeoutAfterClose: 2m

Related Issue(s)

Fixes #8528

How to test

Add the following patch to deploy-to-preview-environment.ts:

diff --git a/.werft/jobs/build/deploy-to-preview-environment.ts b/.werft/jobs/build/deploy-to-preview-environment.ts
index 4c82fa8d..b963703a 100644
--- a/.werft/jobs/build/deploy-to-preview-environment.ts
+++ b/.werft/jobs/build/deploy-to-preview-environment.ts
@@ -392,6 +392,12 @@ async function deployToDevWithInstaller(werft: Werft, jobConfig: JobConfig, depl
             exec('exit 0')
         }
 
+        // TODO: remove me – just for testing purposes
+        exec(`yq w -i ./config.yaml workspace.timeoutDefault "3m"`);
+        exec(`yq w -i ./config.yaml workspace.timeoutExtended "6m"`);
+        exec(`yq w -i ./config.yaml workspace.maxLifetime "9m"`);
+        exec(`yq w -i ./config.yaml workspace.timeoutAfterClose "10s"`);
+
         // validate the config
         exec(`/tmp/installer validate config -c config.yaml`, { slice: installerSlices.INSTALLER_RENDER });

And run a werft build like this:

$ werft run github -s .werft/jobs/build/deploy-to-preview-environment.ts

NOTE: I did this already. As long as the preview env hasn't been removed in the meantime, the preview should be deployed like this.

With this configuration you can observe the following timeouts:

When you start a workspace, it times out after 3 minutes of inactivity. You can see this value when you run (change your workspace ID):
```
$ kubectl describe pod ws-50950ba6-eea9-4d06-8aa9-a98f6a19811d  | grep customTimeout
              gitpod/customTimeout: 3m0s
```
You'll also see the current timeout when you hover over the clock in your workspace status bar:
When you extend your workspace timeout (click on the clock), the new timeout will be 6 minutes (check again with kubectl or the hover message).
When you close the browser, the workspace times out after 10 seconds. (Note: The value that you see with kubectl will not change but you can see that the pod disappears after 10 seconds and the Gitpod dashboard shows stopping.)
The workspace will be stopped after 9 minutes. Even if there is activity. It will stop in every case.

Discussion and Limitations

I tried to eliminate the hard-coded values in the code and add “classes” of timeouts (short, long, and extended). However, the VS Code IDE sends always 180m as a timeout value when a user clicks on the clock. That's why I kept the 180m constant as an alternative value for extended. In the long term, it would make sense to send extended instead.
When the user extends the workspace, the message always says it's extended to “three hours”. That should be changed to a more general wording.
The hover text always shows the proper timeout value but the color of the clock is checked by comparing it with the hard-coded value of 180m. The problem is, that we don't know at this point what the extended timeout value is. Instead of reading listener.info.latestInstance, we could also call
```
const workspaceTimeout = await context.gitpod.server.getWorkspaceTimeout(context.info.getWorkspaceId());
```
instead. That always returns 180m as timeout as constant for the extended value (should be probably changed to extended in the long term, see above). However, I don't know if that has a performance impact.

All of these are little UI flaws that shouldn't be real blockers, in my opinion.

/cc @akosyakov

Release Notes

Allow setting custom workspace timeouts in the Gitpod Installer for self-hosted installations.

codecov · 2022-03-03T16:41:33Z

Codecov Report

Merging #8576 (9f64d53) into main (a567595) will decrease coverage by 0.01%.
The diff coverage is 0.00%.

❗ Current head 9f64d53 differs from pull request most recent head ee9e7bb. Consider uploading reports for the commit ee9e7bb to get more accurate results

@@           Coverage Diff            @@
##            main   #8576      +/-   ##
========================================
- Coverage   7.34%   7.32%   -0.02%     
========================================
  Files         32      32              
  Lines       2234    2238       +4     
========================================
  Hits         164     164              
- Misses      2067    2071       +4     
  Partials       3       3

Flag	Coverage Δ
components-gitpod-cli-app	`11.17% <ø> (ø)`
install-installer-raw-app	`4.25% <0.00%> (-0.02%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...l/installer/pkg/components/ws-manager/configmap.go	`23.17% <0.00%> (-0.58%)`	⬇️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

corneliusludmann · 2022-03-11T16:15:21Z

components/server/ee/src/user/user-service.ts

+    public workspaceTimeoutToDuration(timeout: WorkspaceTimeoutDuration): string {
+        switch (timeout) {
+            case WORKSPACE_TIMEOUT_DEFAULT_SHORT:
+                return "30m";
+            case WORKSPACE_TIMEOUT_DEFAULT_LONG:
+                return this.config.workspaceDefaults.timeoutDefault || "60m";
+            case WORKSPACE_TIMEOUT_EXTENDED:
+            case WORKSPACE_TIMEOUT_EXTENDED_ALT:
+                return this.config.workspaceDefaults.timeoutExtended || "180m";
+        }
+    }
+
+    public durationToWorkspaceTimeout(duration: string): WorkspaceTimeoutDuration {
+        switch (duration) {
+            case "30m":
+                return WORKSPACE_TIMEOUT_DEFAULT_SHORT;
+            case this.config.workspaceDefaults.timeoutDefault || "60m":
+                return WORKSPACE_TIMEOUT_DEFAULT_LONG;
+            case this.config.workspaceDefaults.timeoutExtended || "180m":
+                return WORKSPACE_TIMEOUT_EXTENDED_ALT;
+            default:
+                return WORKSPACE_TIMEOUT_DEFAULT_SHORT;
+        }


I don't know if user-service is a good place for these functions. I was too miserly to create a dedicated service for this that does nothing but provide these values. However, I'm very open to proposals for better places.

I also have no idea if user-service is a good place for this, but if nobody has an opinion on this, then let's say it is a good place. 🙌

princerachit

did not test but code changes lgtm

akosyakov · 2022-03-15T09:53:58Z

All of these are little UI flaws that shouldn't be real blockers, in my opinion.

Could you file a follow-up issue for VS Code please? 🙏 and add to IDE team inbox

akosyakov · 2022-03-15T09:56:17Z

I wonder why the PR is marked wit team: IDE label? There are no changes for IDE components.

@mads-hartmann Should we remove this label? Or the deploy labeller is a smart enough to check only for teams which actually affected?

corneliusludmann · 2022-03-15T10:25:44Z

@akosyakov I requested a review from the IDE team due to the IDE flaws with timeout extends. Just wanted to make sure that the IDE team has a look that is does not break the communication with the IDE.

mads-hartmann · 2022-03-16T08:29:48Z

@akosyakov The label gets added when a team is requested as reviewers - the intention is that CODEOWNERS defines reviewers, but for cases like this where you just want a 2nd pair of eyes I agree it isn't ideal. Removing the label is these cases is the best way forward :)

akosyakov · 2022-03-30T08:08:33Z

A review from webapp team is required here to land correct? cc @geropl

mrsimonemms · 2022-03-30T08:50:52Z

@akosyakov I don't think there's much point at the moment as there's some conflicted files. I'll leave this today in the hope that @corneliusludmann is feeling better by tomorrow, otherwise I'll do my best at resolving the conflicts

mrsimonemms · 2022-04-04T15:13:42Z

Looks good. Can you update the example-config.yaml with the result of the init command? It adds the workspace.maxLifetime to it

corneliusludmann · 2022-04-04T16:12:32Z

Looks good. Can you update the example-config.yaml with the result of the init command? It adds the workspace.maxLifetime to it

Done.

jankeromnes · 2022-04-06T08:12:00Z

Many thanks for this contribution! Taking a look on behalf of team WebApp 👀

jankeromnes

The WebApp component changes look good to me! ✨

I've also tested the latest deployment, and it seems to work flawlessly. 🥇

I've added a few comments/thoughts/questions in-line (mostly about naming things and better understanding this change, but please feel free to dismiss my comments and/or open follow-up issues to be addressed later without blocking your PR).

As a side note, this reminds me that I've always wanted to also make the Prebuild timeouts more "dynamic" (i.e. not necessarily configurable by installers as in your PR, but it would be nice -- in addition to the default 1h timeout -- to also have a 4h timeout for long-building projects, e.g. only for base prebuilds when you have Incremental Prebuilds enabled). No action required here, but sharing my thoughts as they seem somewhat related.

Adding a hold in case you'd like to address some comments, but feel free to unhold any time:

/hold

components/server/ee/src/user/user-service.ts

jankeromnes · 2022-04-06T08:24:30Z

components/server/ee/src/user/user-service.ts

+    public workspaceTimeoutToDuration(timeout: WorkspaceTimeoutDuration): string {
+        switch (timeout) {
+            case WORKSPACE_TIMEOUT_DEFAULT_SHORT:
+                return "30m";
+            case WORKSPACE_TIMEOUT_DEFAULT_LONG:
+                return this.config.workspaceDefaults.timeoutDefault || "60m";
+            case WORKSPACE_TIMEOUT_EXTENDED:
+            case WORKSPACE_TIMEOUT_EXTENDED_ALT:
+                return this.config.workspaceDefaults.timeoutExtended || "180m";
+        }
+    }
+
+    public durationToWorkspaceTimeout(duration: string): WorkspaceTimeoutDuration {
+        switch (duration) {
+            case "30m":
+                return WORKSPACE_TIMEOUT_DEFAULT_SHORT;
+            case this.config.workspaceDefaults.timeoutDefault || "60m":
+                return WORKSPACE_TIMEOUT_DEFAULT_LONG;
+            case this.config.workspaceDefaults.timeoutExtended || "180m":
+                return WORKSPACE_TIMEOUT_EXTENDED_ALT;
+            default:
+                return WORKSPACE_TIMEOUT_DEFAULT_SHORT;
+        }


I also have no idea if user-service is a good place for this, but if nobody has an opinion on this, then let's say it is a good place. 🙌

jankeromnes · 2022-04-06T08:28:22Z

components/server/ee/src/user/user-service.ts

+        }
+    }
+
+    public durationToWorkspaceTimeout(duration: string): WorkspaceTimeoutDuration {


I find the types a little confusing in these method signatures:

duration is a string, and the returned Timeout is a WorkspaceTimeoutDuration? 🤔

Also workspaceTimeoutToDuration takes a WorkspaceTimeoutDuration and turns it into a string (would expect the other way around)

I feel like duration should be of type ...Duration, and timeout of type ...Timeout. 💭

Maybe this could be a follow-up issue though. ⏩

jankeromnes · 2022-04-06T08:30:28Z

components/server/ee/src/workspace/gitpod-server-impl.ts


                return client.setTimeout(ctx, req);
            }),
        );

        const req = new SetTimeoutRequest();
        req.setId(runningInstance.id);
-        req.setDuration(duration);
+        req.setDuration(this.userService.workspaceTimeoutToDuration(duration));


Micro-nit: Unfortunate variable naming 🙈 somethingElseToDuration(duration) ("...wait, isn't it already a duration?")

jankeromnes · 2022-04-06T08:31:41Z

components/server/ee/src/workspace/gitpod-server-impl.ts

@@ -352,7 +355,7 @@ export class GitpodServerEEImpl extends GitpodServerImpl {

        const client = await this.workspaceManagerClientProvider.get(runningInstance.region);
        const desc = await client.describeWorkspace(ctx, req);
-        const duration = desc.getStatus()!.getSpec()!.getTimeout() as WorkspaceTimeoutDuration;
+        const duration = this.userService.durationToWorkspaceTimeout(desc.getStatus()!.getSpec()!.getTimeout());


Micro-nit: Same here, we're calling somethingElseToTimeout(...getTimeout()) 🙈 (as in, turn a timeout into a timeout?)

But let's not fix this now. I understand this came up for historical reasons, and I don't have a good suggestion on how to improve it now.

components/server/src/user/user-service.ts

corneliusludmann · 2022-04-06T09:04:30Z

Thanks, @jankeromnes for your great feedback!

Since your comments are more very useful improvements and no blockers, as suggested, I'll let this pull request merge now and handle your comments in follow-up issues/pull requests. That makes the review process easier because we don't need approval from the other teams for the proposed changes.

Thanks again! 🙏

/unhold

neilkuantm · 2022-05-17T00:31:38Z

Hi @corneliusludmann , how can I setting Default workspace CustomTime after install via kots?

roboquat added do-not-merge/work-in-progress do-not-merge/release-note-label-needed size/M labels Mar 3, 2022

roboquat added size/L and removed size/M labels Mar 4, 2022

corneliusludmann force-pushed the clu/ws-timeout branch 6 times, most recently from ac8b378 to 9e73691 Compare March 9, 2022 13:56

corneliusludmann commented Mar 11, 2022

View reviewed changes

roboquat added release-note and removed do-not-merge/release-note-label-needed labels Mar 11, 2022

corneliusludmann marked this pull request as ready for review March 11, 2022 16:18

corneliusludmann requested review from a team March 11, 2022 16:18

roboquat removed the do-not-merge/work-in-progress label Mar 11, 2022

github-actions bot added team: delivery Issue belongs to the self-hosted team team: webapp Issue belongs to the WebApp team team: workspace Issue belongs to the Workspace team labels Mar 11, 2022

corneliusludmann requested a review from a team March 11, 2022 16:20

github-actions bot added the team: IDE label Mar 11, 2022

princerachit previously approved these changes Mar 14, 2022

View reviewed changes

stale bot added the meta: stale This issue/PR is stale and will be closed soon label Mar 29, 2022

Furisto added meta: never-stale This issue can never become stale and removed meta: stale This issue/PR is stale and will be closed soon labels Mar 29, 2022

corneliusludmann mentioned this pull request Apr 4, 2022

Epic: User Configurable Workspace Timeouts (CLI command + user preference) #9038

Closed

6 tasks

corneliusludmann dismissed stale reviews from mrsimonemms and princerachit via 39906ef April 4, 2022 14:58

corneliusludmann force-pushed the clu/ws-timeout branch from 9e73691 to 39906ef Compare April 4, 2022 14:58

corneliusludmann requested review from mrsimonemms and princerachit April 4, 2022 15:00

github-actions bot added the team: IDE label Apr 4, 2022

[installer] Allow to set default workspace timeout

ee9e7bb

corneliusludmann force-pushed the clu/ws-timeout branch from 39906ef to ee9e7bb Compare April 4, 2022 16:12

mrsimonemms approved these changes Apr 4, 2022

View reviewed changes

princerachit approved these changes Apr 5, 2022

View reviewed changes

jankeromnes self-assigned this Apr 6, 2022

jankeromnes approved these changes Apr 6, 2022

View reviewed changes

roboquat added the do-not-merge/hold label Apr 6, 2022

roboquat removed the do-not-merge/hold label Apr 6, 2022

roboquat merged commit 5166afd into main Apr 6, 2022

roboquat deleted the clu/ws-timeout branch April 6, 2022 09:05

roboquat added deployed: webapp Meta team change is running in production deployed: IDE IDE change is running in production deployed: workspace Workspace team change is running in production labels Apr 7, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[installer] Allow to set default workspace timeout #8576

[installer] Allow to set default workspace timeout #8576

corneliusludmann commented Mar 3, 2022 •

edited

Loading

codecov bot commented Mar 3, 2022 •

edited

Loading

corneliusludmann Mar 11, 2022

jankeromnes Apr 6, 2022

princerachit left a comment

akosyakov commented Mar 15, 2022 •

edited by werft-gitpod-dev-com bot

Loading

akosyakov commented Mar 15, 2022

corneliusludmann commented Mar 15, 2022

mads-hartmann commented Mar 16, 2022

akosyakov commented Mar 30, 2022 •

edited

Loading

mrsimonemms commented Mar 30, 2022

mrsimonemms commented Apr 4, 2022

corneliusludmann commented Apr 4, 2022

jankeromnes commented Apr 6, 2022 •

edited by werft-gitpod-dev-com bot

Loading

jankeromnes left a comment •

edited

Loading

jankeromnes Apr 6, 2022

jankeromnes Apr 6, 2022

jankeromnes Apr 6, 2022 •

edited

Loading

jankeromnes Apr 6, 2022 •

edited

Loading

corneliusludmann commented Apr 6, 2022

neilkuantm commented May 17, 2022

[installer] Allow to set default workspace timeout #8576

[installer] Allow to set default workspace timeout #8576

Conversation

corneliusludmann commented Mar 3, 2022 • edited Loading

Description

Related Issue(s)

How to test

Discussion and Limitations

Release Notes

codecov bot commented Mar 3, 2022 • edited Loading

Codecov Report

corneliusludmann Mar 11, 2022

Choose a reason for hiding this comment

jankeromnes Apr 6, 2022

Choose a reason for hiding this comment

princerachit left a comment

Choose a reason for hiding this comment

akosyakov commented Mar 15, 2022 • edited by werft-gitpod-dev-com bot Loading

akosyakov commented Mar 15, 2022

corneliusludmann commented Mar 15, 2022

mads-hartmann commented Mar 16, 2022

akosyakov commented Mar 30, 2022 • edited Loading

mrsimonemms commented Mar 30, 2022

mrsimonemms commented Apr 4, 2022

corneliusludmann commented Apr 4, 2022

jankeromnes commented Apr 6, 2022 • edited by werft-gitpod-dev-com bot Loading

jankeromnes left a comment • edited Loading

Choose a reason for hiding this comment

jankeromnes Apr 6, 2022

Choose a reason for hiding this comment

jankeromnes Apr 6, 2022

Choose a reason for hiding this comment

jankeromnes Apr 6, 2022 • edited Loading

Choose a reason for hiding this comment

jankeromnes Apr 6, 2022 • edited Loading

Choose a reason for hiding this comment

corneliusludmann commented Apr 6, 2022

neilkuantm commented May 17, 2022

corneliusludmann commented Mar 3, 2022 •

edited

Loading

codecov bot commented Mar 3, 2022 •

edited

Loading

akosyakov commented Mar 15, 2022 •

edited by werft-gitpod-dev-com bot

Loading

akosyakov commented Mar 30, 2022 •

edited

Loading

jankeromnes commented Apr 6, 2022 •

edited by werft-gitpod-dev-com bot

Loading

jankeromnes left a comment •

edited

Loading

jankeromnes Apr 6, 2022 •

edited

Loading

jankeromnes Apr 6, 2022 •

edited

Loading