[SPARK-37145][K8S] Add KubernetesCustom[Driver/Executor]FeatureConfigStep developer api #35345
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Yikun
force-pushed
the
SPARK-37145-alt
branch
from
d658e71
to
e6cab27
Compare
Yikun
changed the title
Yikun
changed the title
Yikun
force-pushed
the
SPARK-37145-alt
branch
from
e6cab27
to
ca0bc33
Compare
Yikun
force-pushed
the
SPARK-37145-alt
branch
from
1bece1a
to
dd4db89
Compare
attilapiros
reviewed
Jan 31, 2022
...s/core/src/main/scala/org/apache/spark/deploy/k8s/features/KubernetesFeatureConfigStep.scala
Outdated
Show resolved
Hide resolved
...s/core/src/main/scala/org/apache/spark/deploy/k8s/features/KubernetesFeatureConfigStep.scala
Outdated
Show resolved
Hide resolved
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/Config.scala
Outdated
Show resolved
Hide resolved
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/Config.scala
Outdated
Show resolved
Hide resolved
...s/core/src/main/scala/org/apache/spark/deploy/k8s/features/KubernetesFeatureConfigStep.scala
Outdated
Show resolved
Hide resolved
...s/core/src/main/scala/org/apache/spark/deploy/k8s/features/KubernetesFeatureConfigStep.scala
Outdated
Show resolved
Hide resolved
...ce-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/KubernetesUtils.scala
Outdated
Show resolved
Hide resolved
|
@attilapiros Thanks for your review, I will address soon. (later) updated. |
Yikun
force-pushed
the
SPARK-37145-alt
branch
4 times, most recently
from
551a9a2
to
d05b3d2
Compare
Yikun
force-pushed
the
SPARK-37145-alt
branch
from
d05b3d2
to
7d3dd65
Compare
Yikun
changed the title
Yikun
changed the title
attilapiros
reviewed
Feb 1, 2022
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/Config.scala
Outdated
Show resolved
Hide resolved
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/Config.scala
Outdated
Show resolved
Hide resolved
...s/core/src/main/scala/org/apache/spark/deploy/k8s/features/KubernetesFeatureConfigStep.scala
Outdated
Show resolved
Hide resolved
...s/core/src/main/scala/org/apache/spark/deploy/k8s/features/KubernetesFeatureConfigStep.scala
Outdated
Show resolved
Hide resolved
...ce-managers/kubernetes/core/src/test/scala/org/apache/spark/deploy/k8s/PodBuilderSuite.scala
Outdated
Show resolved
Hide resolved
...ce-managers/kubernetes/core/src/test/scala/org/apache/spark/deploy/k8s/PodBuilderSuite.scala
Outdated
Show resolved
Hide resolved
Yikun
force-pushed
the
SPARK-37145-alt
branch
from
55be5a7
to
a8c1db7
Compare
|
@attilapiros Thanks for your suggestion, it makes doc and test more clear and simple. Addressed. |
...ernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/KubernetesDriverBuilder.scala
Outdated
Show resolved
Hide resolved
...ernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/KubernetesDriverBuilder.scala
Outdated
Show resolved
Hide resolved
...s/core/src/main/scala/org/apache/spark/deploy/k8s/features/KubernetesFeatureConfigStep.scala
Outdated
Show resolved
Hide resolved
...ce-managers/kubernetes/core/src/test/scala/org/apache/spark/deploy/k8s/PodBuilderSuite.scala
Outdated
Show resolved
Hide resolved
...ernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/KubernetesDriverBuilder.scala
Outdated
Show resolved
Hide resolved
dongjoon-hyun
requested changes
Feb 2, 2022
There was a problem hiding this comment.
Thank you for making this improvement, @Yikun .
I made a few comments.
|
@dongjoon-hyun Thanks for your review, addressed. : ) |
Yikun
force-pushed
the
SPARK-37145-alt
branch
from
206207b
to
c57f527
Compare
Yikun
force-pushed
the
SPARK-37145-alt
branch
from
c57f527
to
c2a67b4
Compare
dongjoon-hyun
approved these changes
Feb 2, 2022
There was a problem hiding this comment.
+1, LGTM. Thank you, @Yikun and all.
Merged to master for Apache Spark 3.3.
|
@dongjoon-hyun @attilapiros Thanks for your help! |
|
Did I miss something from your comments, @attilapiros ? |
|
@dongjoon-hyun It is fine, I just planned to do another review round (the last one) and I got surprised it is already in. |
|
You can make comments still and we can handle it as follow-up PRs. Please let us know if you have any concern, @attilapiros . |
dongjoon-hyun
pushed a commit
that referenced
this pull request
Feb 16, 2022
…ecutor]FeatureConfigStep` ### What changes were proposed in this pull request? Add note for developers to show how to use `KubernetesDriverCustomFeatureConfigStep` and `KubernetesExecutorCustomFeatureConfigStep` (#35345). ### Why are the changes needed? Give an example to show how to use it. ### Does this PR introduce _any_ user-facing change? No, doc only ### How was this patch tested? ci passed Closes #35496 from Yikun/SPARK-37145-followup. Authored-by: Yikun Jiang <yikunkero@gmail.com> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
This patch adds the support for extending user feature steps with configuration by adding 2 developer api:
KubernetesDriverCustomFeatureConfigStep: to help user extend custom feature step in executor sideKubernetesExecutorCustomFeatureConfigStep: to help user extend custom feature step in driver sideBefore this patch user can only add feature step like:
class TestStep extends KubernetesFeatureConfigStep: without any kubernetes confAfter this patch user can add feature step with configuration like:
class TestStepWithDriverConf extends KubernetesDriverCustomFeatureConfigStep: only driverclass TestStepWithExecConf extends KubernetesExecutorCustomFeatureConfigStep: only executorclass TestStepWithK8SConf extends KubernetesDriverCustomFeatureConfigStep with KubernetesExecutorCustomFeatureConfigStep: both driver and executorWhy are the changes needed?
In #30206 , a developer API for custom feature steps has been added, but it didn't support initialize user feature step with kubernetes conf (like
KubernetesConf/KubernetesDriverConf/KubernetesExecutorConf).In most of scenarios, users want to make corresponding changes in their feature steps according to the configuration. Such as, the customized scheduler scenario, user wants to configure pod according to passed job configuration.
Does this PR introduce any user-facing change?
Improve the developer API for for custom feature steps.
How was this patch tested?
build/sbt -Pkubernetes -Pkubernetes-integration-tests -Dtest.exclude.tags=minikube,r "kubernetes-integration-tests/testCloses: #34924