Skip to content

Conversation

@Yikun
Copy link
Member

@Yikun Yikun commented Jul 21, 2022

What changes were proposed in this pull request?

Enable Spark on K8S integration tests in Github Action based on minikube:

  • The K8S IT will always triggered in user fork repo and apache/spark merged commits to master branch

  • This PR does NOT contains Volcano related test due to limited resource of github action.

  • minikube installation is allowed by Apache Infra: INFRA-23000

  • Why setting driver 0.5 cpu, executor 0.2 cpu?

    • Github-hosted runner hardware limited: 2U7G, so cpu resource is very limited.
    • IT Job available CPU = 2U - 0.85U (K8S deploy) = 1.15U
    • There are 1.15 cpu left after k8s installation, to meet the requirement of K8S tests (one driver + max to 3 executors).
    • For memory: 6947 is maximum (Otherwise raise Exiting due to RSRC_OVER_ALLOC_MEM: Requested memory allocation 7168MB is more than your system limit 6947MB.), but this is not integer multiple of 1024, so I just set this to 6144 for better resource statistic.
  • Time cost info:

    • 14 mins to compile related code.
    • 3 mins to build docker images.
    • 20-30 mins to test
    • Total: about 30-40 mins

Why are the changes needed?

This will also improve the efficiency of K8S development and guarantee the quality of spark on K8S and spark docker image in some level.

Does this PR introduce any user-facing change?

No, dev only.

How was this patch tested?

CI passed

Closes #35830

@Yikun
Copy link
Member Author

Yikun commented Jul 21, 2022

cc @dongjoon-hyun @HyukjinKwon

@Yikun Yikun force-pushed the SPARK-38597-k8s-it branch 4 times, most recently from 3f282c9 to 0023bd3 Compare July 21, 2022 15:45
@dongjoon-hyun
Copy link
Member

Thank you for your patience and reviving this, @Yikun .

@dongjoon-hyun
Copy link
Member

I added Closes at the end of your PR description. It will close your PR automatically when this is going to be merged.
Screen Shot 2022-07-21 at 10 36 05 AM

Copy link
Member

Choose a reason for hiding this comment

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

I believe we should enable k8s test always instead of this. core module changes will affect many scheduling tests too.

Copy link
Member

Choose a reason for hiding this comment

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

FYI, historically, log4j2 change also causes K8s integration test failure due to log4j2.properties file changes.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, I will change this soon.

@Yikun Yikun force-pushed the SPARK-38597-k8s-it branch from 0023bd3 to 9190afb Compare July 22, 2022 09:31
@dongjoon-hyun dongjoon-hyun marked this pull request as ready for review July 22, 2022 16:18
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.

+1, LGTM. Thank you, @Yikun . The SQL UT failure is irrelevant to this GitHub Action.
Merged to master for Apache Spark 3.4.

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-38597][K8S][INFRA] Enable Spark on K8S integration tests for master branch [SPARK-38597][K8S][INFRA] Enable Spark on K8S integration tests Jul 22, 2022
@Yikun
Copy link
Member Author

Yikun commented Jul 22, 2022

@dongjoon-hyun Thanks! I will monitor it closely for the next few days.

@dongjoon-hyun
Copy link
Member

Thank you for your consistent participation, @Yikun .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants