Skip to content

Conversation

csweichel
Copy link
Contributor

Description

This PR adds support for workspace classes. Workspace classes add per-workspace support for different resource requests and limits, as well as different templates.

Related Issue(s)

works on #8261

How to test

See unit tests

Release Notes

[ws-manager] Add support for workspace classes

@csweichel
Copy link
Contributor Author

csweichel commented Apr 22, 2022

/werft run

👍 started the job as gitpod-build-cw-ws-class.7

@csweichel csweichel force-pushed the cw/ws-class branch 2 times, most recently from 3c1dd5f to f3b5c4a Compare April 27, 2022 11:21
@csweichel csweichel marked this pull request as ready for review April 27, 2022 15:32
@csweichel csweichel requested a review from a team April 27, 2022 15:32
@csweichel csweichel requested a review from a team April 27, 2022 15:32
@github-actions github-actions bot added team: delivery Issue belongs to the self-hosted team team: workspace Issue belongs to the Workspace team labels Apr 27, 2022
Copy link
Contributor

@princerachit princerachit left a comment

Choose a reason for hiding this comment

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

nit picks

return xerrors.Errorf("workspace class %s: %w", name, err)
}
}

return err
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this redundant return? Should we return nil here?

Copy link
Contributor

@Pothulapati Pothulapati left a comment

Choose a reason for hiding this comment

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

LGTM, except for the naming nit!

@@ -58,6 +61,22 @@ type WorkspaceConfig struct {
PasswordSecret string `json:"passwordSecret"`
} `json:"redisCache"`
} `json:"registryFacade"`

WorkspaceClasses map[string]WorkspaceClass `json:"classes,omitempty"`
Copy link
Contributor

@Pothulapati Pothulapati Apr 29, 2022

Choose a reason for hiding this comment

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

TIOLI: Shouldn't the json field be workspaceClasses? as classes seems a bit ambiguous? 🤔

@roboquat roboquat merged commit fc1b0ac into main Apr 29, 2022
@roboquat roboquat deleted the cw/ws-class branch April 29, 2022 08:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note size/XXL team: delivery Issue belongs to the self-hosted team team: workspace Issue belongs to the Workspace team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants