Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-36061][K8S] Add volcano built-in module and feature step #35495

Closed
wants to merge 1 commit into from

Conversation

Yikun
Copy link
Member

@Yikun Yikun commented Feb 12, 2022

What changes were proposed in this pull request?

This patch added volcano feature step to help user integrate spark with Volcano Scheduler.

  • Add a VolcanoFeatureStep, it can be used in driver and executor side.

After this patch, users can enable this featurestep by submiting job by using

--conf spark.kubernete.driver.scheduler.name=volcano \
--conf spark.kubernetes.driver.pod.featureSteps=org.apache.spark.deploy.k8s.features.scheduler.VolcanoFeatureStep

A PodGroup will be created before driver started, annotations will be set to driver pod to added driver pod to this pod group. Then, Volcano scheduler will help driver pod scheduling instead of deafult kubernetes scheduler.

Why are the changes needed?

This PR help user integrate Spark with Volcano Scheduler.

See also: SPARK-36057

Does this PR introduce any user-facing change?

Yes, introduced a user feature step.

These are used by VolcanoFeatureStep, and also will be used by YunikornFeatureStep in future.

How was this patch tested?

  • UT
  • Integration test: Test all VolcanoSuite (all kubernetes test with volcano + a new podgroup test) and KubernetesSuite
# Deploy Volcano (x86)
kubectl apply -f https://raw.githubusercontent.com/volcano-sh/volcano/master/installer/volcano-development.yaml
# Deploy Volcano (arm64)
kubectl apply -f https://raw.githubusercontent.com/volcano-sh/volcano/master/installer/volcano-development-arm64.yaml
# Test all VolcanoSuite (all kubernetes test with volcano + a new podgroup test) and KubernetesSuite
build/sbt -Pkubernetes -Pkubernetes-integration-tests -Dtest.exclude.tags=minikube,r "kubernetes-integration-tests/test"

@HyukjinKwon
Copy link
Member

Yeah .. I prefer this way but would be good to have some feedback from people like @dongjoon-hyun as you mentioned in another PR.

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

Sorry but -1 for this embedding this experimental dependency into K8s GA module and the default distribution.

If we add this once, we cannot remove this, @HyukjinKwon and @Yikun .

@dongjoon-hyun
Copy link
Member

This feature should be marked as Experimental in the code and documents at Apache Spark 3.3.

@dongjoon-hyun
Copy link
Member

One more thing. In the original PR, @Yikun proposed as a tightly coupled style initially and we revised it into the recent form to be better (more neutral and extensible) in the architectural form.

@HyukjinKwon
Copy link
Member

I am fine. I would defer to @dongjoon-hyun here since he has more background on K8S.

@dongjoon-hyun
Copy link
Member

Thank you, @HyukjinKwon . I'm trying to deliver this feature safely in a step-wise manner. To do that, I'm also actively testing all relevant features in the production environments.

@Yikun
Copy link
Member Author

Yikun commented Feb 13, 2022

Thanks for your discussion and suggestion! I'm also defer to @dongjoon-hyun and @HyukjinKwon decision, so I will close this PR, and continue on profile module PR.

I'm trying to deliver this feature safely in a step-wise manner. To do that, I'm also actively testing all relevant features in the production environments.

@dongjoon-hyun Thanks for taking care this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants