Skip to content

fix(spark): add default service account fallback for Spark Connect - #672

Open
amh1k wants to merge 4 commits into
kubeflow:mainfrom
amh1k:fix/spark-connect-default-sa-fallback
Open

fix(spark): add default service account fallback for Spark Connect#672
amh1k wants to merge 4 commits into
kubeflow:mainfrom
amh1k:fix/spark-connect-default-sa-fallback

Conversation

@amh1k

@amh1k amh1k commented Jul 27, 2026

Copy link
Copy Markdown

What this PR does / why we need it:

This change introduces a fallback to the spark-operator-spark default service account when driver.service_account is not explicitly specified in the Spark Connect API.

This simplifies basic SDK use cases on the default operator installation, where manual specification of a service account on the API becomes cumbersome for end users.

Note: This is a temporary workaround and is meant to be removed once #617 lands.

Which issue(s) this PR fixes (optional, in Fixes #<issue number>, #<issue number>, ... format, will close the issue(s) when PR gets merged):

Fixes #668

Checklist:

  • Ran make verify locally to ensure code quality, formatting compliance, and no linting errors.
  • Ran make test-python locally to ensure unit tests pass.
  • Docs included if any changes are user facing (Not applicable for this internal fallback logic).

Copilot AI review requested due to automatic review settings July 27, 2026 08:51

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 was unable to review this pull request because the user who requested the review has reached their quota limit.

@google-oss-prow

Copy link
Copy Markdown
Contributor

[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 electronic-waste 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:

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

@github-actions

Copy link
Copy Markdown
Contributor

🎉 Welcome to the Kubeflow SDK! 🎉

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! cc @kubeflow/kubeflow-sdk-team

Join the community:

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

@tariq-hasan

Copy link
Copy Markdown
Member

/ok-to-test
/retest

@tariq-hasan tariq-hasan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi @amh1k! Thanks for raising the PR. I have added a few comments.

Please also sign the commits to pass the DCO check.

Comment thread kubeflow/spark/backends/kubernetes/utils.py Outdated
memory=memory,
template=template,
)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please add a pytest parametrized test in utils_test.py to cover all test cases for get_spark_connect_driver_spec.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Working on adding the test

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

have added the test

@amh1k
amh1k force-pushed the fix/spark-connect-default-sa-fallback branch from 15bd5ef to 8388173 Compare July 28, 2026 15:29
@google-oss-prow google-oss-prow Bot added size/M and removed size/S labels Jul 28, 2026
containers=[],
service_account_name=driver.service_account,
)
# Fallback to the the spark-operator chart's default service account if not specified.

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.

Typo in the new comment - "the the spark-operator chart's..." should just be "the spark-operator chart's..."

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done

"expected_cores": 2,
"expected_memory": "4g",
"expected_service_account": "custom-spark-sa",
},

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.

Would you mind adding one more case where service_account is set but resources is left default?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done

@Goku2099

Copy link
Copy Markdown
Contributor

@amh1k Can you please sign your commit to get the DCO check to pass?

amh1k and others added 4 commits July 28, 2026 23:33
This change introduces a fallback to the `spark-operator-spark` default
service account when `driver.service_account` is not explicitly specified
in the Spark Connect API.

Why this is needed:
This simplifies simple SDK use cases on the default operator installation,
where manual specification of a service account on the API becomes cumbersome.

Note: This is a temporary workaround and includes a TODO comment to ensure
it is removed once issue kubeflow#617 is fully resolved.

Signed-off-by: amh1k <abdulmoizx97@gmail.com>
Co-authored-by: Tariq Hasan <mmtariquehsn@gmail.com>
Signed-off-by: Abdul Moiz Hussain <abdulmoizx97@gmail.com>
Signed-off-by: amh1k <abdulmoizx97@gmail.com>
Signed-off-by: amh1k <abdulmoizx97@gmail.com>
@amh1k
amh1k force-pushed the fix/spark-connect-default-sa-fallback branch from af52c17 to 7ea0e33 Compare July 28, 2026 18:35
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.

KEP-107: Default service account for SparkConnect API

4 participants