Refine RetryDelaySec (#39)

Author: yqwang-ms

pkg/apis/frameworkcontroller/v1/funcs.go

@@ -542,7 +542,8 @@ func (rp RetryPolicySpec) ShouldRetry(
 	if cs.Code == CompletionCodePodSpecInvalid ||
 		cs.Code == CompletionCodeStopFrameworkRequested ||
 		cs.Code == CompletionCodeFrameworkAttemptCompletion {
-		return RetryDecision{false, true, 0, cs.Diagnostics}
+		return RetryDecision{false, true, 0, fmt.Sprintf(
+			"CompletionCode is %v, %v", cs.Code, cs.Phrase)}
 	}
 
 	// 1. FancyRetryPolicy

pkg/apis/frameworkcontroller/v1/types.go

@@ -339,8 +339,8 @@ type RetryPolicyStatus struct {
 	// Used to expose the ScheduledRetryTime after which current retry can be
 	// executed.
 	// ScheduledRetryTime = AttemptStatus.CompletionTime + RetryDelaySec
-	// It is available and meaningful if and only if current attempt is in
-	// AttemptCompleted state.
+	// It is not nil only if the retry has been scheduled but not yet executed, i.e.
+	// current attempt is in AttemptCompleted state and is not the last attempt.
 	RetryDelaySec *int64 `json:"retryDelaySec"`
 }
 

pkg/controller/controller.go

@@ -1414,10 +1414,7 @@ func (c *FrameworkController) syncTaskState(
 			apiErr := errorWrap.Cause(err)
 			if apiErrors.IsInvalid(apiErr) {
 				// Should be Framework Error instead of Platform Transient Error.
-				diag := fmt.Sprintf(
-					"Pod Spec is invalid in TaskRole [%v]: "+
-						"Triggered by Task [%v][%v]: Diagnostics: %v",
-					taskRoleName, taskRoleName, taskIndex, apiErr)
+				diag := fmt.Sprintf("%v", apiErr)
 				klog.Infof(logPfx + diag)
 
 				// Ensure pod is deleted in remote to avoid managed pod leak after
@@ -1675,16 +1672,16 @@ func (c *FrameworkController) completeTaskAttempt(
 
 		// To ensure the completed TaskAttempt is persisted before exposed,
 		// we need to wait until next sync to expose it, so manually enqueue a sync.
-		klog.Infof(logPfx + "Waiting the completed TaskAttempt to be persisted")
 		c.enqueueFrameworkSync(f, "TaskAttemptCompleted")
+		klog.Infof(logPfx + "Waiting the completed TaskAttempt to be persisted")
 	} else {
 		f.TransitionTaskState(taskRoleName, taskIndex, ci.TaskAttemptDeletionPending)
 
 		// To ensure the CompletionStatus is persisted before deleting the pod,
 		// we need to wait until next sync to delete the pod, so manually enqueue
 		// a sync.
-		klog.Infof(logPfx + "Waiting the CompletionStatus to be persisted")
 		c.enqueueFrameworkSync(f, "TaskAttemptDeletionPending")
+		klog.Infof(logPfx + "Waiting the CompletionStatus to be persisted")
 	}
 }
 
@@ -1718,6 +1715,7 @@ func (c *FrameworkController) completeFrameworkAttempt(
 					if taskStatus.State != ci.TaskAttemptCompleted {
 						c.completeTaskAttempt(f, taskRoleName, taskIndex, true, nil)
 					}
+					taskStatus.RetryPolicyStatus.RetryDelaySec = nil
 					f.TransitionTaskState(taskRoleName, taskIndex, ci.TaskCompleted)
 				}
 			}
@@ -1737,16 +1735,16 @@ func (c *FrameworkController) completeFrameworkAttempt(
 
 		// To ensure the completed FrameworkAttempt is persisted before exposed,
 		// we need to wait until next sync to expose it, so manually enqueue a sync.
-		klog.Infof(logPfx + "Waiting the completed FrameworkAttempt to be persisted")
 		c.enqueueFrameworkSync(f, "FrameworkAttemptCompleted")
+		klog.Infof(logPfx + "Waiting the completed FrameworkAttempt to be persisted")
 	} else {
 		f.TransitionFrameworkState(ci.FrameworkAttemptDeletionPending)
 
 		// To ensure the CompletionStatus is persisted before deleting the cm,
 		// we need to wait until next sync to delete the cm, so manually enqueue
 		// a sync.
-		klog.Infof(logPfx + "Waiting the CompletionStatus to be persisted")
 		c.enqueueFrameworkSync(f, "FrameworkAttemptDeletionPending")
+		klog.Infof(logPfx + "Waiting the CompletionStatus to be persisted")
 	}
 }