-
Notifications
You must be signed in to change notification settings - Fork 448
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
Change integration test sysytem from KinD Cluster to Minikube Cluster #1899
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,21 +4,17 @@ inputs: | |
kubernetes-version: | ||
required: true | ||
type: string | ||
cluster_name: | ||
required: false | ||
type: string | ||
default: katib-e2e-cluster | ||
|
||
runs: | ||
using: composite | ||
steps: | ||
- name: Set Up KinD Cluster | ||
uses: helm/kind-action@v1.2.0 | ||
- name: Set Up Minikube Cluster | ||
uses: manusa/actions-setup-minikube@v2.6.0 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What are the pros and cons of using this action vs manual steps(like in Kserve)? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I used this action to change to Minikube Cluster with minimal changes. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. WDYT? @johnugeorge There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we keep the manual way as kserve? Do you see any red flags? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is no difference in operation between this action and manual installation. |
||
with: | ||
version: v0.13.0 | ||
node_image: kindest/node:${{ inputs.kubernetes-version }} | ||
cluster_name: ${{ inputs.cluster_name }} | ||
wait: 120s | ||
minikube version: "v1.25.2" | ||
kubernetes version: ${{ inputs.kubernetes-version }} | ||
start args: --driver none --wait-timeout=60s | ||
github token: ${{ env.GITHUB_TOKEN }} | ||
|
||
- name: Set Up Docker Buildx | ||
uses: docker/setup-buildx-action@v1 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need GITHUB_TOKEN? I don't think, it is exposed to pull_request type for forked repos anways.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@johnugeorge Thanks for your review.
According to this document, it seems that actions-setup-minikube calls GitHub REST API.
So, I added GITHUB_TOKEN.
Would you like to remove GITHUB_TOKEN?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://docs.github.com/en/actions/security-guides/automatic-token-authentication#permissions-for-the-github_token Token has only read permission and hence it would be ok. I am thinking if there will be any security concern
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For unauthenticated requests, it is limited to 60 per hour https://docs.github.com/en/rest/overview/resources-in-the-rest-api#requests-from-personal-accounts Hence, we will need GitHub token if we use this action.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it makes sense.
I don't have any concerns about the security issues of GITHUB_TOKEN. Do you have any concerns?