Skip to content

[SPARK-49339][INFRA] Use setup-minikube GitHub Action#47832

Closed
TQJADE wants to merge 2 commits intoapache:masterfrom
TQJADE:SPARK-49339
Closed

[SPARK-49339][INFRA] Use setup-minikube GitHub Action#47832
TQJADE wants to merge 2 commits intoapache:masterfrom
TQJADE:SPARK-49339

Conversation

@TQJADE
Copy link
Contributor

@TQJADE TQJADE commented Aug 21, 2024

What changes were proposed in this pull request?

Use Minikube action

Why are the changes needed?

This is introduced in Minikube doc:

Does this PR introduce any user-facing change?

No, this is a infra change.

How was this patch tested?

Pass the CIs and check the CI log on this PR.

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

No.

@github-actions github-actions bot added the INFRA label Aug 21, 2024
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 (Pending CIs). Thank you, @TQJADE

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-49339]Use minikube action [SPARK-49339][INFRA] Use setup-minikube GitHub Action Aug 21, 2024
sudo install minikube-linux-amd64 /usr/local/bin/minikube
rm minikube-linux-amd64
- name: Start Minikube
uses: medyagh/setup-minikube@v0.0.18
Copy link
Contributor

Choose a reason for hiding this comment

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

why have you pinned the version to 0.0.18 and why not @latest

with this curl -LO https://storage.googleapis.com/minikube/releases/latest/minikube-linux-amd64 we had the latest always but now it its pinned.

Copy link
Member

Choose a reason for hiding this comment

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

To @bjornjorgensen , we use pinned GitHub Action always.

$ git grep setup .github/workflows/build_and_test.yml
.github/workflows/build_and_test.yml:      uses: actions/setup-java@v4
.github/workflows/build_and_test.yml:      uses: actions/setup-python@v5
.github/workflows/build_and_test.yml:        uses: docker/setup-qemu-action@v3
.github/workflows/build_and_test.yml:        uses: docker/setup-buildx-action@v3
.github/workflows/build_and_test.yml:      uses: actions/setup-java@v4
.github/workflows/build_and_test.yml:      uses: actions/setup-java@v4
.github/workflows/build_and_test.yml:        # The followings are also used by `r-lib/actions/setup-r` to avoid
.github/workflows/build_and_test.yml:      uses: bufbuild/buf-setup-action@v1
.github/workflows/build_and_test.yml:      uses: actions/setup-python@v5
.github/workflows/build_and_test.yml:      uses: actions/setup-java@v4
.github/workflows/build_and_test.yml:      uses: actions/setup-java@v4
.github/workflows/build_and_test.yml:      uses: actions/setup-java@v4
.github/workflows/build_and_test.yml:      uses: actions/setup-java@v4
.github/workflows/build_and_test.yml:        uses: actions/setup-java@v4
.github/workflows/build_and_test.yml:        uses: actions/setup-node@v4

In addition, the GitHub Action version is irrelevant to the Minikube version. According to the log, it downloads the latest Minikube 1.33.1 like the picture.
Screenshot 2024-08-21 at 13 25 32

Copy link
Member

Choose a reason for hiding this comment

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

It's the same like setup-java.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently from market place: https://github.com/marketplace/actions/setup-minikube 0.0.18 is the latest.
I think pinning a version will be better to stabilized the test.

@TQJADE TQJADE requested a review from bjornjorgensen August 21, 2024 21:22
@pan3793
Copy link
Member

pan3793 commented Aug 22, 2024

The PR GHA runs in the contributor's repo, is this action allowed by ASF?
https://infra.apache.org/github-actions-policy.html

@dongjoon-hyun
Copy link
Member

dongjoon-hyun commented Aug 22, 2024

Yes, this is allowed, @pan3793 . I asked the same question exactly when @TQJADE contributed this to Apache Spark Kubernetes Operator first. I asked to spin off this separately at that time and verified that it's running fine. After that, I asked her to contribute here together.

@pan3793
Copy link
Member

pan3793 commented Aug 22, 2024

@dongjoon-hyun thanks for information, it's fine then.

@dongjoon-hyun
Copy link
Member

Thank you, @pan3793 .

@dongjoon-hyun
Copy link
Member

For the record, there is only one irrelevant failure and it's a known flaky one.

[info] *** 1 SUITE ABORTED ***
[error] Error: Total 4127, Failed 0, Errors 1, Passed 4126, Ignored 8, Canceled 2
[error] Error during tests:
[error] 	org.apache.spark.util.collection.SorterSuite

@bjornjorgensen
Copy link
Contributor

bjornjorgensen commented Aug 22, 2024

from https://github.com/medyagh/setup-minikube/blob/master/action.yml

kubernetes-version:
    description: 'Choose a specific version of Kubernetes, "stable" for the latest stable build, or "latest" for the latest development build'
    required: false
    default: ''

should be pin latest her?

EDIT:

required: false
so we don't need it.

@dongjoon-hyun
Copy link
Member

Ya, thank you for double-checking, @bjornjorgensen . We don't need that.

Merged to master~

@dongjoon-hyun
Copy link
Member

Thank you again, @TQJADE , @bjornjorgensen , @pan3793 .

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.

4 participants