-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add diagrams Update diagrams Update diagrams Update diagrams Resize diagram images Resize diagram images Resize diagram images Update image size Resize diagram images Fix a format issue Update images Update README.md Update README.md Update README.md Update README.md Update README.md Update README.md Add issue id Update toc Fix typos Fix a format issue Fix format issues Update decoupled-taint-manager KEP Fix a typo in kep.yaml Fix errors Revise README.md Update alternatives Update alternatives Update alterntives Update alternatives Update README Update keps/sig-scheduling/3902-decoupled-taint-manager/README.md Co-authored-by: Wei Huang <weih@hey.com> Address Wei Huang's comments Add Story 3 Remove backup files Address Aldo and Ravi's comments Update Goals Reformat code Reformat code Focus on code split Remove the backup file Address Aldo's comments Fix typos Fix an format issue Fix toc Update README.md Another revison to address comments Fix diagram error Update status in README.md Address sig-app comments Address Wei's comments Update Address final comments from sig-app/sig-scheduling Address sig-node comments Update risk and mitigation Update risk and mitigations
- Loading branch information
Yuan Chen
committed
Jun 6, 2023
1 parent
749b49b
commit 89f2ac7
Showing
5 changed files
with
331 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
kep-number: 3902 | ||
beta: | ||
approver: "@wojtek-t" |
278 changes: 278 additions & 0 deletions
278
keps/sig-scheduling/3902-decoupled-taint-manager/README.md
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,278 @@ | ||
# KEP-3902: Decouple Taint-based Pod Eviction from Node Lifecycle Controller | ||
<!-- toc --> | ||
- [Release Signoff Checklist](#release-signoff-checklist) | ||
- [Summary](#summary) | ||
- [Motivation](#motivation) | ||
- [Goals](#goals) | ||
- [Non-Goals](#non-goals) | ||
- [Proposal](#proposal) | ||
- [User Stories (Optional)](#user-stories-optional) | ||
- [Story](#story) | ||
- [Notes/Constraints/Caveats (Optional)](#notesconstraintscaveats-optional) | ||
- [Risks and Mitigations](#risks-and-mitigations) | ||
- [Design Details](#design-details) | ||
- [Proposed Controllers](#proposed-controllers) | ||
- [Implementation](#implementation) | ||
- [Test Plan](#test-plan) | ||
- [Prerequisite testing updates](#prerequisite-testing-updates) | ||
- [Unit tests](#unit-tests) | ||
- [Integration tests](#integration-tests) | ||
- [E2E tests](#e2e-tests) | ||
- [Graduation Criteria](#graduation-criteria) | ||
- [Alpha](#alpha) | ||
- [Beta](#beta) | ||
- [GA](#ga) | ||
- [Upgrade / Downgrade Strategy](#upgrade--downgrade-strategy) | ||
- [Version Skew Strategy](#version-skew-strategy) | ||
- [Production Readiness Review Questionnaire](#production-readiness-review-questionnaire) | ||
- [Feature Enablement and Rollback](#feature-enablement-and-rollback) | ||
- [Rollout, Upgrade and Rollback Planning](#rollout-upgrade-and-rollback-planning) | ||
- [Monitoring Requirements](#monitoring-requirements) | ||
- [Dependencies](#dependencies) | ||
- [Scalability](#scalability) | ||
- [Troubleshooting](#troubleshooting) | ||
- [Implementation History](#implementation-history) | ||
- [Drawbacks](#drawbacks) | ||
- [Alternatives](#alternatives) | ||
- [Infrastructure Needed (Optional)](#infrastructure-needed-optional) | ||
- [Note](#note) | ||
<!-- /toc --> | ||
|
||
## Release Signoff Checklist | ||
|
||
Items marked with (R) are required *prior to targeting to a milestone / release*. | ||
|
||
- [ ] (R) Enhancement issue in release milestone, which links to KEP dir in [kubernetes/enhancements](https://git.k8s.io/enhancements) (not the initial KEP PR) | ||
- [ ] (R) KEP approvers have approved the KEP status as `implementable` | ||
- [X] (R) Design details are appropriately documented | ||
- [X] (R) Test plan is in place, giving consideration to SIG Architecture and SIG Testing input (including test refactors) | ||
- [ ] e2e Tests for all Beta API Operations (endpoints) | ||
- [ ] (R) Ensure GA e2e tests meet requirements for [Conformance Tests](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/conformance-tests.md) | ||
- [ ] (R) Minimum Two Week Window for GA e2e tests to prove flake free | ||
- [X] (R) Graduation criteria is in place | ||
- [ ] (R) [all GA Endpoints](https://github.com/kubernetes/community/pull/1806) must be hit by [Conformance Tests](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/conformance-tests.md) | ||
- [ ] (R) Production readiness review completed | ||
- [ ] (R) Production readiness review approved | ||
- [ ] "Implementation History" section is up-to-date for milestone | ||
- [ ] User-facing documentation has been created in [kubernetes/website](https://git.k8s.io/website), for publication to [kubernetes.io](https://kubernetes.io/) | ||
- [ ] Supporting documentation—e.g., additional design documents, links to mailing list discussions/SIG meetings, relevant PRs/issues, release notes | ||
|
||
## Summary | ||
|
||
In Kubernetes, `NodeLifecycleController` applies predefined `NoExecute` taints (e.g., `Unreachable`, `NotReady`) when the nodes are determined to be unhealthy. After the nodes get tainted, `TaintManager` does its due diligence to start deleting running pods on those nodes based on `NoExecute` taints, which can be added by anyone. | ||
|
||
In this KEP, we propose to decouple `TaintManager` that performs taint-based pod eviction from `NodeLifecycleController` and make them two separate controllers: `NodeLifecycleController` to add taints to unhealthy nodes and `TaintManager` to perform pod deletion on nodes tainted with NoExecute effect. | ||
|
||
This separation not only improves code organization but also makes it easier to improve `TaintManager` or build custom `TaintManager`. | ||
|
||
## Motivation | ||
`NodeLifecycleController` combines two independent functions: | ||
* Adding a pre-defined set of `NoExecute` taints to nodes based on the node conditions | ||
* Performing pod eviction on `NoExecute` taints | ||
|
||
Splitting the `NodeLifecycleController` based on the above functionalities will help to disentangle code and make future extensions to either component manageable. | ||
|
||
### Goals | ||
|
||
* Move taint-based eviction implementation out of `NodeLifecycleController` into a separate and independent taint-manager to enhance separation of concerns, maintainability. | ||
|
||
### Non-Goals | ||
|
||
The main focus of the KEP is on the separation of concerns between `NodeLifecycleController` and `TaintManager`, which will improve code organization and enable future extensions to `TaintManager`. While it is true that the separation allows cluster operators to build custom `TaintManager` and disable the default `TaintManager`, this is a side-effect of the change rather than the main goal. the KEP emphasizes the benefits of the separation of concerns and briefly mention the option to disable the default TaintManager as a potential side-effect. | ||
|
||
The non-goals are the following. | ||
* Introduce extra maintenance burden or regression after the code reorganization | ||
* Introduce incompatible behavior of `NodeLifecycleController` or `TaintManager` | ||
* Extend and enhance the current built-in APIs for customization of node taint-based eviction, including additional fields, more flexible communication protocols, webhooks and CRD references, etc. | ||
* Actual extension and enhancement of the current `TaintManager`. | ||
|
||
## Proposal | ||
|
||
### User Stories (Optional) | ||
|
||
While this split is for improving the code maintainability, an effect of this change is that it allows cluster-admins to extend and enhance the default `TaintManager` and even replace the default `TaintManager` with a custom implementation. However, discussing such use-cases for extending `TaintManager` or writing custom `TaintManager` is beyond the scope of this KEP. | ||
|
||
#### Story | ||
|
||
### Notes/Constraints/Caveats (Optional) | ||
|
||
### Risks and Mitigations | ||
|
||
Compared with a single combined controller, the risks of using two separate controllers include the following. | ||
* Slightly increase the communication overhead from applying node taints to performing pod eviction. | ||
* Make cancellation of pod eviction and un-tainting nodes harder. | ||
|
||
However, the performance overhead of the split controllers is expected to be small, and pod eviction cancellation and un-tainting nodes are not common use cases. Splitting `NodeLifecycleController` is more beneficial by promoting a cleaner design and enhancing the overall maintainability and extensibility in the long term. | ||
|
||
## Design Details | ||
|
||
In Kubernetes version 1.27 and earlier, the `NoExecuteTaintManager` component is included within the `NodeLifecycleController`. | ||
|
||
![Current Controller](current-nodelifecycle-controller.png) | ||
|
||
### Proposed Controllers | ||
|
||
The proposed design refactors `NodeLifecycleController` into two independent controllers, which are managed by `kube-controller-manager`. | ||
|
||
1. `NodeLifecycleController` monitors node health and adds `NotReady` and `Unreachable` taints to nodes | ||
2. `NoExecuteTaintManager` watches for node and pod updates, and performs `NoExecute` taint based pod eviction | ||
|
||
The existing kube-controller-manager flag `--controllers` can be used to optionally disable the `NoExecuteTaintManager`. | ||
|
||
![New Controllers](new-controllers.png) | ||
|
||
### Implementation | ||
|
||
A new `NodeLifecycleManager` is implemented by removing taint-manager related code from `controller/nodelifecycle/node_lifecycle_controller.go`. | ||
|
||
A new `NoExecuteTaintManager` is created as a top-level controller managed by` kube-controller-manager`. Its implementation is based on the current taint-manager from `controller/nodelifecycle/taint-manager.go`. | ||
|
||
The creation and startup of the default taint-manager is performed by `kube-controller-manager`. A feature gate `SeparateTaintManager` controls whether to use the split `TaintManager` or roll back to the old `NodeLifecycleController`. | ||
|
||
### Test Plan | ||
|
||
[x] I/we understand the owners of the involved components may require updates to existing tests to make this code solid enough prior to committing the changes necessary to implement this enhancement. | ||
|
||
##### Prerequisite testing updates | ||
|
||
Kubernetes already has a good coverage of node `No-Execute` eviction. We will add tests only for the changed code. | ||
|
||
##### Unit tests | ||
- `pkg/controller/taintmanager`: 2023-06-03 - 0% (to add) | ||
- Enable and disable the new `TaintManager` | ||
- The new `TaintManager` acts on node taints properly | ||
- `pkg/controller/apis/config/v1alpha1`: 2023-06-03 - 0% | ||
- Test new configuration | ||
- `pkg/controller/nodelifecycle`: 2023-06-03 - 74.8% | ||
- Test the combined controller | ||
- `pkg/controller/nodelifecycle/scheduler`: 2023-06-03 - 90% | ||
- Test the combined controller | ||
|
||
##### Integration tests | ||
- Verify the ability to disable the default `TaintManager`. | ||
- Verify the new `TaintManager`to act on node taints properly. | ||
|
||
##### E2E tests | ||
|
||
- Verify passing the existing E2E and conformance tests using the split `TaintManager`. | ||
|
||
### Graduation Criteria | ||
|
||
#### Alpha | ||
|
||
Since there are no new API changes, we can skip Alpha and go to Beta directly. | ||
|
||
#### Beta | ||
|
||
* Support `kube-controller-manager` flag `--controllers=-taint-manager` to disable the default taint-manager. | ||
* Unit and e2e tests completed and passed. | ||
* Performance and scalability tests to verify there is non-negligible performance degradation in node taint-based eviction. | ||
* Update documents to reflect the changes. | ||
|
||
#### GA | ||
|
||
* Fix all reported bugs if any. | ||
|
||
### Upgrade / Downgrade Strategy | ||
|
||
A feature gate `SeparateTaintManager` enables and disables the new feature. When the feature is turned on, a user can use `kube-controller-manager`'s flag to disable the default `TaintManager`. | ||
|
||
|
||
### Version Skew Strategy | ||
|
||
## Production Readiness Review Questionnaire | ||
|
||
### Feature Enablement and Rollback | ||
|
||
*This section must be completed when targeting alpha to a release.* | ||
|
||
* **How can this feature be enabled / disabled in a live cluster?** | ||
* Enabled: turn on the new feature gate `SeparateTaintManager` | ||
* Disabled: turn off the new feature gate `SeparateTaintManager` | ||
* **Does enabling the feature change any default behavior?** | ||
* No, the default node-life-cycle controller and taint-manager behavior will stay the same. | ||
* **Can the feature be disabled once it has been enabled (i.e. can we roll back the enablement)?** | ||
* Yes. Once rolled back, `taint-manager` is enabled along with `node-lifecycle-controller` | ||
* **What happens if we re-enable the feature if it was previously rolled back?** | ||
* The feature will work as usual. | ||
* **Are there any tests for feature enablement/disablement?** | ||
* Planned for Beta release | ||
* Appropriate tests have been added in the integration tests. See [Integration tests](#integration-tests) for more details. | ||
|
||
### Rollout, Upgrade and Rollback Planning | ||
|
||
*This section must be completed when targeting beta graduation to a release.* | ||
|
||
* **How can a rollout or rollback fail? Can it impact already running workloads?** | ||
A feature gate `SeparateTaintManager` controls whether to use the split `TaintManager` or the old combined `NodeLifecycleController`. | ||
|
||
* **What specific metrics should inform a rollback?** | ||
N/A | ||
|
||
* **Were upgrade and rollback tested? Was the upgrade→downgrade→upgrade path tested?** | ||
They will be covered in unit and e2e tests. | ||
|
||
* **Is the rollout accompanied by any deprecations and/or removals of features, APIs, fields of API types, flags, etc.?** | ||
No | ||
|
||
### Monitoring Requirements | ||
|
||
*This section must be completed when targeting beta graduation to a release.* | ||
|
||
* **How can an operator determine if the feature is in use by workloads?** | ||
* It can be determined by if the feature gate `SeparateTaintManager` is used. | ||
* **How can someone using this feature know that it is working for their instance?** | ||
* Node taints and taint-based pod eviction work as usual. Admins and users should not see any different behavior if everything works as planned. | ||
* **What are the reasonable SLOs (Service Level Objectives) for the enhancement?** | ||
* The performance of node taint-based eviction should remain the same level as before. | ||
* **What are the SLIs (Service Level Indicators) an operator can use to determine the health of the service?** | ||
* The metrics for both `NodeLifecycleController` and `TaintManager`'s queues should stay the same levels as before. | ||
* **Are there any missing metrics that would be useful to have to improve observability of this feature?** | ||
* No | ||
|
||
### Dependencies | ||
|
||
* **Does this feature depend on any specific services running in the cluster?** | ||
* N/A | ||
|
||
### Scalability | ||
|
||
* **Will enabling / using this feature result in any new API calls?** | ||
* No | ||
* **Will enabling / using this feature result in introducing new API types?** | ||
* No | ||
* **Will enabling / using this feature result in any new calls to the cloud provider?** | ||
* No | ||
* **Will enabling / using this feature result in increasing size or count of the existing API objects?** | ||
* No | ||
* **Will enabling / using this feature result in increasing time taken by any operations covered by existing SLIs/SLOs?** | ||
* No | ||
* **Will enabling / using this feature result in non-negligible increase of resource usage (CPU, RAM, disk, IO, ...) in any components?** | ||
* No | ||
|
||
### Troubleshooting | ||
|
||
* **How does this feature react if the API server and/or etcd is unavailable?** | ||
* The same behavior as the current version: fail to add taint and/or evict pods on tainted nodes | ||
* **What are other known failure modes?** | ||
* No | ||
* **What steps should be taken if SLOs are not being met to determine the problem?** | ||
* If the default or custom taint-manager is working or not. | ||
|
||
## Implementation History | ||
|
||
* 2023-03-06: Initial KEP published. | ||
|
||
## Drawbacks | ||
|
||
## Alternatives | ||
|
||
Keeping the code as-is is an option, but it will make future extension of `TaintManager` harder and less flexible. | ||
|
||
|
||
## Infrastructure Needed (Optional) | ||
|
||
N/A | ||
|
||
## Note | ||
`taint-manager`, `TaintManager` and `NoExecuteTaintManager` are used interchangeably in this doc. |
Binary file added
BIN
+81.9 KB
...ig-scheduling/3902-decoupled-taint-manager/current-nodelifecycle-controller.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
title: Decouple TaintManager from NodeLifeCycleController | ||
kep-number: 3902 | ||
authors: | ||
- "@yuanchen8911" | ||
- "@atosatto" | ||
- "@ddebroy" | ||
- "@ravisantoshgudimetla" | ||
owning-sig: sig-scheduling | ||
participating-sigs: | ||
- sig-apps | ||
- sig-node | ||
status: implementable | ||
creation-date: 2023-03-07 | ||
reviewers: | ||
- "@Huang-Wei" | ||
- "@alculquicondor" | ||
- "@SergeyKanzhelev" | ||
- "@soltysh" | ||
approvers: | ||
- "@Huang-Wei" | ||
- "@alculquicondor" | ||
- "@SergeyKanzhelev" | ||
- "@soltysh" | ||
|
||
see-also: | ||
replaces: | ||
|
||
# The target maturity stage in the current dev cycle for this KEP. | ||
stage: beta | ||
|
||
# The most recent milestone for which work toward delivery of this KEP has been | ||
# done. This can be the current (upcoming) milestone, if it is being actively | ||
# worked on. | ||
latest-milestone: "v1.28" | ||
|
||
# The milestone at which this feature was, or is targeted to be, at each stage. | ||
milestone: | ||
beta: "v1.28" | ||
|
||
# The following PRR answers are required at alpha release | ||
# List the feature gate name and the components for which it must be enabled | ||
feature-gates: | ||
- name: SeparateTaintManager | ||
components: | ||
- kube-controller-manager | ||
disable-supported: true | ||
|
||
# The following PRR answers are required at beta release | ||
metrics: | ||
- TBD |
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.