-
Notifications
You must be signed in to change notification settings - Fork 23
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
Check if response is not nil #215
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Before executing the relevant FollowLink method, make sure the response we got from the engine is not nil, otherwise the response object will be nil and panic. Signed-off-by: Benny Zlotnik <[email protected]>
bennyz
added a commit
to bennyz/csi-driver-operator
that referenced
this pull request
Oct 5, 2020
connection#FollowLink issues an additional HTTP call which may fail if ovirt-engine suddenly becomes unavailable. Currently it will panic, but once [1] is merged, an error will be returned. [1] oVirt/ovirt-engine-sdk-go#215 Signed-off-by: Benny Zlotnik <[email protected]>
bennyz
added a commit
to bennyz/csi-driver-operator
that referenced
this pull request
Oct 5, 2020
connection#FollowLink issues an additional HTTP call which may fail if ovirt-engine suddenly becomes unavailable. Currently it will panic, but once [1] is merged, an error will be returned. [1] oVirt/ovirt-engine-sdk-go#215 Signed-off-by: Benny Zlotnik <[email protected]>
bennyz
added a commit
to bennyz/csi-driver-operator
that referenced
this pull request
Oct 5, 2020
connection#FollowLink issues an additional HTTP call which may fail if ovirt-engine suddenly becomes unavailable. Currently it will panic, but once [1] is merged, an error will be returned. [1] oVirt/ovirt-engine-sdk-go#215 Signed-off-by: Benny Zlotnik <[email protected]>
@bennyz LGTM. Nice catch. Thank you very much for this. Let's merge this. |
Gal-Zaidman
pushed a commit
to Gal-Zaidman/ovirt-csi-driver
that referenced
this pull request
Nov 15, 2020
bumped go-ovirt version to the latest to consume: 1. oVirt/ovirt-engine-sdk-go#217 2. oVirt/ovirt-engine-sdk-go#215 Also ran make vendor which added protobuf Signed-off-by: Gal-Zaidman <[email protected]>
Gal-Zaidman
pushed a commit
to Gal-Zaidman/ovirt-csi-driver
that referenced
this pull request
Nov 15, 2020
bumped go-ovirt version to the latest to consume: 1. oVirt/ovirt-engine-sdk-go#217 2. oVirt/ovirt-engine-sdk-go#215 Also ran make vendor which added protobuf Signed-off-by: Gal-Zaidman <[email protected]>
Gal-Zaidman
pushed a commit
to Gal-Zaidman/ovirt-csi-driver-operator
that referenced
this pull request
Nov 15, 2020
bumped go-ovirt version to the latest to consume: oVirt/ovirt-engine-sdk-go#217 oVirt/ovirt-engine-sdk-go#215 Signed-off-by: Gal-Zaidman <[email protected]>
Gal-Zaidman
pushed a commit
to Gal-Zaidman/ovirt-csi-driver-operator
that referenced
this pull request
Nov 15, 2020
bumped go-ovirt version to the latest to consume: oVirt/ovirt-engine-sdk-go#217 oVirt/ovirt-engine-sdk-go#215 Signed-off-by: Gal-Zaidman <[email protected]>
openshift-cherrypick-robot
pushed a commit
to openshift-cherrypick-robot/ovirt-csi-driver-operator
that referenced
this pull request
Jan 12, 2021
connection#FollowLink issues an additional HTTP call which may fail if ovirt-engine suddenly becomes unavailable. Currently it will panic, but once [1] is merged, an error will be returned. [1] oVirt/ovirt-engine-sdk-go#215 Signed-off-by: Benny Zlotnik <[email protected]>
openshift-cherrypick-robot
pushed a commit
to openshift-cherrypick-robot/ovirt-csi-driver-operator
that referenced
this pull request
Jan 12, 2021
bumped go-ovirt version to the latest to consume: oVirt/ovirt-engine-sdk-go#217 oVirt/ovirt-engine-sdk-go#215 Signed-off-by: Gal-Zaidman <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of the Change
Before executing the relevant FollowLink method, make sure the response we got from the engine is not nil, otherwise the response object will be nil and panic.
Alternate Designs
Benefits
This is a bug fix. If ovirt-engine is not accessible before a FollowLink call, it will panic because the response object will be nil.
Possible Drawbacks
Verification Process
This can be reproduced easily by terminating the ovirt-engine process right before the FollowLink call, an example stack trace would look like this:
I used this snippet[1] to reproduce, with my patch the result is:
https://gist.github.com/bennyz/d5b5f5112aa4b1ea27b6dc41791a8fab
Applicable Issues