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

fix: add more visibility for when we can't consolidate #292

Merged

Conversation

tzneal
Copy link
Contributor

@tzneal tzneal commented Apr 13, 2023

Fixes aws/karpenter-provider-aws#3746

Description

Makes the inability to schedule all pods message more informative.

How was this change tested?

  • Unit testing
  • Scaled up 100+ nodes and did drift/consolidation/expiration.
    • Verified drifted/expired nodes were being replaced at roughly 1/min
    • Verified consolidated nodes were removed rapidly

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

@tzneal tzneal requested a review from a team as a code owner April 13, 2023 13:25
@tzneal tzneal force-pushed the improve-consolidation-visibility branch from 23c41a2 to c9038a5 Compare April 13, 2023 13:53
@tzneal tzneal force-pushed the improve-consolidation-visibility branch from c9038a5 to 2631fef Compare April 13, 2023 14:08
@coveralls
Copy link

coveralls commented Apr 13, 2023

Pull Request Test Coverage Report for Build 4690194224

  • 65 of 82 (79.27%) changed or added relevant lines in 8 files are covered.
  • 9 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-0.06%) to 81.137%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pkg/controllers/deprovisioning/consolidation.go 11 12 91.67%
pkg/controllers/deprovisioning/drift.go 4 5 80.0%
pkg/controllers/deprovisioning/expiration.go 3 4 75.0%
pkg/controllers/provisioning/scheduling/scheduler.go 24 28 85.71%
pkg/controllers/deprovisioning/helpers.go 7 12 58.33%
pkg/controllers/provisioning/provisioner.go 4 9 44.44%
Files with Coverage Reduction New Missed Lines %
pkg/controllers/deprovisioning/drift.go 1 80.0%
pkg/controllers/deprovisioning/expiration.go 1 74.51%
pkg/controllers/provisioning/scheduling/preferences.go 7 86.67%
Totals Coverage Status
Change from base Build 4671766558: -0.06%
Covered Lines: 6706
Relevant Lines: 8265

💛 - Coveralls

Copy link
Member

@jonathan-innis jonathan-innis left a comment

Choose a reason for hiding this comment

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

This is awesome observability work! 🎉

@tzneal tzneal merged commit efcfedb into kubernetes-sigs:main Apr 13, 2023
@tzneal tzneal deleted the improve-consolidation-visibility branch April 13, 2023 17:27
jonathan-innis pushed a commit to jonathan-innis/karpenter that referenced this pull request Apr 14, 2023
njtran pushed a commit to njtran/karpenter that referenced this pull request May 3, 2023
njtran added a commit that referenced this pull request May 4, 2023
* chore: cleanup deprovisioning types

* add inc

* fix: add more visibility for when we can't consolidate (#292)

* feat: Machine Migration (#273)

* Revert "Revert machine migration changes (#176) (#241)"

This reverts commit 9973eac.

* Change owner reference to blockOwnerDeletion

* Remove string logging for machine launch

* Removing FailedInit since machine statusCondition captures it

* Updated eventing on machines

* action const

* testfix

* testfix2

* comments

* rename

* last

* enum

* fixcompile

* export

* fixredundancy

* fixlogic

* comment

* renameVar

---------

Co-authored-by: Todd Neal <tnealt@amazon.com>
Co-authored-by: Jonathan Innis <jonathan.innis.ji@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide more information in the "Not All Pods would Schedule" Event
3 participants