-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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 public API to detect node providers #2069
add public API to detect node providers #2069
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: BenTheElder 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 |
90c007c
to
ef889b6
Compare
Not having people re-implement this to fail out early when there doesn't seem to be a provider available will be more important in the future when we have detection for some provider that is not a mere "$cli_binary is in path", which we'll likely want to do at some point. |
} | ||
return p | ||
} | ||
|
||
// NoNodeProviderDetectedError indicates that we could not autolocate an available | ||
// NodeProvider backend on the host | ||
var NoNodeProviderDetectedError = errors.NewWithoutStack("failed to detect any supported node provider") |
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.
we haven't actually had much in the way of client-handle-able errors yet (usually stuff like "cluster bringup failed in some unexpected way"), so I had to update the error utilities to handle this appropriately.
// NewWithoutStack is like new but does NOT wrap with a stack | ||
// This is useful for exported errors | ||
func NewWithoutStack(message string) error { | ||
return stderrors.New(message) |
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 this will necessarily be for making errors at the bottom of the stack that should not themselves contain a stack, this should be appropriate (a plain error with the message and nothing else).
installing podman is failing in the CI pipeline :| |
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.
/lgtm
hmm it seems to get github actions to run with the latest config you must manually rebase the PR. |
ef889b6
to
f960e32
Compare
/retest
I´m sufferin gthis every day 🙃 |
see: #2067