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

[a7i]

Purpose of this PR

Terminal SparkApplication resources (FAILED or COMPLETED) sometimes have status.terminationTime unset. Expiry and TTL deletion are based on terminationTime, so timeToLiveSeconds never applies and the CR (and related resources) are never garbage-collected.

This change backfills terminationTime when the app is already in a terminal state but the field is missing, preferring lastSubmissionAttemptTime (appropriate for submission-time failures).

Proposed changes:

• Add EnsureTerminationTimeForTerminalState in pkg/util and call it from reconcileTerminatedSparkApplication before IsExpired / TTL handling.
• Add unit tests for the helper.

Example (redacted manifest excerpt: spec.timeToLiveSeconds and status only):

spec:
  timeToLiveSeconds: 3600
status:
  applicationState:
    errorMessage: "failed to submit spark application: failed to run spark-submit: driver pod already exist"
    state: FAILED
  lastSubmissionAttemptTime: "2026-04-26T16:40:39Z"
  submissionAttempts: 1
  terminationTime: null

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

reconcileTerminatedSparkApplication returns early when terminationTime is zero, so TTL requeue and IsExpired never run. Backfilling from lastSubmissionAttemptTime aligns the clock with when the run actually stopped for submission failures, and allows overdue TTLs to delete immediately on the next reconcile.

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

Risk: Low

pkg/util tests were run locally. Controller integration tests require envtest binaries.

[google-oss-prow]

[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 vara-bonthu 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]

🎉 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

Backfills missing status.terminationTime for terminal SparkApplication resources so TTL expiry (spec.timeToLiveSeconds) can be computed and garbage-collection can proceed as intended in the SparkApplication controller.

Changes:

• Added util.EnsureTerminationTimeForTerminalState to populate status.terminationTime when missing for terminal apps, preferring status.lastSubmissionAttemptTime.
• Invoked the helper in reconcileTerminatedSparkApplication before TTL/expiry checks.
• Added unit tests for the new helper.

Reviewed changes

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

File	Description
pkg/util/sparkapplication.go	Adds helper to backfill terminationTime for terminal apps to enable TTL expiry.
internal/controller/sparkapplication/controller.go	Calls the helper before IsExpired/TTL handling in terminated reconcile path.
pkg/util/sparkapplication_test.go	Adds tests validating helper behavior for common cases.

---

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

[tariq-hasan]

Terminal SparkApplication resources (FAILED or COMPLETED) sometimes have status.terminationTime unset.

Thanks for raising the PR. I am wondering if this holds for the recent versions of the operator. If you trace the transitions from parent states to FAILED or COMPLETED you'll notice that the termination time gets set as soon as it becomes obvious that the app is moving to a terminal state. This basically happens either explicitly for the FailedSubmission reconciliation loop or when updating the driver state (which covers all other parent states).

Would you be able to confirm more details as to why you see this issue on your end? I believe we can have this code change more so as a defensive posture rather than framing it as a bug fix.

[a7i]

@tariq-hasan we continue to face this issue, but as you suggested, perhaps it's best to fix the root cause. We have this issue on both spark-operator v1 and v2. It seems to happen when the operator restarts and there is a pipeline with completed/succeeded driver pods.

apiVersion: sparkoperator.k8s.io/v1beta2
kind: SparkApplication
metadata:
  creationTimestamp: "2026-05-01T21:37:52Z"
spec:
  ....
  restartPolicy:
    type: Never
  timeToLiveSeconds: 3600
  type: Scala
  volumes:
  ...
status:
  applicationState:
    errorMessage: 'failed to submit spark application: failed to run spark-submit:
      driver pod already exist'
    state: FAILED
  driverInfo:
    podName: mdycwe5qc7rffuil4sx0tlocy5dueouj-driver
  lastSubmissionAttemptTime: "2026-05-01T21:38:14Z"
  submissionAttempts: 1
  submissionID: e6077d33-c1de-4bdc-baf6-9acfa8a67696
  terminationTime: null

[a7i]

@tariq-hasan tried to take a stab at fixing the root issue here, look forward to your feedback #2934