[eno] Fast track cancellation for out of step synthesis pods#618
[eno] Fast track cancellation for out of step synthesis pods#618ruinan-liu wants to merge 6 commits into
Conversation
| NotReadyStatus = "NotReady" | ||
| ReconcilingStatus = "Reconciling" | ||
| LogSampleCap = 50 | ||
| synthesisIDLabelKey = "eno.azure.io/synthesis-uuid" |
There was a problem hiding this comment.
Nit: This duplicates the unexported synthesisIDLabelKey in internal/controllers/synthesis/pod.go. Not a blocker but consider to keep a single source of truth.
There was a problem hiding this comment.
Done, thanks for pointing this out.
| if pod.DeletionTimestamp != nil { | ||
| continue | ||
| } | ||
| if pod.Status.Phase == corev1.PodSucceeded || pod.Status.Phase == corev1.PodFailed { |
There was a problem hiding this comment.
Failed pod will be restarted by k8s given that the RestartPolicy is OnFailure. Are we sure that we'll cancel it?
There was a problem hiding this comment.
Dropped, the podFailed case. The case where it does nothing will output pod succeeded
if reason, skip := skipSynthesis(comp, syn, env); skip {
logger.Info("synthesis is no longer relevant - skipping", "reason", reason)
return nil
}
ruinan-liu
left a comment
There was a problem hiding this comment.
Replied to comments
| return ctrl.Result{}, nil | ||
| } | ||
|
|
||
| func (c *compositionController) terminalInFlightPod(ctx context.Context, comp *apiv1.Composition) (*corev1.Pod, bool, error) { |
There was a problem hiding this comment.
rename to succeededInfFlightPod?
ruinan-liu
left a comment
There was a problem hiding this comment.
Reply to comment
Instead of waiting for 2 minutes straight, we want to fast track cancellation for out of lock synthesis pods.