Skip to content

Commit

Permalink
adress review remarks
Browse files Browse the repository at this point in the history
  • Loading branch information
atiratree committed Jan 14, 2025
1 parent 7772213 commit 5635ffb
Showing 1 changed file with 11 additions and 14 deletions.
25 changes: 11 additions & 14 deletions keps/sig-apps/4563-eviction-request-api/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -185,24 +185,20 @@ The major issues are:
immediately. This can result in descheduling of incorrect set of pods. This is outlined in the
decheduler [KEP-1397](https://github.com/kubernetes-sigs/descheduler/blob/master/keps/1397-evictions-in-background/README.md)
and [kubernetes-sigs/descheduler#1466](https://github.com/kubernetes-sigs/descheduler/pull/1466).
5. Graceful deletion of DaemonSet pods is currently only supported as part of (Linux) graceful node
5. Kubernetes does not offer resource reservation during a pod migration. We would like to make sure
that we have guaranteed resources for the workload before terminating the pod. This is discussed
in the [kubernetes/kubernetes#129038](https://github.com/kubernetes/kubernetes/issues/129038).
6. Graceful deletion of DaemonSet pods is currently only supported as part of (Linux) graceful node
shutdown. The length of the shutdown is again not application specific and is set cluster-wide
(optionally by priority) by the cluster admin. This does not take into account
`.spec.terminationGracePeriodSeconds` of each pod and may cause premature termination of
the application. This has been discussed in issue [kubernetes/kubernetes#75482](https://github.com/kubernetes/kubernetes/issues/75482)
and in issue [kubernetes-sigs/cluster-api#6158](https://github.com/kubernetes-sigs/cluster-api/issues/6158).
6. Different pod termination mechanisms are not synchronized with each other. So for example, the
7. Different pod termination mechanisms are not synchronized with each other. So for example, the
taint manager may prematurely terminate pods that are currently under Node Graceful Shutdown.
This can also happen with other mechanism (e.g., different types of evictions). This has been
discussed in the issue [kubernetes/kubernetes#124448](https://github.com/kubernetes/kubernetes/issues/124448)
and in the issue [kubernetes/kubernetes#72129](https://github.com/kubernetes/kubernetes/issues/72129).
7. [Affinity Based Eviction](https://github.com/kubernetes/enhancements/issues/4328) is an upcoming
feature that would like to introduce the `RequiredDuringSchedulingRequiredDuringExecution`
nodeAffinity option to remove pods from nodes that do not match this affinity. The controller
proposed by this feature would like to use the EvictionRequest API for the disruption safety and
observability reasons. It can also leave the eviction logic and reconciliation to the eviction
request controller and reducing the maintenance burden. Discussed in the KEP
[kubernetes/enhancements#4329](https://github.com/kubernetes/enhancements/pull/4329).

This KEP is a prerequisite for the [Declarative Node Maintenance KEP](https://github.com/kubernetes/enhancements/pull/4213),
which describes other issues and consequences that would be solved by the EvictionRequest API.
Expand All @@ -223,7 +219,7 @@ the application supports this.

### PodDisruptionBudget Standing Issues

For completeness here is a complete list of upstream open PDB issues. Most are relevant to this KEP.
For completeness here is a complete list of open PDB issues. Most are relevant to this KEP.

- [Mandatorliy specify how the application handle disruptions in the pod spec.](https://github.com/kubernetes/kubernetes/issues/124390)
- [Treat liveness probe-based restarts as voluntary disruptions gated by PodDisruptionBudgets](https://github.com/kubernetes/kubernetes/issues/123204)
Expand All @@ -240,6 +236,7 @@ For completeness here is a complete list of upstream open PDB issues. Most are r
- [New FailedEviction PodDisruptionCondition](https://github.com/kubernetes/kubernetes/issues/128815)
- [Distinguish PDB error separately in eviction API](https://github.com/kubernetes/kubernetes/issues/125500)
- [Confusing use of TooManyRequests error for eviction](https://github.com/kubernetes/kubernetes/issues/106286)
- [New EvictionBlocked PodDisruptionCondition](https://github.com/kubernetes/kubernetes/issues/128815)

### Goals

Expand Down Expand Up @@ -1780,10 +1777,10 @@ also not possible to change the default eviction behavior for safety reasons. So
would not be sufficient, as we need to solve this for all pods.

We could track these requests in the PDB object itself. A PDB can track multiple pods with its
selector. The pods do not have to be from the same application, even though they usually are. These
pods may run in various locations and may have various reasons for their disruption. It would be
difficult to synchronize the PDB object for applications with a large number of pods. This could
result in inconsistent updates and conflicts as many components would race for the PDB updates.
selector. These pods may run in various locations and may have various reasons for their disruption.
It would be difficult to synchronize the PDB object for applications with a large number of pods.
This could result in inconsistent updates and conflicts as many components would race for the PDB
updates.

The updates to the PDB could be done solely by the eviction endpoint, but this is disqualified
because many components override this endpoint with validating admission webhooks. And so we would
Expand Down

0 comments on commit 5635ffb

Please sign in to comment.