Skip to content

[eno] Fast track cancellation for out of step synthesis pods#618

Open
ruinan-liu wants to merge 6 commits into
mainfrom
users/ruinanliu/eno-executor-check
Open

[eno] Fast track cancellation for out of step synthesis pods#618
ruinan-liu wants to merge 6 commits into
mainfrom
users/ruinanliu/eno-executor-check

Conversation

@ruinan-liu

Copy link
Copy Markdown
Collaborator

Instead of waiting for 2 minutes straight, we want to fast track cancellation for out of lock synthesis pods.

NotReadyStatus = "NotReady"
ReconcilingStatus = "Reconciling"
LogSampleCap = 50
synthesisIDLabelKey = "eno.azure.io/synthesis-uuid"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: This duplicates the unexported synthesisIDLabelKey in internal/controllers/synthesis/pod.go. Not a blocker but consider to keep a single source of truth.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, thanks for pointing this out.

Comment thread internal/controllers/composition/controller.go
if pod.DeletionTimestamp != nil {
continue
}
if pod.Status.Phase == corev1.PodSucceeded || pod.Status.Phase == corev1.PodFailed {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Failed pod will be restarted by k8s given that the RestartPolicy is OnFailure. Are we sure that we'll cancel it?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ruinan-liu left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replied to comments

return ctrl.Result{}, nil
}

func (c *compositionController) terminalInFlightPod(ctx context.Context, comp *apiv1.Composition) (*corev1.Pod, bool, error) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename to succeededInfFlightPod?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, thanks

@ruinan-liu ruinan-liu left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reply to comment

@ruinan-liu ruinan-liu requested review from tilnl and xiazhan July 2, 2026 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants