Skip to content

Conversation

@panbingkun
Copy link
Contributor

@panbingkun panbingkun commented Oct 10, 2024

What changes were proposed in this pull request?

The pr aims to install R for ubuntu 24.04 when GA run k8s-integration-tests.

Why are the changes needed?

Does this PR introduce any user-facing change?

No, only for tests.

How was this patch tested?

Pass GA.

Was this patch authored or co-authored using generative AI tooling?

No.

@panbingkun panbingkun changed the title [TEST] Install R for ubuntu 24.04 [SPARK-49920][INFRA][TESTS] Install R for ubuntu 24.04 Oct 10, 2024
if: fromJson(needs.precondition.outputs.required).k8s-integration-tests == 'true'
name: Run Spark on Kubernetes Integration test
runs-on: ubuntu-latest
runs-on: ubuntu-24.04
Copy link
Contributor Author

Choose a reason for hiding this comment

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

actions/runner-images#10636
image
Because the GitHub community is still in the process of migration (sometimes it's ubuntu-22.04, sometimes it's ubuntu-22.04), adding this is to verify the issue. After obtaining approve, I will remove it.


private[spark] trait BasicTestsSuite { k8sSuite: KubernetesSuite =>


Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just to trigger k8s-integration-tests, after obtaining approve, I will remove it.

@panbingkun panbingkun marked this pull request as ready for review October 10, 2024 08:33
@panbingkun
Copy link
Contributor Author

Copy link
Contributor

@LuciferYang LuciferYang 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 (pending tests)

Thanks @panbingkun , this solved the issue I encountered recently.

if: fromJson(needs.precondition.outputs.required).k8s-integration-tests == 'true'
name: Run Spark on Kubernetes Integration test
runs-on: ubuntu-latest
runs-on: ubuntu-24.04
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor

@LuciferYang LuciferYang Oct 10, 2024

Choose a reason for hiding this comment

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

hmm... do you mean we should migrate k8s-integration-tests into container first? k8s-integration-tests does not rely on the base image for execution now.

Copy link
Contributor

@LuciferYang LuciferYang Oct 10, 2024

Choose a reason for hiding this comment

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

r-base \

This Docker file already has r-base installed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, currently it is not running in a container

k8s-integration-tests:
needs: precondition
if: fromJson(needs.precondition.outputs.required).k8s-integration-tests == 'true'
name: Run Spark on Kubernetes Integration test
runs-on: ubuntu-latest
timeout-minutes: 180

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps we could try moving the tests into the container, and then install minikube by default, but only start it when running k8s-integration-tests?

However, this configuration will be reused for the daily tests of branch-3.x, so we need to carefully consider how to ensure compatibility.

Copy link
Member

Choose a reason for hiding this comment

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

ah okok

Copy link
Member

Choose a reason for hiding this comment

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

I misread it. LG

Copy link
Contributor

Choose a reason for hiding this comment

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

haha

@panbingkun
Copy link
Contributor Author

GA has passed, I will restore some testing logic.

@LuciferYang LuciferYang changed the title [SPARK-49920][INFRA][TESTS] Install R for ubuntu 24.04 [SPARK-49920][INFRA] Install R for ubuntu 24.04 when GA run k8s-integration-tests Oct 10, 2024
@LuciferYang LuciferYang changed the title [SPARK-49920][INFRA] Install R for ubuntu 24.04 when GA run k8s-integration-tests [SPARK-49920][INFRA] Install R for ubuntu 24.04 when GA run k8s-integration-tests Oct 10, 2024
@LuciferYang
Copy link
Contributor

image

all test passed

@LuciferYang
Copy link
Contributor

Merged into master. Thanks @panbingkun @HyukjinKwon

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.

3 participants