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

docs: add disruption controls design #516

Merged
merged 4 commits into from
Oct 31, 2023
Merged

docs: add disruption controls design #516

merged 4 commits into from
Oct 31, 2023

Conversation

njtran
Copy link
Contributor

@njtran njtran commented Sep 15, 2023 •
edited
Loading

Fixes

Description
This is a request for comments on the disruption controls design.

Once this is implemented, it should solve the following:

Please leave your comments!

How was this change tested?

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

@njtran njtran requested a review from a team as a code owner September 15, 2023 21:59
@coveralls
Copy link

coveralls commented Sep 15, 2023 •
edited
Loading

Pull Request Test Coverage Report for Build 6699985019

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.02%) to 82.473%
Files with Coverage Reduction New Missed Lines %
pkg/test/cachesyncingclient.go 2 80.21%
Totals Coverage Status
Change from base Build 6672224362: -0.02%
Covered Lines: 9270
Relevant Lines: 11240
💛 - Coveralls

jonathan-innis
jonathan-innis reviewed Sep 20, 2023
View reviewed changes
designs/disruption-controls.md Outdated Show resolved Hide resolved
designs/disruption-controls.md Outdated Show resolved Hide resolved
designs/disruption-controls.md Outdated Show resolved Hide resolved
designs/disruption-controls.md Outdated Show resolved Hide resolved
designs/disruption-controls.md Show resolved Hide resolved
designs/disruption-controls.md Outdated Show resolved Hide resolved
designs/disruption-controls.md Outdated Show resolved Hide resolved
This was referenced Sep 20, 2023
Closed
Closed
Closed
blakepettersson

This comment was marked as off-topic.

Sign in to view

charliedmcb
charliedmcb reviewed Sep 20, 2023
View reviewed changes
Copy link

@charliedmcb charliedmcb left a comment

Choose a reason for hiding this comment

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

Great to see the progress on disruption controls!

Have just a few questions/thoughts on MaxUnavailable

designs/disruption-controls.md Show resolved Hide resolved
designs/disruption-controls.md Show resolved Hide resolved
jonathan-innis
jonathan-innis reviewed Sep 21, 2023
View reviewed changes
designs/disruption-controls.md Outdated Show resolved Hide resolved
designs/disruption-controls.md Outdated Show resolved Hide resolved
designs/disruption-controls.md Outdated Show resolved Hide resolved
designs/disruption-controls.md Show resolved Hide resolved
@chlunde
Copy link

chlunde commented Sep 22, 2023

Are you planning to be able to extend this to more mechanisms/controls with parameters, like "deprovision nodes if there are more than 20% over-allocation" or "more than 2 nodes of spare capacity"?

@njtran
Copy link
Contributor Author

njtran commented Sep 22, 2023

@chlunde

Are you planning to be able to extend this to more mechanisms/controls with parameters, like "deprovision nodes if there are more than 20% over-allocation" or "more than 2 nodes of spare capacity"?

The hope was that defining a disruption block in the NodePool would make future features possibly like these to have a sensible place to live. We should evaluate what each of these features mean and how/if we should implement them, but this is definitely where they would live.

@engedaam engedaam mentioned this pull request Oct 6, 2023
Closed
@njtran njtran mentioned this pull request Oct 18, 2023
Closed
@ellistarn
Copy link
Contributor

Is this ready to merge?

@njtran njtran mentioned this pull request Oct 31, 2023
Closed
@njtran njtran merged commit 462e58e into kubernetes-sigs:main Oct 31, 2023
6 checks passed
@njtran njtran deleted the rfc branch November 1, 2023 20:04
@ssoriche ssoriche mentioned this pull request Nov 2, 2023
Open
@tzneal tzneal mentioned this pull request Nov 14, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chlunde chlunde chlunde left review comments

@sidewinder12s sidewinder12s sidewinder12s left review comments

@paulgmiller paulgmiller paulgmiller left review comments

@johngmyers johngmyers johngmyers left review comments

@jonathan-innis jonathan-innis jonathan-innis left review comments

@charliedmcb charliedmcb charliedmcb left review comments

@engedaam engedaam engedaam left review comments

@garvinp-stripe garvinp-stripe garvinp-stripe left review comments

@blakepettersson blakepettersson blakepettersson approved these changes

@ellistarn ellistarn ellistarn approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.