From 16eaf6c47c2b4651a0a55b76043aa9241c5c6bdc Mon Sep 17 00:00:00 2001 From: hantmac Date: Sat, 6 Jul 2024 10:01:34 +0800 Subject: [PATCH] complete the proposal Signed-off-by: hantmac --- ...loneset-support-progressDeadlineSeconds.md | 49 ++++++++++++++++--- 1 file changed, 41 insertions(+), 8 deletions(-) diff --git a/docs/proposals/20240309-cloneset-support-progressDeadlineSeconds.md b/docs/proposals/20240309-cloneset-support-progressDeadlineSeconds.md index 2d8e068f9d..c13fbb51b1 100644 --- a/docs/proposals/20240309-cloneset-support-progressDeadlineSeconds.md +++ b/docs/proposals/20240309-cloneset-support-progressDeadlineSeconds.md @@ -41,6 +41,7 @@ Firstly, add the `progressDeadlineSeconds` field to the CloneSetSpec. Then add the handle logic in cloneSet controller to handle the `progressDeadlineSeconds` field. ### 1. add .spec.progressDeadlineSeconds field +The default value of `progressDeadlineSeconds` is 600 seconds according to the [official document](https://kubernetes.io/docs/concepts/workloads/controllers/deployment/#progress-deadline-seconds). ```yaml apiVersion: apps.kruise.io/v1alpha1 kind: CloneSet @@ -68,19 +69,28 @@ Here are two possible interpretations of `progressDeadlineSeconds` in the contex 1. `progressDeadlineSeconds` could be redefined as the time taken for the CloneSet to reach completion or the "paused" state due to partition. In this case, the time during which the CloneSet is paused would be included in the progress deadline. 2. Secondly, `progressDeadlineSeconds` could only be supported if the partition is not set. This means that if a partition is set, the `progressDeadlineSeconds` field would not be applicable or has no effect. -These interpretations are open for discussion and we welcome feedback from the community to make a decision. +After the discussion of the community, we choose the first interpretation.So we should re-set the progressDeadlineSeconds when the CloneSet reach completion OR "the paused state". ### 3. handle the logic -In cloneset controller, we should add the logic to handle the `progressDeadlineSeconds` field. +In cloneset controller, we should add the logic to handle the `progressDeadlineSeconds` field. We firstly add a timer to check the progress of the CloneSet. +If the progress exceeds the `progressDeadlineSeconds`, we should add a CloneSetCondition to the CloneSet's `.status.conditions`: ```go -func (c *CloneSetController) syncCloneSetStatus(cloneSet *appsv1alpha1.CloneSet, newStatus *appsv1alpha1.CloneSetStatus) error { - ... - if cloneSet.Spec.ProgressDeadlineSeconds != nil { - // handle the logic +// add a timer to check the progress of the CloneSet +if cloneSet.Spec.ProgressDeadlineSeconds != nil { + // handle the logic + starttime := time.Now() + ... + if time.Now().After(starttime.Add(time.Duration(*cloneSet.Spec.ProgressDeadlineSeconds) * time.Second)) { + newStatus.Conditions = append(newStatus.Conditions, appsv1alpha1.CloneSetCondition{ + Type: appsv1alpha1.CloneSetProgressing, + Status: corev1.ConditionFalse, + Reason: appsv1alpha1.CloneSetProgressDeadlineExceeded, + }) } - ... } ``` + +When the CloneSet reaches the "paused" state, we should reset the timer to avoid the progress deadline being exceeded. And we check the progress of the CloneSet in the `syncCloneSetStatus` function. If the progress exceeds the `progressDeadlineSeconds`, we should add a CloneSetCondition to the CloneSet's `.status.conditions`: ```go @@ -95,7 +105,7 @@ func (c *CloneSetController) syncCloneSetStatus(cloneSet *appsv1alpha1.CloneSet, ... if cloneSet.Spec.ProgressDeadlineSeconds != nil { // handle the logic - if time.Now().After(cloneSet.Status.UpdatedTime.Add(time.Duration(*cloneSet.Spec.ProgressDeadlineSeconds) * time.Second)) { + if time.Now().After(starttime.Add(time.Duration(*cloneSet.Spec.ProgressDeadlineSeconds) * time.Second)) { newStatus.Conditions = append(newStatus.Conditions, appsv1alpha1.CloneSetCondition{ Type: appsv1alpha1.CloneSetProgressing, Status: corev1.ConditionFalse, @@ -107,4 +117,27 @@ func (c *CloneSetController) syncCloneSetStatus(cloneSet *appsv1alpha1.CloneSet, } ``` +When the CloneSet reaches the "paused" state, we should reset the timer to avoid the progress deadline being exceeded. +```go +func (c *CloneSetController) syncCloneSetStatus(cloneSet *appsv1alpha1.CloneSet, newStatus *appsv1alpha1.CloneSetStatus) error { + ... + // reset the starttime when the CloneSet reaches the "paused" state or complete state + if cloneSet.Status.UpdatedReadyReplicas == cloneSet.Status.Replicas { + starttime = time.Now() + } + + if cloneSet.Spec.Paused { + starttime = time.Now() + } + ... + } + + ... +} +``` + +## Implementation History + +- [ ] 06/07/2024: Proposal submission +