Repository: Azure/WALinuxAgent (python)

Issue:
Ensure the Monitor Thead does not Die
The monitoring thread is not (ironically) not monitored.  There have been cases where VMs stop sending telemetry data, which is the responsibility of the monitoring thread.  The working theory is the thread died, and was not automatically restarted.

Pull request description:
restart monitor and environment thread if they die
Closes #1041 

Patch:
diff --git a/azurelinuxagent/common/utils/restutil.py b/azurelinuxagent/common/utils/restutil.py
index c521f63a..807be29c 100644
--- a/azurelinuxagent/common/utils/restutil.py
+++ b/azurelinuxagent/common/utils/restutil.py
@@ -28,7 +28,8 @@ import azurelinuxagent.common.utils.textutil as textutil
 
 from azurelinuxagent.common.exception import HttpError, ResourceGoneError
 from azurelinuxagent.common.future import httpclient, urlparse, ustr
-from azurelinuxagent.common.version import PY_VERSION_MAJOR, AGENT_NAME, GOAL_STATE_AGENT_VERSION
+from azurelinuxagent.common.version import PY_VERSION_MAJOR
+
 
 SECURE_WARNING_EMITTED = False
 
@@ -77,7 +78,6 @@ RETRY_EXCEPTIONS = [
 
 HTTP_PROXY_ENV = "http_proxy"
 HTTPS_PROXY_ENV = "https_proxy"
-HTTP_USER_AGENT = "{0}/{1}".format(AGENT_NAME, GOAL_STATE_AGENT_VERSION)
 
 DEFAULT_PROTOCOL_ENDPOINT='168.63.129.16'
 HOST_PLUGIN_PORT = 32526
@@ -175,13 +175,11 @@ def _http_request(method, host, rel_uri, port=None, data=None, secure=False,
     if port is None:
         port = 443 if secure else 80
 
-    if 'User-Agent' not in headers:
-        headers['User-Agent'] = HTTP_USER_AGENT
-
     if use_proxy:
         conn_host, conn_port = proxy_host, proxy_port
         scheme = "https" if secure else "http"
         url = "{0}://{1}:{2}{3}".format(scheme, host, port, rel_uri)
+
     else:
         conn_host, conn_port = host, port
         url = rel_uri
@@ -192,6 +190,7 @@ def _http_request(method, host, rel_uri, port=None, data=None, secure=False,
                                         timeout=10)
         if use_proxy:
             conn.set_tunnel(host, port)
+
     else:
         conn = httpclient.HTTPConnection(conn_host,
                                         conn_port,
diff --git a/azurelinuxagent/ga/env.py b/azurelinuxagent/ga/env.py
index d9b7d823..fa39b84f 100644
--- a/azurelinuxagent/ga/env.py
+++ b/azurelinuxagent/ga/env.py
@@ -82,6 +82,12 @@ class EnvHandler(object):
         self.dhcp_handler.conf_routes()
         self.hostname = self.osutil.get_hostname_record()
         self.dhcp_id = self.osutil.get_dhcp_pid()
+        self.start()
+
+    def is_alive(self):
+        return self.server_thread.is_alive()
+
+    def start(self):
         self.server_thread = threading.Thread(target=self.monitor)
         self.server_thread.setDaemon(True)
         self.server_thread.start()
diff --git a/azurelinuxagent/ga/monitor.py b/azurelinuxagent/ga/monitor.py
index 71ac9b0b..02767651 100644
--- a/azurelinuxagent/ga/monitor.py
+++ b/azurelinuxagent/ga/monitor.py
@@ -94,13 +94,19 @@ class MonitorHandler(object):
         self.osutil = get_osutil()
         self.protocol_util = get_protocol_util()
         self.sysinfo = []
+        self.event_thread = None
 
     def run(self):
         self.init_sysinfo()
+        self.start()
 
-        event_thread = threading.Thread(target=self.daemon)
-        event_thread.setDaemon(True)
-        event_thread.start()
+    def is_alive(self):
+        return self.event_thread.is_alive()
+
+    def start(self):
+        self.event_thread = threading.Thread(target=self.daemon)
+        self.event_thread.setDaemon(True)
+        self.event_thread.start()
 
     def init_sysinfo(self):
         osversion = "{0}:{1}-{2}-{3}:{4}".format(platform.system(),
diff --git a/azurelinuxagent/ga/update.py b/azurelinuxagent/ga/update.py
index 2e430318..dcd2955d 100644
--- a/azurelinuxagent/ga/update.py
+++ b/azurelinuxagent/ga/update.py
@@ -249,10 +249,12 @@ class UpdateHandler(object):
 
             # Launch monitoring threads
             from azurelinuxagent.ga.monitor import get_monitor_handler
-            get_monitor_handler().run()
+            monitor_thread = get_monitor_handler()
+            monitor_thread.run()
 
             from azurelinuxagent.ga.env import get_env_handler
-            get_env_handler().run()
+            env_thread = get_env_handler()
+            env_thread.run()
 
             from azurelinuxagent.ga.exthandlers import get_exthandlers_handler, migrate_handler_state
             exthandlers_handler = get_exthandlers_handler()
@@ -269,6 +271,14 @@ class UpdateHandler(object):
                                 CURRENT_AGENT)
                     break
 
+                if not monitor_thread.is_alive():
+                    logger.warn(u"Monitor thread died, restarting")
+                    monitor_thread.start()
+
+                if not env_thread.is_alive():
+                    logger.warn(u"Environment thread died, restarting")
+                    env_thread.start()
+
                 if self._upgrade_available():
                     available_agent = self.get_latest_agent()
                     if available_agent is None:


Test patch:
diff --git a/tests/ga/test_monitor.py b/tests/ga/test_monitor.py
index c646cef9..51f12a0d 100644
--- a/tests/ga/test_monitor.py
+++ b/tests/ga/test_monitor.py
@@ -18,6 +18,7 @@
 from tests.tools import *
 from azurelinuxagent.ga.monitor import *
 
+
 class TestMonitor(AgentTestCase):
     def test_parse_xml_event(self):
         data_str = load_data('ext/event.xml')
diff --git a/tests/ga/test_update.py b/tests/ga/test_update.py
index 21c81e98..0726d4c2 100644
--- a/tests/ga/test_update.py
+++ b/tests/ga/test_update.py
@@ -1248,7 +1248,6 @@ class TestUpdate(UpdateTestCase):
                             self.assertEqual(1, mock_env.call_count)
                             self.assertEqual(1, mock_exit.call_count)
 
-
     def test_run(self):
         self._test_run()
 
@@ -1497,6 +1496,151 @@ class TestUpdate(UpdateTestCase):
         self.assertTrue(ga_manifest_2.allowed_versions[1] == '2.2.14')
 
 
+class MonitorThreadTest(AgentTestCase):
+    def setUp(self):
+        AgentTestCase.setUp(self)
+        self.event_patch = patch('azurelinuxagent.common.event.add_event')
+        self.update_handler = get_update_handler()
+        self.update_handler.protocol_util = Mock()
+
+    def _test_run(self, invocations=1):
+        iterations = [0]
+        def iterator(*args, **kwargs):
+            iterations[0] += 1
+            if iterations[0] >= invocations:
+                self.update_handler.running = False
+            return
+
+        with patch('os.getpid', return_value=42):
+            with patch.object(UpdateHandler, '_is_orphaned') as mock_is_orphaned:
+                mock_is_orphaned.__get__ = Mock(return_value=False)
+                with patch('azurelinuxagent.ga.exthandlers.get_exthandlers_handler') as mock_handler:
+                    with patch('time.sleep', side_effect=iterator) as mock_sleep:
+                        with patch('sys.exit') as mock_exit:
+                            self.update_handler.run()
+
+    @patch('azurelinuxagent.ga.monitor.get_monitor_handler')
+    @patch('azurelinuxagent.ga.env.get_env_handler')
+    def test_start_threads(self, mock_env, mock_monitor):
+        self.assertTrue(self.update_handler.running)
+
+        mock_monitor_thread = MagicMock()
+        mock_monitor_thread.run = MagicMock()
+        mock_monitor.return_value = mock_monitor_thread
+
+        mock_env_thread = MagicMock()
+        mock_env_thread.run = MagicMock()
+        mock_env.return_value = mock_env_thread
+
+        self._test_run(invocations=0)
+        self.assertEqual(1, mock_monitor.call_count)
+        self.assertEqual(1, mock_monitor_thread.run.call_count)
+        self.assertEqual(1, mock_env.call_count)
+        self.assertEqual(1, mock_env_thread.run.call_count)
+
+    @patch('azurelinuxagent.ga.monitor.get_monitor_handler')
+    @patch('azurelinuxagent.ga.env.get_env_handler')
+    def test_check_if_monitor_thread_is_alive(self, mock_env, mock_monitor):
+        self.assertTrue(self.update_handler.running)
+
+        mock_monitor_thread = MagicMock()
+        mock_monitor_thread.run = MagicMock()
+        mock_monitor_thread.is_alive = MagicMock(return_value=True)
+        mock_monitor_thread.start = MagicMock()
+        mock_monitor.return_value = mock_monitor_thread
+
+        self._test_run(invocations=0)
+        self.assertEqual(1, mock_monitor.call_count)
+        self.assertEqual(1, mock_monitor_thread.run.call_count)
+        self.assertEqual(1, mock_monitor_thread.is_alive.call_count)
+        self.assertEqual(0, mock_monitor_thread.start.call_count)
+
+    @patch('azurelinuxagent.ga.monitor.get_monitor_handler')
+    @patch('azurelinuxagent.ga.env.get_env_handler')
+    def test_check_if_env_thread_is_alive(self, mock_env, mock_monitor):
+        self.assertTrue(self.update_handler.running)
+
+        mock_env_thread = MagicMock()
+        mock_env_thread.run = MagicMock()
+        mock_env_thread.is_alive = MagicMock(return_value=True)
+        mock_env_thread.start = MagicMock()
+        mock_env.return_value = mock_env_thread
+
+        self._test_run(invocations=1)
+        self.assertEqual(1, mock_env.call_count)
+        self.assertEqual(1, mock_env_thread.run.call_count)
+        self.assertEqual(1, mock_env_thread.is_alive.call_count)
+        self.assertEqual(0, mock_env_thread.start.call_count)
+
+    @patch('azurelinuxagent.ga.monitor.get_monitor_handler')
+    @patch('azurelinuxagent.ga.env.get_env_handler')
+    def test_restart_monitor_thread_if_not_alive(self, mock_env, mock_monitor):
+        self.assertTrue(self.update_handler.running)
+
+        mock_monitor_thread = MagicMock()
+        mock_monitor_thread.run = MagicMock()
+        mock_monitor_thread.is_alive = MagicMock(return_value=False)
+        mock_monitor_thread.start = MagicMock()
+        mock_monitor.return_value = mock_monitor_thread
+
+        self._test_run(invocations=1)
+        self.assertEqual(1, mock_monitor.call_count)
+        self.assertEqual(1, mock_monitor_thread.run.call_count)
+        self.assertEqual(1, mock_monitor_thread.is_alive.call_count)
+        self.assertEqual(1, mock_monitor_thread.start.call_count)
+
+    @patch('azurelinuxagent.ga.monitor.get_monitor_handler')
+    @patch('azurelinuxagent.ga.env.get_env_handler')
+    def test_restart_env_thread_if_not_alive(self, mock_env, mock_monitor):
+        self.assertTrue(self.update_handler.running)
+
+        mock_env_thread = MagicMock()
+        mock_env_thread.run = MagicMock()
+        mock_env_thread.is_alive = MagicMock(return_value=False)
+        mock_env_thread.start = MagicMock()
+        mock_env.return_value = mock_env_thread
+
+        self._test_run(invocations=1)
+        self.assertEqual(1, mock_env.call_count)
+        self.assertEqual(1, mock_env_thread.run.call_count)
+        self.assertEqual(1, mock_env_thread.is_alive.call_count)
+        self.assertEqual(1, mock_env_thread.start.call_count)
+
+    @patch('azurelinuxagent.ga.monitor.get_monitor_handler')
+    @patch('azurelinuxagent.ga.env.get_env_handler')
+    def test_restart_monitor_thread(self, mock_env, mock_monitor):
+        self.assertTrue(self.update_handler.running)
+
+        mock_monitor_thread = MagicMock()
+        mock_monitor_thread.run = MagicMock()
+        mock_monitor_thread.is_alive = MagicMock(return_value=False)
+        mock_monitor_thread.start = MagicMock()
+        mock_monitor.return_value = mock_monitor_thread
+
+        self._test_run(invocations=0)
+        self.assertEqual(True, mock_monitor.called)
+        self.assertEqual(True, mock_monitor_thread.run.called)
+        self.assertEqual(True, mock_monitor_thread.is_alive.called)
+        self.assertEqual(True, mock_monitor_thread.start.called)
+
+    @patch('azurelinuxagent.ga.monitor.get_monitor_handler')
+    @patch('azurelinuxagent.ga.env.get_env_handler')
+    def test_restart_env_thread(self, mock_env, mock_monitor):
+        self.assertTrue(self.update_handler.running)
+
+        mock_env_thread = MagicMock()
+        mock_env_thread.run = MagicMock()
+        mock_env_thread.is_alive = MagicMock(return_value=False)
+        mock_env_thread.start = MagicMock()
+        mock_env.return_value = mock_env_thread
+
+        self._test_run(invocations=0)
+        self.assertEqual(True, mock_env.called)
+        self.assertEqual(True, mock_env_thread.run.called)
+        self.assertEqual(True, mock_env_thread.is_alive.called)
+        self.assertEqual(True, mock_env_thread.start.called)
+
+
 class ChildMock(Mock):
     def __init__(self, return_value=0, side_effect=None):
         Mock.__init__(self, return_value=return_value, side_effect=side_effect)
diff --git a/tests/utils/test_rest_util.py b/tests/utils/test_rest_util.py
index 4f993227..bde0c3d0 100644
--- a/tests/utils/test_rest_util.py
+++ b/tests/utils/test_rest_util.py
@@ -15,11 +15,13 @@
 # Requires Python 2.4+ and Openssl 1.0+
 #
 
+import os
+import unittest
+
 from azurelinuxagent.common.exception import HttpError, \
+                                            ProtocolError, \
                                             ResourceGoneError
-
 import azurelinuxagent.common.utils.restutil as restutil
-from azurelinuxagent.common.utils.restutil import HTTP_USER_AGENT
 
 from azurelinuxagent.common.future import httpclient, ustr
 
@@ -195,7 +197,7 @@ class TestHttpOperations(AgentTestCase):
         ])
         HTTPSConnection.assert_not_called()
         mock_conn.request.assert_has_calls([
-            call(method="GET", url="/bar", body=None, headers={'User-Agent': HTTP_USER_AGENT})
+            call(method="GET", url="/bar", body=None, headers={})
         ])
         mock_conn.getresponse.assert_called_once()
         self.assertNotEquals(None, resp)
@@ -218,7 +220,7 @@ class TestHttpOperations(AgentTestCase):
             call("foo", 443, timeout=10)
         ])
         mock_conn.request.assert_has_calls([
-            call(method="GET", url="/bar", body=None, headers={'User-Agent': HTTP_USER_AGENT})
+            call(method="GET", url="/bar", body=None, headers={})
         ])
         mock_conn.getresponse.assert_called_once()
         self.assertNotEquals(None, resp)
@@ -242,7 +244,7 @@ class TestHttpOperations(AgentTestCase):
         ])
         HTTPSConnection.assert_not_called()
         mock_conn.request.assert_has_calls([
-            call(method="GET", url="http://foo:80/bar", body=None, headers={'User-Agent': HTTP_USER_AGENT})
+            call(method="GET", url="http://foo:80/bar", body=None, headers={})
         ])
         mock_conn.getresponse.assert_called_once()
         self.assertNotEquals(None, resp)
@@ -267,7 +269,7 @@ class TestHttpOperations(AgentTestCase):
             call("foo.bar", 23333, timeout=10)
         ])
         mock_conn.request.assert_has_calls([
-            call(method="GET", url="https://foo:443/bar", body=None, headers={'User-Agent': HTTP_USER_AGENT})
+            call(method="GET", url="https://foo:443/bar", body=None, headers={})
         ])
         mock_conn.getresponse.assert_called_once()
         self.assertNotEquals(None, resp)