From 79cb889d4e6e97bc674749feefb55cab22f402ef Mon Sep 17 00:00:00 2001
From: hantmac <hantmac@outlook.com>
Date: Sat, 6 Jul 2024 10:01:34 +0800
Subject: [PATCH] complete the proposal

---
 ...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
+