Skip to content

Set priorityClassName for Spark tasks#7476

Merged
davidmirror-ops merged 1 commit into
flyteorg:masterfrom
madiyar-wayve:madiyar/spark-priority-class
Jun 5, 2026
Merged

Set priorityClassName for Spark tasks#7476
davidmirror-ops merged 1 commit into
flyteorg:masterfrom
madiyar-wayve:madiyar/spark-priority-class

Conversation

@madiyar-wayve

Copy link
Copy Markdown

Tracking issue

Related to #6348.

Why are the changes needed?

These changes will allow setting priorityClassName for Spark tasks.

What changes were proposed in this pull request?

The Spark K8s plugin now propagates the priorityClassName from the resolved pod spec onto both the driver and executor specs of the generated SparkApplication:

  • In createDriverSpec, set spec.sparkSpec.PriorityClassName from podSpec.PriorityClassName.
  • In createExecutorSpec, set spec.sparkSpec.PriorityClassName from podSpec.PriorityClassName.

The value is set via the existing strPtr helper, which returns nil for an empty string. This means that when no priority class is configured, the field is left unset rather than being populated with an empty string, preserving the previous behavior in that case.

The podSpec.PriorityClassName value honors anything coming from the task's pod template / overlay pod spec, since it is read after MergeOverlayPodSpecOntoBase is applied.

How was this patch tested?

Added unit tests in spark_test.go:

  • TestBuildResourcePriorityClassName: sets priorityClassName on the pod spec and asserts that both the driver and executor specs of the resulting SparkApplication have the expected PriorityClassName.
  • TestBuildResourceNoPriorityClassName: verifies that when no priority class is set, both the driver and executor PriorityClassName fields are left nil (unset) rather than an empty string.

All new and existing Spark plugin unit tests pass.

Labels

  • added

Setup process

N/A

Screenshots

N/A

Check all the applicable boxes

  • I updated the documentation accordingly.
  • All new and existing tests passed.
  • All commits are signed-off.

Related PRs

N/A

Stack

Docs link

N/A

Copilot AI review requested due to automatic review settings June 5, 2026 10:02

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

@github-actions github-actions Bot added the flyte2 label Jun 5, 2026
@madiyar-wayve madiyar-wayve changed the base branch from main to master June 5, 2026 10:09
Signed-off-by: madiyar-wayve <madiyar.aitzhanov@wayve.ai>
@madiyar-wayve madiyar-wayve force-pushed the madiyar/spark-priority-class branch from 929a701 to 3bc838d Compare June 5, 2026 10:11
@github-actions github-actions Bot added the flyte label Jun 5, 2026
@madiyar-wayve madiyar-wayve changed the title Set spark priorityClassName Set priorityClassName for Spark tasks Jun 5, 2026
@madiyar-wayve madiyar-wayve marked this pull request as ready for review June 5, 2026 10:14
@madiyar-wayve

madiyar-wayve commented Jun 5, 2026

Copy link
Copy Markdown
Author

I don't know what happened, but I didn't want to close this PR

@madiyar-wayve madiyar-wayve reopened this Jun 5, 2026

@Tom-Newton Tom-Newton left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. Maybe in future we should switch to using the driver and exectutor pod templates https://github.com/kubeflow/spark-operator/blob/master/docs/api-docs.md#sparkpodspec. I think that would enable a simpler implementation where we don't need to manually add support for new properties.

@davidmirror-ops davidmirror-ops merged commit 58e97c2 into flyteorg:master Jun 5, 2026
93 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants