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

Retrieve supported workspace classes from workspace cluster #11010

Merged
merged 3 commits into from
Jul 6, 2022

Conversation

Furisto
Copy link
Member

@Furisto Furisto commented Jun 29, 2022

Description

Note: This is a stacked PR that depends on #10982

During registration of a workspace cluster the workspace classes that are supported by this cluster will be fetched from the DescribeCluster endpoint of ws-manager. They are also fetched in a configurable interval (currently 60 seconds) so that updates to the supported classes are propagated to the meta side.

I have added a feature flag, so that this change can be deployed even if the new endpoint from #10982 has not been deployed to workspace clusters yet.

Related Issue(s)

Partially fixes ##10842

How to test

  • Currently the feature flag (workspace_classes_backend) is active, so with debug logs activated (gpctl debug logs ws-manager-bridge) you should see messages like reconciling workspace classes...in the log output. After disabling the flag and restarting ws-manager-bridge you should not see any messages.

  • Registration of a workspace cluster can be done with gpctl clusters register --name ws-cluster1 --hint-cordoned --hint-govern --tls-path ./wsman-tls/ --url 34.77.51.162:8081 --kubeconfig ./meta-cluster. After that in the admissionConstraints column of d_b_workspace_cluster you should see an entry similar to this:

[
   {
      "type":"has-class",
      "id":"default",
      "displayName":"default"
   },
   {
      "type":"has-class",
      "id":"gitpodio-internal-xl",
      "displayName":"XL"
   }
]

You can create a workspace cluster with workspace preview. I already have created a VM which you can use for registration. Contact me for the credentials.

Release Notes

None

Werft options:

  • /werft with-preview

@@ -42,6 +42,11 @@ export class WorkspaceClusterDBImpl implements WorkspaceClusterDB {
return repo.findOne(name);
}

async findAll(): Promise<WorkspaceCluster[]> {
const repo = await this.getRepo();
Copy link
Member Author

Choose a reason for hiding this comment

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

I added this in addition to findFiltered because findFiltered does not return the tls config which we need to authenticate against the ws-manager endpoint

Copy link
Member

Choose a reason for hiding this comment

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

Jep. We did so to reduce traffic (bc it turned out to be a perf problem). See comment below.

@Furisto Furisto marked this pull request as ready for review June 30, 2022 09:39
@Furisto Furisto requested review from a team June 30, 2022 09:39
@github-actions github-actions bot added team: delivery Issue belongs to the self-hosted team team: webapp Issue belongs to the WebApp team labels Jun 30, 2022
@Furisto
Copy link
Member Author

Furisto commented Jun 30, 2022

/hold

@geropl geropl self-assigned this Jun 30, 2022
Base automatically changed from fo/describe-cluster to main June 30, 2022 15:56
@roboquat roboquat requested a review from a team June 30, 2022 15:56
@roboquat roboquat added size/XXL and removed size/L labels Jun 30, 2022
@github-actions github-actions bot added team: workspace Issue belongs to the Workspace team and removed size/XXL labels Jun 30, 2022
@sagor999
Copy link
Contributor

Removed myself and workspace team from reviewers, since this PR does not contain any files that we are code owners of.

@geropl
Copy link
Member

geropl commented Jul 1, 2022

FYI: Thomas and I had a quick sync, aligned and discussed some minor changes. @Furisto Please ping for re-review! 🙏

@Furisto
Copy link
Member Author

Furisto commented Jul 1, 2022

@geropl ping

Copy link
Contributor

@corneliusludmann corneliusludmann left a comment

Choose a reason for hiding this comment

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

Changes on Team Self-Hosted owned files look good. Approving to unblock the merge after being reviewed by the other teams.

@geropl
Copy link
Member

geropl commented Jul 5, 2022

/werft run

👍 started the job as gitpod-build-fo-register-cluster.11
(with .werft/ from main)

@geropl
Copy link
Member

geropl commented Jul 5, 2022

@Furisto I'm currently reviewing. Can you provide me the tls config for wsman? 🙏

@geropl
Copy link
Member

geropl commented Jul 6, 2022

@Furisto I received the credentials for your new VM. After setting everything up I still see no update of the admission constraints:

  1. ws cluster is configured and can connect
  2. bridge has run "reconcile workspace classes"
  3. feature flags are 🟢 in ConfigCat
  4. still, the DB says no admission constraint:

image

Could it be that the ws cluster is missing the workspace classes config? 🤔

Copy link
Member

@geropl geropl left a comment

Choose a reason for hiding this comment

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

Turns out the admissionsConstraint is properly updated which we can verify by accessing the DB directly. ✔️

gpctl has to be taught to express workspaceClass admissionConstraints, but that's out-of-scope and worth a follow-up. 🧘

@Furisto
Copy link
Member Author

Furisto commented Jul 6, 2022

/unhold

@roboquat roboquat merged commit 88e5beb into main Jul 6, 2022
@roboquat roboquat deleted the fo/register-cluster branch July 6, 2022 17:45
@roboquat roboquat added deployed: webapp Meta team change is running in production deployed: workspace Workspace team change is running in production labels Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: webapp Meta team change is running in production deployed: workspace Workspace team change is running in production feature: workspace classes release-note-none size/XL team: delivery Issue belongs to the self-hosted team 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.

None yet

5 participants