Fix/63 governor issues

flag_slurper/autolib/governor.py

@@ -50,6 +50,8 @@ def _filter(x: datetime):
         self.limits[ipaddr] = list(filter(_filter, self.limits[ipaddr]))
 
     def attempt(self, ipaddr: str):
+        if not ipaddr:
+            return
         if not self.enabled:
             return
 
@@ -61,4 +63,8 @@ def attempt(self, ipaddr: str):
 
     @staticmethod
     def resolve_url(url: str) -> str:
-        return socket.gethostbyname(url)
+        try:
+            return socket.gethostbyname(url)
+        except socket.gaierror:
+            logger.warning("Failed to resolve url: %s", url)
+            return None

tests/autolib/test_governor.py

@@ -1,8 +1,17 @@
 from datetime import datetime, timedelta
+import socket
+
+import pytest
 
 from flag_slurper.autolib.governor import Governor
 
 
+@pytest.fixture
+def governor():
+    Governor.instance = None
+    return Governor.get_instance(True)
+
+
 def test_governor_singleton():
     Governor.instance = None
     Governor.get_instance(True)
@@ -13,46 +22,40 @@ def test_governor_singleton():
     assert gov.times == 3
 
 
-def test_governor_tracks(mock):
-    Governor.instance = None
-    gov = Governor.get_instance(True)
+def test_governor_tracks(mock, governor):
     sleep = mock.patch('flag_slurper.autolib.governor.time.sleep')
-    gov.attempt('192.168.1.113')
+    governor.attempt('192.168.1.113')
     assert not sleep.called
 
 
-def test_governor_filter():
-    Governor.instance = None
-    gov = Governor.get_instance(True)
-    gov.limits['192.168.1.113'] = [datetime.now() - timedelta(minutes=3),
-                                   datetime.now() - timedelta(minutes=29),
-                                   datetime.now() - timedelta(minutes=1),
-                                   datetime.now() - timedelta(minutes=35)]
-    gov.filter('192.168.1.113')
-    assert len(gov.limits['192.168.1.113']) == 3
+def test_governor_filter(governor):
+    governor.limits['192.168.1.113'] = [datetime.now() - timedelta(minutes=3),
+                                        datetime.now() - timedelta(minutes=29),
+                                        datetime.now() - timedelta(minutes=1),
+                                        datetime.now() - timedelta(minutes=35)]
+    governor.filter('192.168.1.113')
+    assert len(governor.limits['192.168.1.113']) == 3
 
 
-def test_governor_limits(mock):
-    Governor.instance = None
-    gov = Governor.get_instance(True)
-    gov.limits['192.168.1.113'] = [datetime.now() - timedelta(minutes=3),
-                                   datetime.now() - timedelta(minutes=29),
-                                   datetime.now() - timedelta(minutes=1),
-                                   datetime.now() - timedelta(minutes=35)]
+def test_governor_limits(mock, governor):
+    governor.limits['192.168.1.113'] = [datetime.now() - timedelta(minutes=3),
+                                        datetime.now() - timedelta(minutes=29),
+                                        datetime.now() - timedelta(minutes=1),
+                                        datetime.now() - timedelta(minutes=35)]
     sleep = mock.patch('flag_slurper.autolib.governor.time.sleep')
-    gov.attempt('192.168.1.113')
+    governor.attempt('192.168.1.113')
     assert sleep.called
 
 
 def test_governer_doesnt_limit_when_disabled(mock):
     Governor.instance = None
-    gov = Governor.get_instance(False)
-    gov.limits['192.168.1.113'] = [datetime.now() - timedelta(minutes=3),
-                                   datetime.now() - timedelta(minutes=29),
-                                   datetime.now() - timedelta(minutes=1),
-                                   datetime.now() - timedelta(minutes=35)]
+    governor = Governor.get_instance(False)
+    governor.limits['192.168.1.113'] = [datetime.now() - timedelta(minutes=3),
+                                        datetime.now() - timedelta(minutes=29),
+                                        datetime.now() - timedelta(minutes=1),
+                                        datetime.now() - timedelta(minutes=35)]
     sleep = mock.patch('flag_slurper.autolib.governor.time.sleep')
-    gov.attempt('192.168.1.113')
+    governor.attempt('192.168.1.113')
     assert not sleep.called
 
 
@@ -62,3 +65,18 @@ def test_governor_resolve(mock):
     ipaddr = Governor.resolve_url('example.com')
     assert addrhost.called_with('example.com')
     assert ipaddr == '192.168.1.113'
+
+
+def test_governor_resolve_failure(mock):
+    addrhost = mock.patch('flag_slurper.autolib.governor.socket.gethostbyname')
+    addrhost.side_effect = socket.gaierror()
+    ipaddr = Governor.resolve_url('invalid.invalid')
+    assert addrhost.called_with('invalid.invalid')
+    assert ipaddr is None
+
+
+def test_governor_does_not_limit_invalid_ip(mock, governor):
+    governor.attempt(None)
+    sleep = mock.patch('flag_slurper.autolib.governor.time.sleep')
+    assert len(governor.limits) == 0
+    assert not sleep.called