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.

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

Allow specification of workspace class in project settings #10963

Closed
Furisto opened this issue Jun 28, 2022 · 22 comments · Fixed by #14535
Closed

Allow specification of workspace class in project settings #10963

Furisto opened this issue Jun 28, 2022 · 22 comments · Fixed by #14535
Assignees
Labels
feature: workspace classes team: workspace Issue belongs to the Workspace team

Comments

@Furisto
Copy link
Member

Furisto commented Jun 28, 2022

Is your feature request related to a problem? Please describe

Improve usability of #8261.

Describe the behaviour you'd like

As a user of Gitpod, I would like to specify the workspace class that should be used for the repository in gitpod.yml.

The file would like the following:

workspaceRequirements:
  class: g1-standard

or:

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

It should behave as such:

  1. The first workspace class available is used (in prebuilds and regular workspaces).
  2. If no workspace class is set in the .gitpod.yml or those are not available:
    a. On Prebuilds, use the default (g1-standard)
    b. On regular workspaces, use the user preference.

Describe alternatives you've considered

Specify workspace class on a per project basis.

Additional context

Affected components:

  • gitpod-protocol
  • server

Why allow the specification of a list of workspace classes? Workspace classes names and definitions differ between gitpod installations, but the .gitpod.yml is the same for all. As such, we must allow users to define multiple a choose the first one available.

Required actions:

  • Update documentation
  • Talk with @gtsiolis to update the workspace class preferences to include the id, and implement it

Question: Can we have different types (array and string) being processed in the workspaceRequirements.class field? From @WVerlaek: Possible in Go, with a slight workaround: go-yaml/yaml#100

@svenefftinge
Copy link
Member

svenefftinge commented Jun 28, 2022

I think we should only specify system requirements in .gitpod.yml, that the system then tries to satisfy.
Depending on that configuration a Gitpod installation could propose/select a matching workspace class. Workspace classes will vary per installation, per user as well as over time, so I'd rather not reference them from .gitpod.yml.

FWIW .devcontainer does it like that as well [1].

@atduarte atduarte changed the title Support specifying the workspace class in gitpod.yml Support specifying the workspace class per project/repository Jul 28, 2022
@atduarte
Copy link
Contributor

Would the configured values in the .gitpod.yml always take precedence over the user settings? 🤔

Only use case I can think of in which that doesn't fit is whenever you have a project to which you may want to use a different workspace class depending on the type of work you are going to do. For example, you might be opening the project to edit some docs to train a model which require more resources and a GPU, respectively. This is a somewhat similar problem to the fact that long prebuild/command tasks need to run independently of what you are planning to do on the repository, and could be tackled later.

@atduarte atduarte added team: workspace Issue belongs to the Workspace team feature: workspace classes labels Jul 28, 2022
@jldec
Copy link
Contributor

jldec commented Aug 9, 2022

@atduarte I suggest renaming this issue to "Auto-select workspace class based on system requirements in gitpod.yml" to match the comment.

Allowing users to configure their preferred workspace class is a reasonable first step toward automatic workspace class selection, but it also introduces friction e.g. for onboarding large teams where everyone is using non-default workspace classes, or for users who would like to open different workspaces classes for different project repos.

cc: @svenefftinge

@Furisto Furisto changed the title Support specifying the workspace class per project/repository Auto-select workspace class based on system requirements in gitpod.yml Aug 10, 2022
@atduarte
Copy link
Contributor

@jldec I'm still unsure about whether we should auto-select based on system requirements or require users' to specify the class name. The former limits the selection to a few characteristics that may not represent the full set. For example, if there are two workspace classes with the same number of cores but different frequencies it would lead to problems. On the other hand, the .gitpod.yml would be more readable.

Given that both are not incompatible, what if we started with the simplest option (i.e. requiring user to specific the workspace class name) that covers all possible scenarios and later consider introducing the alternative of specifying the workspace class via system requirements?

@jldec
Copy link
Contributor

jldec commented Aug 16, 2022

Starting simple is 😎.
How about allowing a list of preferred workspace classes (for portability / cross-compatibility).

@atduarte
Copy link
Contributor

atduarte commented Aug 17, 2022

💭 Would ensuring that whenever a workspace class is deprecated a "replacement" is defined address your concerns? E.g. when deprecating g1-standard, the admin sets g2-foo-bar as its replacement, so that a .gitpod.yml with g1-standard will now start g2-foo-bar. Whenever no replacement is set, and the class is not available, warning would be showed and user preference used instead.

@jldec
Copy link
Contributor

jldec commented Aug 18, 2022

I was thinking of a scenario where a repo is used in both self-hosted and gitpod.io.
In that case specifying both preferred classes would work.

(the inspiration for allowing a list of multiple classes came from the way fonts work in CSS - you can specify preferred fonts in a list to support system fonts with different names across OSes)

@atduarte
Copy link
Contributor

Makes sense 👌 My concern was that we would make it less readable/intuitive for the majority to address a corner case, but instead of an array we can use a comma-separated string like class: g1-standard, foo-bar.

@atduarte atduarte changed the title Auto-select workspace class based on system requirements in gitpod.yml Allow to specify the workspace class in the gitpod.yml Aug 18, 2022
@jldec
Copy link
Contributor

jldec commented Aug 18, 2022

You have a point, although introducing a new syntax just to avoid some other syntax may be 🤔 .

B.t.w it is valid yaml to allow both string OR sequence of strings.
There's a example in on: event or on: [event1, event2] for github actions - but it makes parsing the yaml tricky.

@jldec
Copy link
Contributor

jldec commented Aug 26, 2022

It's worth mentioning here, that this will address how a workspace class is selected
a. when users start a workspace
and
b. when prebuilds are triggered e.g. by a push event

It may be worth considering the possiblity of allowing a different workspace class for prebuilds (vs. running workspaces for users)

@atduarte
Copy link
Contributor

It may be worth considering the possiblity of allowing a different workspace class for prebuilds (vs. running workspaces for users)

I don't currently see the need for that, and given our inability to downgrade storage it might be confusing.


After thinking about prebuilds' and project workspace class attribution, I have the following proposal:
By default ALL prebuilds use Standard. Only way to override it is by defining the workspace class in the .gitpod.yml. Which impacts prebuilds and workspaces.

It's simpler and easier to explain things such as "your workspace is large, because the prebuild was created as large, because that's your team owner's workspace class preference".

@kylos101
Copy link
Contributor

kylos101 commented Oct 20, 2022

@atduarte we're gonna leave this in breakdown, for now, so that it can be socialized more on Tuesday. The alternatives mention specifying in Project too, but, it's unclear what the hierarchy should be for when setting workspace class (or why we do not want it set in project).

For example, here's sample hierarchy from highest preference to lowest preference:

  1. .gitpod.yml
  2. project
  3. user

edit: is the intent to do this before or after UBP is GA? The reason for asking is so that we can understand how to marshal workspace class on the webapp side. For example, an alternative (which has some plumbing, but isn't done) is to interrogate workspace clusters, aggregate supported classes, and then let webapp make a decision with the result.

@atduarte
Copy link
Contributor

@kylos101 the alternative was written, I believe, to denote that it was thought about but it's not the plan.

I may have missed something. Is there any reason not to do as proposed, and explore the alternative or a mix of both?

Regarding timing: Preferably it would be for UBP GA as it would reduce confusion during the launch. But of course only if it's reasonable.

One thing I forgot to add to the description, is that we need to ensure that people that are already using g1-large for prebuilds need to be warned and must update their .gitpod.yml.

@kylos101
Copy link
Contributor

To be safe, it would be good to socialize this with @Furisto during Tuesday's session.

Some additional questions and concerns I can think of that may influence the issue description:

  1. What is the use case for specifying two workspace classes class: g1-standard, g2-foo-bar?
  2. We should update the docs, too [1][2]
  3. There may not be any actual Workspace Team work here, in which case we'd want to inbox this to Webapp. @Furisto will be a good judge of this.

Also, I skimmed the comments in this issue (above) and have some additional thoughts (below).

Is there any reason not to do as proposed, and explore the alternative or a mix of both?

No, we just simply ran out of time during refinement. Everything else was moved from Breakdown to Scheduled.

By default ALL prebuilds use Standard. Only way to override it is by defining the workspace class in the .gitpod.yml. Which impacts prebuilds and workspaces.

This is no longer true, right? For example, if I created a project, and my user preference was large, the prebuilds would be done with Large (our docs say this, too).

Or, are you saying you'd want prebuilds to be done with standard (even though the creator of project is specifying Large as a user preference, with no specification in the .gitpod.yml file)? I ask so that we can update the issue description accordingly or create separate issues.

@atduarte
Copy link
Contributor

Thanks, @kylos101! Updated the description for clarity. In summary:

  • "By default ALL prebuilds use Standard" is a proposed behaviour change.
  • Added an explanation on why we should have a list of workspace classes.
  • Added updating the documentation as required action

@WVerlaek
Copy link
Member

Question: Can we have different types (array and string) being processed in the workspaceRequirements.class field?

Possible in Go, with a slight workaround: go-yaml/yaml#100

@atduarte atduarte moved this from Breakdown to Scheduled in 🌌 Workspace Team Oct 25, 2022
@Furisto Furisto self-assigned this Oct 28, 2022
@Furisto Furisto moved this from Scheduled to In Progress in 🌌 Workspace Team Oct 28, 2022
@Furisto Furisto changed the title Allow to specify the workspace class in the gitpod.yml Allow specification of workspace class in gitpod.yml Oct 31, 2022
@svenefftinge
Copy link
Member

coming back to my initial commit on this: I propose to introduce a project setting overriding a possible user setting. And do not change anything wrt the gitpod.yml

@gtsiolis
Copy link
Contributor

gtsiolis commented Nov 1, 2022

I propose to introduce a project setting overriding a possible user setting. And do not change anything wrt the gitpod.yml

Yes, please! 💯

@gtsiolis
Copy link
Contributor

gtsiolis commented Nov 2, 2022

Reposing for visibility an early design draft on how moving this to project settings as described above in #10963 (comment) could look like, see relevant discussion (internal).

This would 🅰️ require a project to exist for a repository to allow such configuration but also 🅱️ work towards #9898 which would require a project to enable prebuilds.

Selecting a workspace class for prebuilds for a project
Screenshot 2022-11-02 at 2 06 46 PM (2)

@svenefftinge
Copy link
Member

@gtsiolis this setting is not only for prebuilds but for all workspaces. So I opted for moving it into the workspace section.
Can you review? #14535

@kylos101
Copy link
Contributor

kylos101 commented Nov 8, 2022

@Furisto as Sven is taking this over, may I ask you to:

  1. Remove this issue from the Workspace project
  2. Close your draft PR?

This way our board has less "in-flight". 🙇

@kylos101
Copy link
Contributor

kylos101 commented Nov 9, 2022

@Furisto I closed in #14257 in favor of #14535, and am also removing from our project.

@atduarte for awaress, as webapp is delivering this change, it'll be on their board.

Repository owner moved this from In Progress to In Validation in 🍎 WebApp Team Nov 10, 2022
@svenefftinge svenefftinge moved this from In Validation to Done in 🍎 WebApp Team Nov 14, 2022
@jldec jldec changed the title Allow specification of workspace class in gitpod.yml Allow specification of workspace class in project settings Nov 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: workspace classes team: workspace Issue belongs to the Workspace team
Projects
Status: Done
7 participants