WIP: KEP-3973: Consider Terminating Pods in replica sets #3974
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: kannon92 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
kind/kep
k8s-ci-robot
added
the
size/XL
kannon92
changed the title
k8s-ci-robot
added
the
do-not-merge/work-in-progress
| should be approved by the remaining approvers and/or the owning SIG (or | ||
| SIG Architecture for cross-cutting KEPs). | ||
| --> | ||
| # KEP-3973: Consider Terminating Pods in Replica Set Active Count |
There was a problem hiding this comment.
| # KEP-3973: Consider Terminating Pods in Replica Set Active Count | |
| # KEP-3973: Consider Terminating Pods in Deployments |
this KEP should explore pod management holistically for deployments and transitively for replica sets to correctly account for terminating pods
| // as active. If the field is true, then the Deployment controller will include active pods | ||
| // to mean running or terminating pods | ||
| // +optional | ||
| TerminatingAsActive *bool |
There was a problem hiding this comment.
As I mentioned in a KEP earlier I do not think it is useful to have this field on a replica set: #3940 (comment) .
| // as active. If the field is true, then the Deployment controller will include active pods | ||
| // to mean running or terminating pods | ||
| // +optional | ||
| TerminatingAsActive *bool |
There was a problem hiding this comment.
can you update this to an enum to reflect discussions in #3940 ?
There was a problem hiding this comment.
Pasting some relevant discussions here for when I come back to this KEP: #3940 (comment)
| ## Summary | ||
|
|
||
| Currently, Deployments start replacement Pods as soon as previously created Pods are marked for terminating. Terminating pods are in a transitory state where they are neither active nor really fully terminated. | ||
| This KEP proposes a new field for the Deployment/ReplicaSet controllers that counts terminating |
There was a problem hiding this comment.
I think we should describe the expected behaviour at the API level here, rather than an internal implementation that is not certain at this time. At the API level there is no such concept as active replicas.
| #### Open Questions on Deployment Controller | ||
|
|
||
| The Deployment API is open for discussion. We put the field in Deployment/ReplicaSet because it is related to RolloutStrategy. | ||
| It is not clear if `recreate` and/or `rollingupdate` need this API for both rollout options. |
There was a problem hiding this comment.
as a result of #3940 (comment) we can safely say that the feature should be implemented for both of these strategies, so we can put the field into the .spec
| The Deployment API is open for discussion. We put the field in Deployment/ReplicaSet because it is related to RolloutStrategy. | ||
| It is not clear if `recreate` and/or `rollingupdate` need this API for both rollout options. | ||
|
|
||
| Another open question is if we want to include Deployments in the initial release of this feature. There is some discussion about releasing the Job API first and then follow up with Deployment. |
| - TerminatingAsActive | ||
| - ActiveUntilTerminal | ||
| - DelayPodRecreationUntilTerminal | ||
| - ? |
There was a problem hiding this comment.
choices for the field and values: #3940 (comment)
| } | ||
| ``` | ||
|
|
||
| ### Implementation |
There was a problem hiding this comment.
The implementation part will be done in a deployment and not in a replica set and is probably not feasible to write it fully here. So IMO the expected behaviours and replicasets transitiions should be discussed mainly here. And then we can decide if some important parts of the implementation should be highlighted.
|
As discussed with @kannon92, I will be taking over this KEP. /assign dejanzele |
|
@dejanzele I had a similar discussion with @kannon92 some time ago about this as well. And I have the KEP mostly written, just need to finish it.. |
|
@atiratree aha, ok, I am open to helping with implementation if needed |
|
@dejanzele Thanks, I will try to revisit the KEP this week. Let's then see how the KEP and the implementation phase will go. |
|
@kannon92 @dejanzele I have opened the new version of the KEP in #4357. Reviews would be appreciated. |
|
/close |
|
@kannon92: Closed this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Users can consider terminating pods in their count of active replicas when controlling rollouts.