-
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
Add a machine readable reason to all error paths #9126
Add a machine readable reason to all error paths #9126
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tstromberg 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 |
/ok-to-test |
kvm2 Driver |
kvm2 Driver Times for Minikube (PR 9126): 63.5s 64.0s 62.7s Averages Time Per Log
docker Driver Times for Minikube (PR 9126): 27.0s 28.2s 26.4s Averages Time Per Log
|
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.
This is awesome! Just a few comments but the code is looking way cleaner after this refactor :)
kvm2 Driver Times for Minikube (PR 9126): 63.1s 63.6s 65.7s Averages Time Per Log
docker Driver Times for Minikube (PR 9126): 30.0s 28.2s 28.2s Averages Time Per Log
|
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.
great refactor
This gargantuan PR updates all error paths to provide a machine readable reason. This PR also significantly expands the dynamic range of exit codes, providing a framework to use most of the ~125 exit codes available in UNIX.
This replaces the previous "problem" error matching package with a new general purpose "reason" package that can be used directly from anywhere. As such, this PR gets rid of many attempts to provide output that looked similar to the problem package. Like the old problem package, the reason ID is propagated into both the JSON output and display output.
Unlike the previous problem implementation, reasons no longer require an error regexp, and now supports custom exit codes and styles.
Fixes #9080
Example 1: Not enough Docker storage
Old (exit code 251, UNDEFINED)
In JSON form:
New (exit code 26, ExInsufficientStorage)
In JSON form:
Example 2: Not enough Docker cores
Old (exit status 64, EX_USAGE)
New (exit status 29, ExInsufficientCores)
📘 https://docs.docker.com/config/containers/resource_constraints/
💣 Ensure your Docker for Desktop system has enough CPUs. The minimum allowed is 2 CPUs.
Example 3: Docker is not running (exit status 69: EX_UNAVAILABLE)
Old
New (exit status 63: ExProviderNotRunning)
Example 4: Permissions to $HOME/.kube/config
Old (exit 78, EX_CONFIG)
New (exit 37, ExHostPermission)
Example 5: Docker memory allocation warning
Old
New