From c1041869b34bf03f76489a164d2dca7a63bd68de Mon Sep 17 00:00:00 2001 From: larohra Date: Wed, 29 Apr 2020 14:25:24 -0700 Subject: [PATCH 1/6] Removing infinite download of extension manifest without a new GS --- azurelinuxagent/ga/exthandlers.py | 17 ++++++++--------- tests/ga/test_extension.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 9 deletions(-) diff --git a/azurelinuxagent/ga/exthandlers.py b/azurelinuxagent/ga/exthandlers.py index 6eba19aaf7..fb07f42f72 100644 --- a/azurelinuxagent/ga/exthandlers.py +++ b/azurelinuxagent/ga/exthandlers.py @@ -423,6 +423,14 @@ def handle_ext_handler(self, ext_handler, etag): try: state = ext_handler.properties.state + + self.get_artifact_error_state.reset() + if self.last_etag == etag: + if self.log_etag: + ext_handler_i.logger.verbose("Incarnation {0} did not change, not processing GoalState", etag) + self.log_etag = False + return + if ext_handler_i.decide_version(target_state=state) is None: version = ext_handler_i.ext_handler.properties.version name = ext_handler_i.ext_handler.name @@ -432,15 +440,6 @@ def handle_ext_handler(self, ext_handler, etag): ext_handler_i.report_event(message=ustr(err_msg), is_success=False) return - self.get_artifact_error_state.reset() - if self.last_etag == etag: - if self.log_etag: - ext_handler_i.logger.verbose("Version {0} is current for etag {1}", - ext_handler_i.pkg.version, - etag) - self.log_etag = False - return - self.log_etag = True ext_handler_i.logger.info("Target handler state: {0} [incarnation {1}]", state, etag) diff --git a/tests/ga/test_extension.py b/tests/ga/test_extension.py index 16cc5a3c82..650249457d 100644 --- a/tests/ga/test_extension.py +++ b/tests/ga/test_extension.py @@ -488,6 +488,36 @@ def test_ext_handler(self, *args): exthandlers_handler.run() self._assert_no_handler_status(protocol.report_vm_status) + def test_it_should_only_download_extension_manifest_once_per_goal_state(self, *args): + + def _assert_handler_status_and_manifest_download_count(protocol, test_data, manifest_count): + self._assert_handler_status(protocol.report_vm_status, "Ready", 1, "1.0.0") + self._assert_ext_status(protocol.report_ext_status, "success", 0) + self.assertEqual(test_data.call_counts['manifest.xml'], manifest_count, + "We should have downloaded extension manifest {0} times".format(manifest_count)) + + test_data = mockwiredata.WireProtocolData(mockwiredata.DATA_FILE) + exthandlers_handler, protocol = self._create_mock(test_data, *args) + exthandlers_handler.run() + _assert_handler_status_and_manifest_download_count(protocol, test_data, 1) + + for _ in range(5): + exthandlers_handler.run() + # The extension manifest should only be downloaded once as incarnation did not change + _assert_handler_status_and_manifest_download_count(protocol, test_data, 1) + + # Update Incarnation + test_data.set_incarnation(2) + protocol.update_goal_state() + + exthandlers_handler.run() + _assert_handler_status_and_manifest_download_count(protocol, test_data, 2) + + for _ in range(5): + exthandlers_handler.run() + # The extension manifest should be downloaded twice now as incarnation changed once + _assert_handler_status_and_manifest_download_count(protocol, test_data, 2) + def test_ext_zip_file_packages_removed_in_update_case(self, *args): # Test enable scenario. test_data = mockwiredata.WireProtocolData(mockwiredata.DATA_FILE) From b6ae983411fe20f22f8367c412b1497633831709 Mon Sep 17 00:00:00 2001 From: larohra Date: Mon, 4 May 2020 16:42:46 -0700 Subject: [PATCH 2/6] Removed get_artifact_error_state for operations that only occur on a per GS basis --- azurelinuxagent/ga/exthandlers.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/azurelinuxagent/ga/exthandlers.py b/azurelinuxagent/ga/exthandlers.py index fb07f42f72..e6e2d51ad1 100644 --- a/azurelinuxagent/ga/exthandlers.py +++ b/azurelinuxagent/ga/exthandlers.py @@ -424,7 +424,6 @@ def handle_ext_handler(self, ext_handler, etag): try: state = ext_handler.properties.state - self.get_artifact_error_state.reset() if self.last_etag == etag: if self.log_etag: ext_handler_i.logger.verbose("Incarnation {0} did not change, not processing GoalState", etag) @@ -473,12 +472,8 @@ def handle_ext_handler_download_error(self, ext_handler_i, e, code=-1): msg = ustr(e) ext_handler_i.set_handler_status(message=msg, code=code) - self.get_artifact_error_state.incr() - if self.get_artifact_error_state.is_triggered(): - report_event(op=WALAEventOperation.Download, is_success=False, log_event=True, - message="Failed to get artifact for over " - "{0}: {1}".format(self.get_artifact_error_state.min_timedelta, msg)) - self.get_artifact_error_state.reset() + report_event(op=WALAEventOperation.Download, is_success=False, log_event=True, + message="Failed to download artifacts: {0}".format(msg)) def handle_enable(self, ext_handler_i): self.log_process = True From 0893dd1c09cf48b0f18bdeac0f39bffebd6c372f Mon Sep 17 00:00:00 2001 From: larohra Date: Thu, 14 May 2020 15:05:02 -0700 Subject: [PATCH 3/6] Removed error state and not useful tests. --- azurelinuxagent/ga/exthandlers.py | 46 ++++++++++---------- tests/ga/test_extension.py | 71 ++++++++++++++++++++----------- tests/ga/test_exthandlers.py | 23 ---------- 3 files changed, 70 insertions(+), 70 deletions(-) diff --git a/azurelinuxagent/ga/exthandlers.py b/azurelinuxagent/ga/exthandlers.py index e6e2d51ad1..204b9237d5 100644 --- a/azurelinuxagent/ga/exthandlers.py +++ b/azurelinuxagent/ga/exthandlers.py @@ -236,30 +236,30 @@ def __init__(self, protocol): self.log_process = False self.report_status_error_state = ErrorState() - self.get_artifact_error_state = ErrorState(min_timedelta=ERROR_STATE_DELTA_INSTALL) + # self.get_artifact_error_state = ErrorState(min_timedelta=ERROR_STATE_DELTA_INSTALL) def run(self): - self.ext_handlers, etag = None, None - try: - self.ext_handlers, etag = self.protocol.get_ext_handlers() - self.get_artifact_error_state.reset() - except Exception as e: - msg = u"Exception retrieving extension handlers: {0}".format(ustr(e)) - detailed_msg = '{0} {1}'.format(msg, traceback.extract_tb(get_traceback(e))) - - self.get_artifact_error_state.incr() - - if self.get_artifact_error_state.is_triggered(): - add_event(AGENT_NAME, - version=CURRENT_VERSION, - op=WALAEventOperation.GetArtifactExtended, - is_success=False, - message="Failed to get extension artifact for over " - "{0}: {1}".format(self.get_artifact_error_state.min_timedelta, msg)) - self.get_artifact_error_state.reset() - - add_event(AGENT_NAME, version=CURRENT_VERSION, op=WALAEventOperation.ExtensionProcessing, is_success=False, message=detailed_msg) - return + self.ext_handlers, etag = self.protocol.get_ext_handlers()# None, None + # try: + # self.ext_handlers, etag = self.protocol.get_ext_handlers() + # self.get_artifact_error_state.reset() + # except Exception as e: + # msg = u"Exception retrieving extension handlers: {0}".format(ustr(e)) + # detailed_msg = '{0} {1}'.format(msg, traceback.extract_tb(get_traceback(e))) + # + # self.get_artifact_error_state.incr() + # + # if self.get_artifact_error_state.is_triggered(): + # add_event(AGENT_NAME, + # version=CURRENT_VERSION, + # op=WALAEventOperation.GetArtifactExtended, + # is_success=False, + # message="Failed to get extension artifact for over " + # "{0}: {1}".format(self.get_artifact_error_state.min_timedelta, msg)) + # self.get_artifact_error_state.reset() + # + # add_event(AGENT_NAME, version=CURRENT_VERSION, op=WALAEventOperation.ExtensionProcessing, is_success=False, message=detailed_msg) + # return try: msg = u"Handle extensions updates for incarnation {0}".format(etag) @@ -1382,6 +1382,8 @@ def set_handler_status(self, status="NotReady", message="", code=0): try: handler_status_json = json.dumps(get_properties(handler_status)) if handler_status_json is not None: + # if not os.path.exists(state_dir): + # fileutil.mkdir(state_dir, mode=0o700) fileutil.write_file(status_file, handler_status_json) else: self.logger.error("Failed to create JSON document of handler status for {0} version {1}".format( diff --git a/tests/ga/test_extension.py b/tests/ga/test_extension.py index 650249457d..af17043a14 100644 --- a/tests/ga/test_extension.py +++ b/tests/ga/test_extension.py @@ -950,39 +950,60 @@ def test_ext_handler_download_failure_permanent_ProtocolError(self, mock_add_eve self.assertTrue("Failed to get ext handler pkgs" in kw['message']) self.assertTrue("ProtocolError" in kw['message']) - @patch('azurelinuxagent.common.errorstate.ErrorState.is_triggered') - @patch('azurelinuxagent.common.event.add_event') - def test_ext_handler_download_failure_permanent_with_ExtensionDownloadError_and_triggered(self, mock_add_event, - mock_error_state, *args): + def test_ext_handler_download_errors_should_be_reported_only_on_new_goal_state(self, *args): test_data = mockwiredata.WireProtocolData(mockwiredata.DATA_FILE) exthandlers_handler, protocol = self._create_mock(test_data, *args) protocol.get_ext_handler_pkgs = Mock(side_effect=ExtensionDownloadError) - mock_error_state.return_value = True - exthandlers_handler.run() - self.assertEquals(1, mock_add_event.call_count) - args, kw = mock_add_event.call_args_list[0] - self.assertEquals(False, kw['is_success']) - self.assertTrue("Failed to get artifact for over" in kw['message']) - self.assertTrue("ExtensionDownloadError" in kw['message']) - self.assertEquals("Download", kw['op']) - - @patch('azurelinuxagent.common.errorstate.ErrorState.is_triggered') - @patch('azurelinuxagent.common.event.add_event') - def test_ext_handler_download_failure_permanent_with_ExtensionDownloadError_and_not_triggered(self, mock_add_event, - mock_error_state, - *args): - test_data = mockwiredata.WireProtocolData(mockwiredata.DATA_FILE) - exthandlers_handler, protocol = self._create_mock(test_data, *args) - protocol.get_ext_handler_pkgs = Mock(side_effect=ExtensionDownloadError) - - mock_error_state.return_value = False + # self.assertEquals(1, mock_add_event.call_count) + # args, kw = mock_add_event.call_args_list[0] + # self.assertEquals(False, kw['is_success']) + # self.assertTrue("Failed to get artifact for over" in kw['message']) + # self.assertTrue("ExtensionDownloadError" in kw['message']) + # self.assertEquals("Download", kw['op']) - exthandlers_handler.run() + self._assert_handler_status(protocol.report_vm_status, "Ready", 1, "1.0.0") + self._assert_ext_status(protocol.report_ext_status, "success", 0) - self.assertEquals(0, mock_add_event.call_count) + # Re-run exthandler.run and ensure we dont report error + # Change incarnation and then re-check we report error + + + # @patch('azurelinuxagent.common.errorstate.ErrorState.is_triggered') + # @patch('azurelinuxagent.common.event.add_event') + # def test_ext_handler_download_failure_permanent_with_ExtensionDownloadError_and_triggered(self, mock_add_event, + # mock_error_state, *args): + # test_data = mockwiredata.WireProtocolData(mockwiredata.DATA_FILE) + # exthandlers_handler, protocol = self._create_mock(test_data, *args) + # protocol.get_ext_handler_pkgs = Mock(side_effect=ExtensionDownloadError) + # + # mock_error_state.return_value = True + # + # exthandlers_handler.run() + # + # self.assertEquals(1, mock_add_event.call_count) + # args, kw = mock_add_event.call_args_list[0] + # self.assertEquals(False, kw['is_success']) + # self.assertTrue("Failed to get artifact for over" in kw['message']) + # self.assertTrue("ExtensionDownloadError" in kw['message']) + # self.assertEquals("Download", kw['op']) + + # @patch('azurelinuxagent.common.errorstate.ErrorState.is_triggered') + # @patch('azurelinuxagent.common.event.add_event') + # def test_ext_handler_download_failure_permanent_with_ExtensionDownloadError_and_not_triggered(self, mock_add_event, + # mock_error_state, + # *args): + # test_data = mockwiredata.WireProtocolData(mockwiredata.DATA_FILE) + # exthandlers_handler, protocol = self._create_mock(test_data, *args) + # protocol.get_ext_handler_pkgs = Mock(side_effect=ExtensionDownloadError) + # + # mock_error_state.return_value = False + # + # exthandlers_handler.run() + # + # self.assertEquals(0, mock_add_event.call_count) @patch('azurelinuxagent.ga.exthandlers.fileutil') def test_ext_handler_io_error(self, mock_fileutil, *args): diff --git a/tests/ga/test_exthandlers.py b/tests/ga/test_exthandlers.py index b18d5a80ba..9758c5b09e 100644 --- a/tests/ga/test_exthandlers.py +++ b/tests/ga/test_exthandlers.py @@ -249,29 +249,6 @@ def test_extension_sequence_number(self): disk_sequence_number=3, expected_sequence_number=-1) - @patch("azurelinuxagent.ga.exthandlers.add_event") - @patch("azurelinuxagent.common.errorstate.ErrorState.is_triggered") - def test_it_should_report_an_error_if_the_wireserver_cannot_be_reached(self, patch_is_triggered, patch_add_event): - test_message = "TEST MESSAGE" - - patch_is_triggered.return_value = True # protocol errors are reported only after a delay; force the error to be reported now - - protocol = WireProtocol("foo.bar") - protocol.get_ext_handlers = MagicMock(side_effect=ProtocolError(test_message)) - - get_exthandlers_handler(protocol).run() - - self.assertEquals(patch_add_event.call_count, 2) - - _, first_call_args = patch_add_event.call_args_list[0] - self.assertEquals(first_call_args['op'], WALAEventOperation.GetArtifactExtended) - self.assertEquals(first_call_args['is_success'], False) - - _, second_call_args = patch_add_event.call_args_list[1] - self.assertEquals(second_call_args['op'], WALAEventOperation.ExtensionProcessing) - self.assertEquals(second_call_args['is_success'], False) - self.assertIn(test_message, second_call_args['message']) - class LaunchCommandTestCase(AgentTestCase): """ From df5096588b508a25503300c24e17308706b2eddf Mon Sep 17 00:00:00 2001 From: larohra Date: Wed, 20 May 2020 16:49:51 -0700 Subject: [PATCH 4/6] Removed unused artifact_error_state and modified tests to check complete scenario rather than specific functions --- azurelinuxagent/ga/exthandlers.py | 27 ++----------- tests/ga/test_extension.py | 66 ++++++++++--------------------- 2 files changed, 24 insertions(+), 69 deletions(-) diff --git a/azurelinuxagent/ga/exthandlers.py b/azurelinuxagent/ga/exthandlers.py index 204b9237d5..d52c1535ce 100644 --- a/azurelinuxagent/ga/exthandlers.py +++ b/azurelinuxagent/ga/exthandlers.py @@ -236,32 +236,11 @@ def __init__(self, protocol): self.log_process = False self.report_status_error_state = ErrorState() - # self.get_artifact_error_state = ErrorState(min_timedelta=ERROR_STATE_DELTA_INSTALL) def run(self): - self.ext_handlers, etag = self.protocol.get_ext_handlers()# None, None - # try: - # self.ext_handlers, etag = self.protocol.get_ext_handlers() - # self.get_artifact_error_state.reset() - # except Exception as e: - # msg = u"Exception retrieving extension handlers: {0}".format(ustr(e)) - # detailed_msg = '{0} {1}'.format(msg, traceback.extract_tb(get_traceback(e))) - # - # self.get_artifact_error_state.incr() - # - # if self.get_artifact_error_state.is_triggered(): - # add_event(AGENT_NAME, - # version=CURRENT_VERSION, - # op=WALAEventOperation.GetArtifactExtended, - # is_success=False, - # message="Failed to get extension artifact for over " - # "{0}: {1}".format(self.get_artifact_error_state.min_timedelta, msg)) - # self.get_artifact_error_state.reset() - # - # add_event(AGENT_NAME, version=CURRENT_VERSION, op=WALAEventOperation.ExtensionProcessing, is_success=False, message=detailed_msg) - # return try: + self.ext_handlers, etag = self.protocol.get_ext_handlers() msg = u"Handle extensions updates for incarnation {0}".format(etag) logger.verbose(msg) # Log status report success on new config @@ -1382,8 +1361,8 @@ def set_handler_status(self, status="NotReady", message="", code=0): try: handler_status_json = json.dumps(get_properties(handler_status)) if handler_status_json is not None: - # if not os.path.exists(state_dir): - # fileutil.mkdir(state_dir, mode=0o700) + if not os.path.exists(state_dir): + fileutil.mkdir(state_dir, mode=0o700) fileutil.write_file(status_file, handler_status_json) else: self.logger.error("Failed to create JSON document of handler status for {0} version {1}".format( diff --git a/tests/ga/test_extension.py b/tests/ga/test_extension.py index af17043a14..c8a8ae8d27 100644 --- a/tests/ga/test_extension.py +++ b/tests/ga/test_extension.py @@ -950,60 +950,36 @@ def test_ext_handler_download_failure_permanent_ProtocolError(self, mock_add_eve self.assertTrue("Failed to get ext handler pkgs" in kw['message']) self.assertTrue("ProtocolError" in kw['message']) - def test_ext_handler_download_errors_should_be_reported_only_on_new_goal_state(self, *args): + @patch('azurelinuxagent.common.event.add_event') + def test_ext_handler_download_errors_should_be_reported_only_on_new_goal_state(self, mock_add_event, *args): + + def _assert_mock_add_event_call(expected_call_count): + self.assertEquals(expected_call_count, mock_add_event.call_count, "Call count do not match") + args, kw = mock_add_event.call_args + self.assertEquals(False, kw['is_success'], "The event should have failed") + self.assertTrue("Failed to download artifacts" in kw['message'], "Incorrect failure, it should have failed to download") + self.assertTrue("ExtensionDownloadError" in kw['message'], "Incorrect error, it should be a DownloadError") + self.assertEquals("Download", kw['op'], "Incorrect Operation, it should be a download error") + test_data = mockwiredata.WireProtocolData(mockwiredata.DATA_FILE) exthandlers_handler, protocol = self._create_mock(test_data, *args) protocol.get_ext_handler_pkgs = Mock(side_effect=ExtensionDownloadError) exthandlers_handler.run() + _assert_mock_add_event_call(expected_call_count=1) + self._assert_handler_status(protocol.report_vm_status, "NotReady", 0, "1.0.0") - # self.assertEquals(1, mock_add_event.call_count) - # args, kw = mock_add_event.call_args_list[0] - # self.assertEquals(False, kw['is_success']) - # self.assertTrue("Failed to get artifact for over" in kw['message']) - # self.assertTrue("ExtensionDownloadError" in kw['message']) - # self.assertEquals("Download", kw['op']) - - self._assert_handler_status(protocol.report_vm_status, "Ready", 1, "1.0.0") - self._assert_ext_status(protocol.report_ext_status, "success", 0) + # Re-run exthandler.run without updating the GS and ensure we dont report error + exthandlers_handler.run() + _assert_mock_add_event_call(expected_call_count=1) - # Re-run exthandler.run and ensure we dont report error # Change incarnation and then re-check we report error + test_data.set_incarnation(2) + protocol.update_goal_state() - - # @patch('azurelinuxagent.common.errorstate.ErrorState.is_triggered') - # @patch('azurelinuxagent.common.event.add_event') - # def test_ext_handler_download_failure_permanent_with_ExtensionDownloadError_and_triggered(self, mock_add_event, - # mock_error_state, *args): - # test_data = mockwiredata.WireProtocolData(mockwiredata.DATA_FILE) - # exthandlers_handler, protocol = self._create_mock(test_data, *args) - # protocol.get_ext_handler_pkgs = Mock(side_effect=ExtensionDownloadError) - # - # mock_error_state.return_value = True - # - # exthandlers_handler.run() - # - # self.assertEquals(1, mock_add_event.call_count) - # args, kw = mock_add_event.call_args_list[0] - # self.assertEquals(False, kw['is_success']) - # self.assertTrue("Failed to get artifact for over" in kw['message']) - # self.assertTrue("ExtensionDownloadError" in kw['message']) - # self.assertEquals("Download", kw['op']) - - # @patch('azurelinuxagent.common.errorstate.ErrorState.is_triggered') - # @patch('azurelinuxagent.common.event.add_event') - # def test_ext_handler_download_failure_permanent_with_ExtensionDownloadError_and_not_triggered(self, mock_add_event, - # mock_error_state, - # *args): - # test_data = mockwiredata.WireProtocolData(mockwiredata.DATA_FILE) - # exthandlers_handler, protocol = self._create_mock(test_data, *args) - # protocol.get_ext_handler_pkgs = Mock(side_effect=ExtensionDownloadError) - # - # mock_error_state.return_value = False - # - # exthandlers_handler.run() - # - # self.assertEquals(0, mock_add_event.call_count) + exthandlers_handler.run() + _assert_mock_add_event_call(expected_call_count=2) + self._assert_handler_status(protocol.report_vm_status, "NotReady", 0, "1.0.0") @patch('azurelinuxagent.ga.exthandlers.fileutil') def test_ext_handler_io_error(self, mock_fileutil, *args): From fd06d5f96d9c35735c926122cce0e5fcfe0dd96f Mon Sep 17 00:00:00 2001 From: larohra Date: Wed, 20 May 2020 17:03:57 -0700 Subject: [PATCH 5/6] Modified a failing test which would pass once cleanup logic is fixed --- tests/ga/test_extension.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/ga/test_extension.py b/tests/ga/test_extension.py index c8a8ae8d27..2223dded51 100644 --- a/tests/ga/test_extension.py +++ b/tests/ga/test_extension.py @@ -972,6 +972,7 @@ def _assert_mock_add_event_call(expected_call_count): # Re-run exthandler.run without updating the GS and ensure we dont report error exthandlers_handler.run() _assert_mock_add_event_call(expected_call_count=1) + self._assert_handler_status(protocol.report_vm_status, "NotReady", 0, "1.0.0") # Change incarnation and then re-check we report error test_data.set_incarnation(2) From ad4e16875296bb8c4d1c1a839c80662199685050 Mon Sep 17 00:00:00 2001 From: larohra Date: Thu, 21 May 2020 16:51:20 -0700 Subject: [PATCH 6/6] Addressed PR comments, made test more reliable and removed dead code --- azurelinuxagent/common/event.py | 1 - azurelinuxagent/ga/exthandlers.py | 4 ++-- tests/ga/test_extension.py | 24 +++++++++++++----------- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/azurelinuxagent/common/event.py b/azurelinuxagent/common/event.py index a2e3f9950d..408c7edf40 100644 --- a/azurelinuxagent/common/event.py +++ b/azurelinuxagent/common/event.py @@ -83,7 +83,6 @@ class WALAEventOperation: ExtensionProcessing = "ExtensionProcessing" FetchGoalState = "FetchGoalState" Firewall = "Firewall" - GetArtifactExtended = "GetArtifactExtended" HealthCheck = "HealthCheck" HealthObservation = "HealthObservation" HeartBeat = "HeartBeat" diff --git a/azurelinuxagent/ga/exthandlers.py b/azurelinuxagent/ga/exthandlers.py index d52c1535ce..8f5b990ff8 100644 --- a/azurelinuxagent/ga/exthandlers.py +++ b/azurelinuxagent/ga/exthandlers.py @@ -401,14 +401,14 @@ def handle_ext_handler(self, ext_handler, etag): ext_handler_i = ExtHandlerInstance(ext_handler, self.protocol) try: - state = ext_handler.properties.state - if self.last_etag == etag: if self.log_etag: ext_handler_i.logger.verbose("Incarnation {0} did not change, not processing GoalState", etag) self.log_etag = False return + state = ext_handler.properties.state + if ext_handler_i.decide_version(target_state=state) is None: version = ext_handler_i.ext_handler.properties.version name = ext_handler_i.ext_handler.name diff --git a/tests/ga/test_extension.py b/tests/ga/test_extension.py index 2223dded51..ba6e1a14d8 100644 --- a/tests/ga/test_extension.py +++ b/tests/ga/test_extension.py @@ -22,6 +22,7 @@ import tempfile import time import unittest +import uuid import zipfile import datetime @@ -953,25 +954,26 @@ def test_ext_handler_download_failure_permanent_ProtocolError(self, mock_add_eve @patch('azurelinuxagent.common.event.add_event') def test_ext_handler_download_errors_should_be_reported_only_on_new_goal_state(self, mock_add_event, *args): - def _assert_mock_add_event_call(expected_call_count): - self.assertEquals(expected_call_count, mock_add_event.call_count, "Call count do not match") - args, kw = mock_add_event.call_args - self.assertEquals(False, kw['is_success'], "The event should have failed") - self.assertTrue("Failed to download artifacts" in kw['message'], "Incorrect failure, it should have failed to download") - self.assertTrue("ExtensionDownloadError" in kw['message'], "Incorrect error, it should be a DownloadError") - self.assertEquals("Download", kw['op'], "Incorrect Operation, it should be a download error") + def _assert_mock_add_event_call(expected_download_failed_event_count, err_msg_guid): + event_occurrences = [kw for _, kw in mock_add_event.call_args_list if + "Failed to download artifacts: [ExtensionDownloadError] {0}".format(err_msg_guid) in kw['message']] + self.assertEquals(expected_download_failed_event_count, len(event_occurrences), "Call count do not match") + self.assertFalse(any([kw['is_success'] for kw in event_occurrences]), "The events should have failed") + self.assertEqual(expected_download_failed_event_count, len([kw['op'] for kw in event_occurrences]), + "Incorrect Operation, all events should be a download errors") test_data = mockwiredata.WireProtocolData(mockwiredata.DATA_FILE) exthandlers_handler, protocol = self._create_mock(test_data, *args) - protocol.get_ext_handler_pkgs = Mock(side_effect=ExtensionDownloadError) + unique_error_message_guid = str(uuid.uuid4()) + protocol.get_ext_handler_pkgs = Mock(side_effect=ExtensionDownloadError(unique_error_message_guid)) exthandlers_handler.run() - _assert_mock_add_event_call(expected_call_count=1) + _assert_mock_add_event_call(expected_download_failed_event_count=1, err_msg_guid=unique_error_message_guid) self._assert_handler_status(protocol.report_vm_status, "NotReady", 0, "1.0.0") # Re-run exthandler.run without updating the GS and ensure we dont report error exthandlers_handler.run() - _assert_mock_add_event_call(expected_call_count=1) + _assert_mock_add_event_call(expected_download_failed_event_count=1, err_msg_guid=unique_error_message_guid) self._assert_handler_status(protocol.report_vm_status, "NotReady", 0, "1.0.0") # Change incarnation and then re-check we report error @@ -979,7 +981,7 @@ def _assert_mock_add_event_call(expected_call_count): protocol.update_goal_state() exthandlers_handler.run() - _assert_mock_add_event_call(expected_call_count=2) + _assert_mock_add_event_call(expected_download_failed_event_count=2, err_msg_guid=unique_error_message_guid) self._assert_handler_status(protocol.report_vm_status, "NotReady", 0, "1.0.0") @patch('azurelinuxagent.ga.exthandlers.fileutil')