Skip to content

[installer] Update installer to use credits config #14362

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

Jump to bottom
Merged
merged 1 commit into from
Nov 14, 2022
Merged

[installer] Update installer to use credits config #14362

merged 1 commit into from
Nov 14, 2022

Conversation

laushinka
Copy link
Contributor

@laushinka laushinka commented Nov 2, 2022 •
edited
Loading

Description

Use the credits config in workspaceClasses

Related Issue(s)

Depends on https://github.com/gitpod-io/ops/pull/6597 ✅ and https://github.com/gitpod-io/ops/pull/6601 ✅
Relates #13362

How to test

  1. In the workspace, run leeway run dev/preview:build
  2. If build is successful, run leeway run dev/preview:deploy-gitpod
  3. This will generate a /tmp file that looks like /tmp/[some-gibberish].gitpod.config.yaml
  4. cat the file and see whether the config is as expected.

Release Notes

NONE

Documentation

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

@laushinka laushinka mentioned this pull request Nov 3, 2022
Closed
5 tasks
@werft-gitpod-dev-com
Copy link

cannot start job - please talk to whoever's in charge of your Werft installation

@laushinka laushinka force-pushed the lau/credits-config-installer branch from 5a3ca1c to 885b5a7 Compare November 3, 2022 15:07
@roboquat roboquat added size/M and removed size/XS labels Nov 3, 2022
@laushinka laushinka force-pushed the lau/credits-config-installer branch from 885b5a7 to 3d8dbf1 Compare November 3, 2022 15:09
@laushinka
Copy link
Contributor Author

laushinka commented Nov 4, 2022 •
edited
Loading

@mads-hartmann Need your pointers to adjust what I would've added into installer.ts for werft, based on the recent changes[1] 🙏🏽

@laushinka laushinka force-pushed the lau/credits-config-installer branch 2 times, most recently from ae72de9 to 2c3574c Compare November 4, 2022 15:12
@laushinka laushinka marked this pull request as ready for review November 4, 2022 15:19
@laushinka laushinka requested review from a team November 4, 2022 15:19
@github-actions github-actions bot added team: SID team: devx team: webapp Issue belongs to the WebApp team labels Nov 4, 2022
@laushinka laushinka force-pushed the lau/credits-config-installer branch from 2c3574c to 66610cb Compare November 4, 2022 15:23
easyCZ
easyCZ reviewed Nov 4, 2022
View reviewed changes
install/installer/pkg/components/usage/configmap.go Outdated
@@ -62,7 +63,12 @@ func configmap(ctx *common.RenderContext) ([]runtime.Object, error) {
if expUsageConfig.DefaultSpendingLimit != nil {
cfg.DefaultSpendingLimit = *expUsageConfig.DefaultSpendingLimit
}
cfg.CreditsPerMinuteByWorkspaceClass = expUsageConfig.CreditsPerMinuteByWorkspaceClass
cfg.CreditsPerMinuteByWorkspaceClass = make(map[string]float64)
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't need to be inside the expUsageConfig because it doensn't relly on the expUsageConfig being set. It relies on the workspaceClassConfig variable only.

dev/preview/workflow/preview/deploy-gitpod.sh Outdated
@@ -421,8 +421,8 @@ yq w -i "${INSTALLER_CONFIG_PATH}" experimental.webapp.usage.billInstancesAfter
yq w -i "${INSTALLER_CONFIG_PATH}" experimental.webapp.usage.defaultSpendingLimit.forUsers "500"
yq w -i "${INSTALLER_CONFIG_PATH}" experimental.webapp.usage.defaultSpendingLimit.forTeams "0"
yq w -i "${INSTALLER_CONFIG_PATH}" experimental.webapp.usage.defaultSpendingLimit.minForUsersOnStripe "1000"
yq w -i "${INSTALLER_CONFIG_PATH}" experimental.webapp.usage.creditsPerMinuteByWorkspaceClass['default'] "0.1666666667"
yq w -i "${INSTALLER_CONFIG_PATH}" experimental.webapp.usage.creditsPerMinuteByWorkspaceClass['gitpodio-internal-xl'] "0.3333333333"
yq w -i "${INSTALLER_CONFIG_PATH}" experimental.webapp.workspaceClasses[2].credits.perMinute "0.1666666667"
Copy link
Member

Choose a reason for hiding this comment

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

It shouldn't be defined here as part of usage, it should be set as part of setting the WS classes elsewhere

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was confusing because originally it was default and gitpodio-internal-xl but the workspaceClasses config were default and small. Fixed!

@laushinka laushinka force-pushed the lau/credits-config-installer branch from 66610cb to a6ebf42 Compare November 4, 2022 16:31
mads-hartmann
mads-hartmann approved these changes Nov 7, 2022
View reviewed changes
Copy link
Contributor

@mads-hartmann mads-hartmann left a comment

Choose a reason for hiding this comment

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

:shipit:

@easyCZ
Copy link
Member

easyCZ commented Nov 7, 2022

Before we land this, we need to ensure that all existing config does contain the new perMinute credits configuration. For example, we don't have it defined in staging yet

@laushinka
Copy link
Contributor Author

Before we land this, we need to ensure that all existing config does contain the new perMinute credits configuration. For example, we don't have it defined in staging yet

Ahh, thank you for catching this!

@nandajavarma
Copy link
Contributor

nandajavarma commented Nov 7, 2022 •
edited by werft-gitpod-dev-com bot
Loading

/werft run with-sh-preview

👍 started the job as gitpod-build-lau-credits-config-installer.7
(with .werft/ from main)

@laushinka
Copy link
Contributor Author

Before we land this, we need to ensure that all existing config does contain the new perMinute credits configuration. For example, we don't have it defined in staging yet

Created https://github.com/gitpod-io/ops/pull/6597.

@laushinka laushinka force-pushed the lau/credits-config-installer branch from a6ebf42 to 23dcc26 Compare November 7, 2022 11:32
@laushinka
Copy link
Contributor Author

laushinka commented Nov 7, 2022 •
edited
Loading

Holding for question[1] for the Workspace team this PR[1].

/hold

ArthurSens
ArthurSens approved these changes Nov 8, 2022
View reviewed changes
Copy link
Contributor

@ArthurSens ArthurSens left a comment

Choose a reason for hiding this comment

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

All good from the installer perspective 🙂

@easyCZ easyCZ mentioned this pull request Nov 10, 2022
Closed
4 tasks
@laushinka laushinka force-pushed the lau/credits-config-installer branch from 23dcc26 to daa8fd5 Compare November 11, 2022 12:00
@laushinka
Copy link
Contributor Author

Thanks for all the approvals! I've re-tested and will unhold on Monday, just to be safe..

@roboquat roboquat merged commit c9496a0 into main Nov 14, 2022
@roboquat roboquat deleted the lau/credits-config-installer branch November 14, 2022 11:02
@roboquat roboquat added the deployed: webapp Meta team change is running in production label Nov 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mads-hartmann mads-hartmann mads-hartmann approved these changes

@easyCZ easyCZ easyCZ approved these changes

@ArthurSens ArthurSens ArthurSens approved these changes

Assignees
No one assigned
Labels
deployed: webapp Meta team change is running in production release-note-none size/M team: devx team: SID team: webapp Issue belongs to the WebApp team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@laushinka @easyCZ @nandajavarma @mads-hartmann @ArthurSens @roboquat