-
Notifications
You must be signed in to change notification settings - Fork 5k
Skip Kubernetes preloads and binary downloads when --no-kubernetes is… #21107 #21139
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
base: master
Are you sure you want to change the base?
Skip Kubernetes preloads and binary downloads when --no-kubernetes is… #21107 #21139
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: divysinghvi The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Hi @divysinghvi. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Can one of the admins verify this patch? |
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.
thank you @divysinghvi please consider adding an integraiton test sub-test to
before {"StartNoK8sWithVersion", validateStartNoK8sWithVersion}, you can do --download-only and check using -minikube ssh -- ls path/to/binary/- that the file doesnt exist
@medyagh Thanks for the suggestion! Just to clarify, for the integration test you mentioned, which exact Kubernetes binary path should I check for in the VM after running minikube with |
Lets also try not setting k8s version to 0.0.0 and see what breaks and why did we have to that |
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.
lets add a sub test that that checks this folder should not exist
~/.minikube/cache/linux/amd64/v0.0.0/
ok-to-test |
defer PostMortemLogs(t, profile) | ||
|
||
// Start minikube with --no-kubernetes flag | ||
args := append([]string{"start", "-p", profile, "--no-kubernetes", "--memory=2048"}, StartArgs()...) |
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.
no need to start another minikube, use the Existing started minikube and simply add a sub test
like this
{"StartNoK8sWithVersion", validateStartNoK8sWithVersion},
---> {"VerifyNok8sNoK8sDownloads", },
{"StartWithK8s", validateStartWithK8S},
{"StartWithStopK8s", validateStartWithStopK8s},
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.
since in StartNoK8sWithVersion we pass a Kubernetes version with --no-kubernetes isn't that suppose to exits with an error and that won't leave a running cluster to reuse i guess.
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.
maybe we are reuse this instance {"Start", validateStartNoK8S}, since it will be fresh working instance
This PR improves the Binary function in binary.go to better handle Kubernetes binary downloads. It adds logic to skip downloads when the --no-kubernetes flag is set, ensures not downloading unnecessary downloads with version v0.0.0


Before
After
This pr Fixes the issue #21107