Wrap reverse dns lookup in try/except block #281
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: lmiccini The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/da69c46862c24cf5827d5d1bea9a0b48 ✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 50m 35s |
lmiccini
force-pushed
the
kdump-dns-fix
branch
from
baebed4 to
8f15204
Compare
|
/lgtm |
| name = socket.gethostbyaddr(address[0])[0].split('.', 1)[0] | ||
| try: | ||
| name = socket.gethostbyaddr(address[0])[0].split('.', 1)[0] | ||
| except Exception as msg: |
There was a problem hiding this comment.
It would be better to catch something more specific than general exception.
There was a problem hiding this comment.
With the generic exception we will catch:
ERROR:root:Failed reverse dns lookup for: 8.8.8.88 - [Errno 1] Unknown host (valid dns server, wrong or unknown ip)
ERROR:root:Failed reverse dns lookup for: 8.8.8.88 - [Errno 2] Host name lookup failure (dns server unreachable or bogus response)
that is the same returned by except socket.herror or similar afaik.
|
obsolete, will be fixed in a single PR tackling two more issues we found. |
If DNS is broken calling socket.gethostbyaddr() could result in an unexpected pod crash.
Let's wrap the call in try/except with the expectation that a reverse dns lookup failure should not prevent evacuation, worst case scenario we would not capture the memory dump and the user will have a "ERROR Could not perform reverse dns lookup for: X" in the logs hinting at resolution not working.