-
Notifications
You must be signed in to change notification settings - Fork 5.2k
fix: return non-zero exit code when status unavailable #22494
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?
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: thc1006 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Welcome @thc1006! |
|
Hi @thc1006. 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. DetailsInstructions 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. |
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.
Pull request overview
This PR fixes a bug where minikube status incorrectly returns exit code 0 when the Docker container does not exist. The root cause is that cluster.GetStatus() returns nil when it fails, and the exitCode() function's loop doesn't execute for nil/empty slices, resulting in an incorrect 0 return.
Changes:
- Added nil/empty check in
exitCode()to return exit code 7 (all components not running) - Added test cases for nil and empty status slices to ensure proper coverage
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| cmd/minikube/cmd/status.go | Added early return in exitCode() to handle nil/empty statuses by returning exit code 7 |
| cmd/minikube/cmd/status_test.go | Added test cases for nil and empty status scenarios to verify correct exit code behavior |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Can one of the admins verify this patch? |
When cluster.GetStatus() fails (e.g., Docker container missing), the statuses slice is nil. Previously exitCode() returned 0 for nil/empty statuses. Now it returns 7 (all components not running). Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
e9a8fe7 to
cbaaddb
Compare
What this PR does
Fixes
minikube statusreturning exit code 0 when the Docker container does not exist.Root cause: When
cluster.GetStatus()fails, it returnsnilstatuses. TheexitCode()function's for-loop doesn't execute for nil/empty slices, returning 0.Fix: Add nil/empty check at the start of
exitCode()to return 7 (all components not running).Changes
cmd/minikube/cmd/status.go: Add nil check inexitCode()(+3 lines)cmd/minikube/cmd/status_test.go: Add test cases for nil and empty statuses (+16 lines)Verification
exitCode()function: 100% test coverageFixes #22466