Skip to content

Commit

Permalink
[change] Add "timed out" to ignored connection failures
Browse files Browse the repository at this point in the history
Plus minor cleanup.
  • Loading branch information
nemesifier committed Aug 7, 2020
1 parent 944d9b7 commit a6d3c0a
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 27 deletions.
8 changes: 5 additions & 3 deletions openwisp_monitoring/device/apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,11 @@ def is_working_changed_receiver(
return
# if device is down because of connectivity issues, it's probably due
# to reboot caused by firmware upgrade, avoid notifications
if 'Unable to connect' in failure_reason:
device_monitoring.save()
return
ignored_failures = ['Unable to connect', 'timed out']
for message in ignored_failures:
if message in failure_reason:
device_monitoring.save()
return
initial_status = device_monitoring.status
status = 'ok' if is_working else 'problem'
# do not send notificatons if recovery made after firmware upgrade
Expand Down
62 changes: 38 additions & 24 deletions openwisp_monitoring/device/tests/test_models.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
import json
import socket
from copy import deepcopy
from unittest.mock import patch

from django.core.exceptions import ObjectDoesNotExist, ValidationError
from openwisp_notifications.signals import notify
from paramiko.ssh_exception import NoValidConnectionsError
from swapper import load_model

from openwisp_controller.config.signals import config_modified
Expand All @@ -23,6 +21,7 @@


class BaseTestCase(DeviceMonitoringTestCase):
_PING = 'openwisp_monitoring.check.classes.Ping'
_sample_data = {
"type": "DeviceMonitoring",
"general": {
Expand Down Expand Up @@ -589,8 +588,7 @@ def test_is_working_false_true(self, notify_send, perform_check):
dm = d.monitoring
dm.status = 'unknown'
dm.save()
ping_path = 'openwisp_monitoring.check.classes.Ping'
Check.objects.create(name='Check', content_object=d, check=ping_path)
Check.objects.create(name='Check', content_object=d, check=self._PING)
c = Credentials.objects.create()
dc = DeviceConnection.objects.create(credentials=c, device=d, is_working=False)
self.assertFalse(dc.is_working)
Expand All @@ -606,8 +604,7 @@ def test_is_working_changed_to_false(self, notify_send, perform_check):
dm = d.monitoring
dm.status = 'ok'
dm.save()
ping_path = 'openwisp_monitoring.check.classes.Ping'
Check.objects.create(name='Check', content_object=d, check=ping_path)
Check.objects.create(name='Check', content_object=d, check=self._PING)
c = Credentials.objects.create()
dc = DeviceConnection.objects.create(credentials=c, device=d)
dc.is_working = False
Expand All @@ -622,8 +619,7 @@ def test_is_working_none_true(self, notify_send, perform_check):
dm = d.monitoring
dm.status = 'unknown'
dm.save()
ping_path = 'openwisp_monitoring.check.classes.Ping'
Check.objects.create(name='Check', content_object=d, check=ping_path)
Check.objects.create(name='Check', content_object=d, check=self._PING)
c = Credentials.objects.create()
dc = DeviceConnection.objects.create(credentials=c, device=d)
self.assertIsNone(dc.is_working)
Expand All @@ -634,24 +630,43 @@ def test_is_working_none_true(self, notify_send, perform_check):

@patch.object(Check, 'perform_check')
@patch.object(notify, 'send')
def test_is_working_connectivity_failure(self, notify_send, perform_check):
def test_is_working_changed_unable_to_connect(self, notify_send, perform_check):
ckey = self._create_credentials_with_key(port=self.ssh_server.port)
dc = self._create_device_connection(credentials=ckey)
dc.is_working = True
dc.save()
notify_send.assert_not_called()
perform_check.assert_not_called()

d = self.device_model.objects.first()
d.monitoring.update_status('unknown')
ping_path = 'openwisp_monitoring.check.classes.Ping'
Check.objects.create(name='Check', content_object=d, check=ping_path)
self.assertIsNone(dc.is_working)
d.monitoring.update_status('ok')
Check.objects.create(name='Check', content_object=d, check=self._PING)
dc.is_working = False
dc.failure_reason = '[Errno None] Unable to connect to port 5555 on 127.0.0.1'
dc.full_clean()
dc.save()

notify_send.assert_not_called()
perform_check.assert_not_called()

@patch.object(Check, 'perform_check')
@patch.object(notify, 'send')
def test_is_working_changed_timed_out(self, notify_send, perform_check):
ckey = self._create_credentials_with_key(port=self.ssh_server.port)
dc = self._create_device_connection(credentials=ckey)
dc.is_working = True
e = NoValidConnectionsError({'error': socket.error})
self.assertEqual(dc.failure_reason, '')
with patch.object(dc.connector_instance, 'connect', return_value=e):
dc.save()
dc.connect()
self.assertEqual(
dc.failure_reason,
'[Errno None] Unable to connect to port 5555 on 127.0.0.1',
)
dc.save()
notify_send.assert_not_called()
perform_check.assert_not_called()

d = self.device_model.objects.first()
d.monitoring.update_status('ok')
Check.objects.create(name='Check', content_object=d, check=self._PING)
dc.is_working = False
dc.failure_reason = 'timed out'
dc.full_clean()
dc.save()

notify_send.assert_not_called()
perform_check.assert_not_called()

Expand All @@ -663,8 +678,7 @@ def test_is_working_no_recovery_notification(self, notify_send, perform_check):
d = self.device_model.objects.first()
d.monitoring.update_status('ok')
dc.refresh_from_db()
ping_path = 'openwisp_monitoring.check.classes.Ping'
Check.objects.create(name='Check', content_object=d, check=ping_path)
Check.objects.create(name='Check', content_object=d, check=self._PING)
failure_reason = '[Errno None] Unable to connect to port 5555 on 127.0.0.1'
self.assertTrue(dc.is_working)
dc.failure_reason = failure_reason
Expand Down

0 comments on commit a6d3c0a

Please sign in to comment.