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

[ws-manager] Improve workspaces PodAffinity #7472

Merged
merged 3 commits into from
Jan 18, 2022
Merged

Conversation

aledbf
Copy link
Member

@aledbf aledbf commented Jan 6, 2022

Description

If EnforceWorkspaceNodeAffinity is configured, ensure the workspaces are scheduled in nodes where ws-daemon and registry-facade are running.

Release Notes

[ws-manager] Improve workspaces PodAffinity

How to test

  • scale down ws-scheduler replicas to 0
  • change ws-manager configmap and remove schedulerName
  • restart ws-manager
  • start a workspace
  • check the workspace contains the affinity
nodeSelectorTerms:
- matchExpressions:
  - key: gitpod.io/ws-daemon_ready_ns_<namespace>
    operator: Exists
  - key: gitpod.io/registry-facade_ready_ns_<namespace>
    operator: Exists

/werft with-clean-slate-deployment

@aledbf aledbf requested a review from csweichel January 6, 2022 12:19
@roboquat roboquat added release-note team: workspace Issue belongs to the Workspace team size/S labels Jan 6, 2022
@aledbf aledbf force-pushed the aledbf/podAffinity branch from 46659f6 to 4650322 Compare January 6, 2022 12:22
@roboquat roboquat added size/M and removed size/S labels Jan 6, 2022
@codecov
Copy link

codecov bot commented Jan 6, 2022

Codecov Report

Merging #7472 (5e6487a) into main (972dfeb) will decrease coverage by 0.80%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##            main   #7472      +/-   ##
========================================
- Coverage   8.69%   7.88%   -0.81%     
========================================
  Files         33      31       -2     
  Lines       2323    2155     -168     
========================================
- Hits         202     170      -32     
+ Misses      2117    1983     -134     
+ Partials       4       2       -2     
Flag Coverage Δ
components-gitpod-cli-app 10.38% <ø> (ø)
components-local-app-app-darwin-amd64 ?
components-local-app-app-darwin-arm64 ?
components-local-app-app-linux-amd64 ?
components-local-app-app-linux-arm64 ?
components-local-app-app-windows-386 ?
components-local-app-app-windows-amd64 ?
components-local-app-app-windows-arm64 ?
installer-raw-app 5.76% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
components/local-app/pkg/auth/pkce.go
components/local-app/pkg/auth/auth.go

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 972dfeb...5e6487a. Read the comment docs.

@csweichel
Copy link
Contributor

Code looks good. However, when I try this by

  • scaling ws-scheduler to zero replicas
  • removing the schedulerName entry from the ws-manager config and restarting ws-manager,
  • ensuring that no registry-facade instances are running in my namespace

The pod still gets scheduled, even though there's no registry-facade present.
image

The workspace pod had the correct podAffinity and schedulerName set. It's doubtful this is caused by the namespace setup in core-dev either, considering we're running on 1.21 and PodAffinityNamespaceSelector was introduced in 1.22.

@aledbf aledbf force-pushed the aledbf/podAffinity branch from f02d48b to fb825c2 Compare January 6, 2022 18:10
@roboquat roboquat added size/L and removed size/M labels Jan 6, 2022
@aledbf
Copy link
Member Author

aledbf commented Jan 6, 2022

/werft run

👍 started the job as gitpod-build-aledbf-podaffinity.9

@aledbf aledbf force-pushed the aledbf/podAffinity branch from b512fe3 to fb825c2 Compare January 6, 2022 19:05
@aledbf aledbf force-pushed the aledbf/podAffinity branch from c588a73 to c7ebd0b Compare January 7, 2022 15:56
@roboquat roboquat added size/XL team: delivery Issue belongs to the self-hosted team and removed size/L labels Jan 7, 2022
@aledbf aledbf force-pushed the aledbf/podAffinity branch 2 times, most recently from f39c68c to 98a146a Compare January 7, 2022 23:05
@aledbf aledbf force-pushed the aledbf/podAffinity branch 2 times, most recently from 13b224a to 16c2c57 Compare January 14, 2022 13:02
@roboquat roboquat added size/L and removed size/M labels Jan 14, 2022
@aledbf aledbf force-pushed the aledbf/podAffinity branch 3 times, most recently from e754d0d to 2005504 Compare January 14, 2022 14:11
@aledbf aledbf force-pushed the aledbf/podAffinity branch 2 times, most recently from 35049fb to 8b37a6c Compare January 14, 2022 14:40
@aledbf aledbf force-pushed the aledbf/podAffinity branch from 8b37a6c to 5e6487a Compare January 14, 2022 14:48
@aledbf
Copy link
Member Author

aledbf commented Jan 15, 2022

/werft run

👍 started the job as gitpod-build-aledbf-podaffinity.59

@aledbf
Copy link
Member Author

aledbf commented Jan 15, 2022

/werft run

👍 started the job as gitpod-build-aledbf-podaffinity.60

@aledbf
Copy link
Member Author

aledbf commented Jan 15, 2022

/werft with-clean-slate-deployment

👎 unknown command: with-clean-slate-deployment
Use /werft help to list the available commands

@aledbf
Copy link
Member Author

aledbf commented Jan 15, 2022

/werft run

👍 started the job as gitpod-build-aledbf-podaffinity.62

@csweichel
Copy link
Contributor

/lgtm

@roboquat
Copy link
Contributor

LGTM label has been added.

Git tree hash: 26b7b7df863a120b0c9d49f8c839bf3cfb84441b

@aledbf
Copy link
Member Author

aledbf commented Jan 17, 2022

/assign @mrsimonemms

@corneliusludmann
Copy link
Contributor

/approve no-issue

@roboquat
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: corneliusludmann, csweichel

Associated issue requirement bypassed by: corneliusludmann

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@roboquat roboquat merged commit 314413a into main Jan 18, 2022
@roboquat roboquat deleted the aledbf/podAffinity branch January 18, 2022 08:29
@roboquat roboquat added the deployed: workspace Workspace team change is running in production label Jan 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved deployed: workspace Workspace team change is running in production release-note size/L 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.

5 participants