-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Fix overwriting err for OCI "minikube start" #14506
Fix overwriting err for OCI "minikube start" #14506
Conversation
Welcome @criztovyl! |
Hi @criztovyl. 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/test-infra 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.
Hi @criztovyl, thanks for the PR! Looking at this I see a cleaner way to resolve this issue.
Line 402, if the if
is moved to the front of the line like how line 400 is, that will keep the scope of the error within that block, leaving the error from StartContainer
untouched
Could you make that change and revert the variable names to what they were, thanks!
Thx, yes, I'll look into that :) |
kubernetes#14424 podman start minikube returned with exit code 125
913868c
to
444f21f
Compare
I could verify locally minikube now reports an start-up error properly:
(that's containers/podman#4900, it's fixed upstream but not yet in my distro.) |
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.
Thanks for the PR!
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: criztovyl, spowelljr The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
I hope I am not too impatient; when is the next release, containing this fix, planned? :) |
We're considering doing a point/patch release, if we do that it'll be included then. Otherwise it would be included with the next minor release with will in a month and a half or so. |
I see 1.26.1 was released with this fix, thank you! |
fixes #14424