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

feat: reserved capacity support #1911

Merged
merged 10 commits into from
Feb 26, 2025
Merged

feat: reserved capacity support #1911

merged 10 commits into from
Feb 26, 2025

Conversation

jmdeal
Copy link
Member

@jmdeal jmdeal commented Jan 13, 2025 •
edited
Loading

Fixes #N/A

Description
Implements the changes described in #1760.

How was this change tested?

  • make test
  • integration tests in the AWS provider (WIP)

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

@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. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jan 13, 2025
@k8s-ci-robot k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jan 13, 2025
@enxebre enxebre mentioned this pull request Jan 27, 2025
Open
@jmdeal jmdeal force-pushed the feat/reserved-capacity-2 branch from 1bd2ce9 to afe2a47 Compare January 29, 2025 01:02
@k8s-ci-robot k8s-ci-robot added 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 size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 29, 2025
@jmdeal jmdeal force-pushed the feat/reserved-capacity-2 branch from c1255e1 to e523238 Compare January 29, 2025 07:15
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 5, 2025
@jmdeal jmdeal force-pushed the feat/reserved-capacity-2 branch 2 times, most recently from b67556e to 5be3fb3 Compare February 12, 2025 20:13
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 12, 2025
@jmdeal jmdeal force-pushed the feat/reserved-capacity-2 branch from 5be3fb3 to 0547af3 Compare February 12, 2025 21:39
jonathan-innis
jonathan-innis reviewed Feb 13, 2025
View reviewed changes
@coveralls
Copy link

coveralls commented Feb 13, 2025 •
edited
Loading

Pull Request Test Coverage Report for Build 13541340534

Details

  • 302 of 333 (90.69%) changed or added relevant lines in 14 files are covered.
  • 14 unchanged lines in 5 files lost coverage.
  • Overall coverage increased (+0.07%) to 81.6%
Changes Missing Coverage Covered Lines Changed/Added Lines %
pkg/controllers/nodeclaim/disruption/drift.go 11 14 78.57%
pkg/controllers/provisioning/scheduling/scheduler.go 46 49 93.88%
pkg/controllers/disruption/helpers.go 3 7 42.86%
pkg/controllers/provisioning/scheduling/nodeclaim.go 77 83 92.77%
pkg/controllers/disruption/consolidation.go 2 9 22.22%
pkg/controllers/provisioning/scheduling/reservationmanager.go 31 39 79.49%
Files with Coverage Reduction New Missed Lines %
pkg/controllers/provisioning/scheduling/nodeclaim.go 1 88.85%
pkg/test/cachesyncingclient.go 2 82.29%
pkg/utils/termination/termination.go 2 88.24%
pkg/controllers/disruption/consolidation.go 4 85.55%
pkg/controllers/node/termination/controller.go 5 69.54%
Totals Coverage Status
Change from base Build 13509538416: 0.07%
Covered Lines: 9468
Relevant Lines: 11603
💛 - Coveralls

@jmdeal jmdeal changed the title [WIP] feat: implement reserved capacity feat: implement reserved capacity Feb 14, 2025
@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 Feb 14, 2025
@jmdeal jmdeal changed the title feat: implement reserved capacity feat: reserved capacity support Feb 14, 2025
@jmdeal jmdeal force-pushed the feat/reserved-capacity-2 branch 2 times, most recently from 7aa3395 to 95834ee Compare February 14, 2025 08:07
jonathan-innis
jonathan-innis reviewed Feb 15, 2025
View reviewed changes
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Feb 18, 2025
@jmdeal jmdeal force-pushed the feat/reserved-capacity-2 branch 3 times, most recently from e7c9da4 to 290a7dc Compare February 20, 2025 00:21
@jmdeal jmdeal force-pushed the feat/reserved-capacity-2 branch from c07b539 to 55b41ce Compare February 21, 2025 00:54
jonathan-innis
jonathan-innis reviewed Feb 21, 2025
View reviewed changes
@kubernetes-sigs kubernetes-sigs deleted a comment from jmdeal Feb 21, 2025
@jmdeal jmdeal force-pushed the feat/reserved-capacity-2 branch from 4c25410 to 37a26ee Compare February 21, 2025 11:02
@jmdeal jmdeal force-pushed the feat/reserved-capacity-2 branch from 37a26ee to 74040ed Compare February 21, 2025 14:03
jonathan-innis
jonathan-innis approved these changes Feb 21, 2025
View reviewed changes
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.

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Feb 21, 2025
@jonathan-innis
Copy link
Member

I can see the light at the end of the tunnel. Thanks for your hard work on this @jmdeal!

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 24, 2025
@jmdeal jmdeal force-pushed the feat/reserved-capacity-2 branch from a3b0f5e to d34d715 Compare February 25, 2025 00:39
@jmdeal
drop drift tests
bbbe054 
drift is now handled dynamically by the cloudprovider - dropping from
upstream
jonathan-innis
jonathan-innis approved these changes Feb 26, 2025
View reviewed changes
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.

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 26, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jmdeal, jonathan-innis

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

@jmdeal
Copy link
Member Author

jmdeal commented Feb 26, 2025

/unhold

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 26, 2025
@k8s-ci-robot k8s-ci-robot merged commit a863104 into kubernetes-sigs:main Feb 26, 2025
14 checks passed
@DerekFrank DerekFrank mentioned this pull request Feb 27, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonathan-innis jonathan-innis jonathan-innis approved these changes

@jackfrancis jackfrancis Awaiting requested review from jackfrancis

@tallaxes tallaxes Awaiting requested review from tallaxes

Assignees

@jonathan-innis jonathan-innis

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jmdeal @coveralls @jonathan-innis @k8s-ci-robot