Repository: Azure/WALinuxAgent (python)

Issue:
Emit Telemetry Event if PUT to Status Blob Fails
There have been cases where the status blob does not exist, but we do not have telemetry to track this. Please consider sending a telemetry event if the PUT to the status blob fails.

Pull request:
Add telemetry for put status failures
- adds specific telemetry for failures on `put status`
- cleanup of python imports
- adds unit tests
- fixes #623 

Patch:
diff --git a/azurelinuxagent/common/event.py b/azurelinuxagent/common/event.py
index fb2c97c7..ce79adfa 100644
--- a/azurelinuxagent/common/event.py
+++ b/azurelinuxagent/common/event.py
@@ -46,6 +46,7 @@ class WALAEventOperation:
     Install = "Install"
     InitializeHostPlugin = "InitializeHostPlugin"
     Provision = "Provision"
+    ReportStatus = "ReportStatus"
     Restart = "Restart"
     UnhandledError = "UnhandledError"
     UnInstall = "UnInstall"
@@ -110,6 +111,15 @@ class EventLogger(object):
 __event_logger__ = EventLogger()
 
 
+def report_event(op, is_success=True, message=''):
+    from azurelinuxagent.common.version import AGENT_NAME, CURRENT_VERSION
+    add_event(AGENT_NAME,
+              version=CURRENT_VERSION,
+              is_success=is_success,
+              message=message,
+              op=op)
+
+
 def add_event(name, op="", is_success=True, duration=0, version=CURRENT_VERSION,
               message="", evt_type="", is_internal=False,
               reporter=__event_logger__):
diff --git a/azurelinuxagent/common/protocol/hostplugin.py b/azurelinuxagent/common/protocol/hostplugin.py
index d76326d4..acabd5b0 100644
--- a/azurelinuxagent/common/protocol/hostplugin.py
+++ b/azurelinuxagent/common/protocol/hostplugin.py
@@ -18,9 +18,14 @@
 #
 
 import base64
+import json
 
-from azurelinuxagent.common.protocol.wire import *
+from azurelinuxagent.common import logger
+from azurelinuxagent.common.exception import ProtocolError, HttpError
+from azurelinuxagent.common.future import ustr, httpclient
+from azurelinuxagent.common.utils import restutil
 from azurelinuxagent.common.utils import textutil
+from azurelinuxagent.common.utils.textutil import remove_bom
 from azurelinuxagent.common.version import PY_VERSION_MAJOR
 
 HOST_PLUGIN_PORT = 32526
@@ -65,11 +70,9 @@ class HostPluginProtocol(object):
             self.api_versions = self.get_api_versions()
             self.is_available = API_VERSION in self.api_versions
             self.is_initialized = True
-
-            from azurelinuxagent.common.event import add_event, WALAEventOperation
-            add_event(name="WALA",
-                      op=WALAEventOperation.InitializeHostPlugin,
-                      is_success=self.is_available)
+            from azurelinuxagent.common.event import WALAEventOperation, report_event
+            report_event(WALAEventOperation.InitializeHostPlugin,
+                         is_success=self.is_available)
         return self.is_available
 
     def get_api_versions(self):
@@ -89,8 +92,7 @@ class HostPluginProtocol(object):
                 return_val = ustr(remove_bom(response.read()), encoding='utf-8')
 
         except HttpError as e:
-            logger.error("HostGAPlugin: Exception Get API versions: " \
-                "{0}".format(e))
+            logger.error("HostGAPlugin: Exception Get API versions: {0}".format(e))
 
         return return_val
 
@@ -173,8 +175,16 @@ class HostPluginProtocol(object):
             else:
                 self._put_page_blob_status(sas_url, status_blob)
 
+            if not HostPluginProtocol.is_default_channel():
+                logger.info("HostGAPlugin: Setting host plugin as default channel")
+                HostPluginProtocol.set_default_channel(True)
         except Exception as e:
-            logger.error("HostGAPlugin: Exception Put VM status: {0}", e)
+            message = "HostGAPlugin: Exception Put VM status: {0}".format(e)
+            logger.error(message)
+            from azurelinuxagent.common.event import WALAEventOperation, report_event
+            report_event(op=WALAEventOperation.ReportStatus,
+                         is_success=False,
+                         message=message)
 
     def _put_block_blob_status(self, sas_url, status_blob):
         url = URI_FORMAT_PUT_VM_STATUS.format(self.endpoint, HOST_PLUGIN_PORT)
diff --git a/azurelinuxagent/common/protocol/wire.py b/azurelinuxagent/common/protocol/wire.py
index 051dfa1b..265e2dd7 100644
--- a/azurelinuxagent/common/protocol/wire.py
+++ b/azurelinuxagent/common/protocol/wire.py
@@ -387,7 +387,12 @@ class StatusBlob(object):
             else:
                 raise ProtocolError("Unknown blob type: {0}".format(self.type))
         except HttpError as e:
-            logger.warn("Initial upload failed [{0}]".format(e))
+            message = "Initial upload failed [{0}]".format(e)
+            logger.warn(message)
+            from azurelinuxagent.common.event import WALAEventOperation, report_event
+            report_event(op=WALAEventOperation.ReportStatus,
+                         is_success=False,
+                         message=message)
         else:
             logger.verbose("Uploading status blob succeeded")
             upload_successful = True
@@ -863,9 +868,6 @@ class WireClient(object):
                 host.put_vm_status(self.status_blob,
                                    ext_conf.status_upload_blob,
                                    ext_conf.status_upload_blob_type)
-                if not HostPluginProtocol.is_default_channel():
-                    logger.info("Setting host plugin as default channel")
-                    HostPluginProtocol.set_default_channel(True)
 
     """
     Emit an event to determine if the type in the extension config
diff --git a/azurelinuxagent/pa/rdma/centos.py b/azurelinuxagent/pa/rdma/centos.py
index 942d225f..10b7c817 100644
--- a/azurelinuxagent/pa/rdma/centos.py
+++ b/azurelinuxagent/pa/rdma/centos.py
@@ -82,8 +82,7 @@ class CentOSRDMAHandler(RDMAHandler):
         # Example match (pkg name, -, followed by 3 segments, fw_version and -):
         # - pkg=microsoft-hyper-v-rdma-4.1.0.142-20160323.x86_64
         # - fw_version=142
-        pattern = '{0}-(\d+\.){3,}({1})-'.format(
-            self.rdma_user_mode_package_name, fw_version)
+        pattern = '{0}-(\d+\.){{3,}}({1})-'.format(self.rdma_user_mode_package_name, fw_version)
         return re.match(pattern, pkg)
 
     @staticmethod


Test patch:
diff --git a/tests/common/test_event.py b/tests/common/test_event.py
index 726474cd..f5354110 100644
--- a/tests/common/test_event.py
+++ b/tests/common/test_event.py
@@ -17,8 +17,9 @@
 
 from __future__ import print_function
 
+from azurelinuxagent.common.event import init_event_logger, add_event
+from azurelinuxagent.common.future import ustr
 from tests.tools import *
-from azurelinuxagent.common.event import *
 
 
 class TestEvent(AgentTestCase):
diff --git a/tests/common/test_version.py b/tests/common/test_version.py
index df4f0e36..c01430b6 100644
--- a/tests/common/test_version.py
+++ b/tests/common/test_version.py
@@ -21,8 +21,9 @@ import textwrap
 
 import mock
 
-import azurelinuxagent.common.version as version
-from azurelinuxagent.common.version import *
+from azurelinuxagent.common.version import set_current_agent, \
+    AGENT_LONG_VERSION, AGENT_VERSION, AGENT_NAME, AGENT_NAME_PATTERN, \
+    get_f5_platform
 from tests.tools import *
 
 
@@ -67,6 +68,7 @@ class TestCurrentAgentName(AgentTestCase):
         self.assertEqual(version, str(current_version))
         return
 
+
 class TestGetF5Platforms(AgentTestCase):
     def test_get_f5_platform_bigip_12_1_1(self):
         version_file = textwrap.dedent("""
@@ -83,7 +85,7 @@ class TestGetF5Platforms(AgentTestCase):
 
         mo = mock.mock_open(read_data=version_file)
         with patch(open_patch(), mo):
-            platform = version.get_f5_platform()
+            platform = get_f5_platform()
             self.assertTrue(platform[0] == 'bigip')
             self.assertTrue(platform[1] == '12.1.1')
             self.assertTrue(platform[2] == 'bigip')
@@ -104,7 +106,7 @@ class TestGetF5Platforms(AgentTestCase):
 
         mo = mock.mock_open(read_data=version_file)
         with patch(open_patch(), mo):
-            platform = version.get_f5_platform()
+            platform = get_f5_platform()
             self.assertTrue(platform[0] == 'bigip')
             self.assertTrue(platform[1] == '12.1.0')
             self.assertTrue(platform[2] == 'bigip')
@@ -125,7 +127,7 @@ class TestGetF5Platforms(AgentTestCase):
 
         mo = mock.mock_open(read_data=version_file)
         with patch(open_patch(), mo):
-            platform = version.get_f5_platform()
+            platform = get_f5_platform()
             self.assertTrue(platform[0] == 'bigip')
             self.assertTrue(platform[1] == '12.0.0')
             self.assertTrue(platform[2] == 'bigip')
@@ -146,7 +148,7 @@ class TestGetF5Platforms(AgentTestCase):
 
         mo = mock.mock_open(read_data=version_file)
         with patch(open_patch(), mo):
-            platform = version.get_f5_platform()
+            platform = get_f5_platform()
             self.assertTrue(platform[0] == 'iworkflow')
             self.assertTrue(platform[1] == '2.0.1')
             self.assertTrue(platform[2] == 'iworkflow')
@@ -167,7 +169,7 @@ class TestGetF5Platforms(AgentTestCase):
 
         mo = mock.mock_open(read_data=version_file)
         with patch(open_patch(), mo):
-            platform = version.get_f5_platform()
+            platform = get_f5_platform()
             self.assertTrue(platform[0] == 'bigiq')
             self.assertTrue(platform[1] == '5.1.0')
             self.assertTrue(platform[2] == 'bigiq')
diff --git a/tests/daemon/test_daemon.py b/tests/daemon/test_daemon.py
index 77b4e3ed..dd31fd70 100644
--- a/tests/daemon/test_daemon.py
+++ b/tests/daemon/test_daemon.py
@@ -14,8 +14,7 @@
 #
 # Requires Python 2.4+ and Openssl 1.0+
 #
-
-from azurelinuxagent.daemon import *
+from azurelinuxagent.daemon import get_daemon_handler
 from tests.tools import *
 
 
diff --git a/tests/protocol/test_hostplugin.py b/tests/protocol/test_hostplugin.py
index 8ebb468c..ae90692e 100644
--- a/tests/protocol/test_hostplugin.py
+++ b/tests/protocol/test_hostplugin.py
@@ -16,11 +16,12 @@
 #
 
 import base64
-import sys
 import json
-import unittest
+import sys
+
+
 
-if sys.version_info[0]== 3:
+if sys.version_info[0] == 3:
     import http.client as httpclient
     bytebuffer = memoryview
 elif sys.version_info[0] == 2:
@@ -31,10 +32,9 @@ import azurelinuxagent.common.protocol.restapi as restapi
 import azurelinuxagent.common.protocol.wire as wire
 import azurelinuxagent.common.protocol.hostplugin as hostplugin
 
+from azurelinuxagent.common import event
 from azurelinuxagent.common.protocol.hostplugin import API_VERSION
 from azurelinuxagent.common.utils import restutil
-from azurelinuxagent.common.utils import textutil
-from azurelinuxagent.common.version import PY_VERSION_MAJOR
 
 from tests.protocol.mockwiredata import WireProtocolData, DATA_FILE
 from tests.tools import *
@@ -118,19 +118,51 @@ class TestHostPlugin(AgentTestCase):
         """
         test_goal_state = wire.GoalState(WireProtocolData(DATA_FILE).goal_state)
         status = restapi.VMStatus(status="Ready", message="Guest Agent is running")
-        with patch.object(wire.HostPluginProtocol, "put_vm_status") as patch_put:
+        with patch.object(wire.HostPluginProtocol,
+                          "ensure_initialized",
+                          return_value=True):
             with patch.object(wire.StatusBlob, "upload", return_value=False) as patch_upload:
-                wire_protocol_client = wire.WireProtocol(wireserver_url).client
-                wire_protocol_client.get_goal_state = Mock(return_value=test_goal_state)
-                wire_protocol_client.ext_conf = wire.ExtensionsConfig(None)
-                wire_protocol_client.ext_conf.status_upload_blob = sas_url
-                wire_protocol_client.status_blob.set_vm_status(status)
-                wire_protocol_client.upload_status_blob()
-                self.assertTrue(patch_put.call_count == 1,
-                                "Fallback was not engaged")
-                self.assertTrue(patch_put.call_args[0][1] == sas_url)
-                self.assertTrue(wire.HostPluginProtocol.is_default_channel())
-                wire.HostPluginProtocol.set_default_channel(False)
+                with patch.object(wire.HostPluginProtocol,
+                                  "_put_page_blob_status") as patch_put:
+                    wire_protocol_client = wire.WireProtocol(wireserver_url).client
+                    wire_protocol_client.get_goal_state = Mock(return_value=test_goal_state)
+                    wire_protocol_client.ext_conf = wire.ExtensionsConfig(None)
+                    wire_protocol_client.ext_conf.status_upload_blob = sas_url
+                    wire_protocol_client.status_blob.set_vm_status(status)
+                    wire_protocol_client.upload_status_blob()
+                    self.assertTrue(patch_put.call_count == 1,
+                                    "Fallback was not engaged")
+                    self.assertTrue(patch_put.call_args[0][0] == sas_url)
+                    self.assertTrue(wire.HostPluginProtocol.is_default_channel())
+                    wire.HostPluginProtocol.set_default_channel(False)
+
+    def test_put_status_error_reporting(self):
+        """
+        Validate the telemetry when uploading status fails
+        """
+        test_goal_state = wire.GoalState(WireProtocolData(DATA_FILE).goal_state)
+        status = restapi.VMStatus(status="Ready",
+                                  message="Guest Agent is running")
+        with patch.object(wire.StatusBlob,
+                          "upload",
+                          return_value=False):
+            wire_protocol_client = wire.WireProtocol(wireserver_url).client
+            wire_protocol_client.get_goal_state = Mock(return_value=test_goal_state)
+            wire_protocol_client.ext_conf = wire.ExtensionsConfig(None)
+            wire_protocol_client.ext_conf.status_upload_blob = sas_url
+            wire_protocol_client.status_blob.set_vm_status(status)
+            put_error = wire.HttpError("put status http error")
+            with patch.object(event,
+                              "add_event") as patch_add_event:
+                with patch.object(restutil,
+                                  "http_put",
+                                  side_effect=put_error) as patch_http_put:
+                    with patch.object(wire.HostPluginProtocol,
+                                      "ensure_initialized", return_value=True):
+                        wire_protocol_client.upload_status_blob()
+                        self.assertFalse(wire.HostPluginProtocol.is_default_channel())
+                        self.assertTrue(patch_add_event.call_count == 1)
+
 
     def test_validate_http_request(self):
         """Validate correct set of data is sent to HostGAPlugin when reporting VM status"""
diff --git a/tests/protocol/test_wire.py b/tests/protocol/test_wire.py
index e36fc652..dda7a2b8 100644
--- a/tests/protocol/test_wire.py
+++ b/tests/protocol/test_wire.py
@@ -14,7 +14,7 @@
 #
 # Requires Python 2.4+ and Openssl 1.0+
 #
-
+from azurelinuxagent.common import event
 from azurelinuxagent.common.protocol.wire import *
 from tests.protocol.mockwiredata import *
 
@@ -157,6 +157,7 @@ class TestWireProtocolGetters(AgentTestCase):
         with patch.object(WireClient, "get_goal_state") as patch_get_goal_state:
             with patch.object(HostPluginProtocol, "put_vm_status") as patch_host_ga_plugin_upload:
                 with patch.object(StatusBlob, "upload", return_value=True) as patch_default_upload:
+                    HostPluginProtocol.set_default_channel(False)
                     wire_protocol_client.upload_status_blob()
 
                     patch_default_upload.assert_called_once_with(testurl)
@@ -172,16 +173,55 @@ class TestWireProtocolGetters(AgentTestCase):
         wire_protocol_client.status_blob.vm_status = vmstatus
         goal_state = GoalState(WireProtocolData(DATA_FILE).goal_state)
 
-        with patch.object(HostPluginProtocol, "put_vm_status") as patch_host_ga_plugin_upload:
-            with patch.object(StatusBlob, "upload", return_value=False) as patch_default_upload:
-                wire_protocol_client.get_goal_state = Mock(return_value=goal_state)
-                wire_protocol_client.upload_status_blob()
+        with patch.object(HostPluginProtocol,
+                          "ensure_initialized",
+                          return_value=True):
+            with patch.object(StatusBlob,
+                              "upload",
+                              return_value=False) as patch_default_upload:
+                with patch.object(HostPluginProtocol,
+                                  "_put_block_blob_status") as patch_http:
+                    HostPluginProtocol.set_default_channel(False)
+                    wire_protocol_client.get_goal_state = Mock(return_value=goal_state)
+                    wire_protocol_client.upload_status_blob()
+                    patch_default_upload.assert_called_once_with(testurl)
+                    wire_protocol_client.get_goal_state.assert_called_once()
+                    patch_http.assert_called_once_with(testurl, wire_protocol_client.status_blob)
+                    self.assertTrue(HostPluginProtocol.is_default_channel())
+                    HostPluginProtocol.set_default_channel(False)
+
+    def test_upload_status_blob_error_reporting(self, *args):
+        vmstatus = VMStatus(message="Ready", status="Ready")
+        wire_protocol_client = WireProtocol(wireserver_url).client
+        wire_protocol_client.ext_conf = ExtensionsConfig(None)
+        wire_protocol_client.ext_conf.status_upload_blob = testurl
+        wire_protocol_client.ext_conf.status_upload_blob_type = testtype
+        wire_protocol_client.status_blob.vm_status = vmstatus
+        goal_state = GoalState(WireProtocolData(DATA_FILE).goal_state)
 
-                patch_default_upload.assert_called_once_with(testurl)
-                wire_protocol_client.get_goal_state.assert_called_once()
-                patch_host_ga_plugin_upload.assert_called_once_with(wire_protocol_client.status_blob, testurl, testtype)
-                self.assertTrue(HostPluginProtocol.is_default_channel())
-                HostPluginProtocol.set_default_channel(False)
+        with patch.object(HostPluginProtocol,
+                          "ensure_initialized",
+                          return_value=True):
+                 with patch.object(StatusBlob,
+                                   "put_block_blob",
+                                   side_effect=HttpError("error")):
+                     with patch.object(StatusBlob,
+                                       "get_blob_type",
+                                       return_value='BlockBlob'):
+                         with patch.object(HostPluginProtocol,
+                                           "put_vm_status"):
+                             with patch.object(WireClient,
+                                               "report_blob_type",
+                                               side_effect=MagicMock()):
+                                 with patch.object(event,
+                                                   "add_event") as patch_add_event:
+                                    HostPluginProtocol.set_default_channel(False)
+                                    wire_protocol_client.get_goal_state = Mock(return_value=goal_state)
+                                    wire_protocol_client.upload_status_blob()
+                                    wire_protocol_client.get_goal_state.assert_called_once()
+                                    self.assertTrue(patch_add_event.call_count == 1)
+                                    self.assertTrue(patch_add_event.call_args_list[0][1]['op'] == 'ReportStatus')
+                                    self.assertFalse(HostPluginProtocol.is_default_channel())
 
     def test_get_in_vm_artifacts_profile_blob_not_available(self, *args):
         wire_protocol_client = WireProtocol(wireserver_url).client
@@ -255,6 +295,7 @@ class TestWireProtocolGetters(AgentTestCase):
                 with patch.object(HostPluginProtocol,
                                   "get_artifact_request",
                                   return_value=[host_uri, {}]):
+                    HostPluginProtocol.set_default_channel(False)
                     self.assertRaises(ProtocolError, client.fetch_manifest, uris)
                     self.assertEqual(patch_fetch.call_count, 2)
                     self.assertEqual(patch_fetch.call_args_list[0][0][0], uri1.uri)