Skip to content

Conversation

@Furisto
Copy link
Member

@Furisto Furisto commented Oct 28, 2022

Description

Add support for specifying the workspace class in gitpod.yaml

workspaceRequirements:
  class: g1-standard
workspaceRequirements:
  class: 
    - g1-standard
    - g2-foo-bar

Related Issue(s)

Fixes #10963

How to test

  1. Prebuild class is taken from gitpod.yml (single workspace class specified)
  2. Prebuild class is taken from gitpod.yml (multiple workspace classes specified)
  3. Default class is used for Prebuild if gitpod.yml does not specify a class
    • Trigger a prebuild of a random repository in the preview environment. Prebuild should run with default workspace class

-> Delete all prebuilds in the database to ensure that the prebuild class is not used for regular workspaces

  1. Regular workspace class is taken from gitpod.yml (single workspace class specified)
  2. Regular workspace class is taken from gitpod.yml (multiple workspace classes specified)

Release Notes

The workspace class for a workspace can now be specified in gitpod.yml

Documentation

Does this PR require updates to the documentation at www.gitpod.io/docs?
Yes: https://github.com/gitpod-io/website/issues/2973

Werft options:

  • /werft with-local-preview
    If enabled this will build install/preview
  • /werft with-preview
  • /werft with-large-vm
  • /werft with-integration-tests=all
    Valid options are all, workspace, webapp, ide

@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-fo-class-repo-config.9 because the annotations in the pull request description changed
(with .werft/ from main)

@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-fo-class-repo-config.10 because the annotations in the pull request description changed
(with .werft/ from main)

@Furisto Furisto force-pushed the fo/class-repo-config branch from 903363d to 4a72c3b Compare October 28, 2022 15:46
@roboquat roboquat added size/L and removed size/M labels Oct 28, 2022
@Furisto Furisto marked this pull request as ready for review October 31, 2022 12:23
@Furisto Furisto requested a review from a team October 31, 2022 12:23
@github-actions github-actions bot added the team: webapp Issue belongs to the WebApp team label Oct 31, 2022
"additionalProperties": false,
"properties": {
"class": {
"type": ["string", "array"],
Copy link
Member

Choose a reason for hiding this comment

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

I feel this overloaded option makes it unnecessarily complicated. I feel the UX wouldn't suffer much if we always made it a list.

Most of the time, I expect customers would be copying this from the docs so it doesn't matter to them if it's a list or just a string.

},
"workspaceRequirements": {
"type": "object",
"description": "Configure the requirements of the workspace.",
Copy link
Member

Choose a reason for hiding this comment

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

Here, we talk about requirements. But when those requirements are not satisfied, we fall back to the default. Under this behaviour, it's no longer a requirement, it's a preference.

Can we change the name to workspacePreferences?

Copy link
Member

Choose a reason for hiding this comment

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

Possibly worth adding info about what happens when no workspace class matches in here as well.

@svenefftinge
Copy link
Contributor

svenefftinge commented Oct 31, 2022

I really think referencing workspace class names here is not the right approach, because it is not declarative/descriptive and I doubt they will be stable. We should instead state what requirements in terms of resources (RAM, VCPU, ...) are needed and then match it against the available classes.

A setting for picking a certain workspace class would make more sense in the project configuration. I have added a comment to the issue.

@atduarte
Copy link
Contributor

atduarte commented Nov 1, 2022

@svenefftinge thanks for your feedback. Let me share a bit more on why I think setting the workspace class right now makes sense:

  1. both options are not mutually exclusive. The .gitpod.yml can allow to define the workspace classes explicitly or (later) set minimum resource requirements as you described.
  2. we need a quick win, because the current prebuild workspace class selection is terrible. Currently, prebuild workspace class is picked based on the user preference of whoever triggers it. This is the main motivation to push this change through asap

Re "I doubt they will be stable": I'm pretty sure of that, but that doesn't need to be a problem if when deprecating a workspace class we specify what new workspace class should be used instead.

I also doubt that we are aware of all the resource dimensions we will have, particularly on customers' installations. E.g. different CPU frequencies and performance, type of GPU, disk IOPS, network bandwidth, "security level", network proxy and access, ... So having the option to define explicitly the workspace class sounds reasonable and beneficial.

A setting for picking a certain workspace class would make more sense in the project configuration

Mixing project configurations and .gitpod.yml sounds like a very bad idea to me. 😬 I understand the benefit of being able to change what shows in the UI/dropdown, and assume that's the reason for your preference (?) but I don't believe the interface should be the deciding factor on whether something is at the repository (file) or the project (UI) level.

@easyCZ
Copy link
Member

easyCZ commented Nov 1, 2022

FWIW, I'm also not convinced this is the right approach. I'm not sure what the alternative is, but I can at least outline how I think we should think about workspace class selection.

A customer is starting a workspace, they should be able to express preferences, and requirements of what they need in a Workspace.
This can take the form of

preferences:
  cpu:
    min: 4 vCPU
  memory:
    min: 6 GBi
  architecture:
    - ...
  
  classChoiceStrategy:
    - cheapest
 
requirements:
  os:
    - linux
  gpu: true

Given such a request, apply constraint satisfaction to arrive at a set of possible workspace classes. These classes must strictly match requirements, and they should (if possible) satisfy preferences.
Given the set of eligible classes, we choose based on the strategy specified - cheapest, most powerful, whatever...

What this does is it flips the product space to the customers needs, rather than forcing them to know (and keep up-to-date) on the possible workspace classes, and whether they work for them.

Yes, it's not easy, or necessarily possible, to do this right now. But with this as a north-star, we probably wouldn't choose to put class preferences on the gitpod yaml.

@easyCZ
Copy link
Member

easyCZ commented Nov 8, 2022

Moving to draft due to ongoing discussion in https://gitpod.slack.com/archives/C02F19UUW6S/p1667389673842289. Feel free to move back to in-review if this should be actioned.

@easyCZ easyCZ marked this pull request as draft November 8, 2022 13:30
@svenefftinge
Copy link
Contributor

svenefftinge commented Nov 8, 2022

As discussed I'm taking this over and move the implementation to be a project setting.
PR :#14535

@kylos101
Copy link
Contributor

kylos101 commented Nov 9, 2022

Closing in favor of #14535

@kylos101 kylos101 closed this Nov 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/work-in-progress release-note size/L team: webapp Issue belongs to the WebApp team team: workspace Issue belongs to the Workspace team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow specification of workspace class in project settings

7 participants