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.

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

[KEP-2400]: Swap eviction signal #128137

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

[KEP-2400]: Swap eviction signal #128137

wants to merge 5 commits into from

Conversation

kannon92
Copy link
Contributor

@kannon92 kannon92 commented Oct 16, 2024 •
edited
Loading

What type of PR is this?

/kind feature

What this PR does / why we need it:

Make eviction manager aware of swap as a resource

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

We introduce a new eviction signal called swap.available which tracks the swap usage. Once this swap usage is passed the user specified eviction signal, then evictions will be based on swap usage.

I added a commit for Swap KEP that explains these changes: kubernetes/enhancements@85e36dc

Does this PR introduce a user-facing change?

Add a new eviction signal swap.available to track the amount of swap and evict once that threshold is surpassed

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Oct 16, 2024
@k8s-ci-robot
Copy link
Contributor

This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Oct 16, 2024
@k8s-ci-robot k8s-ci-robot added area/kubelet kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API sig/apps Categorizes an issue or PR as relevant to SIG Apps. sig/node Categorizes an issue or PR as relevant to SIG Node. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 16, 2024
@kannon92 kannon92 force-pushed the swap-eviction-signal branch 2 times, most recently from c7b66d0 to 0ca319f Compare October 20, 2024 16:44
@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Oct 22, 2024
@kannon92 kannon92 changed the title wip: Swap eviction signal Swap eviction signal Oct 22, 2024
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 22, 2024
@kannon92
Copy link
Contributor Author

/hold cancel

@kannon92 kannon92 force-pushed the swap-eviction-signal branch from 0ca319f to 5f3097a Compare October 22, 2024 19:08
rphillips
rphillips reviewed Oct 22, 2024
View reviewed changes
pkg/controller/daemon/util/daemonset_util.go Outdated
@@ -80,6 +80,12 @@ func AddOrUpdateDaemonPodTolerations(spec *v1.PodSpec) {
Effect: v1.TaintEffectNoSchedule,
})

v1helper.AddOrUpdateTolerationInPodSpec(spec, &v1.Toleration{
Copy link
Member

Choose a reason for hiding this comment

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

Needs to be feature gated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

@kannon92
Copy link
Contributor Author

kannon92 commented Nov 1, 2024

/test pull-kubernetes-node-swap-fedora-serial
/test pull-kubernetes-node-swap-ubuntu-serial

@kannon92 kannon92 force-pushed the swap-eviction-signal branch from ae12042 to beb3776 Compare November 1, 2024 18:54
@kannon92
Copy link
Contributor Author

kannon92 commented Nov 1, 2024

/test pull-kubernetes-node-swap-fedora-serial
/test pull-kubernetes-node-swap-ubuntu-serial

4 similar comments
@kannon92
Copy link
Contributor Author

kannon92 commented Nov 1, 2024

/test pull-kubernetes-node-swap-fedora-serial
/test pull-kubernetes-node-swap-ubuntu-serial

@kannon92
Copy link
Contributor Author

kannon92 commented Nov 1, 2024

/test pull-kubernetes-node-swap-fedora-serial
/test pull-kubernetes-node-swap-ubuntu-serial

@kannon92
Copy link
Contributor Author

kannon92 commented Nov 2, 2024

/test pull-kubernetes-node-swap-fedora-serial
/test pull-kubernetes-node-swap-ubuntu-serial

@kannon92
Copy link
Contributor Author

kannon92 commented Nov 3, 2024

/test pull-kubernetes-node-swap-fedora-serial
/test pull-kubernetes-node-swap-ubuntu-serial

@kannon92 kannon92 force-pushed the swap-eviction-signal branch from 95a93e4 to 7ce1289 Compare November 3, 2024 14:57
@kannon92
Copy link
Contributor Author

kannon92 commented Nov 3, 2024

/test pull-kubernetes-node-swap-fedora-serial
/test pull-kubernetes-node-swap-ubuntu-serial

2 similar comments
@kannon92
Copy link
Contributor Author

kannon92 commented Nov 3, 2024

/test pull-kubernetes-node-swap-fedora-serial
/test pull-kubernetes-node-swap-ubuntu-serial

@kannon92
Copy link
Contributor Author

kannon92 commented Nov 3, 2024

/test pull-kubernetes-node-swap-fedora-serial
/test pull-kubernetes-node-swap-ubuntu-serial

@kannon92 kannon92 force-pushed the swap-eviction-signal branch from 6176cc5 to 13c9971 Compare November 3, 2024 22:37
@kannon92 kannon92 requested a review from yujuhong November 3, 2024 22:38
@kannon92
Copy link
Contributor Author

kannon92 commented Nov 3, 2024

/test pull-kubernetes-node-swap-fedora-serial
/test pull-kubernetes-node-swap-ubuntu-serial

@kannon92
Copy link
Contributor Author

kannon92 commented Nov 3, 2024

@yujuhong I worked on some e2e tests and I got them working.

@kannon92 kannon92 force-pushed the swap-eviction-signal branch from 13c9971 to 3fb95ff Compare November 3, 2024 22:41
@kannon92
Copy link
Contributor Author

kannon92 commented Nov 3, 2024

/test pull-kubernetes-node-swap-fedora-serial
/test pull-kubernetes-node-swap-ubuntu-serial

@kannon92
Copy link
Contributor Author

kannon92 commented Nov 4, 2024

/retest

@kannon92
Copy link
Contributor Author

kannon92 commented Nov 4, 2024

/test pull-kubernetes-node-swap-fedora-serial
/test pull-kubernetes-node-swap-ubuntu-serial

@kannon92
Copy link
Contributor Author

kannon92 commented Nov 4, 2024

Tests still seem to be a work in progress..

@kannon92
Copy link
Contributor Author

kannon92 commented Nov 4, 2024

/test pull-kubernetes-node-swap-fedora-serial
/test pull-kubernetes-node-swap-ubuntu-serial

@kannon92
Copy link
Contributor Author

kannon92 commented Nov 6, 2024

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 6, 2024
@kannon92
Copy link
Contributor Author

kannon92 commented Nov 6, 2024

I am using kubernetes/test-infra#33753 to test the eviction tests and I am running into some difficulties.

I think we should not push this for this release and we will continue to look into this.

@yujuhong
Copy link
Contributor

yujuhong commented Nov 6, 2024

I am using kubernetes/test-infra#33753 to test the eviction tests and I am running into some difficulties.

ACK. Thanks for the update.

@k8s-ci-robot
Copy link
Contributor

@kannon92: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-kubernetes-node-swap-ubuntu-serial 2aed4f7 link false /test pull-kubernetes-node-swap-ubuntu-serial
pull-kubernetes-node-swap-fedora-serial 2aed4f7 link false /test pull-kubernetes-node-swap-fedora-serial
pull-kubernetes-e2e-kind da37277 link true /test pull-kubernetes-e2e-kind
pull-kubernetes-e2e-gce da37277 link true /test pull-kubernetes-e2e-gce

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrewsykim andrewsykim Awaiting requested review from andrewsykim

@kow3ns kow3ns Awaiting requested review from kow3ns

@rphillips rphillips Awaiting requested review from rphillips

@yujuhong yujuhong Awaiting requested review from yujuhong

Assignees

@dchen1107 dchen1107

@SergeyKanzhelev SergeyKanzhelev

@yujuhong yujuhong

Labels
area/kubelet area/test cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API needs-priority Indicates a PR lacks a `priority/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/apps Categorizes an issue or PR as relevant to SIG Apps. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
SIG Apps
Status: Needs Triage
SIG Node CI/Test Board
Status: Archive-it
SIG Node: code and documentation PRs
Status: Needs Reviewer
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@kannon92 @k8s-ci-robot @k8s-triage-robot @AnishShah @yujuhong @rphillips @dchen1107 @SergeyKanzhelev