Skip to content
This repository has been archived by the owner on Aug 29, 2018. It is now read-only.

routing-daemon: F5: Rescue more network errors #6407

Conversation

Miciah
Copy link
Contributor

@Miciah Miciah commented Jul 26, 2016

F5IControlRestLoadBalancerModel#rest_request: Rescue several additional exceptions that failure to connect to the F5 BIG-IP host can raise, and attempt retries for these exceptions.

Newer versions of the restclient gem should wrap these exceptions, but there has been at least one report where restclient let through an unwrapped exception (Errno::ETIMEDOUT), and as a result, F5IControlRestLoadBalancerModel#rest_request did not rescue the exception and thus did not follow the retry logic.

This commit is related to commit f9d4782.

This commit is related to bug 1227472.

https://bugzilla.redhat.com/show_bug.cgi?id=1227472

@Miciah Miciah force-pushed the bug-1227472-routing-daemon-f5-rescue-more-network-errors branch from 326caa0 to 410d468 Compare July 26, 2016 13:27
F5IControlRestLoadBalancerModel#rest_request: Rescue several additional
exceptions that failure to connect to the F5 BIG-IP host can raise, and
attempt retries for these exceptions.

Newer versions of the restclient gem should wrap these exceptions, but
there has been at least one report where restclient let through an
unwrapped exception (Errno::ETIMEDOUT), and as a result,
F5IControlRestLoadBalancerModel#rest_request did not rescue the exception
and thus did not follow the retry logic.

This commit is related to commit f9d4782.

This commit is related to bug 1227472.

https://bugzilla.redhat.com/show_bug.cgi?id=1227472
@Miciah Miciah force-pushed the bug-1227472-routing-daemon-f5-rescue-more-network-errors branch from 410d468 to dc1be12 Compare July 26, 2016 13:28
@tiwillia
Copy link
Member

I don't see any issues with this approach. LGTM

@tiwillia
Copy link
Member

We don't have any tests that test the routing daemon at all, right?

@Miciah
Copy link
Contributor Author

Miciah commented Jul 27, 2016

Thanks! That's right. 😢

@tiwillia
Copy link
Member

Well then lets go ahead and [merge]

@openshift-bot
Copy link

openshift-bot commented Jul 27, 2016

Online Merge Results: SUCCESS (https://ci.dev.openshift.redhat.com/jenkins/job/test_pull_requests/9345/) (Image: devenv_5807)

@openshift-bot
Copy link

Evaluated for online merge up to dc1be12

@openshift-bot openshift-bot merged commit 825e3c5 into openshift:master Jul 27, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants