Skip to content
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.

Sign up for GitHub

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

[installer] Allow to set default workspace timeout #8576

Merged
merged 1 commit into from
Apr 6, 2022
Merged

[installer] Allow to set default workspace timeout #8576

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 10 additions & 1 deletion components/gitpod-protocol/src/gitpod-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,16 @@ export interface ClientHeaderFields {
clientRegion?: string;
}

export const WorkspaceTimeoutValues = ["30m", "60m", "180m"] as const;
export const WORKSPACE_TIMEOUT_DEFAULT_SHORT = "short";
export const WORKSPACE_TIMEOUT_DEFAULT_LONG = "long";
export const WORKSPACE_TIMEOUT_EXTENDED = "extended";
export const WORKSPACE_TIMEOUT_EXTENDED_ALT = "180m"; // for backwards compatibility since the IDE uses this
export const WorkspaceTimeoutValues = [
WORKSPACE_TIMEOUT_DEFAULT_SHORT,
WORKSPACE_TIMEOUT_DEFAULT_LONG,
WORKSPACE_TIMEOUT_EXTENDED,
WORKSPACE_TIMEOUT_EXTENDED_ALT,
] as const;

export const createServiceMock = function <C extends GitpodClient, S extends GitpodServer>(
methods: Partial<JsonRpcProxy<S>>,
Expand Down
6 changes: 3 additions & 3 deletions components/server/ee/src/user/eligibility-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import { inject, injectable } from "inversify";
import { TeamSubscriptionDB, UserDB } from "@gitpod/gitpod-db/lib";
import { TokenProvider } from "../../../src/user/token-provider";
import { User, WorkspaceTimeoutDuration, WorkspaceInstance } from "@gitpod/gitpod-protocol";
import { User, WorkspaceTimeoutDuration, WorkspaceInstance, WORKSPACE_TIMEOUT_DEFAULT_LONG, WORKSPACE_TIMEOUT_DEFAULT_SHORT } from "@gitpod/gitpod-protocol";
import { RemainingHours } from "@gitpod/gitpod-protocol/lib/accounting-protocol";
import { log } from "@gitpod/gitpod-protocol/lib/util/logging";
import { Plans, MAX_PARALLEL_WORKSPACES } from "@gitpod/gitpod-protocol/lib/plans";
Expand Down Expand Up @@ -251,9 +251,9 @@ export class EligibilityService {
*/
async getDefaultWorkspaceTimeout(user: User, date: Date = new Date()): Promise<WorkspaceTimeoutDuration> {
if (await this.maySetTimeout(user, date)) {
return "60m";
return WORKSPACE_TIMEOUT_DEFAULT_LONG;
} else {
return "30m";
return WORKSPACE_TIMEOUT_DEFAULT_SHORT;
}
}

Expand Down
33 changes: 30 additions & 3 deletions components/server/ee/src/user/user-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import { UserService, CheckSignUpParams, CheckTermsParams } from "../../../src/user/user-service";
import { User, WorkspaceTimeoutDuration } from "@gitpod/gitpod-protocol";
import { User, WorkspaceTimeoutDuration, WORKSPACE_TIMEOUT_EXTENDED, WORKSPACE_TIMEOUT_EXTENDED_ALT, WORKSPACE_TIMEOUT_DEFAULT_LONG, WORKSPACE_TIMEOUT_DEFAULT_SHORT } from "@gitpod/gitpod-protocol";
import { inject } from "inversify";
import { LicenseEvaluator } from "@gitpod/licensor/lib";
import { Feature } from "@gitpod/licensor/lib/api";
Expand All @@ -14,13 +14,15 @@ import { EligibilityService } from "./eligibility-service";
import { SubscriptionService } from "@gitpod/gitpod-payment-endpoint/lib/accounting";
import { OssAllowListDB } from "@gitpod/gitpod-db/lib/oss-allowlist-db";
import { HostContextProvider } from "../../../src/auth/host-context-provider";
import { Config } from "../../../src/config";

export class UserServiceEE extends UserService {
@inject(LicenseEvaluator) protected readonly licenseEvaluator: LicenseEvaluator;
@inject(EligibilityService) protected readonly eligibilityService: EligibilityService;
@inject(SubscriptionService) protected readonly subscriptionService: SubscriptionService;
@inject(OssAllowListDB) protected readonly OssAllowListDb: OssAllowListDB;
@inject(HostContextProvider) protected readonly hostContextProvider: HostContextProvider;
@inject(Config) protected readonly config: Config;

async getDefaultWorkspaceTimeout(user: User, date: Date): Promise<WorkspaceTimeoutDuration> {
if (this.config.enablePayment) {
Expand All @@ -32,10 +34,35 @@ export class UserServiceEE extends UserService {

// the self-hosted case
if (!this.licenseEvaluator.isEnabled(Feature.FeatureSetTimeout, userCount)) {
return "30m";
return WORKSPACE_TIMEOUT_DEFAULT_SHORT;
}

return "60m";
return WORKSPACE_TIMEOUT_DEFAULT_LONG;
}

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";
corneliusludmann marked this conversation as resolved.
Show resolved Hide resolved
case WORKSPACE_TIMEOUT_EXTENDED:
case WORKSPACE_TIMEOUT_EXTENDED_ALT:
return this.config.workspaceDefaults.timeoutExtended || "180m";
}
}

public durationToWorkspaceTimeout(duration: string): WorkspaceTimeoutDuration {
Copy link
Contributor

Choose a reason for hiding this comment

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

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. ⏩

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;
}
Comment on lines +43 to +65
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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. 🙌

}

async userGetsMoreResources(user: User): Promise<boolean> {
Expand Down
13 changes: 8 additions & 5 deletions components/server/ee/src/workspace/gitpod-server-impl.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import {
Workspace,
FindPrebuildsParams,
TeamMemberRole,
WORKSPACE_TIMEOUT_DEFAULT_SHORT,
} from "@gitpod/gitpod-protocol";
import { ResponseError } from "vscode-jsonrpc";
import {
Expand Down Expand Up @@ -77,7 +78,7 @@ import {
import { Plans } from "@gitpod/gitpod-protocol/lib/plans";
import * as pThrottle from "p-throttle";
import { formatDate } from "@gitpod/gitpod-protocol/lib/util/date-time";
import { FindUserByIdentityStrResult } from "../../../src/user/user-service";
import { FindUserByIdentityStrResult, UserService } from "../../../src/user/user-service";
import {
Accounting,
AccountService,
Expand Down Expand Up @@ -133,6 +134,8 @@ export class GitpodServerEEImpl extends GitpodServerImpl {

@inject(UserCounter) protected readonly userCounter: UserCounter;

@inject(UserService) protected readonly userService: UserService;

initialize(
client: GitpodClient | undefined,
user: User | undefined,
Expand Down Expand Up @@ -312,15 +315,15 @@ export class GitpodServerEEImpl extends GitpodServerImpl {
instancesWithReset.map((i) => {
const req = new SetTimeoutRequest();
req.setId(i.id);
req.setDuration(defaultTimeout);
req.setDuration(this.userService.workspaceTimeoutToDuration(defaultTimeout));

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

const req = new SetTimeoutRequest();
req.setId(runningInstance.id);
req.setDuration(duration);
req.setDuration(this.userService.workspaceTimeoutToDuration(duration));
Copy link
Contributor

@jankeromnes jankeromnes Apr 6, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

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

await client.setTimeout(ctx, req);

return {
Expand All @@ -343,7 +346,7 @@ export class GitpodServerEEImpl extends GitpodServerImpl {
const runningInstance = await this.workspaceDb.trace(ctx).findRunningInstance(workspaceId);
if (!runningInstance) {
log.warn({ userId: user.id, workspaceId }, "Can only get keep-alive for running workspaces");
return { duration: "30m", canChange };
return { duration: WORKSPACE_TIMEOUT_DEFAULT_SHORT, canChange };
}
await this.guardAccess({ kind: "workspaceInstance", subject: runningInstance, workspace: workspace }, "get");

Expand All @@ -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());
Copy link
Contributor

@jankeromnes jankeromnes Apr 6, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

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.

return { duration, canChange };
}

Expand Down
2 changes: 2 additions & 0 deletions components/server/src/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ export interface WorkspaceDefaults {
workspaceImage: string;
previewFeatureFlags: NamedWorkspaceFeatureFlag[];
defaultFeatureFlags: NamedWorkspaceFeatureFlag[];
timeoutDefault?: string;
timeoutExtended?: string;
}

export interface WorkspaceGarbageCollection {
Expand Down
29 changes: 27 additions & 2 deletions components/server/src/user/user-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import { injectable, inject } from "inversify";
import { User, Identity, WorkspaceTimeoutDuration, UserEnvVarValue, Token } from "@gitpod/gitpod-protocol";
import { User, Identity, WorkspaceTimeoutDuration, UserEnvVarValue, Token, WORKSPACE_TIMEOUT_DEFAULT_SHORT, WORKSPACE_TIMEOUT_DEFAULT_LONG, WORKSPACE_TIMEOUT_EXTENDED, WORKSPACE_TIMEOUT_EXTENDED_ALT } from "@gitpod/gitpod-protocol";
import { TermsAcceptanceDB, UserDB } from "@gitpod/gitpod-db/lib";
import { HostContextProvider } from "../auth/host-context-provider";
import { log } from "@gitpod/gitpod-protocol/lib/util/logging";
Expand Down Expand Up @@ -155,7 +155,32 @@ export class UserService {
* @param date The date for which we want to know the default workspace timeout
*/
async getDefaultWorkspaceTimeout(user: User, date: Date = new Date()): Promise<WorkspaceTimeoutDuration> {
return "30m";
return WORKSPACE_TIMEOUT_DEFAULT_SHORT;
}

public workspaceTimeoutToDuration(timeout: WorkspaceTimeoutDuration): string {
switch (timeout) {
case WORKSPACE_TIMEOUT_DEFAULT_SHORT:
return "30m";
case WORKSPACE_TIMEOUT_DEFAULT_LONG:
return "60m";
case WORKSPACE_TIMEOUT_EXTENDED:
case WORKSPACE_TIMEOUT_EXTENDED_ALT:
return "180m";
}
}
corneliusludmann marked this conversation as resolved.
Show resolved Hide resolved

public durationToWorkspaceTimeout(duration: string): WorkspaceTimeoutDuration {
switch (duration) {
case "30m":
return WORKSPACE_TIMEOUT_DEFAULT_SHORT;
case "60m":
return WORKSPACE_TIMEOUT_DEFAULT_LONG;
case "180m":
return WORKSPACE_TIMEOUT_EXTENDED_ALT;
default:
return WORKSPACE_TIMEOUT_DEFAULT_SHORT;
}
}

/**
Expand Down
2 changes: 1 addition & 1 deletion components/server/src/workspace/workspace-starter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1191,7 +1191,7 @@ export class WorkspaceStarter {
spec.setWorkspaceLocation(workspace.config.workspaceLocation || spec.getCheckoutLocation());
spec.setFeatureFlagsList(this.toWorkspaceFeatureFlags(featureFlags));
if (workspace.type === "regular") {
spec.setTimeout(await userTimeoutPromise);
spec.setTimeout(this.userService.workspaceTimeoutToDuration(await userTimeoutPromise));
}
spec.setAdmission(admissionLevel);
return spec;
Expand Down
1 change: 1 addition & 0 deletions install/installer/example-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ openVSX:
url: https://open-vsx.org
repository: eu.gcr.io/gitpod-core-dev/build
workspace:
maxLifetime: 36h0m0s
resources:
requests:
cpu: "1"
Expand Down
2 changes: 2 additions & 0 deletions install/installer/pkg/components/server/configmap.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ func configmap(ctx *common.RenderContext) ([]runtime.Object, error) {
WorkspaceImage: common.ImageName(common.ThirdPartyContainerRepo(ctx.Config.Repository, ""), workspace.DefaultWorkspaceImage, workspace.DefaultWorkspaceImageVersion),
PreviewFeatureFlags: []NamedWorkspaceFeatureFlag{},
DefaultFeatureFlags: []NamedWorkspaceFeatureFlag{},
TimeoutDefault: ctx.Config.Workspace.TimeoutDefault,
TimeoutExtended: ctx.Config.Workspace.TimeoutExtended,
},
Session: Session{
MaxAgeMs: 259200000,
Expand Down
7 changes: 6 additions & 1 deletion install/installer/pkg/components/server/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@

package server

import "github.com/gitpod-io/gitpod/installer/pkg/config/v1"
import (
"github.com/gitpod-io/gitpod/common-go/util"
"github.com/gitpod-io/gitpod/installer/pkg/config/v1"
)

// These types are from TypeScript files

Expand Down Expand Up @@ -120,6 +123,8 @@ type WorkspaceDefaults struct {
WorkspaceImage string `json:"workspaceImage"`
PreviewFeatureFlags []NamedWorkspaceFeatureFlag `json:"previewFeatureFlags"`
DefaultFeatureFlags []NamedWorkspaceFeatureFlag `json:"defaultFeatureFlags"`
TimeoutDefault *util.Duration `json:"timeoutDefault,omitempty"`
TimeoutExtended *util.Duration `json:"timeoutExtended,omitempty"`
}

type NamedWorkspaceFeatureFlag string
Expand Down
9 changes: 7 additions & 2 deletions install/installer/pkg/components/ws-manager/configmap.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ func configmap(ctx *common.RenderContext) ([]runtime.Object, error) {
return (&q).String()
}

timeoutAfterClose := util.Duration(2 * time.Minute)
if ctx.Config.Workspace.TimeoutAfterClose != nil {
timeoutAfterClose = *ctx.Config.Workspace.TimeoutAfterClose
}

wsmcfg := config.ServiceConfiguration{
Manager: config.Configuration{
Namespace: ctx.Namespace,
Expand Down Expand Up @@ -81,11 +86,11 @@ func configmap(ctx *common.RenderContext) ([]runtime.Object, error) {
WorkspaceHostPath: wsdaemon.HostWorkingArea,
WorkspacePodTemplate: templatesCfg,
Timeouts: config.WorkspaceTimeoutConfiguration{
AfterClose: util.Duration(2 * time.Minute),
AfterClose: timeoutAfterClose,
HeadlessWorkspace: util.Duration(1 * time.Hour),
Initialization: util.Duration(30 * time.Minute),
RegularWorkspace: util.Duration(30 * time.Minute),
MaxLifetime: util.Duration(36 * time.Hour),
MaxLifetime: ctx.Config.Workspace.MaxLifetime,
TotalStartup: util.Duration(1 * time.Hour),
ContentFinalization: util.Duration(1 * time.Hour),
Stopping: util.Duration(1 * time.Hour),
Expand Down
16 changes: 16 additions & 0 deletions install/installer/pkg/config/v1/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
package config

import (
"time"

"github.com/gitpod-io/gitpod/common-go/util"
"github.com/gitpod-io/gitpod/installer/pkg/config"
"github.com/gitpod-io/gitpod/installer/pkg/config/v1/experimental"
"github.com/gitpod-io/gitpod/ws-daemon/pkg/cpulimit"
Expand Down Expand Up @@ -53,6 +56,7 @@ func (v version) Defaults(in interface{}) error {
cfg.Workspace.Runtime.FSShiftMethod = FSShiftFuseFS
cfg.Workspace.Runtime.ContainerDSocket = "/run/containerd/containerd.sock"
cfg.Workspace.Runtime.ContainerDRuntimeDir = "/var/lib/containerd/io.containerd.runtime.v2.task/k8s.io"
cfg.Workspace.MaxLifetime = util.Duration(36 * time.Hour)
cfg.OpenVSX.URL = "https://open-vsx.org"
cfg.DisableDefinitelyGP = false

Expand Down Expand Up @@ -224,6 +228,18 @@ type Workspace struct {
Runtime WorkspaceRuntime `json:"runtime" validate:"required"`
Resources Resources `json:"resources" validate:"required"`
Templates *WorkspaceTemplates `json:"templates,omitempty"`

// MaxLifetime is the maximum time a workspace is allowed to run. After that, the workspace times out despite activity
MaxLifetime util.Duration `json:"maxLifetime" validate:"required"`

// TimeoutDefault is the default timeout of a regular workspace
TimeoutDefault *util.Duration `json:"timeoutDefault,omitempty"`

// TimeoutExtended is the workspace timeout that a user can extend to for one workspace
TimeoutExtended *util.Duration `json:"timeoutExtended,omitempty"`

// TimeoutAfterClose is the time a workspace timed out after it has been closed (“closed” means that it does not get a heartbeat from an IDE anymore)
TimeoutAfterClose *util.Duration `json:"timeoutAfterClose,omitempty"`
}

type OpenVSX struct {
Expand Down