Skip to content
This repository has been archived by the owner on Jun 29, 2022. It is now read-only.

Add cpu_manager_policy to workers #1406

Merged
merged 3 commits into from Sep 3, 2021
Merged

Conversation

knrt10
Copy link
Member

@knrt10 knrt10 commented Mar 1, 2021

This add flag to worker pools also added a knob for the user to configure
it.

closes: #1337
Signed-off-by: knrt10 kautilya@kinvolk.io

@knrt10 knrt10 force-pushed the knrt10/cpu-pinnig-worker-pool branch from c7db768 to e0de8f7 Compare March 1, 2021 08:14
@surajssd surajssd force-pushed the knrt10/cpu-pinnig-worker-pool branch from e0de8f7 to 6f1abaa Compare July 14, 2021 14:12
@surajssd surajssd changed the title Add --cpu-manager-policy to kubelet Add cpu_manager_policy to workers Jul 15, 2021
@surajssd surajssd marked this pull request as ready for review July 15, 2021 07:00
@surajssd surajssd requested review from invidian and ipochi July 22, 2021 07:32
@surajssd
Copy link
Member

I was adding this feature to baremetal and was wondering how to restrict this to certain workers since there are no worker pools for it as of now. WDYT if I skip adding support to baremetal?

@invidian
Copy link
Member

I was adding this feature to baremetal and was wondering how to restrict this to certain workers since there are no worker pools for it as of now. WDYT if I skip adding support to baremetal?

Lack of worker pools on Bare metal (actually Matchbox) seems like ongoing issue with feature parity.

@surajssd surajssd force-pushed the knrt10/cpu-pinnig-worker-pool branch from 6f1abaa to 2738400 Compare July 28, 2021 07:22
@surajssd surajssd requested a review from invidian July 28, 2021 07:23
@surajssd surajssd requested a review from invidian August 5, 2021 10:28
pkg/platform/aws/aws.go Show resolved Hide resolved
pkg/platform/aws/template.go Show resolved Hide resolved
@surajssd
Copy link
Member

surajssd commented Aug 6, 2021

@invidian one more round of review?

@invidian
Copy link
Member

invidian commented Aug 6, 2021

@surajssd I already left some comments, I don't see any replies or new code to review. Also, PR has conflicts which must be resolved.

@surajssd
Copy link
Member

surajssd commented Aug 6, 2021

@surajssd I already left some comments, I don't see any replies or new code to review. Also, PR has conflicts which must be resolved.

My bad, I had this page open for many days and did not see those comments.

@surajssd surajssd force-pushed the knrt10/cpu-pinnig-worker-pool branch from 2738400 to b6b3120 Compare August 6, 2021 14:10
pkg/platform/aws/aws.go Show resolved Hide resolved
pkg/platform/aws/aws.go Show resolved Hide resolved
@surajssd surajssd force-pushed the knrt10/cpu-pinnig-worker-pool branch from b6b3120 to c26fc73 Compare August 13, 2021 06:00
@surajssd surajssd requested a review from invidian August 18, 2021 07:47
pkg/platform/aws/aws.go Outdated Show resolved Hide resolved
pkg/platform/aws/aws_internal_test.go Outdated Show resolved Hide resolved
pkg/platform/aws/aws_internal_test.go Outdated Show resolved Hide resolved
pkg/platform/aws/aws_internal_test.go Outdated Show resolved Hide resolved
pkg/platform/packet/packet.go Outdated Show resolved Hide resolved
@surajssd surajssd force-pushed the knrt10/cpu-pinnig-worker-pool branch from c26fc73 to 8e9d3bb Compare August 24, 2021 02:16
@surajssd surajssd requested a review from invidian August 24, 2021 02:16
Copy link
Member

@invidian invidian left a comment

Choose a reason for hiding this comment

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

Perhaps we could use some other reviewers for this PR :)

pkg/platform/platform_test.go Outdated Show resolved Hide resolved
pkg/platform/packet/packet.go Show resolved Hide resolved
pkg/platform/platform.go Outdated Show resolved Hide resolved
@surajssd surajssd force-pushed the knrt10/cpu-pinnig-worker-pool branch from 8e9d3bb to 6d4e20d Compare August 25, 2021 02:45
@surajssd surajssd requested a review from invidian August 25, 2021 07:56
surajssd and others added 2 commits September 2, 2021 07:58
Signed-off-by: Suraj Deshmukh <suraj@kinvolk.io>
This allows a user to choose the cpu manager policy on a worker pool.
Possible values are: `none` and `static`.

To make this work, kubelet also needs a static allocation of system
reserved and kubernetes reserved CPUs to be defined. So this commit also
adds the default of 300m cores for kube-reserved-cpu and 1500m cores for
system-reserved-cpu when `cpu_manager_policy` is set.

closes: #1337

Signed-off-by: knrt10 <kautilya@kinvolk.io>
Co-authored-by: Suraj Deshmukh <suraj@kinvolk.io>
@surajssd surajssd force-pushed the knrt10/cpu-pinnig-worker-pool branch from 6d4e20d to 3897bda Compare September 2, 2021 02:32
invidian
invidian previously approved these changes Sep 2, 2021
Copy link
Member

@invidian invidian left a comment

Choose a reason for hiding this comment

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

LGTM

This allows a user to choose the cpu manager policy on a worker pool.
Possible values are: `none` and `static`.

To make this work, kubelet also needs a static allocation of system
reserved and kubernetes reserved CPUs to be defined. So this commit also
adds the default of 300m cores for kube-reserved-cpu and 1500m cores for
system-reserved-cpu when `cpu_manager_policy` is set.

Signed-off-by: Suraj Deshmukh <suraj@kinvolk.io>
@surajssd surajssd merged commit 4828d2d into master Sep 3, 2021
@surajssd surajssd deleted the knrt10/cpu-pinnig-worker-pool branch September 3, 2021 06:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow setting CPU pinning per worker pool
3 participants