Export mutateSparkPodOption and its implementations#2857
Export mutateSparkPodOption and its implementations#2857everpeace wants to merge 1 commit intokubeflow:masterfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
ce4933e to
811ca12
Compare
There was a problem hiding this comment.
Pull request overview
This PR moves the Spark pod mutation option type and its mutation implementations out of internal/webhook into pkg/webhook, so external integrations (e.g., Kueue) can reuse the same driver/executor pod shaping logic as the operator’s mutating webhook.
Changes:
- Added a new public
pkg/webhookpackage file that containsMutateSparkPodOptionand the Spark pod mutation functions. - Refactored
internal/webhookSpark pod defaulter to call the exported mutation functions frompkg/webhook.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/webhook/mutate_spark_option.go | Introduces exported pod mutation option type and exported mutation/helper functions for reuse by external tools. |
| internal/webhook/sparkpod_defaulter.go | Switches the internal webhook to use the exported mutation options from pkg/webhook. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
811ca12 to
8ca9f0e
Compare
… in pkg/webhook so that external tools(e.g. kueue) can re-use the mutation logic. Signed-off-by: Shingo Omura <everpeace@gmail.com>
8ca9f0e to
e28433d
Compare
kannon92
left a comment
There was a problem hiding this comment.
Since we are exporting mutate_spark_operaton, we should make sure to have unit tests.
Actually, these methods are well tested here: https://github.com/kubeflow/spark-operator/blob/master/internal/webhook/sparkpod_defaulter_test.go I will move the tests into |
Purpose of this PR
Proposed changes:
mutateSparkPodOption, which was originally defined ininternal/webhook, inpkg/webhookso that so that external tools(e.g. kueue) can re-use the mutation logicChange Category
Rationale
Spark operator integration in Kueue should calculate spark driver/executor pods' shape(resource requests/limits, volume, etc.) so that kueue correctly decide the workload should be admitted or not.
Thus,
mutateSparkPodOptionlogics are needed to re-use in Kueue integration.ref: kubernetes-sigs/kueue#7268 (comment)
Checklist
Additional Notes