fix(controller): cancel in-flight spark-submit on controller shutdown by a7i · Pull Request #2934 · kubeflow/spark-operator

a7i · 2026-05-08T01:37:09Z

Purpose of this PR

Reduce duplicate driver submissions when the controller restarts mid-submit (refs #2788). Complementary to adoption in #2932: adoption recovers races this cannot avoid; this PR narrows the window by making in-flight spark-submit cancellable on shutdown.

On SIGTERM, controller-runtime cancels the reconcile context immediately. The post-submit status write uses that same context, so the real lever is interrupting the spark-submit JVM before it creates a driver pod the controller will never record. Previously runSparkSubmit used exec.Command without context propagation.

Proposed changes:

SparkSubmitter.runSparkSubmit: use exec.CommandContext, send SIGTERM on cancellation (not SIGKILL), and wait up to ShutdownGracePeriod before force-killing the child.
Reuse --driver-pod-creation-grace-period for SparkSubmitter.ShutdownGracePeriod (WaitDelay after SIGTERM). Leave Manager.GracefulShutdownTimeout at controller-runtime's 30s default (not overridden). No separate --graceful-shutdown-timeout flag.
Helm: controller.terminationGracePeriodSeconds (default 60), must exceed the 30s manager shutdown window.
Startup validation: reject driver-pod-creation-grace-period >= 30s so WaitDelay stays below GracefulShutdownTimeout.
Unit tests for runSparkSubmit (SIGTERM on cancel, WaitDelay before force-kill) and chart deployment rendering.

How the defaults were chosen

Setting	Default	Role
`driverPodCreationGracePeriod`	`10s`	Retry "driver pod not found" after submit; `cmd.WaitDelay` after SIGTERM to spark-submit
`GracefulShutdownTimeout`	`30s` (controller-runtime default)	Manager cleanup window after SIGTERM; unchanged from today
`terminationGracePeriodSeconds`	`60`	Kubelet ceiling; must stay above GracefulShutdownTimeout so the manager can reap in-flight JVMs before SIGKILL

Ordering: driverPodCreationGracePeriod < GracefulShutdownTimeout (30s) < terminationGracePeriodSeconds.

Typical spark-submit in kubernetes mode with submission-wait-app-completion=false takes ~5-10s; worst case on a busy cluster ~15-20s. Tune driverPodCreationGracePeriod up if your cluster needs more headroom (must stay below 30s).

Change Category

Bugfix (non-breaking change which fixes an issue)
Feature (non-breaking change which adds functionality)
Breaking change (fix or feature that could affect existing functionality)
Documentation update

Rationale

Refs #2788. Complementary to #2932.

Checklist

I have conducted a self-review of my own code.
I have updated documentation accordingly.
I have added tests that prove my changes are effective or that my feature works.
Existing unit tests pass locally with my changes.

Additional Notes

Locally validated:

go test ./internal/controller/sparkapplication/ -run TestRunSparkSubmit -timeout 120s
helm unittest charts/spark-operator-chart -f 'tests/controller/deployment_test.yaml'

google-oss-prow · 2026-05-08T01:37:15Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign chenyi015 for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

github-actions · 2026-05-08T01:37:18Z

🎉 Welcome to the Kubeflow Spark Operator! 🎉

Thanks for opening your first PR! We're happy to have you as part of our community 🚀

Here's what happens next:

If you haven't already, please check out our Contributing Guide for repo-specific guidelines and the Kubeflow Contributor Guide for general community standards.
Our team will review your PR soon!

Join the community:

Slack: Join our #kubeflow-spark-on-kubernetes Slack channel.

Feel free to ask questions in the comments if you need any help or clarification!
Thanks again for contributing to Kubeflow! 🙏

Copilot

Pull request overview

This PR reduces the likelihood of losing the post-spark-submit status update during controller shutdown/restarts by (1) increasing the controller-runtime graceful shutdown window and (2) propagating reconcile cancellation into the spark-submit subprocess.

Changes:

Add a --graceful-shutdown-timeout CLI flag (default 60s) and wire it to controller-runtime GracefulShutdownTimeout.
Make spark-submit execution cancellable by using exec.CommandContext(ctx, ...) and threading the reconcile context into submission.
Extend the Helm chart to configure controller.gracefulShutdownTimeout (default 60s) and controller.terminationGracePeriodSeconds (default 90), with unit tests and README updates.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
internal/controller/sparkapplication/submission.go	Pass reconcile context into `spark-submit` via `exec.CommandContext` to support cancellation.
cmd/operator/controller/start.go	Add `--graceful-shutdown-timeout` flag and configure controller-runtime manager shutdown timeout.
charts/spark-operator-chart/values.yaml	Introduce default values for graceful shutdown timeout and pod termination grace period.
charts/spark-operator-chart/templates/controller/deployment.yaml	Render the new controller arg and set `terminationGracePeriodSeconds` on the controller Pod spec.
charts/spark-operator-chart/tests/controller/deployment_test.yaml	Add Helm unit tests for the new arg and pod termination grace period settings.
charts/spark-operator-chart/README.md	Document the new Helm values.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

a7i · 2026-05-13T12:22:37Z

+	// cancellation) terminates the child process instead of letting it run to completion
+	// and create a driver pod whose status update will never be persisted.


Reworded in b41b544 to drop the implication that cancellation prevents a driver pod from being created; the comment now says cancellation only narrows the window for losing the post-submit status update.

tariq-hasan

Hi @a7i! I have left a few questions. Thanks!

/cc @nabuskey @ChenYi015

tariq-hasan · 2026-05-13T09:35:19Z

+  # has time to drain in-flight reconciles, including any that are still writing status
+  # after a successful spark-submit.
+  terminationGracePeriodSeconds: 90
+


I am wondering if in-flight reconciles can really finish if a termination signal is already propagated down the controller-runtime chain to the reconcile loop before it hits the spark-submit.

My understanding is that the real crux of the code changes here is that JVM processes are being terminated or killed off through signal propagation so we have a better chance of avoiding driver pod creation altogether. Reconciles seem to be cancelled immediately and are simply given a grace period for cleanup rather than being allowed to finish.

Good point, you are right. The controller-runtime cancels the reconcile context as soon as SIGTERM arrives, and updateSparkApplicationStatus uses that same context, so the window does not let a normal reconcile finish writing status. The real value is interrupting the in-flight spark-submit JVM before it creates a driver pod the controller will never get to record. I reworded both gracefulShutdownTimeout and terminationGracePeriodSeconds in b41b544 to reflect that (cleanup/reap window, not "let reconciles finish"). Regenerated the chart README as well.

a7i · 2026-06-04T14:22:08Z

Production context

We hit the same failure mode in production on 2026-06-03 (details in #2788). Two SparkApplication resources were created while the controller was rolling to a new version; both followed the same sequence.

How this PR helps: SIGTERM during rollout interrupted an in-flight submit after the driver pod often already existed. Making spark-submit cancellable and widening the shutdown window should reduce orphan submissions and lost status writes on normal shutdown. That aligns with the clarified intent in review (cleanup/reap, not letting reconciles run to completion).

What still needs #2932: In our trace the driver was already created before cancel, and the next leader logged driver pod already exist and marked the CR FAILED. Adoption/idempotent submit (#2932) is what recovers that case; this PR and #2932 are complementary.

We are also planning maxUnavailable: 0 / maxSurge: 1 on the controller Deployment and chart wiring for --graceful-shutdown-timeout / terminationGracePeriodSeconds once this lands.

a7i · 2026-06-17T15:10:02Z

Hi @tariq-hasan @nabuskey @ChenYi015 - this is a pretty low risk PR, would you consider reviewing this?

We continue to have issues in production when the operator pods are rotated or we do a new operator rollout. This causes pipelines that are mid-flight (in particular driver pod creation via spark-submit) to be cancelled.

a7i · 2026-06-17T15:11:57Z

If you prefer, I can remove the graceful-shutdown-timeout" flag and instead ensure that the context is not cancelled with the --driver-pod-creation-grace-period as the wait delay

a7i · 2026-06-25T15:17:43Z

If you prefer, I can remove the graceful-shutdown-timeout" flag and instead ensure that the context is not cancelled with the --driver-pod-creation-grace-period as the wait delay

ended up doing this to reduce the changes and flags

nabuskey · 2026-06-25T16:07:58Z

 		LeaseDuration:           &leaderElectionLeaseDuration,
 		RenewDeadline:           &leaderElectionRenewDeadline,
 		RetryPeriod:             &leaderElectionRetryPeriod,
+		GracefulShutdownTimeout: &driverPodCreationGracePeriod,


This changes the current (implicit) behaviour of GracefulShutdownTimeout. It currently sets to 30 seconds viasetOptionsDefaults in controller manager.

So creation grace period setting is applied to all controller runtime managed context.

Is this needed? As far as I can tell, it should be: driverPodCreationGracePeriod < GracefulShutdownTimeout < terminationGracePeriodSeconds

If this must be in exactly this order, we could either derive it from a value or validate it at startup to protect end users.

yeah initially I added a new flag called gracefulShutdownTimeout but removed it thinking that it would simplify the implementation. let me take another look

Addressed in 8a7a6a2: GracefulShutdownTimeout is now driverPodCreationGracePeriod + 1s, so the ordering is driverPodCreationGracePeriod < GracefulShutdownTimeout < terminationGracePeriodSeconds with defaults 10s / 11s / 60s. Chart values and flag help text document the constraint.

Correction in 22469f5: we no longer override GracefulShutdownTimeout. It stays at controller-runtime's 30s default; driverPodCreationGracePeriod only sets runSparkSubmit WaitDelay. Startup rejects grace period >= 30s to preserve ordering.

nabuskey · 2026-06-25T16:13:35Z

+	cmd.Cancel = func() error {
+		return cmd.Process.Signal(syscall.SIGTERM)
+	}
+	cmd.WaitDelay = s.ShutdownGracePeriod


Is this a race condition? We wait for spark-submit command to exit and ShutdownGracePeriod for exactly the same time. So there's no guarantee that one happens before the other?

Good catch. WaitDelay and GracefulShutdownTimeout were both 10s before, so the manager could exit while runSparkSubmit was still in its WaitDelay window. GracefulShutdownTimeout is now driverPodCreationGracePeriod + 1s, and a test asserts we wait the full ShutdownGracePeriod before force-kill when SIGTERM is ignored.

When the controller leader receives SIGTERM during spark-submit, the reconcile context is cancelled immediately and the JVM subprocess may keep running until the process exits or the kubelet sends SIGKILL. That can create a driver pod the controller never records, contributing to duplicate submissions on restart. - Add SparkSubmitter.runSparkSubmit using exec.CommandContext; send SIGTERM on cancellation and wait up to ShutdownGracePeriod before force-killing. - Reuse --driver-pod-creation-grace-period for ShutdownGracePeriod and Manager.GracefulShutdownTimeout (no separate graceful-shutdown flag). - Helm: controller pod terminationGracePeriodSeconds default 60. - Add runSparkSubmit unit tests and chart deployment tests. Refs kubeflow#2788 Signed-off-by: Amir Alavi <amiralavi7@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>

Set GracefulShutdownTimeout to driverPodCreationGracePeriod plus one second so the manager outlives runSparkSubmit's WaitDelay window. Document the ordering constraint and add a test for force-kill after WaitDelay. Co-authored-by: Cursor <cursoragent@cursor.com>

Do not override GracefulShutdownTimeout with driverPodCreationGracePeriod. The manager default stays 30s; driverPodCreationGracePeriod only caps runSparkSubmit WaitDelay. Reject grace periods at or above 30s at startup. Co-authored-by: Cursor <cursoragent@cursor.com>

google-oss-prow Bot added the do-not-merge/work-in-progress label May 8, 2026

google-oss-prow Bot requested review from ImpSy and nabuskey May 8, 2026 01:37

google-oss-prow Bot added the size/M label May 8, 2026

a7i force-pushed the fix/controller-shutdown-grace-window branch from ae91caf to 71f74fc Compare May 8, 2026 01:45

a7i marked this pull request as ready for review May 8, 2026 01:50

Copilot AI review requested due to automatic review settings May 8, 2026 01:50

google-oss-prow Bot removed the do-not-merge/work-in-progress label May 8, 2026

Copilot started reviewing on behalf of a7i May 8, 2026 01:50 View session

Copilot AI reviewed May 8, 2026

View reviewed changes

a7i mentioned this pull request May 12, 2026

fix(sparkapplication): backfill terminationTime when missing so TTL applies #2926

Closed

8 tasks

tariq-hasan reviewed May 13, 2026

View reviewed changes

google-oss-prow Bot requested a review from ChenYi015 May 13, 2026 11:37

google-oss-prow Bot added size/L and removed size/M labels May 13, 2026

a7i mentioned this pull request Jun 4, 2026

Restart of Spark Operator may Result in Duplicated Submission #2788

Open

1 task

a7i force-pushed the fix/controller-shutdown-grace-window branch from b41b544 to 5524fb2 Compare June 25, 2026 13:36

a7i changed the title ~~fix(controller): widen shutdown grace window for in-flight submissions~~ fix(controller): cancel in-flight spark-submit on controller shutdown Jun 25, 2026

a7i force-pushed the fix/controller-shutdown-grace-window branch from 5524fb2 to 4cd9268 Compare June 25, 2026 15:24

nabuskey reviewed Jun 25, 2026

View reviewed changes

a7i and others added 2 commits July 14, 2026 08:45

a7i force-pushed the fix/controller-shutdown-grace-window branch from 4cd9268 to 8a7a6a2 Compare July 14, 2026 12:47

		// cancellation) terminates the child process instead of letting it run to completion
		// and create a driver pod whose status update will never be persisted.

Uh oh!

Conversation

a7i commented May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose of this PR

How the defaults were chosen

Change Category

Rationale

Checklist

Additional Notes

Uh oh!

google-oss-prow Bot commented May 8, 2026

Uh oh!

github-actions Bot commented May 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

a7i May 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tariq-hasan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tariq-hasan May 13, 2026

Choose a reason for hiding this comment

Uh oh!

a7i May 13, 2026

Choose a reason for hiding this comment

Uh oh!

a7i commented Jun 4, 2026

Production context

Uh oh!

a7i commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

a7i commented Jun 17, 2026

Uh oh!

a7i commented Jun 25, 2026

Uh oh!

nabuskey Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

a7i Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

a7i Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

a7i Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

nabuskey Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

a7i Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

a7i commented May 8, 2026 •

edited

Loading

a7i commented Jun 17, 2026 •

edited

Loading

nabuskey Jun 25, 2026 •

edited

Loading

nabuskey Jun 25, 2026 •

edited

Loading