feat: expose nodeclaim disruption through new disruption condition, improves pod eviction event message #1370

cnmcavoy · 2024-06-28T21:29:14Z

Fixes #N/A

Description

Add's a new nodeclaim condition DisruptionCandidate which is set when a nodeclaim is being disrupted, and is applied after the disruption taint is set. The DisruptionCandidate nodeclaim condition contains the reason why the nodeclaim is being terminated (e.g node worker-mgn6n/ip-10-115-200-242.us-east-2.compute.internal was single node consolidated).

The motivation for this new nodeclaim condition is so that when evicting pods, we can look up this condition and use the condition's message in the pod event.

Example of what the pod events look like now from testing in our clusters:

keda                                          85s         Normal    Evicted                           pod/keda-admission-webhooks-5bd6b554ff-tn25h                                                Evicted pod: node worker-qa-czn28/ip-10-115-195-50.us-east-2.compute.internal drifted
keda                                          86s         Normal    Evicted                           pod/keda-operator-688bc9b887-7t4gd                                                          Evicted pod: node worker-qa-czn28/ip-10-115-195-50.us-east-2.compute.internal drifted

How was this change tested?

Built Karpenter with this change locally and tested in our clusters. Also make presubmit

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

coveralls · 2024-06-28T21:46:53Z

Pull Request Test Coverage Report for Build 9718717778

Details

32 of 54 (59.26%) changed or added relevant lines in 4 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.1%) to 78.704%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pkg/operator/operator.go	0	3	0.0%
pkg/controllers/disruption/controller.go	14	23	60.87%
pkg/controllers/node/termination/terminator/eviction.go	16	26	61.54%

Totals
Change from base Build 9718332603:	-0.1%
Covered Lines:	8622
Relevant Lines:	10955

💛 - Coveralls

coveralls · 2024-07-01T22:26:38Z

Pull Request Test Coverage Report for Build 9750492374

Details

32 of 54 (59.26%) changed or added relevant lines in 4 files are covered.
4 unchanged lines in 2 files lost coverage.
Overall coverage decreased (-0.1%) to 78.684%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pkg/operator/operator.go	0	3	0.0%
pkg/controllers/disruption/controller.go	14	23	60.87%
pkg/controllers/node/termination/terminator/eviction.go	16	26	61.54%

Files with Coverage Reduction	New Missed Lines	%
pkg/test/expectations/expectations.go	2	93.69%
pkg/utils/atomic/lazy.go	2	87.88%

Totals
Change from base Build 9748897421:	-0.1%
Covered Lines:	8619
Relevant Lines:	10954

💛 - Coveralls

pkg/controllers/disruption/controller.go

coveralls · 2024-07-03T16:31:20Z

Pull Request Test Coverage Report for Build 9781364600

Details

39 of 57 (68.42%) changed or added relevant lines in 9 files are covered.
2 unchanged lines in 1 file lost coverage.
Overall coverage decreased (-0.05%) to 78.695%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pkg/operator/operator.go	0	3	0.0%
pkg/controllers/disruption/controller.go	6	11	54.55%
pkg/controllers/node/termination/terminator/eviction.go	16	26	61.54%

Files with Coverage Reduction	New Missed Lines	%
pkg/controllers/provisioning/scheduling/nodeclaim.go	2	89.13%

Totals
Change from base Build 9772878439:	-0.05%
Covered Lines:	8636
Relevant Lines:	10974

💛 - Coveralls

coveralls · 2024-07-08T18:49:26Z

Pull Request Test Coverage Report for Build 12399190527

Details

66 of 82 (80.49%) changed or added relevant lines in 6 files are covered.
7 unchanged lines in 3 files lost coverage.
Overall coverage decreased (-0.05%) to 81.247%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pkg/controllers/node/termination/terminator/eviction.go	20	22	90.91%
pkg/controllers/state/statenode.go	18	22	81.82%
pkg/controllers/disruption/controller.go	24	34	70.59%

Files with Coverage Reduction	New Missed Lines	%
pkg/controllers/disruption/controller.go	1	71.13%
pkg/utils/termination/termination.go	2	92.31%
pkg/controllers/disruption/consolidation.go	4	88.55%

Totals
Change from base Build 12303032942:	-0.05%
Covered Lines:	9068
Relevant Lines:	11161

💛 - Coveralls

cnmcavoy · 2024-07-08T21:41:54Z

/remove-needs-rebase

pkg/controllers/disruption/controller.go

pkg/controllers/state/statenode.go

pkg/controllers/nodeclaim/lifecycle/controller.go

pkg/controllers/node/termination/terminator/eviction.go

pkg/controllers/disruption/controller.go

njtran

Nice work, think we're getting close!

pkg/controllers/disruption/controller.go

pkg/controllers/state/statenode.go

pkg/controllers/nodeclaim/lifecycle/controller.go

pkg/controllers/node/termination/terminator/eviction.go

pkg/controllers/disruption/controller.go

github-actions · 2024-12-10T12:02:22Z

This PR has been inactive for 14 days. StaleBot will close this stale PR after 14 more days of inactivity.

engedaam · 2024-12-12T17:12:56Z

/assign @njtran

…dd eviction message from condition Signed-off-by: Cameron McAvoy <cmcavoy@indeed.com>

github-actions · 2025-01-02T12:02:08Z

This PR has been inactive for 14 days. StaleBot will close this stale PR after 14 more days of inactivity.

njtran

/lgtm
/approve

thanks for the contribution!

k8s-ci-robot · 2025-01-02T18:55:37Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cnmcavoy, njtran

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:

~~OWNERS~~ [njtran]

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

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 28, 2024

k8s-ci-robot requested review from engedaam and jackfrancis June 28, 2024 21:29

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jun 28, 2024

cnmcavoy force-pushed the cmcavoy/eviction-reason branch from 83de6f8 to a7f19d9 Compare June 28, 2024 21:30

cnmcavoy force-pushed the cmcavoy/eviction-reason branch from a7f19d9 to 3e04007 Compare July 1, 2024 20:15

Bryce-Soghigian reviewed Jul 2, 2024

View reviewed changes

pkg/controllers/disruption/controller.go Outdated Show resolved Hide resolved

Bryce-Soghigian reviewed Jul 2, 2024

View reviewed changes

pkg/controllers/disruption/controller.go Outdated Show resolved Hide resolved

cnmcavoy force-pushed the cmcavoy/eviction-reason branch 5 times, most recently from 9819849 to 3041c3e Compare July 3, 2024 16:14

cnmcavoy requested a review from Bryce-Soghigian July 3, 2024 16:41

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 4, 2024

cnmcavoy force-pushed the cmcavoy/eviction-reason branch from 3041c3e to 0d31c2d Compare July 8, 2024 18:32

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jul 9, 2024

cnmcavoy force-pushed the cmcavoy/eviction-reason branch from 0d31c2d to ef86002 Compare July 12, 2024 17:52

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 12, 2024

cnmcavoy force-pushed the cmcavoy/eviction-reason branch from ef86002 to 2b235b5 Compare July 12, 2024 18:03

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 26, 2024

cnmcavoy force-pushed the cmcavoy/eviction-reason branch from 2b235b5 to 0807939 Compare July 26, 2024 19:23

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jul 26, 2024

cnmcavoy force-pushed the cmcavoy/eviction-reason branch 2 times, most recently from ab5dbb4 to 22ed1fe Compare October 2, 2024 20:48

cnmcavoy requested a review from njtran October 2, 2024 20:48

njtran reviewed Oct 8, 2024

View reviewed changes

cnmcavoy force-pushed the cmcavoy/eviction-reason branch 2 times, most recently from 5743ceb to 5381d84 Compare October 8, 2024 20:04

njtran reviewed Oct 21, 2024

View reviewed changes

cnmcavoy force-pushed the cmcavoy/eviction-reason branch 2 times, most recently from 8f0d122 to 23471fa Compare November 6, 2024 20:55

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 9, 2024

cnmcavoy force-pushed the cmcavoy/eviction-reason branch from 23471fa to 001c05c Compare November 19, 2024 18:59

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 19, 2024

cnmcavoy requested a review from njtran November 19, 2024 20:36

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 24, 2024

cnmcavoy force-pushed the cmcavoy/eviction-reason branch from 001c05c to ed1453b Compare November 25, 2024 15:41

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 25, 2024

github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 10, 2024

k8s-ci-robot assigned njtran Dec 12, 2024

github-actions bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 13, 2024

Expose nodeclaim disruption candidates through nodeclaim condition, a…

4dd4e79

…dd eviction message from condition Signed-off-by: Cameron McAvoy <cmcavoy@indeed.com>

cnmcavoy force-pushed the cmcavoy/eviction-reason branch from ed1453b to 4dd4e79 Compare December 18, 2024 18:36

github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 2, 2025

njtran reviewed Jan 2, 2025

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 2, 2025

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 2, 2025

k8s-ci-robot merged commit cfda355 into kubernetes-sigs:main Jan 2, 2025
12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: expose nodeclaim disruption through new disruption condition, improves pod eviction event message #1370

feat: expose nodeclaim disruption through new disruption condition, improves pod eviction event message #1370

cnmcavoy commented Jun 28, 2024 •

edited

Loading

coveralls commented Jun 28, 2024 •

edited

Loading

coveralls commented Jul 1, 2024 •

edited

Loading

coveralls commented Jul 3, 2024 •

edited

Loading

coveralls commented Jul 8, 2024 •

edited

Loading

cnmcavoy commented Jul 8, 2024

njtran left a comment

github-actions bot commented Dec 10, 2024

engedaam commented Dec 12, 2024

github-actions bot commented Jan 2, 2025

njtran left a comment

k8s-ci-robot commented Jan 2, 2025

feat: expose nodeclaim disruption through new disruption condition, improves pod eviction event message #1370

feat: expose nodeclaim disruption through new disruption condition, improves pod eviction event message #1370

Conversation

cnmcavoy commented Jun 28, 2024 • edited Loading

coveralls commented Jun 28, 2024 • edited Loading

Pull Request Test Coverage Report for Build 9718717778

Details

💛 - Coveralls

coveralls commented Jul 1, 2024 • edited Loading

Pull Request Test Coverage Report for Build 9750492374

Details

💛 - Coveralls

coveralls commented Jul 3, 2024 • edited Loading

Pull Request Test Coverage Report for Build 9781364600

Details

💛 - Coveralls

coveralls commented Jul 8, 2024 • edited Loading

Pull Request Test Coverage Report for Build 12399190527

Details

💛 - Coveralls

cnmcavoy commented Jul 8, 2024

njtran left a comment

Choose a reason for hiding this comment

github-actions bot commented Dec 10, 2024

engedaam commented Dec 12, 2024

github-actions bot commented Jan 2, 2025

njtran left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Jan 2, 2025

cnmcavoy commented Jun 28, 2024 •

edited

Loading

coveralls commented Jun 28, 2024 •

edited

Loading

coveralls commented Jul 1, 2024 •

edited

Loading

coveralls commented Jul 3, 2024 •

edited

Loading

coveralls commented Jul 8, 2024 •

edited

Loading