From 8e3ba2f5934e57b9f87b40fc7afa6f0b47f7b2d0 Mon Sep 17 00:00:00 2001 From: William Pearson Date: Fri, 16 Mar 2018 14:43:36 -0700 Subject: [PATCH 01/14] Add parsing for dependsOn element. Move dependencyLevel parsing from plugin inside plugins to dependsOn Use tuples of handler name and extension name to represent dependencies --- azurelinuxagent/common/protocol/restapi.py | 12 +++++++--- azurelinuxagent/common/protocol/wire.py | 28 ++++++++++++++++++---- tests/data/wire/ext_conf_sequencing.xml | 9 +++++-- tests/ga/test_extension.py | 16 ++++++------- 4 files changed, 47 insertions(+), 18 deletions(-) diff --git a/azurelinuxagent/common/protocol/restapi.py b/azurelinuxagent/common/protocol/restapi.py index 5497e76465..e5f861948f 100644 --- a/azurelinuxagent/common/protocol/restapi.py +++ b/azurelinuxagent/common/protocol/restapi.py @@ -151,12 +151,16 @@ def __init__(self, sequenceNumber=None, publicSettings=None, protectedSettings=None, - certificateThumbprint=None): + certificateThumbprint=None, + dependencyLevel=0, + dependencies=None): self.name = name self.sequenceNumber = sequenceNumber self.publicSettings = publicSettings self.protectedSettings = protectedSettings self.certificateThumbprint = certificateThumbprint + self.dependencyLevel = dependencyLevel + self.dependencies = [] if dependencies is None else dependencies class ExtHandlerProperties(DataContract): @@ -179,9 +183,11 @@ def __init__(self, name=None): self.versionUris = DataContractList(ExtHandlerVersionUri) def sort_key(self): - level = self.properties.dependencyLevel - if level is None: + levels = [e.dependencyLevel for e in self.properties.extensions] + if len(levels) == 0: level = 0 + else: + level = min(levels) # Process uninstall or disabled before enabled, in reverse order # remap 0 to -1, 1 to -2, 2 to -3, etc if self.properties.state != u"enabled": diff --git a/azurelinuxagent/common/protocol/wire.py b/azurelinuxagent/common/protocol/wire.py index 752f62c601..072459eb50 100644 --- a/azurelinuxagent/common/protocol/wire.py +++ b/azurelinuxagent/common/protocol/wire.py @@ -1591,11 +1591,6 @@ def parse_plugin(self, plugin): ext_handler.properties.version = getattrib(plugin, "version") ext_handler.properties.state = getattrib(plugin, "state") - try: - ext_handler.properties.dependencyLevel = int(getattrib(plugin, "dependencyLevel")) - except ValueError: - ext_handler.properties.dependencyLevel = 0 - location = getattrib(plugin, "location") failover_location = getattrib(plugin, "failoverlocation") for uri in [location, failover_location]: @@ -1627,6 +1622,26 @@ def parse_plugin_settings(self, ext_handler, plugin_settings): logger.error("Invalid extension settings") return + depends_on = None + depends_on_node = find(settings[0], "DependsOn") + + depends_on_name = getattrib(depends_on_node, "name") + if depends_on_name == "": + depends_on_name = ext_handler.name + + try: + depends_on_level = int(getattrib(depends_on_node, "dependencyLevel")) + except (ValueError, TypeError): + depends_on_level = 0 + + dependencies = [] + for dependency in findall(depends_on_node, "DependsOnExtension"): + handler_name = gettext(dependency) + ext_name = getattrib(dependency, "name") + if ext_name == "": + ext_name = handler_name + dependencies.append((handler_name, ext_name)) + for plugin_settings_list in runtime_settings["runtimeSettings"]: handler_settings = plugin_settings_list["handlerSettings"] ext = Extension() @@ -1636,6 +1651,9 @@ def parse_plugin_settings(self, ext_handler, plugin_settings): ext.sequenceNumber = seqNo ext.publicSettings = handler_settings.get("publicSettings") ext.protectedSettings = handler_settings.get("protectedSettings") + if ext.name == depends_on_name: + ext.dependencies = dependencies + ext.dependencyLevel = depends_on_level thumbprint = handler_settings.get( "protectedSettingsCertThumbprint") ext.certificateThumbprint = thumbprint diff --git a/tests/data/wire/ext_conf_sequencing.xml b/tests/data/wire/ext_conf_sequencing.xml index 7f4c9bc82d..7ee7c09809 100644 --- a/tests/data/wire/ext_conf_sequencing.xml +++ b/tests/data/wire/ext_conf_sequencing.xml @@ -15,14 +15,19 @@ - - + + + + {"runtimeSettings":[{"handlerSettings":{"protectedSettingsCertThumbprint":"4037FBF5F1F3014F99B5D6C7799E9B20E6871CB3","protectedSettings":"MIICWgYJK","publicSettings":{"foo":"bar"}}}]} + + OSTCExtensions.ExampleHandlerLinux + {"runtimeSettings":[{"handlerSettings":{"protectedSettingsCertThumbprint":"4037FBF5F1F3014F99B5D6C7799E9B20E6871CB3","protectedSettings":"MIICWgYJK","publicSettings":{"foo":"bar"}}}]} diff --git a/tests/ga/test_extension.py b/tests/ga/test_extension.py index 8940419d3a..1ad53a6ffd 100644 --- a/tests/ga/test_extension.py +++ b/tests/ga/test_extension.py @@ -409,8 +409,8 @@ def test_ext_handler_sequencing(self, *args): self.assertTrue(exthandlers_handler.ext_handlers is not None) self.assertTrue(exthandlers_handler.ext_handlers.extHandlers is not None) self.assertEqual(len(exthandlers_handler.ext_handlers.extHandlers), 2) - self.assertEqual(exthandlers_handler.ext_handlers.extHandlers[0].properties.dependencyLevel, 1) - self.assertEqual(exthandlers_handler.ext_handlers.extHandlers[1].properties.dependencyLevel, 2) + self.assertEqual(exthandlers_handler.ext_handlers.extHandlers[0].properties.extensions[0].dependencyLevel, 1) + self.assertEqual(exthandlers_handler.ext_handlers.extHandlers[1].properties.extensions[0].dependencyLevel, 2) #Test goal state not changed exthandlers_handler.run() @@ -431,8 +431,8 @@ def test_ext_handler_sequencing(self, *args): self._assert_ext_status(protocol.report_ext_status, "success", 1) self.assertEqual(len(exthandlers_handler.ext_handlers.extHandlers), 2) - self.assertEqual(exthandlers_handler.ext_handlers.extHandlers[0].properties.dependencyLevel, 3) - self.assertEqual(exthandlers_handler.ext_handlers.extHandlers[1].properties.dependencyLevel, 4) + self.assertEqual(exthandlers_handler.ext_handlers.extHandlers[0].properties.extensions[0].dependencyLevel, 3) + self.assertEqual(exthandlers_handler.ext_handlers.extHandlers[1].properties.extensions[0].dependencyLevel, 4) #Test disable test_data.goal_state = test_data.goal_state.replace("2<", @@ -443,8 +443,8 @@ def test_ext_handler_sequencing(self, *args): 1, "1.0.0", expected_handler_name="OSTCExtensions.OtherExampleHandlerLinux") self.assertEqual(len(exthandlers_handler.ext_handlers.extHandlers), 2) - self.assertEqual(exthandlers_handler.ext_handlers.extHandlers[0].properties.dependencyLevel, 4) - self.assertEqual(exthandlers_handler.ext_handlers.extHandlers[1].properties.dependencyLevel, 3) + self.assertEqual(exthandlers_handler.ext_handlers.extHandlers[0].properties.extensions[0].dependencyLevel, 4) + self.assertEqual(exthandlers_handler.ext_handlers.extHandlers[1].properties.extensions[0].dependencyLevel, 3) #Test uninstall test_data.goal_state = test_data.goal_state.replace("3<", @@ -457,8 +457,8 @@ def test_ext_handler_sequencing(self, *args): exthandlers_handler.run() self._assert_no_handler_status(protocol.report_vm_status) self.assertEqual(len(exthandlers_handler.ext_handlers.extHandlers), 2) - self.assertEqual(exthandlers_handler.ext_handlers.extHandlers[0].properties.dependencyLevel, 6) - self.assertEqual(exthandlers_handler.ext_handlers.extHandlers[1].properties.dependencyLevel, 5) + self.assertEqual(exthandlers_handler.ext_handlers.extHandlers[0].properties.extensions[0].dependencyLevel, 6) + self.assertEqual(exthandlers_handler.ext_handlers.extHandlers[1].properties.extensions[0].dependencyLevel, 5) def test_ext_handler_rollingupgrade(self, *args): test_data = WireProtocolData(DATA_FILE_EXT_ROLLINGUPGRADE) From d32fafd49897b1c33168695843058e6247238f25 Mon Sep 17 00:00:00 2001 From: William Pearson Date: Fri, 16 Mar 2018 16:54:57 -0700 Subject: [PATCH 02/14] Make extensions wait for dependencies An extension's dependencies must report a success status before the agent should install and enable an extension. --- azurelinuxagent/common/protocol/wire.py | 30 ++++++++++++++++++++++ azurelinuxagent/ga/exthandlers.py | 34 +++++++++++++++++++++---- tests/data/wire/ext_conf_sequencing.xml | 2 +- 3 files changed, 60 insertions(+), 6 deletions(-) diff --git a/azurelinuxagent/common/protocol/wire.py b/azurelinuxagent/common/protocol/wire.py index 072459eb50..cdd6ce18e5 100644 --- a/azurelinuxagent/common/protocol/wire.py +++ b/azurelinuxagent/common/protocol/wire.py @@ -1530,6 +1530,21 @@ def write_to_tmp_file(self, index, suffix, buf): return file_name +class Dependency(object): + """ + Represents an extension upon which another extension is dependent + handler - the ExtHandler of the dependency + exts - the Extension(s) of the dependency + timeout - the timeout for the extension to get to the Ready state + timeout is expressed in minutes. + """ + + def __init__(self, handler=None, exts=None, timeout=None): + self.handler = handler + self.exts = exts + self.timeout = timeout + + class ExtensionsConfig(object): """ parse ExtensionsConfig, downloading and unpacking them to /var/lib/waagent. @@ -1576,6 +1591,21 @@ def parse(self, xml_text): self.ext_handlers.extHandlers.append(ext_handler) self.parse_plugin_settings(ext_handler, plugin_settings) + for ext_handler in self.ext_handlers.extHandlers: + for extension in ext_handler.properties.extensions: + resolved_dependencies = [] + for (handler_name, ext_name) in extension.dependencies: + # Gracefully handle garbage cases where handler name does + # not exist or is not unique. Same for the extension name. + handlers = [handler for handler in self.ext_handlers.extHandlers + if handler_name == handler.name] + for handler in handlers: + exts = [e for e in handler.properties.extensions + if ext_name == e.name] + resolved_dependencies.append( + Dependency(handler=handler, exts=exts)) + extension.dependencies = resolved_dependencies + self.status_upload_blob = findtext(xml_doc, "StatusUploadBlob") self.artifacts_profile_blob = findtext(xml_doc, "InVMArtifactsProfileBlob") diff --git a/azurelinuxagent/ga/exthandlers.py b/azurelinuxagent/ga/exthandlers.py index 497f8d9776..73118552b8 100644 --- a/azurelinuxagent/ga/exthandlers.py +++ b/azurelinuxagent/ga/exthandlers.py @@ -120,9 +120,7 @@ def parse_ext_status(ext_status, data): ext_status.code = status_data.get('code', 0) formatted_message = status_data.get('formattedMessage') ext_status.message = parse_formatted_message(formatted_message) - substatus_list = status_data.get('substatus') - if substatus_list is None: - return + substatus_list = status_data.get('substatus', []) for substatus in substatus_list: if substatus is not None: ext_status.substatusList.append(parse_ext_substatus(substatus)) @@ -312,9 +310,35 @@ def handle_ext_handlers(self, etag=None): self.ext_handlers.extHandlers.sort(key=operator.methodcaller('sort_key')) for ext_handler in self.ext_handlers.extHandlers: - # TODO: handle install in sequence, enable in parallel + self.wait_on_ext_handler_dependencies(ext_handler) self.handle_ext_handler(ext_handler, etag) - + + def wait_on_ext_handler_dependencies(self, ext_handler): + dependencies = sum([e.dependencies for e in ext_handler.properties.extensions], []) + for dependency in dependencies: + handler_i = ExtHandlerInstance(dependency.handler, self.protocol) + timeout = 90 if dependency.timeout is None else dependency.timeout + timeout_delta = datetime.timedelta(seconds=(timeout * 60)) + begin = datetime.datetime.utcnow() + for ext in dependency.exts: + while timeout_delta > (datetime.datetime.utcnow() - begin): + status = handler_i.collect_ext_status(ext) + if status is None: + break + all_success = status.status == "success" + for substatus in status.substatusList: + if substatus.status != "success": + all_success = False + break + if all_success: + break + time.sleep(10) + if (datetime.datetime.utcnow() - begin) > timeout_delta: + raise ExtensionError("Timeout waiting for success status " + "from dependency {}/{} for {}", + dependency.handler, ext, + ext_handler.name) + def handle_ext_handler(self, ext_handler, etag): ext_handler_i = ExtHandlerInstance(ext_handler, self.protocol) diff --git a/tests/data/wire/ext_conf_sequencing.xml b/tests/data/wire/ext_conf_sequencing.xml index 7ee7c09809..70c79fe534 100644 --- a/tests/data/wire/ext_conf_sequencing.xml +++ b/tests/data/wire/ext_conf_sequencing.xml @@ -21,12 +21,12 @@ + OSTCExtensions.OtherExampleHandlerLinux {"runtimeSettings":[{"handlerSettings":{"protectedSettingsCertThumbprint":"4037FBF5F1F3014F99B5D6C7799E9B20E6871CB3","protectedSettings":"MIICWgYJK","publicSettings":{"foo":"bar"}}}]} - OSTCExtensions.ExampleHandlerLinux {"runtimeSettings":[{"handlerSettings":{"protectedSettingsCertThumbprint":"4037FBF5F1F3014F99B5D6C7799E9B20E6871CB3","protectedSettings":"MIICWgYJK","publicSettings":{"foo":"bar"}}}]} From 77bab20ba6cfb01f4d52f426918c44f9aec9221e Mon Sep 17 00:00:00 2001 From: William Pearson Date: Fri, 30 Mar 2018 13:42:03 -0700 Subject: [PATCH 03/14] Add unit tests for extensions status wait method --- azurelinuxagent/ga/exthandlers.py | 7 ++-- tests/ga/test_extension.py | 60 +++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 3 deletions(-) diff --git a/azurelinuxagent/ga/exthandlers.py b/azurelinuxagent/ga/exthandlers.py index 73118552b8..7fa6bc1cf4 100644 --- a/azurelinuxagent/ga/exthandlers.py +++ b/azurelinuxagent/ga/exthandlers.py @@ -335,9 +335,10 @@ def wait_on_ext_handler_dependencies(self, ext_handler): time.sleep(10) if (datetime.datetime.utcnow() - begin) > timeout_delta: raise ExtensionError("Timeout waiting for success status " - "from dependency {}/{} for {}", - dependency.handler, ext, - ext_handler.name) + "from dependency {}/{} for {}" + "status was: {}".format( + dependency.handler.name, ext.name, + ext_handler.name, status)) def handle_ext_handler(self, ext_handler, etag): ext_handler_i = ExtHandlerInstance(ext_handler, self.protocol) diff --git a/tests/ga/test_extension.py b/tests/ga/test_extension.py index 1ad53a6ffd..c49d882041 100644 --- a/tests/ga/test_extension.py +++ b/tests/ga/test_extension.py @@ -18,6 +18,9 @@ import os.path from tests.protocol.mockwiredata import * + +from azurelinuxagent.common.protocol.wire import Dependency +from azurelinuxagent.common.protocol.restapi import Extension from azurelinuxagent.ga.exthandlers import * from azurelinuxagent.common.protocol.wire import WireProtocol @@ -666,6 +669,63 @@ def test_ext_handler_no_reporting_status(self, *args): self._assert_handler_status(protocol.report_vm_status, "Ready", 1, "1.0.0") self._assert_ext_status(protocol.report_ext_status, "error", 0) + def _test_wait_on_ext_handler_dependencies(self, exthandlers_handler, timeout=None): + handler_name = "Handler" + exthandler = ExtHandler(name=handler_name) + dependency_ext = Extension(name=handler_name) + extension = Extension(name=handler_name) + + dependency = Dependency(handler=exthandler, exts=[ dependency_ext ], timeout=timeout) + extension.dependencies.append(dependency) + exthandler.properties.extensions.append(extension) + + fun = exthandlers_handler.wait_on_ext_handler_dependencies + if timeout is None: + fun(exthandler) + else: + expected_msg = "Timeout.*{0}/{1} for {2}".format(handler_name, handler_name, handler_name) + self.assertRaisesRegexp(ExtensionError, expected_msg, fun, exthandler) + + def test_wait_on_ext_handler_dependencies_none(self, *args): + test_data = WireProtocolData(DATA_FILE) + exthandlers_handler, protocol = self._create_mock(test_data, *args) + + ExtHandlerInstance.collect_ext_status = MagicMock(return_value=None) + self._test_wait_on_ext_handler_dependencies(exthandlers_handler) + + def test_wait_on_ext_handler_dependencies_success_status(self, *args): + test_data = WireProtocolData(DATA_FILE) + exthandlers_handler, protocol = self._create_mock(test_data, *args) + + status = ExtensionStatus(seq_no=0) + status.status = "success" + + ExtHandlerInstance.collect_ext_status = MagicMock(return_value=status) + self._test_wait_on_ext_handler_dependencies(exthandlers_handler) + + def test_wait_on_ext_handler_dependencies_success_status_with_substatus(self, *args): + test_data = WireProtocolData(DATA_FILE) + exthandlers_handler, protocol = self._create_mock(test_data, *args) + + status = ExtensionStatus(seq_no=0) + status.status = "success" + substatus = ExtensionSubStatus() + substatus.status = "success" + status.substatusList.append(substatus) + + ExtHandlerInstance.collect_ext_status = MagicMock(return_value=status) + self._test_wait_on_ext_handler_dependencies(exthandlers_handler) + + def test_wait_on_ext_handler_dependencies_timeout(self, *args): + test_data = WireProtocolData(DATA_FILE) + exthandlers_handler, protocol = self._create_mock(test_data, *args) + + status = ExtensionStatus(seq_no=0) + status.status = "error" + + ExtHandlerInstance.collect_ext_status = MagicMock(return_value=status) + self._test_wait_on_ext_handler_dependencies(exthandlers_handler, timeout=1) + def test_ext_handler_version_decide_autoupgrade_internalversion(self, *args): for internal in [False, True]: for autoupgrade in [False, True]: From 670b5bd24af33e388bae4ebe407fd5c18ed35cd5 Mon Sep 17 00:00:00 2001 From: William Pearson Date: Fri, 30 Mar 2018 16:05:13 -0700 Subject: [PATCH 04/14] Refactor dependency resolution --- azurelinuxagent/common/protocol/restapi.py | 32 ++++++++++++++++++++++ azurelinuxagent/common/protocol/wire.py | 28 +------------------ 2 files changed, 33 insertions(+), 27 deletions(-) diff --git a/azurelinuxagent/common/protocol/restapi.py b/azurelinuxagent/common/protocol/restapi.py index e5f861948f..ad4b011e97 100644 --- a/azurelinuxagent/common/protocol/restapi.py +++ b/azurelinuxagent/common/protocol/restapi.py @@ -145,6 +145,21 @@ def __init__(self): self.vmAgentManifests = DataContractList(VMAgentManifest) +class Dependency(object): + """ + Represents an extension upon which another extension is dependent + handler - the ExtHandler of the dependency + exts - the Extension(s) of the dependency + timeout - the timeout for the extension to get to the Ready state + timeout is expressed in minutes. + """ + + def __init__(self, handler=None, exts=None, timeout=None): + self.handler = handler + self.exts = exts + self.timeout = timeout + + class Extension(DataContract): def __init__(self, name=None, @@ -162,6 +177,23 @@ def __init__(self, self.dependencyLevel = dependencyLevel self.dependencies = [] if dependencies is None else dependencies + def resolve_dependencies(self, ext_handlers): + resolved_dependencies = [] + for (handler_name, ext_name) in self.dependencies: + # Gracefully handle garbage cases where handler name does + # not exist or is not unique. Same for the extension name. + for handler in ext_handlers: + if handler_name != handler.name: + continue + resolved_dependencies.append( + Dependency( + handler = handler, + exts = [ext for ext in handler.properties.extensions + if ext.name == ext_name] + ) + ) + self.dependencies = resolved_dependencies + class ExtHandlerProperties(DataContract): def __init__(self): diff --git a/azurelinuxagent/common/protocol/wire.py b/azurelinuxagent/common/protocol/wire.py index cdd6ce18e5..68be575e9d 100644 --- a/azurelinuxagent/common/protocol/wire.py +++ b/azurelinuxagent/common/protocol/wire.py @@ -1530,21 +1530,6 @@ def write_to_tmp_file(self, index, suffix, buf): return file_name -class Dependency(object): - """ - Represents an extension upon which another extension is dependent - handler - the ExtHandler of the dependency - exts - the Extension(s) of the dependency - timeout - the timeout for the extension to get to the Ready state - timeout is expressed in minutes. - """ - - def __init__(self, handler=None, exts=None, timeout=None): - self.handler = handler - self.exts = exts - self.timeout = timeout - - class ExtensionsConfig(object): """ parse ExtensionsConfig, downloading and unpacking them to /var/lib/waagent. @@ -1593,18 +1578,7 @@ def parse(self, xml_text): for ext_handler in self.ext_handlers.extHandlers: for extension in ext_handler.properties.extensions: - resolved_dependencies = [] - for (handler_name, ext_name) in extension.dependencies: - # Gracefully handle garbage cases where handler name does - # not exist or is not unique. Same for the extension name. - handlers = [handler for handler in self.ext_handlers.extHandlers - if handler_name == handler.name] - for handler in handlers: - exts = [e for e in handler.properties.extensions - if ext_name == e.name] - resolved_dependencies.append( - Dependency(handler=handler, exts=exts)) - extension.dependencies = resolved_dependencies + extension.resolve_dependencies(self.ext_handlers.extHandlers) self.status_upload_blob = findtext(xml_doc, "StatusUploadBlob") self.artifacts_profile_blob = findtext(xml_doc, "InVMArtifactsProfileBlob") From ded2e0a0ad8b71454ba403b25c5ffb6777d34ebc Mon Sep 17 00:00:00 2001 From: William Pearson Date: Tue, 3 Apr 2018 14:18:30 -0700 Subject: [PATCH 05/14] Fix up unit tests for dependencies --- azurelinuxagent/ga/exthandlers.py | 1 + tests/ga/test_extension.py | 45 ++++++++++++++++++++++++++++--- 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/azurelinuxagent/ga/exthandlers.py b/azurelinuxagent/ga/exthandlers.py index 7fa6bc1cf4..c71a52f246 100644 --- a/azurelinuxagent/ga/exthandlers.py +++ b/azurelinuxagent/ga/exthandlers.py @@ -321,6 +321,7 @@ def wait_on_ext_handler_dependencies(self, ext_handler): timeout_delta = datetime.timedelta(seconds=(timeout * 60)) begin = datetime.datetime.utcnow() for ext in dependency.exts: + status = None while timeout_delta > (datetime.datetime.utcnow() - begin): status = handler_i.collect_ext_status(ext) if status is None: diff --git a/tests/ga/test_extension.py b/tests/ga/test_extension.py index c49d882041..3ae9f45796 100644 --- a/tests/ga/test_extension.py +++ b/tests/ga/test_extension.py @@ -414,6 +414,10 @@ def test_ext_handler_sequencing(self, *args): self.assertEqual(len(exthandlers_handler.ext_handlers.extHandlers), 2) self.assertEqual(exthandlers_handler.ext_handlers.extHandlers[0].properties.extensions[0].dependencyLevel, 1) self.assertEqual(exthandlers_handler.ext_handlers.extHandlers[1].properties.extensions[0].dependencyLevel, 2) + self.assertEqual(exthandlers_handler.ext_handlers.extHandlers[1].properties.extensions[0].dependencies[0].handler, + exthandlers_handler.ext_handlers.extHandlers[0]) + self.assertEqual(exthandlers_handler.ext_handlers.extHandlers[1].properties.extensions[0].dependencies[0].exts[0], + exthandlers_handler.ext_handlers.extHandlers[0].properties.extensions[0]) #Test goal state not changed exthandlers_handler.run() @@ -669,12 +673,42 @@ def test_ext_handler_no_reporting_status(self, *args): self._assert_handler_status(protocol.report_vm_status, "Ready", 1, "1.0.0") self._assert_ext_status(protocol.report_ext_status, "error", 0) + def test_wait_on_ext_handler_dependencies_empty_exts(self, *args): + test_data = WireProtocolData(DATA_FILE) + exthandlers_handler, protocol = self._create_mock(test_data, *args) + + handler_name = "Handler" + exthandler = ExtHandler(name=handler_name) + extension = Extension(name=handler_name) + + dependency = Dependency(handler=exthandler, exts=[]) + extension.dependencies.append(dependency) + exthandler.properties.extensions.append(extension) + + exthandlers_handler.wait_on_ext_handler_dependencies(exthandler) + + def test_wait_on_ext_handler_dependencies_two_exts(self, *args): + test_data = WireProtocolData(DATA_FILE) + exthandlers_handler, protocol = self._create_mock(test_data, *args) + + handler_name = "Handler" + exthandler = ExtHandler(name=handler_name) + dependency_exts = [Extension(name=handler_name), Extension(name=handler_name)] + extension = Extension(name=handler_name) + + dependency = Dependency(handler=exthandler, exts=dependency_exts) + extension.dependencies.append(dependency) + exthandler.properties.extensions.append(extension) + + ExtHandlerInstance.collect_ext_status = MagicMock(return_value=None) + exthandlers_handler.wait_on_ext_handler_dependencies(exthandler) + def _test_wait_on_ext_handler_dependencies(self, exthandlers_handler, timeout=None): handler_name = "Handler" exthandler = ExtHandler(name=handler_name) dependency_ext = Extension(name=handler_name) extension = Extension(name=handler_name) - + dependency = Dependency(handler=exthandler, exts=[ dependency_ext ], timeout=timeout) extension.dependencies.append(dependency) exthandler.properties.extensions.append(extension) @@ -684,8 +718,11 @@ def _test_wait_on_ext_handler_dependencies(self, exthandlers_handler, timeout=No fun(exthandler) else: expected_msg = "Timeout.*{0}/{1} for {2}".format(handler_name, handler_name, handler_name) - self.assertRaisesRegexp(ExtensionError, expected_msg, fun, exthandler) - + try: + self.assertRaisesRegexp(ExtensionError, expected_msg, fun, exthandler) + except AttributeError: + pass # Python 2.6 doesn't like assertRaisesRegexp + def test_wait_on_ext_handler_dependencies_none(self, *args): test_data = WireProtocolData(DATA_FILE) exthandlers_handler, protocol = self._create_mock(test_data, *args) @@ -724,7 +761,7 @@ def test_wait_on_ext_handler_dependencies_timeout(self, *args): status.status = "error" ExtHandlerInstance.collect_ext_status = MagicMock(return_value=status) - self._test_wait_on_ext_handler_dependencies(exthandlers_handler, timeout=1) + self._test_wait_on_ext_handler_dependencies(exthandlers_handler, timeout=0) def test_ext_handler_version_decide_autoupgrade_internalversion(self, *args): for internal in [False, True]: From 7056831d89bf940c817e539e6997defdc29054bf Mon Sep 17 00:00:00 2001 From: William Pearson Date: Thu, 19 Apr 2018 15:09:51 -0700 Subject: [PATCH 06/14] Address PR comments --- azurelinuxagent/common/protocol/restapi.py | 2 +- azurelinuxagent/ga/exthandlers.py | 43 ++++++++++++---------- tests/ga/test_extension.py | 8 ++++ 3 files changed, 32 insertions(+), 21 deletions(-) diff --git a/azurelinuxagent/common/protocol/restapi.py b/azurelinuxagent/common/protocol/restapi.py index ad4b011e97..9d6e8d746d 100644 --- a/azurelinuxagent/common/protocol/restapi.py +++ b/azurelinuxagent/common/protocol/restapi.py @@ -156,7 +156,7 @@ class Dependency(object): def __init__(self, handler=None, exts=None, timeout=None): self.handler = handler - self.exts = exts + self.exts = [] if exts is None else exts self.timeout = timeout diff --git a/azurelinuxagent/ga/exthandlers.py b/azurelinuxagent/ga/exthandlers.py index c71a52f246..90ddb20d69 100644 --- a/azurelinuxagent/ga/exthandlers.py +++ b/azurelinuxagent/ga/exthandlers.py @@ -318,28 +318,20 @@ def wait_on_ext_handler_dependencies(self, ext_handler): for dependency in dependencies: handler_i = ExtHandlerInstance(dependency.handler, self.protocol) timeout = 90 if dependency.timeout is None else dependency.timeout - timeout_delta = datetime.timedelta(seconds=(timeout * 60)) + timeout = datetime.timedelta(seconds=(timeout * 60)) begin = datetime.datetime.utcnow() for ext in dependency.exts: - status = None - while timeout_delta > (datetime.datetime.utcnow() - begin): - status = handler_i.collect_ext_status(ext) - if status is None: - break - all_success = status.status == "success" - for substatus in status.substatusList: - if substatus.status != "success": - all_success = False - break - if all_success: - break - time.sleep(10) - if (datetime.datetime.utcnow() - begin) > timeout_delta: - raise ExtensionError("Timeout waiting for success status " - "from dependency {}/{} for {}" - "status was: {}".format( - dependency.handler.name, ext.name, - ext_handler.name, status)) + success_status, status = handler_i.is_ext_status_success(ext) + while not success_status: + if (datetime.datetime.utcnow() - begin) > timeout: + raise ExtensionError("Timeout waiting for success status " + "from dependency {}/{} for {}" + "status was: {}".format( + dependency.handler.name, ext.name, + ext_handler.name, status)) + else: + time.sleep(10) + success_status, status = handler_i.is_ext_status_success(ext) def handle_ext_handler(self, ext_handler, etag): ext_handler_i = ExtHandlerInstance(ext_handler, self.protocol) @@ -897,6 +889,17 @@ def collect_ext_status(self, ext): return ext_status + def is_ext_status_success(self, ext): + status = self.collect_ext_status(ext) + if status is None: + return (True, status) + if status.status != "success": + return (False, status) + for substatus in status.substatusList: + if substatus.status != "success": + return (False, status) + return (True, status) + def report_ext_status(self): active_exts = [] # TODO Refactor or remove this common code pattern (for each extension subordinate to an ext_handler, do X). diff --git a/tests/ga/test_extension.py b/tests/ga/test_extension.py index 3ae9f45796..5024764588 100644 --- a/tests/ga/test_extension.py +++ b/tests/ga/test_extension.py @@ -429,6 +429,7 @@ def test_ext_handler_sequencing(self, *args): "2<") test_data.ext_conf = test_data.ext_conf.replace("seqNo=\"0\"", "seqNo=\"1\"") + # Swap the dependency ordering test_data.ext_conf = test_data.ext_conf.replace("dependencyLevel=\"2\"", "dependencyLevel=\"3\"") test_data.ext_conf = test_data.ext_conf.replace("dependencyLevel=\"1\"", @@ -442,6 +443,9 @@ def test_ext_handler_sequencing(self, *args): self.assertEqual(exthandlers_handler.ext_handlers.extHandlers[1].properties.extensions[0].dependencyLevel, 4) #Test disable + # In the case of disable, the last extension to be enabled should be + # the first extension disabled. The first extension enabled should be + # the last one disabled. test_data.goal_state = test_data.goal_state.replace("2<", "3<") test_data.ext_conf = test_data.ext_conf.replace("enabled", "disabled") @@ -454,9 +458,13 @@ def test_ext_handler_sequencing(self, *args): self.assertEqual(exthandlers_handler.ext_handlers.extHandlers[1].properties.extensions[0].dependencyLevel, 3) #Test uninstall + # In the case of uninstall, the last extension to be installed should be + # the first extension uninstalled. The first extension installed + # should be the last one uninstalled. test_data.goal_state = test_data.goal_state.replace("3<", "4<") test_data.ext_conf = test_data.ext_conf.replace("disabled", "uninstall") + # Swap the dependency ordering AGAIN test_data.ext_conf = test_data.ext_conf.replace("dependencyLevel=\"3\"", "dependencyLevel=\"6\"") test_data.ext_conf = test_data.ext_conf.replace("dependencyLevel=\"4\"", From ca0453ebca97fc4e750087757784ffa37511643a Mon Sep 17 00:00:00 2001 From: William Pearson Date: Thu, 3 May 2018 16:38:24 -0700 Subject: [PATCH 07/14] Minor adjustments to schema --- azurelinuxagent/common/protocol/wire.py | 4 ++-- tests/data/wire/ext_conf_sequencing.xml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/azurelinuxagent/common/protocol/wire.py b/azurelinuxagent/common/protocol/wire.py index 68be575e9d..751cd302b8 100644 --- a/azurelinuxagent/common/protocol/wire.py +++ b/azurelinuxagent/common/protocol/wire.py @@ -1640,8 +1640,8 @@ def parse_plugin_settings(self, ext_handler, plugin_settings): dependencies = [] for dependency in findall(depends_on_node, "DependsOnExtension"): - handler_name = gettext(dependency) - ext_name = getattrib(dependency, "name") + handler_name = getattrib(dependency, "handler") + ext_name = getattrib(dependency, "extension") if ext_name == "": ext_name = handler_name dependencies.append((handler_name, ext_name)) diff --git a/tests/data/wire/ext_conf_sequencing.xml b/tests/data/wire/ext_conf_sequencing.xml index 70c79fe534..3120a979b7 100644 --- a/tests/data/wire/ext_conf_sequencing.xml +++ b/tests/data/wire/ext_conf_sequencing.xml @@ -21,7 +21,7 @@ - OSTCExtensions.OtherExampleHandlerLinux + {"runtimeSettings":[{"handlerSettings":{"protectedSettingsCertThumbprint":"4037FBF5F1F3014F99B5D6C7799E9B20E6871CB3","protectedSettings":"MIICWgYJK","publicSettings":{"foo":"bar"}}}]} From 8a0c6ee49cfad273cfcce083b4c6163d71334afb Mon Sep 17 00:00:00 2001 From: William Pearson Date: Thu, 3 May 2018 16:59:29 -0700 Subject: [PATCH 08/14] Adjust dependency timeout waiting The platform gives up to 90 minutes of processing for each dependency level. The first level should have no dependencies, so any waiting should return a status right away. The dependencies of each subsequent level are allocated an additional 90 minutes of processing time to produce a valid status. If the agent has been processing extensions for more than (90 minutes) * (number of levels processed so far) then it should timeout. --- azurelinuxagent/ga/exthandlers.py | 20 ++++++++++++++------ tests/ga/test_extension.py | 8 ++++---- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/azurelinuxagent/ga/exthandlers.py b/azurelinuxagent/ga/exthandlers.py index 90ddb20d69..24926227d9 100644 --- a/azurelinuxagent/ga/exthandlers.py +++ b/azurelinuxagent/ga/exthandlers.py @@ -66,6 +66,7 @@ HANDLER_PKG_EXT = ".zip" HANDLER_PKG_PATTERN = re.compile(HANDLER_PATTERN + r"\.zip$", re.IGNORECASE) +DEFAULT_EXT_TIMEOUT_MINUTES = 90 def validate_has_key(obj, key, fullname): if key not in obj: @@ -308,22 +309,29 @@ def handle_ext_handlers(self, etag=None): logger.info("Extension handling is on hold") return + dep_level = None + deps_wait_until = None + handlers_wait_until = datetime.datetime.utcnow() + self.ext_handlers.extHandlers.sort(key=operator.methodcaller('sort_key')) for ext_handler in self.ext_handlers.extHandlers: - self.wait_on_ext_handler_dependencies(ext_handler) + if dep_level != ext_handler.sort_key(): + dep_level = ext_handler.sort_key() + deps_wait_until = handlers_wait_until + handlers_wait_until += datetime.timedelta(seconds=(DEFAULT_EXT_TIMEOUT_MINUTES * 60)) + self.wait_on_ext_handler_dependencies(ext_handler, deps_wait_until) self.handle_ext_handler(ext_handler, etag) - def wait_on_ext_handler_dependencies(self, ext_handler): + def wait_on_ext_handler_dependencies(self, ext_handler, wait_until): dependencies = sum([e.dependencies for e in ext_handler.properties.extensions], []) for dependency in dependencies: handler_i = ExtHandlerInstance(dependency.handler, self.protocol) - timeout = 90 if dependency.timeout is None else dependency.timeout - timeout = datetime.timedelta(seconds=(timeout * 60)) - begin = datetime.datetime.utcnow() + if dependency.timeout is not None and dependency.timeout < DEFAULT_EXT_TIMEOUT_MINUTES: + wait_until -= datetime.timedelta(seconds=( (DEFAULT_EXT_TIMEOUT_MINUTES - dependency.timeout) * 60)) for ext in dependency.exts: success_status, status = handler_i.is_ext_status_success(ext) while not success_status: - if (datetime.datetime.utcnow() - begin) > timeout: + if datetime.datetime.utcnow() > wait_until: raise ExtensionError("Timeout waiting for success status " "from dependency {}/{} for {}" "status was: {}".format( diff --git a/tests/ga/test_extension.py b/tests/ga/test_extension.py index 5024764588..57e0aa1e93 100644 --- a/tests/ga/test_extension.py +++ b/tests/ga/test_extension.py @@ -693,7 +693,7 @@ def test_wait_on_ext_handler_dependencies_empty_exts(self, *args): extension.dependencies.append(dependency) exthandler.properties.extensions.append(extension) - exthandlers_handler.wait_on_ext_handler_dependencies(exthandler) + exthandlers_handler.wait_on_ext_handler_dependencies(exthandler, datetime.datetime.utcnow()) def test_wait_on_ext_handler_dependencies_two_exts(self, *args): test_data = WireProtocolData(DATA_FILE) @@ -709,7 +709,7 @@ def test_wait_on_ext_handler_dependencies_two_exts(self, *args): exthandler.properties.extensions.append(extension) ExtHandlerInstance.collect_ext_status = MagicMock(return_value=None) - exthandlers_handler.wait_on_ext_handler_dependencies(exthandler) + exthandlers_handler.wait_on_ext_handler_dependencies(exthandler, datetime.datetime.utcnow()) def _test_wait_on_ext_handler_dependencies(self, exthandlers_handler, timeout=None): handler_name = "Handler" @@ -723,11 +723,11 @@ def _test_wait_on_ext_handler_dependencies(self, exthandlers_handler, timeout=No fun = exthandlers_handler.wait_on_ext_handler_dependencies if timeout is None: - fun(exthandler) + fun(exthandler, datetime.datetime.utcnow()) else: expected_msg = "Timeout.*{0}/{1} for {2}".format(handler_name, handler_name, handler_name) try: - self.assertRaisesRegexp(ExtensionError, expected_msg, fun, exthandler) + self.assertRaisesRegexp(ExtensionError, expected_msg, fun, exthandler, datetime.datetime.utcnow()) except AttributeError: pass # Python 2.6 doesn't like assertRaisesRegexp From 3cff99a51cd0039f49084f57f01d61bf94d086ca Mon Sep 17 00:00:00 2001 From: Sirajudeen Sahul Hameed Date: Mon, 20 Aug 2018 15:43:40 -0700 Subject: [PATCH 09/14] Adding implementation of extension sequencing - addressing PR comments --- azurelinuxagent/common/protocol/restapi.py | 12 +-- azurelinuxagent/ga/exthandlers.py | 72 ++++++++++++----- tests/ga/test_extension.py | 90 ++++++++++++++++------ 3 files changed, 126 insertions(+), 48 deletions(-) diff --git a/azurelinuxagent/common/protocol/restapi.py b/azurelinuxagent/common/protocol/restapi.py index 9d6e8d746d..afd8551058 100644 --- a/azurelinuxagent/common/protocol/restapi.py +++ b/azurelinuxagent/common/protocol/restapi.py @@ -149,15 +149,12 @@ class Dependency(object): """ Represents an extension upon which another extension is dependent handler - the ExtHandler of the dependency - exts - the Extension(s) of the dependency - timeout - the timeout for the extension to get to the Ready state - timeout is expressed in minutes. + ext - the Extension of the dependency """ - def __init__(self, handler=None, exts=None, timeout=None): + def __init__(self, handler=None, ext=None): self.handler = handler - self.exts = [] if exts is None else exts - self.timeout = timeout + self.ext = ext class Extension(DataContract): @@ -188,8 +185,7 @@ def resolve_dependencies(self, ext_handlers): resolved_dependencies.append( Dependency( handler = handler, - exts = [ext for ext in handler.properties.extensions - if ext.name == ext_name] + ext = next((ext for ext in handler.properties.extensions if ext.name == ext_name), None) ) ) self.dependencies = resolved_dependencies diff --git a/azurelinuxagent/ga/exthandlers.py b/azurelinuxagent/ga/exthandlers.py index 24926227d9..31b3c6f397 100644 --- a/azurelinuxagent/ga/exthandlers.py +++ b/azurelinuxagent/ga/exthandlers.py @@ -312,34 +312,70 @@ def handle_ext_handlers(self, etag=None): dep_level = None deps_wait_until = None handlers_wait_until = datetime.datetime.utcnow() + skipped_handler_names = [] self.ext_handlers.extHandlers.sort(key=operator.methodcaller('sort_key')) for ext_handler in self.ext_handlers.extHandlers: + # Use updated time limit for each dependency level if dep_level != ext_handler.sort_key(): dep_level = ext_handler.sort_key() deps_wait_until = handlers_wait_until - handlers_wait_until += datetime.timedelta(seconds=(DEFAULT_EXT_TIMEOUT_MINUTES * 60)) - self.wait_on_ext_handler_dependencies(ext_handler, deps_wait_until) - self.handle_ext_handler(ext_handler, etag) + handlers_wait_until += datetime.timedelta(minutes=DEFAULT_EXT_TIMEOUT_MINUTES) - def wait_on_ext_handler_dependencies(self, ext_handler, wait_until): + # Make sure the previous level extensions are installed. + # If installed successfully, continue with the current handler. Otherwise, skip it. + if self.wait_on_ext_handler_dependencies(ext_handler, deps_wait_until, skipped_handler_names): + self.handle_ext_handler(ext_handler, etag) + else: + skipped_handler_names.append(ext_handler.name) + + ''' + Collect all the dependencies in the given handler. + Check the status of the extension in each dependency. + Wait until it becomes success or times out. + Return True if the dependencies are installed successfully. False if timed out. + ''' + def wait_on_ext_handler_dependencies(self, ext_handler, wait_until, skipped_handler_names=[]): dependencies = sum([e.dependencies for e in ext_handler.properties.extensions], []) + + # Check if any of the dependencies are already skipped. + # This allows us to fail fast in that case. for dependency in dependencies: + if dependency.handler.name in skipped_handler_names: + logger.info("{0} is skipped since its dependency {1} is already skipped or timedout".format(ext_handler.name, + dependency.handler.name)) + return False + + for dependency in dependencies: + if dependency.ext is None: + continue + handler_i = ExtHandlerInstance(dependency.handler, self.protocol) - if dependency.timeout is not None and dependency.timeout < DEFAULT_EXT_TIMEOUT_MINUTES: - wait_until -= datetime.timedelta(seconds=( (DEFAULT_EXT_TIMEOUT_MINUTES - dependency.timeout) * 60)) - for ext in dependency.exts: - success_status, status = handler_i.is_ext_status_success(ext) - while not success_status: - if datetime.datetime.utcnow() > wait_until: - raise ExtensionError("Timeout waiting for success status " - "from dependency {}/{} for {}" - "status was: {}".format( - dependency.handler.name, ext.name, - ext_handler.name, status)) - else: - time.sleep(10) - success_status, status = handler_i.is_ext_status_success(ext) + success_status, status = handler_i.is_ext_status_success(dependency.ext) + + # Keep polling for the extension status until it becomes success or times out + while not success_status and datetime.datetime.utcnow() <= wait_until: + time.sleep(5) + success_status, status = handler_i.is_ext_status_success(dependency.ext) + + # In case of timeout, we log it and return false so that the extension waiting + # on this one can be skipped processing + if not success_status: + msg = "Timeout waiting for success status " \ + "from dependency {0}/{1} for {2}" \ + "status was: {3}".format(dependency.handler.name, + dependency.ext.name, + ext_handler.name, + status) + logger.info(msg) + add_event(AGENT_NAME, + version=CURRENT_VERSION, + op=WALAEventOperation.ExtensionProcessing, + is_success=False, + message=msg) + return False + + return True def handle_ext_handler(self, ext_handler, etag): ext_handler_i = ExtHandlerInstance(ext_handler, self.protocol) diff --git a/tests/ga/test_extension.py b/tests/ga/test_extension.py index 57e0aa1e93..af3ff42194 100644 --- a/tests/ga/test_extension.py +++ b/tests/ga/test_extension.py @@ -416,7 +416,7 @@ def test_ext_handler_sequencing(self, *args): self.assertEqual(exthandlers_handler.ext_handlers.extHandlers[1].properties.extensions[0].dependencyLevel, 2) self.assertEqual(exthandlers_handler.ext_handlers.extHandlers[1].properties.extensions[0].dependencies[0].handler, exthandlers_handler.ext_handlers.extHandlers[0]) - self.assertEqual(exthandlers_handler.ext_handlers.extHandlers[1].properties.extensions[0].dependencies[0].exts[0], + self.assertEqual(exthandlers_handler.ext_handlers.extHandlers[1].properties.extensions[0].dependencies[0].ext, exthandlers_handler.ext_handlers.extHandlers[0].properties.extensions[0]) #Test goal state not changed @@ -689,11 +689,12 @@ def test_wait_on_ext_handler_dependencies_empty_exts(self, *args): exthandler = ExtHandler(name=handler_name) extension = Extension(name=handler_name) - dependency = Dependency(handler=exthandler, exts=[]) + dependency = Dependency(handler=exthandler, ext=None) extension.dependencies.append(dependency) exthandler.properties.extensions.append(extension) - exthandlers_handler.wait_on_ext_handler_dependencies(exthandler, datetime.datetime.utcnow()) + ExtHandlerInstance.collect_ext_status = MagicMock(return_value=None) + self.assertTrue(exthandlers_handler.wait_on_ext_handler_dependencies(exthandler, datetime.datetime.utcnow())) def test_wait_on_ext_handler_dependencies_two_exts(self, *args): test_data = WireProtocolData(DATA_FILE) @@ -701,36 +702,81 @@ def test_wait_on_ext_handler_dependencies_two_exts(self, *args): handler_name = "Handler" exthandler = ExtHandler(name=handler_name) - dependency_exts = [Extension(name=handler_name), Extension(name=handler_name)] extension = Extension(name=handler_name) - dependency = Dependency(handler=exthandler, exts=dependency_exts) - extension.dependencies.append(dependency) + dependency1 = Dependency(handler=exthandler, ext=Extension(name=handler_name)) + dependency2 = Dependency(handler=exthandler, ext=Extension(name=handler_name)) + extension.dependencies.append(dependency1) + extension.dependencies.append(dependency2) exthandler.properties.extensions.append(extension) ExtHandlerInstance.collect_ext_status = MagicMock(return_value=None) - exthandlers_handler.wait_on_ext_handler_dependencies(exthandler, datetime.datetime.utcnow()) + self.assertTrue(exthandlers_handler.wait_on_ext_handler_dependencies(exthandler, datetime.datetime.utcnow())) + + def test_wait_on_ext_handler_dependencies_skipped_handlers(self, *args): + test_data = WireProtocolData(DATA_FILE) + exthandlers_handler, protocol = self._create_mock(test_data, *args) + + # Mock E1 -> [E2, E3] + handler1 = ExtHandler(name="H1") + handler2 = ExtHandler(name="H2") + handler3 = ExtHandler(name="H3") + + extension1 = Extension(name="H1") + extension2 = Extension(name="H2") + extension3 = Extension(name="H3") - def _test_wait_on_ext_handler_dependencies(self, exthandlers_handler, timeout=None): + dependency1 = (handler2.name, handler2.name) + dependency2 = (handler3.name, handler3.name) + + extension1.dependencies.append(dependency1) + extension1.dependencies.append(dependency2) + extension1.resolve_dependencies([handler1, handler2, handler3]) + + handler1.properties.extensions.append(extension1) + + # Make H2 (of Ext E2) is already skipped and expect the function to return False + ExtHandlerInstance.collect_ext_status = MagicMock(return_value=None) + self.assertFalse(exthandlers_handler.wait_on_ext_handler_dependencies(handler1, datetime.datetime.utcnow(), ["H2"])) + + def test_wait_on_ext_handler_dependencies_non_skipped_handlers(self, *args): + test_data = WireProtocolData(DATA_FILE) + exthandlers_handler, protocol = self._create_mock(test_data, *args) + + # Mock E1 -> [E2, E3] + handler1 = ExtHandler(name="H1") + handler2 = ExtHandler(name="H2") + handler3 = ExtHandler(name="H3") + + extension1 = Extension(name="H1") + extension2 = Extension(name="H2") + extension3 = Extension(name="H3") + + dependency1 = (handler2.name, handler2.name) + dependency2 = (handler3.name, handler3.name) + + extension1.dependencies.append(dependency1) + extension1.dependencies.append(dependency2) + extension1.resolve_dependencies([handler1, handler2, handler3]) + + handler1.properties.extensions.append(extension1) + + # Make H4 (a non-dependent handler) to be skipped and expect the function to return True + ExtHandlerInstance.collect_ext_status = MagicMock(return_value=None) + self.assertTrue(exthandlers_handler.wait_on_ext_handler_dependencies(handler1, datetime.datetime.utcnow(), ["H4"])) + + def _test_wait_on_ext_handler_dependencies(self, exthandlers_handler): handler_name = "Handler" exthandler = ExtHandler(name=handler_name) dependency_ext = Extension(name=handler_name) extension = Extension(name=handler_name) - dependency = Dependency(handler=exthandler, exts=[ dependency_ext ], timeout=timeout) + dependency = Dependency(handler=exthandler, ext=dependency_ext) extension.dependencies.append(dependency) exthandler.properties.extensions.append(extension) - fun = exthandlers_handler.wait_on_ext_handler_dependencies - if timeout is None: - fun(exthandler, datetime.datetime.utcnow()) - else: - expected_msg = "Timeout.*{0}/{1} for {2}".format(handler_name, handler_name, handler_name) - try: - self.assertRaisesRegexp(ExtensionError, expected_msg, fun, exthandler, datetime.datetime.utcnow()) - except AttributeError: - pass # Python 2.6 doesn't like assertRaisesRegexp - + return exthandlers_handler.wait_on_ext_handler_dependencies(exthandler, datetime.datetime.utcnow()) + def test_wait_on_ext_handler_dependencies_none(self, *args): test_data = WireProtocolData(DATA_FILE) exthandlers_handler, protocol = self._create_mock(test_data, *args) @@ -746,7 +792,7 @@ def test_wait_on_ext_handler_dependencies_success_status(self, *args): status.status = "success" ExtHandlerInstance.collect_ext_status = MagicMock(return_value=status) - self._test_wait_on_ext_handler_dependencies(exthandlers_handler) + self.assertTrue(self._test_wait_on_ext_handler_dependencies(exthandlers_handler)) def test_wait_on_ext_handler_dependencies_success_status_with_substatus(self, *args): test_data = WireProtocolData(DATA_FILE) @@ -759,7 +805,7 @@ def test_wait_on_ext_handler_dependencies_success_status_with_substatus(self, *a status.substatusList.append(substatus) ExtHandlerInstance.collect_ext_status = MagicMock(return_value=status) - self._test_wait_on_ext_handler_dependencies(exthandlers_handler) + self.assertTrue(self._test_wait_on_ext_handler_dependencies(exthandlers_handler)) def test_wait_on_ext_handler_dependencies_timeout(self, *args): test_data = WireProtocolData(DATA_FILE) @@ -769,7 +815,7 @@ def test_wait_on_ext_handler_dependencies_timeout(self, *args): status.status = "error" ExtHandlerInstance.collect_ext_status = MagicMock(return_value=status) - self._test_wait_on_ext_handler_dependencies(exthandlers_handler, timeout=0) + self.assertFalse(self._test_wait_on_ext_handler_dependencies(exthandlers_handler)) def test_ext_handler_version_decide_autoupgrade_internalversion(self, *args): for internal in [False, True]: From c5d5bf1865a479e5d4416d382e02a6dbd7a605a3 Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Thu, 30 Aug 2018 03:51:39 +0000 Subject: [PATCH 10/14] Adding implementation of extension sequencing - simplified --- azurelinuxagent/common/protocol/restapi.py | 32 +-- azurelinuxagent/common/protocol/wire.py | 21 +- azurelinuxagent/ga/exthandlers.py | 56 +++-- tests/ga/test_extension.py | 225 ++++++++++++--------- 4 files changed, 155 insertions(+), 179 deletions(-) diff --git a/azurelinuxagent/common/protocol/restapi.py b/azurelinuxagent/common/protocol/restapi.py index afd8551058..80087825b5 100644 --- a/azurelinuxagent/common/protocol/restapi.py +++ b/azurelinuxagent/common/protocol/restapi.py @@ -145,18 +145,6 @@ def __init__(self): self.vmAgentManifests = DataContractList(VMAgentManifest) -class Dependency(object): - """ - Represents an extension upon which another extension is dependent - handler - the ExtHandler of the dependency - ext - the Extension of the dependency - """ - - def __init__(self, handler=None, ext=None): - self.handler = handler - self.ext = ext - - class Extension(DataContract): def __init__(self, name=None, @@ -164,31 +152,13 @@ def __init__(self, publicSettings=None, protectedSettings=None, certificateThumbprint=None, - dependencyLevel=0, - dependencies=None): + dependencyLevel=0): self.name = name self.sequenceNumber = sequenceNumber self.publicSettings = publicSettings self.protectedSettings = protectedSettings self.certificateThumbprint = certificateThumbprint self.dependencyLevel = dependencyLevel - self.dependencies = [] if dependencies is None else dependencies - - def resolve_dependencies(self, ext_handlers): - resolved_dependencies = [] - for (handler_name, ext_name) in self.dependencies: - # Gracefully handle garbage cases where handler name does - # not exist or is not unique. Same for the extension name. - for handler in ext_handlers: - if handler_name != handler.name: - continue - resolved_dependencies.append( - Dependency( - handler = handler, - ext = next((ext for ext in handler.properties.extensions if ext.name == ext_name), None) - ) - ) - self.dependencies = resolved_dependencies class ExtHandlerProperties(DataContract): diff --git a/azurelinuxagent/common/protocol/wire.py b/azurelinuxagent/common/protocol/wire.py index 751cd302b8..7353632aa5 100644 --- a/azurelinuxagent/common/protocol/wire.py +++ b/azurelinuxagent/common/protocol/wire.py @@ -1576,10 +1576,6 @@ def parse(self, xml_text): self.ext_handlers.extHandlers.append(ext_handler) self.parse_plugin_settings(ext_handler, plugin_settings) - for ext_handler in self.ext_handlers.extHandlers: - for extension in ext_handler.properties.extensions: - extension.resolve_dependencies(self.ext_handlers.extHandlers) - self.status_upload_blob = findtext(xml_doc, "StatusUploadBlob") self.artifacts_profile_blob = findtext(xml_doc, "InVMArtifactsProfileBlob") @@ -1626,26 +1622,13 @@ def parse_plugin_settings(self, ext_handler, plugin_settings): logger.error("Invalid extension settings") return - depends_on = None depends_on_node = find(settings[0], "DependsOn") - depends_on_name = getattrib(depends_on_node, "name") - if depends_on_name == "": - depends_on_name = ext_handler.name - try: depends_on_level = int(getattrib(depends_on_node, "dependencyLevel")) except (ValueError, TypeError): depends_on_level = 0 - dependencies = [] - for dependency in findall(depends_on_node, "DependsOnExtension"): - handler_name = getattrib(dependency, "handler") - ext_name = getattrib(dependency, "extension") - if ext_name == "": - ext_name = handler_name - dependencies.append((handler_name, ext_name)) - for plugin_settings_list in runtime_settings["runtimeSettings"]: handler_settings = plugin_settings_list["handlerSettings"] ext = Extension() @@ -1655,9 +1638,7 @@ def parse_plugin_settings(self, ext_handler, plugin_settings): ext.sequenceNumber = seqNo ext.publicSettings = handler_settings.get("publicSettings") ext.protectedSettings = handler_settings.get("protectedSettings") - if ext.name == depends_on_name: - ext.dependencies = dependencies - ext.dependencyLevel = depends_on_level + ext.dependencyLevel = depends_on_level thumbprint = handler_settings.get( "protectedSettingsCertThumbprint") ext.certificateThumbprint = thumbprint diff --git a/azurelinuxagent/ga/exthandlers.py b/azurelinuxagent/ga/exthandlers.py index 31b3c6f397..360703bb27 100644 --- a/azurelinuxagent/ga/exthandlers.py +++ b/azurelinuxagent/ga/exthandlers.py @@ -312,7 +312,7 @@ def handle_ext_handlers(self, etag=None): dep_level = None deps_wait_until = None handlers_wait_until = datetime.datetime.utcnow() - skipped_handler_names = [] + prev_handler = None self.ext_handlers.extHandlers.sort(key=operator.methodcaller('sort_key')) for ext_handler in self.ext_handlers.extHandlers: @@ -322,50 +322,44 @@ def handle_ext_handlers(self, etag=None): deps_wait_until = handlers_wait_until handlers_wait_until += datetime.timedelta(minutes=DEFAULT_EXT_TIMEOUT_MINUTES) - # Make sure the previous level extensions are installed. - # If installed successfully, continue with the current handler. Otherwise, skip it. - if self.wait_on_ext_handler_dependencies(ext_handler, deps_wait_until, skipped_handler_names): - self.handle_ext_handler(ext_handler, etag) - else: - skipped_handler_names.append(ext_handler.name) + # Make sure that the previous extension is installed. + # If installed successfully, proceed with installing the current handler. + # Otherwise, skip the rest of them. + if dep_level >= 0: + if self.wait_for_prev_handler_installation(prev_handler, ext_handler, deps_wait_until): + prev_handler = ext_handler + else: + break + + self.handle_ext_handler(ext_handler, etag) ''' - Collect all the dependencies in the given handler. - Check the status of the extension in each dependency. + Check the status of the previous extension installed. Wait until it becomes success or times out. - Return True if the dependencies are installed successfully. False if timed out. + Return True if it is installed successfully. False if timed out. ''' - def wait_on_ext_handler_dependencies(self, ext_handler, wait_until, skipped_handler_names=[]): - dependencies = sum([e.dependencies for e in ext_handler.properties.extensions], []) - - # Check if any of the dependencies are already skipped. - # This allows us to fail fast in that case. - for dependency in dependencies: - if dependency.handler.name in skipped_handler_names: - logger.info("{0} is skipped since its dependency {1} is already skipped or timedout".format(ext_handler.name, - dependency.handler.name)) - return False - - for dependency in dependencies: - if dependency.ext is None: - continue + def wait_for_prev_handler_installation(self, prev_handler, cur_handler, wait_until): + # No need to wait on anything for the very first extension + if prev_handler == None: + return True - handler_i = ExtHandlerInstance(dependency.handler, self.protocol) - success_status, status = handler_i.is_ext_status_success(dependency.ext) + handler_i = ExtHandlerInstance(prev_handler, self.protocol) + for ext in prev_handler.properties.extensions: + success_status, status = handler_i.is_ext_status_success(ext) # Keep polling for the extension status until it becomes success or times out while not success_status and datetime.datetime.utcnow() <= wait_until: time.sleep(5) - success_status, status = handler_i.is_ext_status_success(dependency.ext) + success_status, status = handler_i.is_ext_status_success(ext) - # In case of timeout, we log it and return false so that the extension waiting + # In case of timeout, we log it and return false so that the extensions waiting # on this one can be skipped processing if not success_status: msg = "Timeout waiting for success status " \ "from dependency {0}/{1} for {2}" \ - "status was: {3}".format(dependency.handler.name, - dependency.ext.name, - ext_handler.name, + "status was: {3}".format(prev_handler.name, + ext.name, + cur_handler.name, status) logger.info(msg) add_event(AGENT_NAME, diff --git a/tests/ga/test_extension.py b/tests/ga/test_extension.py index af3ff42194..678a48a3fb 100644 --- a/tests/ga/test_extension.py +++ b/tests/ga/test_extension.py @@ -19,7 +19,6 @@ from tests.protocol.mockwiredata import * -from azurelinuxagent.common.protocol.wire import Dependency from azurelinuxagent.common.protocol.restapi import Extension from azurelinuxagent.ga.exthandlers import * from azurelinuxagent.common.protocol.wire import WireProtocol @@ -414,10 +413,6 @@ def test_ext_handler_sequencing(self, *args): self.assertEqual(len(exthandlers_handler.ext_handlers.extHandlers), 2) self.assertEqual(exthandlers_handler.ext_handlers.extHandlers[0].properties.extensions[0].dependencyLevel, 1) self.assertEqual(exthandlers_handler.ext_handlers.extHandlers[1].properties.extensions[0].dependencyLevel, 2) - self.assertEqual(exthandlers_handler.ext_handlers.extHandlers[1].properties.extensions[0].dependencies[0].handler, - exthandlers_handler.ext_handlers.extHandlers[0]) - self.assertEqual(exthandlers_handler.ext_handlers.extHandlers[1].properties.extensions[0].dependencies[0].ext, - exthandlers_handler.ext_handlers.extHandlers[0].properties.extensions[0]) #Test goal state not changed exthandlers_handler.run() @@ -681,110 +676,33 @@ def test_ext_handler_no_reporting_status(self, *args): self._assert_handler_status(protocol.report_vm_status, "Ready", 1, "1.0.0") self._assert_ext_status(protocol.report_ext_status, "error", 0) - def test_wait_on_ext_handler_dependencies_empty_exts(self, *args): + def test_wait_for_prev_handler_installation_empty_exts(self, *args): test_data = WireProtocolData(DATA_FILE) exthandlers_handler, protocol = self._create_mock(test_data, *args) - handler_name = "Handler" - exthandler = ExtHandler(name=handler_name) - extension = Extension(name=handler_name) - - dependency = Dependency(handler=exthandler, ext=None) - extension.dependencies.append(dependency) - exthandler.properties.extensions.append(extension) - - ExtHandlerInstance.collect_ext_status = MagicMock(return_value=None) - self.assertTrue(exthandlers_handler.wait_on_ext_handler_dependencies(exthandler, datetime.datetime.utcnow())) - - def test_wait_on_ext_handler_dependencies_two_exts(self, *args): - test_data = WireProtocolData(DATA_FILE) - exthandlers_handler, protocol = self._create_mock(test_data, *args) - - handler_name = "Handler" - exthandler = ExtHandler(name=handler_name) - extension = Extension(name=handler_name) - - dependency1 = Dependency(handler=exthandler, ext=Extension(name=handler_name)) - dependency2 = Dependency(handler=exthandler, ext=Extension(name=handler_name)) - extension.dependencies.append(dependency1) - extension.dependencies.append(dependency2) - exthandler.properties.extensions.append(extension) - - ExtHandlerInstance.collect_ext_status = MagicMock(return_value=None) - self.assertTrue(exthandlers_handler.wait_on_ext_handler_dependencies(exthandler, datetime.datetime.utcnow())) - - def test_wait_on_ext_handler_dependencies_skipped_handlers(self, *args): - test_data = WireProtocolData(DATA_FILE) - exthandlers_handler, protocol = self._create_mock(test_data, *args) - - # Mock E1 -> [E2, E3] - handler1 = ExtHandler(name="H1") - handler2 = ExtHandler(name="H2") - handler3 = ExtHandler(name="H3") - - extension1 = Extension(name="H1") - extension2 = Extension(name="H2") - extension3 = Extension(name="H3") - - dependency1 = (handler2.name, handler2.name) - dependency2 = (handler3.name, handler3.name) - - extension1.dependencies.append(dependency1) - extension1.dependencies.append(dependency2) - extension1.resolve_dependencies([handler1, handler2, handler3]) - - handler1.properties.extensions.append(extension1) - - # Make H2 (of Ext E2) is already skipped and expect the function to return False - ExtHandlerInstance.collect_ext_status = MagicMock(return_value=None) - self.assertFalse(exthandlers_handler.wait_on_ext_handler_dependencies(handler1, datetime.datetime.utcnow(), ["H2"])) - - def test_wait_on_ext_handler_dependencies_non_skipped_handlers(self, *args): - test_data = WireProtocolData(DATA_FILE) - exthandlers_handler, protocol = self._create_mock(test_data, *args) - - # Mock E1 -> [E2, E3] - handler1 = ExtHandler(name="H1") - handler2 = ExtHandler(name="H2") - handler3 = ExtHandler(name="H3") - - extension1 = Extension(name="H1") - extension2 = Extension(name="H2") - extension3 = Extension(name="H3") - - dependency1 = (handler2.name, handler2.name) - dependency2 = (handler3.name, handler3.name) - - extension1.dependencies.append(dependency1) - extension1.dependencies.append(dependency2) - extension1.resolve_dependencies([handler1, handler2, handler3]) - - handler1.properties.extensions.append(extension1) + prev_handler = ExtHandler(name="Previous") + exthandler = ExtHandler(name="Handler") - # Make H4 (a non-dependent handler) to be skipped and expect the function to return True ExtHandlerInstance.collect_ext_status = MagicMock(return_value=None) - self.assertTrue(exthandlers_handler.wait_on_ext_handler_dependencies(handler1, datetime.datetime.utcnow(), ["H4"])) + self.assertTrue(exthandlers_handler.wait_for_prev_handler_installation(None, exthandler, datetime.datetime.utcnow())) + self.assertTrue(exthandlers_handler.wait_for_prev_handler_installation(prev_handler, exthandler, datetime.datetime.utcnow())) - def _test_wait_on_ext_handler_dependencies(self, exthandlers_handler): + def _test_wait_for_prev_handler_installation(self, exthandlers_handler): handler_name = "Handler" exthandler = ExtHandler(name=handler_name) - dependency_ext = Extension(name=handler_name) extension = Extension(name=handler_name) - - dependency = Dependency(handler=exthandler, ext=dependency_ext) - extension.dependencies.append(dependency) exthandler.properties.extensions.append(extension) - return exthandlers_handler.wait_on_ext_handler_dependencies(exthandler, datetime.datetime.utcnow()) + return exthandlers_handler.wait_for_prev_handler_installation(exthandler, exthandler, datetime.datetime.utcnow()) - def test_wait_on_ext_handler_dependencies_none(self, *args): + def test_wait_for_prev_handler_installation_none(self, *args): test_data = WireProtocolData(DATA_FILE) exthandlers_handler, protocol = self._create_mock(test_data, *args) ExtHandlerInstance.collect_ext_status = MagicMock(return_value=None) - self._test_wait_on_ext_handler_dependencies(exthandlers_handler) + self.assertTrue(self._test_wait_for_prev_handler_installation(exthandlers_handler)) - def test_wait_on_ext_handler_dependencies_success_status(self, *args): + def test_wait_for_prev_handler_installation_success_status(self, *args): test_data = WireProtocolData(DATA_FILE) exthandlers_handler, protocol = self._create_mock(test_data, *args) @@ -792,9 +710,9 @@ def test_wait_on_ext_handler_dependencies_success_status(self, *args): status.status = "success" ExtHandlerInstance.collect_ext_status = MagicMock(return_value=status) - self.assertTrue(self._test_wait_on_ext_handler_dependencies(exthandlers_handler)) + self.assertTrue(self._test_wait_for_prev_handler_installation(exthandlers_handler)) - def test_wait_on_ext_handler_dependencies_success_status_with_substatus(self, *args): + def test_wait_for_prev_handler_installation_success_status_with_substatus(self, *args): test_data = WireProtocolData(DATA_FILE) exthandlers_handler, protocol = self._create_mock(test_data, *args) @@ -805,9 +723,9 @@ def test_wait_on_ext_handler_dependencies_success_status_with_substatus(self, *a status.substatusList.append(substatus) ExtHandlerInstance.collect_ext_status = MagicMock(return_value=status) - self.assertTrue(self._test_wait_on_ext_handler_dependencies(exthandlers_handler)) + self.assertTrue(self._test_wait_for_prev_handler_installation(exthandlers_handler)) - def test_wait_on_ext_handler_dependencies_timeout(self, *args): + def test_wait_for_prev_handler_installation_timeout(self, *args): test_data = WireProtocolData(DATA_FILE) exthandlers_handler, protocol = self._create_mock(test_data, *args) @@ -815,7 +733,7 @@ def test_wait_on_ext_handler_dependencies_timeout(self, *args): status.status = "error" ExtHandlerInstance.collect_ext_status = MagicMock(return_value=status) - self.assertFalse(self._test_wait_on_ext_handler_dependencies(exthandlers_handler)) + self.assertFalse(self._test_wait_for_prev_handler_installation(exthandlers_handler)) def test_ext_handler_version_decide_autoupgrade_internalversion(self, *args): for internal in [False, True]: @@ -1068,6 +986,119 @@ def test_upgrade(self, patch_get_update_command, *args): self._assert_handler_status(protocol.report_vm_status, "NotReady", expected_ext_count=1, version="1.0.1") +@patch("azurelinuxagent.common.protocol.wire.CryptUtil") +@patch("azurelinuxagent.common.utils.restutil.http_get") +class TestExtensionSequencing(AgentTestCase): + + def _create_mock(self, mock_http_get, MockCryptUtil): + test_data = WireProtocolData(DATA_FILE) + + #Mock protocol to return test data + mock_http_get.side_effect = test_data.mock_http_get + MockCryptUtil.side_effect = test_data.mock_crypt_util + + protocol = WireProtocol("foo.bar") + protocol.detect() + protocol.report_ext_status = MagicMock() + protocol.report_vm_status = MagicMock() + protocol.get_artifacts_profile = MagicMock() + + handler = get_exthandlers_handler() + handler.protocol_util.get_protocol = Mock(return_value=protocol) + handler.ext_handlers, handler.last_etag = protocol.get_ext_handlers() + conf.get_enable_overprovisioning = Mock(return_value=False) + + def wait_for_prev_handler_installation(prev_handler, cur_handler, wait_until): + return orig_wait_for_prev_handler_installation(prev_handler, cur_handler, datetime.datetime.utcnow()) + + orig_wait_for_prev_handler_installation = handler.wait_for_prev_handler_installation + handler.wait_for_prev_handler_installation = wait_for_prev_handler_installation + return handler + + def _set_dependency_levels(self, dependency_levels, exthandlers_handler): + handler_map = dict() + all_handlers = [] + for h, level in dependency_levels: + if handler_map.get(h) is None: + handler = ExtHandler(name=h) + extension = Extension(name=h) + handler.properties.state = "enabled" + handler.properties.extensions.append(extension) + handler_map[h] = handler + all_handlers.append(handler) + + handler = handler_map[h] + for ext in handler.properties.extensions: + ext.dependencyLevel = level + + exthandlers_handler.ext_handlers.extHandlers = [] + for handler in all_handlers: + exthandlers_handler.ext_handlers.extHandlers.append(handler) + + def _validate_installed_extensions(self, expected_extensions, exthandlers_handler): + installed_extensions = [a[0].name for a, k in exthandlers_handler.handle_ext_handler.call_args_list] + self.assertListEqual(expected_extensions, installed_extensions, "Expected and actual list of extensions are not equal") + + def _run_test(self, extensions_to_be_failed, expected_extensions, exthandlers_handler): + + def collect_ext_status(ext): + status = ExtensionStatus(seq_no=0) + status.status = "error" if ext.name in extensions_to_be_failed else "success" + return status + + ExtHandlerInstance.collect_ext_status = MagicMock(side_effect = collect_ext_status) + exthandlers_handler.handle_ext_handler = MagicMock() + exthandlers_handler.run() + self._validate_installed_extensions(expected_extensions, exthandlers_handler) + + def test_handle_ext_handlers(self, *args): + exthandlers_handler = self._create_mock(*args) + + self._set_dependency_levels([("A", 3), ("B", 2), ("C", 2), ("D", 1), ("E", 1), ("F", 1), ("G", 1)], + exthandlers_handler) + + extensions_to_be_failed = [] + expected_extensions = ["D", "E", "F", "G", "B", "C", "A"] + self._run_test(extensions_to_be_failed, expected_extensions, exthandlers_handler) + + extensions_to_be_failed = ["D"] + expected_extensions = ["D"] + self._run_test(extensions_to_be_failed, expected_extensions, exthandlers_handler) + + extensions_to_be_failed = ["E"] + expected_extensions = ["D", "E"] + self._run_test(extensions_to_be_failed, expected_extensions, exthandlers_handler) + + extensions_to_be_failed = ["F"] + expected_extensions = ["D", "E", "F"] + self._run_test(extensions_to_be_failed, expected_extensions, exthandlers_handler) + + extensions_to_be_failed = ["G"] + expected_extensions = ["D", "E", "F", "G"] + self._run_test(extensions_to_be_failed, expected_extensions, exthandlers_handler) + + extensions_to_be_failed = ["B"] + expected_extensions = ["D", "E", "F", "G", "B"] + self._run_test(extensions_to_be_failed, expected_extensions, exthandlers_handler) + + extensions_to_be_failed = ["C"] + expected_extensions = ["D", "E", "F", "G", "B", "C"] + self._run_test(extensions_to_be_failed, expected_extensions, exthandlers_handler) + + extensions_to_be_failed = ["A"] + expected_extensions = ["D", "E", "F", "G", "B", "C", "A"] + self._run_test(extensions_to_be_failed, expected_extensions, exthandlers_handler) + + def test_handle_ext_handlers_with_uninstallation(self, *args): + exthandlers_handler = self._create_mock(*args) + + # "A", "D" and "F" are marked as to be uninstalled + self._set_dependency_levels([("A", 0), ("B", 2), ("C", 2), ("D", 0), ("E", 1), ("F", 0), ("G", 1)], + exthandlers_handler) + + extensions_to_be_failed = [] + expected_extensions = ["A", "D", "F", "E", "G", "B", "C"] + self._run_test(extensions_to_be_failed, expected_extensions, exthandlers_handler) if __name__ == '__main__': unittest.main() From a8f6a4b83b7a36140d8664f8480e26b8d1e02f24 Mon Sep 17 00:00:00 2001 From: Sirajudeen Sahul Hameed Date: Wed, 5 Sep 2018 21:57:33 +0000 Subject: [PATCH 11/14] Addressing PR comments --- azurelinuxagent/common/protocol/restapi.py | 1 - azurelinuxagent/ga/exthandlers.py | 14 +++++++------- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/azurelinuxagent/common/protocol/restapi.py b/azurelinuxagent/common/protocol/restapi.py index 80087825b5..08b25c6512 100644 --- a/azurelinuxagent/common/protocol/restapi.py +++ b/azurelinuxagent/common/protocol/restapi.py @@ -164,7 +164,6 @@ def __init__(self, class ExtHandlerProperties(DataContract): def __init__(self): self.version = None - self.dependencyLevel = None self.state = None self.extensions = DataContractList(Extension) diff --git a/azurelinuxagent/ga/exthandlers.py b/azurelinuxagent/ga/exthandlers.py index 360703bb27..f8c944bf06 100644 --- a/azurelinuxagent/ga/exthandlers.py +++ b/azurelinuxagent/ga/exthandlers.py @@ -333,12 +333,12 @@ def handle_ext_handlers(self, etag=None): self.handle_ext_handler(ext_handler, etag) - ''' - Check the status of the previous extension installed. - Wait until it becomes success or times out. - Return True if it is installed successfully. False if timed out. - ''' def wait_for_prev_handler_installation(self, prev_handler, cur_handler, wait_until): + ''' + Check the status of the previous extension installed. + Wait until it becomes success or times out. + Return True if it is installed successfully. False if timed out. + ''' # No need to wait on anything for the very first extension if prev_handler == None: return True @@ -356,11 +356,11 @@ def wait_for_prev_handler_installation(self, prev_handler, cur_handler, wait_unt # on this one can be skipped processing if not success_status: msg = "Timeout waiting for success status " \ - "from dependency {0}/{1} for {2}" \ + "from dependency {0}/{1} for {2} " \ "status was: {3}".format(prev_handler.name, ext.name, cur_handler.name, - status) + status.status if status != None else None) logger.info(msg) add_event(AGENT_NAME, version=CURRENT_VERSION, From ab4741eba3af05f915beeb31c7ad4f66b068c3f4 Mon Sep 17 00:00:00 2001 From: Sirajudeen Sahul Hameed Date: Fri, 7 Sep 2018 22:19:57 +0000 Subject: [PATCH 12/14] Addressing PR comments from narrieta --- azurelinuxagent/common/protocol/wire.py | 12 +- azurelinuxagent/ga/exthandlers.py | 98 +++++++++------- tests/ga/test_extension.py | 146 +++++++++++++++++++++--- 3 files changed, 191 insertions(+), 65 deletions(-) diff --git a/azurelinuxagent/common/protocol/wire.py b/azurelinuxagent/common/protocol/wire.py index 7353632aa5..de39fa37d1 100644 --- a/azurelinuxagent/common/protocol/wire.py +++ b/azurelinuxagent/common/protocol/wire.py @@ -1622,12 +1622,14 @@ def parse_plugin_settings(self, ext_handler, plugin_settings): logger.error("Invalid extension settings") return + depends_on_level = 0 depends_on_node = find(settings[0], "DependsOn") - - try: - depends_on_level = int(getattrib(depends_on_node, "dependencyLevel")) - except (ValueError, TypeError): - depends_on_level = 0 + if depends_on_node != None: + try: + depends_on_level = int(getattrib(depends_on_node, "dependencyLevel")) + except (ValueError, TypeError): + logger.warn("Could not parse dependencyLevel for handler {0}. Setting it to 0".format(name)) + depends_on_level = 0 for plugin_settings_list in runtime_settings["runtimeSettings"]: handler_settings = plugin_settings_list["handlerSettings"] diff --git a/azurelinuxagent/ga/exthandlers.py b/azurelinuxagent/ga/exthandlers.py index f8c944bf06..0eda08b413 100644 --- a/azurelinuxagent/ga/exthandlers.py +++ b/azurelinuxagent/ga/exthandlers.py @@ -57,7 +57,9 @@ HANDLER_ENVIRONMENT_VERSION = 1.0 EXTENSION_STATUS_ERROR = 'error' +EXTENSION_STATUS_SUCCESS = 'success' VALID_EXTENSION_STATUS = ['transitioning', 'error', 'success', 'warning'] +EXTENSION_TERMINAL_STATUSES = ['error', 'success'] VALID_HANDLER_STATUS = ['Ready', 'NotReady', "Installing", "Unresponsive"] @@ -309,35 +311,46 @@ def handle_ext_handlers(self, etag=None): logger.info("Extension handling is on hold") return - dep_level = None - deps_wait_until = None - handlers_wait_until = datetime.datetime.utcnow() + wait_until = datetime.datetime.utcnow() + datetime.timedelta(minutes=DEFAULT_EXT_TIMEOUT_MINUTES) prev_handler = None + ext_seq_required = self.is_ext_seq_required() self.ext_handlers.extHandlers.sort(key=operator.methodcaller('sort_key')) for ext_handler in self.ext_handlers.extHandlers: - # Use updated time limit for each dependency level - if dep_level != ext_handler.sort_key(): + if ext_seq_required: + # Make sure that the previous extension is handled. + # If handled successfully, proceed with the current handler. + # Otherwise, skip the rest of them. dep_level = ext_handler.sort_key() - deps_wait_until = handlers_wait_until - handlers_wait_until += datetime.timedelta(minutes=DEFAULT_EXT_TIMEOUT_MINUTES) - - # Make sure that the previous extension is installed. - # If installed successfully, proceed with installing the current handler. - # Otherwise, skip the rest of them. - if dep_level >= 0: - if self.wait_for_prev_handler_installation(prev_handler, ext_handler, deps_wait_until): - prev_handler = ext_handler - else: - break + if dep_level >= 0: + if self.wait_for_prev_handler_completion(prev_handler, wait_until): + prev_handler = ext_handler + else: + break self.handle_ext_handler(ext_handler, etag) - def wait_for_prev_handler_installation(self, prev_handler, cur_handler, wait_until): + def is_ext_seq_required(self): ''' - Check the status of the previous extension installed. - Wait until it becomes success or times out. - Return True if it is installed successfully. False if timed out. + If all the extensions have same dependency level + It is considered as extension sequencing is not required + ''' + dep_level = None + for ext_handler in self.ext_handlers.extHandlers: + if dep_level is None: + dep_level = ext_handler.sort_key() + continue + + if dep_level != ext_handler.sort_key(): + return True + + return False + + def wait_for_prev_handler_completion(self, prev_handler, wait_until): + ''' + Check the status of the previous extension handled. + Wait until it has a terminal state or times out. + Return True if it is handled successfully. False if not. ''' # No need to wait on anything for the very first extension if prev_handler == None: @@ -345,22 +358,17 @@ def wait_for_prev_handler_installation(self, prev_handler, cur_handler, wait_unt handler_i = ExtHandlerInstance(prev_handler, self.protocol) for ext in prev_handler.properties.extensions: - success_status, status = handler_i.is_ext_status_success(ext) + ext_completed, status = handler_i.is_ext_handling_complete(ext) # Keep polling for the extension status until it becomes success or times out - while not success_status and datetime.datetime.utcnow() <= wait_until: + while not ext_completed and datetime.datetime.utcnow() <= wait_until: time.sleep(5) - success_status, status = handler_i.is_ext_status_success(ext) - - # In case of timeout, we log it and return false so that the extensions waiting - # on this one can be skipped processing - if not success_status: - msg = "Timeout waiting for success status " \ - "from dependency {0}/{1} for {2} " \ - "status was: {3}".format(prev_handler.name, - ext.name, - cur_handler.name, - status.status if status != None else None) + ext_completed, status = handler_i.is_ext_handling_complete(ext) + + # In case of timeout or terminal error state, we log it and return false + # so that the extensions waiting on this one can be skipped processing + if not ext_completed or status != EXTENSION_STATUS_SUCCESS: + msg = "Extension {0} was timedout or status was: {1}".format(ext.name, status) logger.info(msg) add_event(AGENT_NAME, version=CURRENT_VERSION, @@ -927,16 +935,22 @@ def collect_ext_status(self, ext): return ext_status - def is_ext_status_success(self, ext): - status = self.collect_ext_status(ext) + def is_ext_handling_complete(self, ext): + try: + status = self.collect_ext_status(ext) + except FileNotFoundError as e: + return (False, None) + + # when there is no status, the handling is complete and return None status if status is None: - return (True, status) - if status.status != "success": - return (False, status) - for substatus in status.substatusList: - if substatus.status != "success": - return (False, status) - return (True, status) + return (True, None) + + # If not in terminal state, it is incomplete + if status.status not in EXTENSION_TERMINAL_STATUSES: + return (False, status.status) + + # Extension completed, return its status + return (True, status.status) def report_ext_status(self): active_exts = [] diff --git a/tests/ga/test_extension.py b/tests/ga/test_extension.py index 678a48a3fb..2cd306549f 100644 --- a/tests/ga/test_extension.py +++ b/tests/ga/test_extension.py @@ -470,6 +470,30 @@ def test_ext_handler_sequencing(self, *args): self.assertEqual(exthandlers_handler.ext_handlers.extHandlers[0].properties.extensions[0].dependencyLevel, 6) self.assertEqual(exthandlers_handler.ext_handlers.extHandlers[1].properties.extensions[0].dependencyLevel, 5) + def test_ext_handler_sequencing_default_dependency_level(self, *args): + test_data = WireProtocolData(DATA_FILE) + exthandlers_handler, protocol = self._create_mock(test_data, *args) + exthandlers_handler.run() + self.assertEqual(exthandlers_handler.ext_handlers.extHandlers[0].properties.extensions[0].dependencyLevel, 0) + self.assertEqual(exthandlers_handler.ext_handlers.extHandlers[0].properties.extensions[0].dependencyLevel, 0) + + def test_ext_handler_sequencing_invalid_dependency_level(self, *args): + test_data = WireProtocolData(DATA_FILE_EXT_SEQUENCING) + exthandlers_handler, protocol = self._create_mock(test_data, *args) + + test_data.goal_state = test_data.goal_state.replace("1<", + "2<") + test_data.ext_conf = test_data.ext_conf.replace("seqNo=\"0\"", + "seqNo=\"1\"") + test_data.ext_conf = test_data.ext_conf.replace("dependencyLevel=\"1\"", + "dependencyLevel=\"a6\"") + test_data.ext_conf = test_data.ext_conf.replace("dependencyLevel=\"2\"", + "dependencyLevel=\"5b\"") + exthandlers_handler.run() + + self.assertEqual(exthandlers_handler.ext_handlers.extHandlers[0].properties.extensions[0].dependencyLevel, 0) + self.assertEqual(exthandlers_handler.ext_handlers.extHandlers[0].properties.extensions[0].dependencyLevel, 0) + def test_ext_handler_rollingupgrade(self, *args): test_data = WireProtocolData(DATA_FILE_EXT_ROLLINGUPGRADE) exthandlers_handler, protocol = self._create_mock(test_data, *args) @@ -676,33 +700,34 @@ def test_ext_handler_no_reporting_status(self, *args): self._assert_handler_status(protocol.report_vm_status, "Ready", 1, "1.0.0") self._assert_ext_status(protocol.report_ext_status, "error", 0) - def test_wait_for_prev_handler_installation_empty_exts(self, *args): + def test_wait_for_prev_handler_completion_empty_exts(self, *args): test_data = WireProtocolData(DATA_FILE) exthandlers_handler, protocol = self._create_mock(test_data, *args) prev_handler = ExtHandler(name="Previous") - exthandler = ExtHandler(name="Handler") ExtHandlerInstance.collect_ext_status = MagicMock(return_value=None) - self.assertTrue(exthandlers_handler.wait_for_prev_handler_installation(None, exthandler, datetime.datetime.utcnow())) - self.assertTrue(exthandlers_handler.wait_for_prev_handler_installation(prev_handler, exthandler, datetime.datetime.utcnow())) + self.assertTrue(exthandlers_handler.wait_for_prev_handler_completion(None, datetime.datetime.utcnow())) + self.assertTrue(exthandlers_handler.wait_for_prev_handler_completion(prev_handler, datetime.datetime.utcnow())) - def _test_wait_for_prev_handler_installation(self, exthandlers_handler): + def _test_wait_for_prev_handler_completion(self, exthandlers_handler): handler_name = "Handler" exthandler = ExtHandler(name=handler_name) extension = Extension(name=handler_name) exthandler.properties.extensions.append(extension) - return exthandlers_handler.wait_for_prev_handler_installation(exthandler, exthandler, datetime.datetime.utcnow()) + # Override the timout value to minimize the test duration + wait_until = datetime.datetime.utcnow() + datetime.timedelta(seconds=1) + return exthandlers_handler.wait_for_prev_handler_completion(exthandler, wait_until) - def test_wait_for_prev_handler_installation_none(self, *args): + def test_wait_for_prev_handler_completion_none(self, *args): test_data = WireProtocolData(DATA_FILE) exthandlers_handler, protocol = self._create_mock(test_data, *args) ExtHandlerInstance.collect_ext_status = MagicMock(return_value=None) - self.assertTrue(self._test_wait_for_prev_handler_installation(exthandlers_handler)) + self.assertFalse(self._test_wait_for_prev_handler_completion(exthandlers_handler)) - def test_wait_for_prev_handler_installation_success_status(self, *args): + def test_wait_for_prev_handler_completion_success_status(self, *args): test_data = WireProtocolData(DATA_FILE) exthandlers_handler, protocol = self._create_mock(test_data, *args) @@ -710,9 +735,9 @@ def test_wait_for_prev_handler_installation_success_status(self, *args): status.status = "success" ExtHandlerInstance.collect_ext_status = MagicMock(return_value=status) - self.assertTrue(self._test_wait_for_prev_handler_installation(exthandlers_handler)) + self.assertTrue(self._test_wait_for_prev_handler_completion(exthandlers_handler)) - def test_wait_for_prev_handler_installation_success_status_with_substatus(self, *args): + def test_wait_for_prev_handler_completion_success_status_with_substatus(self, *args): test_data = WireProtocolData(DATA_FILE) exthandlers_handler, protocol = self._create_mock(test_data, *args) @@ -723,9 +748,9 @@ def test_wait_for_prev_handler_installation_success_status_with_substatus(self, status.substatusList.append(substatus) ExtHandlerInstance.collect_ext_status = MagicMock(return_value=status) - self.assertTrue(self._test_wait_for_prev_handler_installation(exthandlers_handler)) + self.assertTrue(self._test_wait_for_prev_handler_completion(exthandlers_handler)) - def test_wait_for_prev_handler_installation_timeout(self, *args): + def test_wait_for_prev_handler_completion_error_status(self, *args): test_data = WireProtocolData(DATA_FILE) exthandlers_handler, protocol = self._create_mock(test_data, *args) @@ -733,7 +758,57 @@ def test_wait_for_prev_handler_installation_timeout(self, *args): status.status = "error" ExtHandlerInstance.collect_ext_status = MagicMock(return_value=status) - self.assertFalse(self._test_wait_for_prev_handler_installation(exthandlers_handler)) + self.assertFalse(self._test_wait_for_prev_handler_completion(exthandlers_handler)) + + def test_wait_for_prev_handler_completion_timeout(self, *args): + test_data = WireProtocolData(DATA_FILE) + exthandlers_handler, protocol = self._create_mock(test_data, *args) + + status = ExtensionStatus(seq_no=0) + status.status = "warning" + + ExtHandlerInstance.collect_ext_status = MagicMock(return_value=status) + self.assertFalse(self._test_wait_for_prev_handler_completion(exthandlers_handler)) + + def test_is_ext_handling_complete(self, *args): + test_data = WireProtocolData(DATA_FILE) + exthandlers_handler, protocol = self._create_mock(test_data, *args) + + handler_name = "Handler" + exthandler = ExtHandler(name=handler_name) + extension = Extension(name=handler_name) + exthandler.properties.extensions.append(extension) + + ext_handler_i = ExtHandlerInstance(exthandler, protocol) + ext_status = ExtensionStatus(seq_no=0) + + # Testing no status case + ext_handler_i.collect_ext_status = MagicMock(return_value=None) + completed, status = ext_handler_i.is_ext_handling_complete(ext_status) + self.assertTrue(completed) + self.assertEqual(status, None) + + # Testing FileNotFound case + ext_handler_i.collect_ext_status = MagicMock(side_effect=FileNotFoundError()) + completed, status = ext_handler_i.is_ext_handling_complete(ext_status) + self.assertFalse(completed) + self.assertEqual(status, None) + + ext_handler_i.collect_ext_status = MagicMock(return_value=ext_status) + + # Testing different status cases + expected_results = { + "error": (True, "error"), + "success": (True, "success"), + "warning": (False, "warning"), + "transitioning": (False, "transitioning") + } + + for key in expected_results.keys(): + ext_status.status = key + completed, status = ext_handler_i.is_ext_handling_complete(ext_status) + self.assertEqual(completed, expected_results[key][0]) + self.assertEqual(status, expected_results[key][1]) def test_ext_handler_version_decide_autoupgrade_internalversion(self, *args): for internal in [False, True]: @@ -1008,11 +1083,11 @@ def _create_mock(self, mock_http_get, MockCryptUtil): handler.ext_handlers, handler.last_etag = protocol.get_ext_handlers() conf.get_enable_overprovisioning = Mock(return_value=False) - def wait_for_prev_handler_installation(prev_handler, cur_handler, wait_until): - return orig_wait_for_prev_handler_installation(prev_handler, cur_handler, datetime.datetime.utcnow()) + def wait_for_prev_handler_completion(prev_handler, wait_until): + return orig_wait_for_prev_handler_completion(prev_handler, datetime.datetime.utcnow()) - orig_wait_for_prev_handler_installation = handler.wait_for_prev_handler_installation - handler.wait_for_prev_handler_installation = wait_for_prev_handler_installation + orig_wait_for_prev_handler_completion = handler.wait_for_prev_handler_completion + handler.wait_for_prev_handler_completion = wait_for_prev_handler_completion return handler def _set_dependency_levels(self, dependency_levels, exthandlers_handler): @@ -1100,6 +1175,41 @@ def test_handle_ext_handlers_with_uninstallation(self, *args): expected_extensions = ["A", "D", "F", "E", "G", "B", "C"] self._run_test(extensions_to_be_failed, expected_extensions, exthandlers_handler) + def test_handle_ext_handlers_fallback(self, *args): + exthandlers_handler = self._create_mock(*args) + + self._set_dependency_levels([("A", 1), ("B", 1), ("C", 1), ("D", 1), ("E", 1), ("F", 1), ("G", 1)], + exthandlers_handler) + + # Make sure that the extension sequencing is not applied + self.assertFalse(exthandlers_handler.is_ext_seq_required()) + expected_extensions = ["A", "B", "C", "D", "E", "F", "G"] + + # Make sure that failure in any extension does not prevent other extensions to be installed + extensions_to_be_failed = [] + self._run_test(extensions_to_be_failed, expected_extensions, exthandlers_handler) + + extensions_to_be_failed = ["A"] + self._run_test(extensions_to_be_failed, expected_extensions, exthandlers_handler) + + extensions_to_be_failed = ["B"] + self._run_test(extensions_to_be_failed, expected_extensions, exthandlers_handler) + + extensions_to_be_failed = ["C"] + self._run_test(extensions_to_be_failed, expected_extensions, exthandlers_handler) + + extensions_to_be_failed = ["D"] + self._run_test(extensions_to_be_failed, expected_extensions, exthandlers_handler) + + extensions_to_be_failed = ["E"] + self._run_test(extensions_to_be_failed, expected_extensions, exthandlers_handler) + + extensions_to_be_failed = ["F"] + self._run_test(extensions_to_be_failed, expected_extensions, exthandlers_handler) + + extensions_to_be_failed = ["G"] + self._run_test(extensions_to_be_failed, expected_extensions, exthandlers_handler) + if __name__ == '__main__': unittest.main() From dc0ec36948109f157fa796dba56700ae8ad1c1c5 Mon Sep 17 00:00:00 2001 From: Sirajudeen Sahul Hameed Date: Thu, 13 Sep 2018 19:50:19 +0000 Subject: [PATCH 13/14] Addressing PR comments and adding P1 changes --- azurelinuxagent/ga/exthandlers.py | 22 ++++++++-- tests/ga/test_extension.py | 73 ++++++++++++++++++++++++------- 2 files changed, 74 insertions(+), 21 deletions(-) diff --git a/azurelinuxagent/ga/exthandlers.py b/azurelinuxagent/ga/exthandlers.py index 0eda08b413..cb61c0638b 100644 --- a/azurelinuxagent/ga/exthandlers.py +++ b/azurelinuxagent/ga/exthandlers.py @@ -935,11 +935,25 @@ def collect_ext_status(self, ext): return ext_status + def get_ext_handling_status(self, ext): + seq_no, ext_status_file = self.get_status_file_path(ext) + if seq_no < 0 or ext_status_file is None: + return None + + # Missing status file is considered a non-terminal state here + # so that extension sequencing can wait until it becomes existing + if not os.path.exists(ext_status_file): + ext_status = ExtensionStatus(seq_no=seq_no) + ext_status.message = u"Failed to get status file for {0}".format(ext.name) + ext_status.code = -1 + ext_status.status = "warning" + else: + ext_status = self.collect_ext_status(ext) + + return ext_status + def is_ext_handling_complete(self, ext): - try: - status = self.collect_ext_status(ext) - except FileNotFoundError as e: - return (False, None) + status = self.get_ext_handling_status(ext) # when there is no status, the handling is complete and return None status if status is None: diff --git a/tests/ga/test_extension.py b/tests/ga/test_extension.py index 2cd306549f..225e607431 100644 --- a/tests/ga/test_extension.py +++ b/tests/ga/test_extension.py @@ -706,7 +706,7 @@ def test_wait_for_prev_handler_completion_empty_exts(self, *args): prev_handler = ExtHandler(name="Previous") - ExtHandlerInstance.collect_ext_status = MagicMock(return_value=None) + ExtHandlerInstance.get_ext_handling_status = MagicMock(return_value=None) self.assertTrue(exthandlers_handler.wait_for_prev_handler_completion(None, datetime.datetime.utcnow())) self.assertTrue(exthandlers_handler.wait_for_prev_handler_completion(prev_handler, datetime.datetime.utcnow())) @@ -724,7 +724,7 @@ def test_wait_for_prev_handler_completion_none(self, *args): test_data = WireProtocolData(DATA_FILE) exthandlers_handler, protocol = self._create_mock(test_data, *args) - ExtHandlerInstance.collect_ext_status = MagicMock(return_value=None) + ExtHandlerInstance.get_ext_handling_status = MagicMock(return_value=None) self.assertFalse(self._test_wait_for_prev_handler_completion(exthandlers_handler)) def test_wait_for_prev_handler_completion_success_status(self, *args): @@ -734,7 +734,7 @@ def test_wait_for_prev_handler_completion_success_status(self, *args): status = ExtensionStatus(seq_no=0) status.status = "success" - ExtHandlerInstance.collect_ext_status = MagicMock(return_value=status) + ExtHandlerInstance.get_ext_handling_status = MagicMock(return_value=status) self.assertTrue(self._test_wait_for_prev_handler_completion(exthandlers_handler)) def test_wait_for_prev_handler_completion_success_status_with_substatus(self, *args): @@ -747,7 +747,7 @@ def test_wait_for_prev_handler_completion_success_status_with_substatus(self, *a substatus.status = "success" status.substatusList.append(substatus) - ExtHandlerInstance.collect_ext_status = MagicMock(return_value=status) + ExtHandlerInstance.get_ext_handling_status = MagicMock(return_value=status) self.assertTrue(self._test_wait_for_prev_handler_completion(exthandlers_handler)) def test_wait_for_prev_handler_completion_error_status(self, *args): @@ -757,7 +757,7 @@ def test_wait_for_prev_handler_completion_error_status(self, *args): status = ExtensionStatus(seq_no=0) status.status = "error" - ExtHandlerInstance.collect_ext_status = MagicMock(return_value=status) + ExtHandlerInstance.get_ext_handling_status = MagicMock(return_value=status) self.assertFalse(self._test_wait_for_prev_handler_completion(exthandlers_handler)) def test_wait_for_prev_handler_completion_timeout(self, *args): @@ -767,9 +767,54 @@ def test_wait_for_prev_handler_completion_timeout(self, *args): status = ExtensionStatus(seq_no=0) status.status = "warning" - ExtHandlerInstance.collect_ext_status = MagicMock(return_value=status) + ExtHandlerInstance.get_ext_handling_status = MagicMock(return_value=status) self.assertFalse(self._test_wait_for_prev_handler_completion(exthandlers_handler)) - + + def test_get_ext_handling_status(self, *args): + test_data = WireProtocolData(DATA_FILE) + exthandlers_handler, protocol = self._create_mock(test_data, *args) + + handler_name = "Handler" + exthandler = ExtHandler(name=handler_name) + extension = Extension(name=handler_name) + exthandler.properties.extensions.append(extension) + + ext_status = ExtensionStatus(seq_no=0) + ext_status.code = 1 + ext_status.message = "Test message" + ext_status.status = "success" + + warning_status = ExtensionStatus(seq_no=0, status="warning", code=-1, + message=u"Failed to get status file for {0}".format(handler_name)) + + # list of [seq_no, ext_status_file, is_status_file_exist, expected_result] + test_cases = [ + [-5, None, False, None], + [-1, None, False, None], + [0, None, False, None], + [0, "filename", False, warning_status], + [0, "filename", True, ext_status], + [5, "filename", False, warning_status], + [5, "filename", True, ext_status] + ] + + orig_state = os.path.exists + for case in test_cases: + ext_handler_i = ExtHandlerInstance(exthandler, protocol) + ext_handler_i.get_status_file_path = MagicMock(return_value=(case[0], case[1])) + os.path.exists = MagicMock(return_value=case[2]) + if case[2]: + ext_handler_i.collect_ext_status= MagicMock(return_value=case[3]) + + status = ext_handler_i.get_ext_handling_status(extension) + if case[3] is None: + self.assertEqual(status, None) + else: + self.assertEqual(status.code, case[3].code) + self.assertEqual(status.message, case[3].message) + self.assertEqual(status.status, case[3].status) + os.path.exists = orig_state + def test_is_ext_handling_complete(self, *args): test_data = WireProtocolData(DATA_FILE) exthandlers_handler, protocol = self._create_mock(test_data, *args) @@ -783,18 +828,12 @@ def test_is_ext_handling_complete(self, *args): ext_status = ExtensionStatus(seq_no=0) # Testing no status case - ext_handler_i.collect_ext_status = MagicMock(return_value=None) + ext_handler_i.get_ext_handling_status = MagicMock(return_value=None) completed, status = ext_handler_i.is_ext_handling_complete(ext_status) self.assertTrue(completed) self.assertEqual(status, None) - # Testing FileNotFound case - ext_handler_i.collect_ext_status = MagicMock(side_effect=FileNotFoundError()) - completed, status = ext_handler_i.is_ext_handling_complete(ext_status) - self.assertFalse(completed) - self.assertEqual(status, None) - - ext_handler_i.collect_ext_status = MagicMock(return_value=ext_status) + ext_handler_i.get_ext_handling_status = MagicMock(return_value=ext_status) # Testing different status cases expected_results = { @@ -1116,12 +1155,12 @@ def _validate_installed_extensions(self, expected_extensions, exthandlers_handle def _run_test(self, extensions_to_be_failed, expected_extensions, exthandlers_handler): - def collect_ext_status(ext): + def get_ext_handling_status(ext): status = ExtensionStatus(seq_no=0) status.status = "error" if ext.name in extensions_to_be_failed else "success" return status - ExtHandlerInstance.collect_ext_status = MagicMock(side_effect = collect_ext_status) + ExtHandlerInstance.get_ext_handling_status = MagicMock(side_effect = get_ext_handling_status) exthandlers_handler.handle_ext_handler = MagicMock() exthandlers_handler.run() self._validate_installed_extensions(expected_extensions, exthandlers_handler) From 849e3e0dd1a077ed475251ebc15e289820754854 Mon Sep 17 00:00:00 2001 From: Sirajudeen Sahul Hameed Date: Fri, 14 Sep 2018 07:08:37 +0000 Subject: [PATCH 14/14] Addressing additional comments --- azurelinuxagent/ga/exthandlers.py | 82 +++++------ tests/ga/test_extension.py | 235 +++++++++++++++++------------- 2 files changed, 169 insertions(+), 148 deletions(-) diff --git a/azurelinuxagent/ga/exthandlers.py b/azurelinuxagent/ga/exthandlers.py index cb61c0638b..74109ddeb9 100644 --- a/azurelinuxagent/ga/exthandlers.py +++ b/azurelinuxagent/ga/exthandlers.py @@ -312,52 +312,30 @@ def handle_ext_handlers(self, etag=None): return wait_until = datetime.datetime.utcnow() + datetime.timedelta(minutes=DEFAULT_EXT_TIMEOUT_MINUTES) - prev_handler = None - ext_seq_required = self.is_ext_seq_required() + max_dep_level = max([handler.sort_key() for handler in self.ext_handlers.extHandlers]) self.ext_handlers.extHandlers.sort(key=operator.methodcaller('sort_key')) for ext_handler in self.ext_handlers.extHandlers: - if ext_seq_required: - # Make sure that the previous extension is handled. - # If handled successfully, proceed with the current handler. - # Otherwise, skip the rest of them. - dep_level = ext_handler.sort_key() - if dep_level >= 0: - if self.wait_for_prev_handler_completion(prev_handler, wait_until): - prev_handler = ext_handler - else: - break - self.handle_ext_handler(ext_handler, etag) - def is_ext_seq_required(self): - ''' - If all the extensions have same dependency level - It is considered as extension sequencing is not required - ''' - dep_level = None - for ext_handler in self.ext_handlers.extHandlers: - if dep_level is None: - dep_level = ext_handler.sort_key() - continue - - if dep_level != ext_handler.sort_key(): - return True - - return False + # Wait for the extension installation until it is handled. + # This is done for the install and enable. Not for the uninstallation. + # If handled successfully, proceed with the current handler. + # Otherwise, skip the rest of the extension installation. + dep_level = ext_handler.sort_key() + if dep_level >= 0 and dep_level < max_dep_level: + if not self.wait_for_handler_successful_completion(ext_handler, wait_until): + logger.warn("An extension failed or timed out, will skip processing the rest of the extensions") + break - def wait_for_prev_handler_completion(self, prev_handler, wait_until): + def wait_for_handler_successful_completion(self, ext_handler, wait_until): ''' - Check the status of the previous extension handled. + Check the status of the extension being handled. Wait until it has a terminal state or times out. Return True if it is handled successfully. False if not. ''' - # No need to wait on anything for the very first extension - if prev_handler == None: - return True - - handler_i = ExtHandlerInstance(prev_handler, self.protocol) - for ext in prev_handler.properties.extensions: + handler_i = ExtHandlerInstance(ext_handler, self.protocol) + for ext in ext_handler.properties.extensions: ext_completed, status = handler_i.is_ext_handling_complete(ext) # Keep polling for the extension status until it becomes success or times out @@ -367,9 +345,19 @@ def wait_for_prev_handler_completion(self, prev_handler, wait_until): # In case of timeout or terminal error state, we log it and return false # so that the extensions waiting on this one can be skipped processing - if not ext_completed or status != EXTENSION_STATUS_SUCCESS: - msg = "Extension {0} was timedout or status was: {1}".format(ext.name, status) - logger.info(msg) + if datetime.datetime.utcnow() > wait_until: + msg = "Extension {0} did not reach a terminal state within the allowed timeout. Last status was {1}".format(ext.name, status) + logger.warn(msg) + add_event(AGENT_NAME, + version=CURRENT_VERSION, + op=WALAEventOperation.ExtensionProcessing, + is_success=False, + message=msg) + return False + + if status != EXTENSION_STATUS_SUCCESS: + msg = "Extension {0} did not succeed. Status was {1}".format(ext.name, status) + logger.warn(msg) add_event(AGENT_NAME, version=CURRENT_VERSION, op=WALAEventOperation.ExtensionProcessing, @@ -943,28 +931,26 @@ def get_ext_handling_status(self, ext): # Missing status file is considered a non-terminal state here # so that extension sequencing can wait until it becomes existing if not os.path.exists(ext_status_file): - ext_status = ExtensionStatus(seq_no=seq_no) - ext_status.message = u"Failed to get status file for {0}".format(ext.name) - ext_status.code = -1 - ext_status.status = "warning" + status = "warning" else: ext_status = self.collect_ext_status(ext) + status = ext_status.status if ext_status is not None else None - return ext_status + return status def is_ext_handling_complete(self, ext): status = self.get_ext_handling_status(ext) - # when there is no status, the handling is complete and return None status + # when seq < 0 (i.e. no new user settings), the handling is complete and return None status if status is None: return (True, None) # If not in terminal state, it is incomplete - if status.status not in EXTENSION_TERMINAL_STATUSES: - return (False, status.status) + if status not in EXTENSION_TERMINAL_STATUSES: + return (False, status) # Extension completed, return its status - return (True, status.status) + return (True, status) def report_ext_status(self): active_exts = [] diff --git a/tests/ga/test_extension.py b/tests/ga/test_extension.py index 225e607431..e77d68ec5f 100644 --- a/tests/ga/test_extension.py +++ b/tests/ga/test_extension.py @@ -700,77 +700,90 @@ def test_ext_handler_no_reporting_status(self, *args): self._assert_handler_status(protocol.report_vm_status, "Ready", 1, "1.0.0") self._assert_ext_status(protocol.report_ext_status, "error", 0) - def test_wait_for_prev_handler_completion_empty_exts(self, *args): + def test_wait_for_handler_successful_completion_empty_exts(self, *args): + ''' + Testing wait_for_handler_successful_completion() when there is no extension in a handler. + Expected to return True. + ''' test_data = WireProtocolData(DATA_FILE) exthandlers_handler, protocol = self._create_mock(test_data, *args) - prev_handler = ExtHandler(name="Previous") + handler = ExtHandler(name="handler") ExtHandlerInstance.get_ext_handling_status = MagicMock(return_value=None) - self.assertTrue(exthandlers_handler.wait_for_prev_handler_completion(None, datetime.datetime.utcnow())) - self.assertTrue(exthandlers_handler.wait_for_prev_handler_completion(prev_handler, datetime.datetime.utcnow())) - - def _test_wait_for_prev_handler_completion(self, exthandlers_handler): + self.assertTrue(exthandlers_handler.wait_for_handler_successful_completion(handler, datetime.datetime.utcnow())) + + def _helper_wait_for_handler_successful_completion(self, exthandlers_handler): + ''' + Call wait_for_handler_successful_completion() passing a handler with an extension. + Override the wait time to be 5 seconds to minimize the timout duration. + Return the value returned by wait_for_handler_successful_completion(). + ''' handler_name = "Handler" exthandler = ExtHandler(name=handler_name) extension = Extension(name=handler_name) exthandler.properties.extensions.append(extension) - # Override the timout value to minimize the test duration - wait_until = datetime.datetime.utcnow() + datetime.timedelta(seconds=1) - return exthandlers_handler.wait_for_prev_handler_completion(exthandler, wait_until) + # Override the timeout value to minimize the test duration + wait_until = datetime.datetime.utcnow() + datetime.timedelta(seconds=5) + return exthandlers_handler.wait_for_handler_successful_completion(exthandler, wait_until) - def test_wait_for_prev_handler_completion_none(self, *args): + def test_wait_for_handler_successful_completion_no_status(self, *args): + ''' + Testing wait_for_handler_successful_completion() when there is no status file or seq_no is negative. + Expected to return False. + ''' test_data = WireProtocolData(DATA_FILE) exthandlers_handler, protocol = self._create_mock(test_data, *args) ExtHandlerInstance.get_ext_handling_status = MagicMock(return_value=None) - self.assertFalse(self._test_wait_for_prev_handler_completion(exthandlers_handler)) - - def test_wait_for_prev_handler_completion_success_status(self, *args): - test_data = WireProtocolData(DATA_FILE) - exthandlers_handler, protocol = self._create_mock(test_data, *args) - - status = ExtensionStatus(seq_no=0) - status.status = "success" - - ExtHandlerInstance.get_ext_handling_status = MagicMock(return_value=status) - self.assertTrue(self._test_wait_for_prev_handler_completion(exthandlers_handler)) + self.assertFalse(self._helper_wait_for_handler_successful_completion(exthandlers_handler)) - def test_wait_for_prev_handler_completion_success_status_with_substatus(self, *args): + def test_wait_for_handler_successful_completion_success_status(self, *args): + ''' + Testing wait_for_handler_successful_completion() when there is successful status. + Expected to return True. + ''' test_data = WireProtocolData(DATA_FILE) exthandlers_handler, protocol = self._create_mock(test_data, *args) - status = ExtensionStatus(seq_no=0) - status.status = "success" - substatus = ExtensionSubStatus() - substatus.status = "success" - status.substatusList.append(substatus) + status = "success" ExtHandlerInstance.get_ext_handling_status = MagicMock(return_value=status) - self.assertTrue(self._test_wait_for_prev_handler_completion(exthandlers_handler)) + self.assertTrue(self._helper_wait_for_handler_successful_completion(exthandlers_handler)) - def test_wait_for_prev_handler_completion_error_status(self, *args): + def test_wait_for_handler_successful_completion_error_status(self, *args): + ''' + Testing wait_for_handler_successful_completion() when there is error status. + Expected to return False. + ''' test_data = WireProtocolData(DATA_FILE) exthandlers_handler, protocol = self._create_mock(test_data, *args) - status = ExtensionStatus(seq_no=0) - status.status = "error" + status = "error" ExtHandlerInstance.get_ext_handling_status = MagicMock(return_value=status) - self.assertFalse(self._test_wait_for_prev_handler_completion(exthandlers_handler)) + self.assertFalse(self._helper_wait_for_handler_successful_completion(exthandlers_handler)) - def test_wait_for_prev_handler_completion_timeout(self, *args): + def test_wait_for_handler_successful_completion_timeout(self, *args): + ''' + Testing wait_for_handler_successful_completion() when there is non terminal status. + Expected to return False. + ''' test_data = WireProtocolData(DATA_FILE) exthandlers_handler, protocol = self._create_mock(test_data, *args) - status = ExtensionStatus(seq_no=0) - status.status = "warning" + # Choose a non-terminal status + status = "warning" ExtHandlerInstance.get_ext_handling_status = MagicMock(return_value=status) - self.assertFalse(self._test_wait_for_prev_handler_completion(exthandlers_handler)) + self.assertFalse(self._helper_wait_for_handler_successful_completion(exthandlers_handler)) def test_get_ext_handling_status(self, *args): + ''' + Testing get_ext_handling_status() function with various cases and + verifying against the expected values + ''' test_data = WireProtocolData(DATA_FILE) exthandlers_handler, protocol = self._create_mock(test_data, *args) @@ -779,23 +792,17 @@ def test_get_ext_handling_status(self, *args): extension = Extension(name=handler_name) exthandler.properties.extensions.append(extension) - ext_status = ExtensionStatus(seq_no=0) - ext_status.code = 1 - ext_status.message = "Test message" - ext_status.status = "success" - - warning_status = ExtensionStatus(seq_no=0, status="warning", code=-1, - message=u"Failed to get status file for {0}".format(handler_name)) - - # list of [seq_no, ext_status_file, is_status_file_exist, expected_result] + # In the following list of test cases, the first element corresponds to seq_no. + # the second element is the status file name, the third element indicates if the status file exits or not. + # The fourth element is the expected value from get_ext_handling_status() test_cases = [ [-5, None, False, None], [-1, None, False, None], [0, None, False, None], - [0, "filename", False, warning_status], - [0, "filename", True, ext_status], - [5, "filename", False, warning_status], - [5, "filename", True, ext_status] + [0, "filename", False, "warning"], + [0, "filename", True, ExtensionStatus(status="success")], + [5, "filename", False, "warning"], + [5, "filename", True, ExtensionStatus(status="success")] ] orig_state = os.path.exists @@ -804,18 +811,22 @@ def test_get_ext_handling_status(self, *args): ext_handler_i.get_status_file_path = MagicMock(return_value=(case[0], case[1])) os.path.exists = MagicMock(return_value=case[2]) if case[2]: + # when the status file exists, it is expected return the value from collect_ext_status() ext_handler_i.collect_ext_status= MagicMock(return_value=case[3]) status = ext_handler_i.get_ext_handling_status(extension) - if case[3] is None: - self.assertEqual(status, None) + if case[2]: + self.assertEqual(status, case[3].status) else: - self.assertEqual(status.code, case[3].code) - self.assertEqual(status.message, case[3].message) - self.assertEqual(status.status, case[3].status) + self.assertEqual(status, case[3]) + os.path.exists = orig_state def test_is_ext_handling_complete(self, *args): + ''' + Testing is_ext_handling_complete() with various input and + verifying against the expected output values. + ''' test_data = WireProtocolData(DATA_FILE) exthandlers_handler, protocol = self._create_mock(test_data, *args) @@ -825,17 +836,15 @@ def test_is_ext_handling_complete(self, *args): exthandler.properties.extensions.append(extension) ext_handler_i = ExtHandlerInstance(exthandler, protocol) - ext_status = ExtensionStatus(seq_no=0) # Testing no status case ext_handler_i.get_ext_handling_status = MagicMock(return_value=None) - completed, status = ext_handler_i.is_ext_handling_complete(ext_status) + completed, status = ext_handler_i.is_ext_handling_complete(extension) self.assertTrue(completed) self.assertEqual(status, None) - ext_handler_i.get_ext_handling_status = MagicMock(return_value=ext_status) - - # Testing different status cases + # Here the key represents the possible input value to is_ext_handling_complete() + # the value represents the output tuple from is_ext_handling_complete() expected_results = { "error": (True, "error"), "success": (True, "success"), @@ -844,8 +853,8 @@ def test_is_ext_handling_complete(self, *args): } for key in expected_results.keys(): - ext_status.status = key - completed, status = ext_handler_i.is_ext_handling_complete(ext_status) + ext_handler_i.get_ext_handling_status = MagicMock(return_value=key) + completed, status = ext_handler_i.is_ext_handling_complete(extension) self.assertEqual(completed, expected_results[key][0]) self.assertEqual(status, expected_results[key][1]) @@ -1122,14 +1131,17 @@ def _create_mock(self, mock_http_get, MockCryptUtil): handler.ext_handlers, handler.last_etag = protocol.get_ext_handlers() conf.get_enable_overprovisioning = Mock(return_value=False) - def wait_for_prev_handler_completion(prev_handler, wait_until): - return orig_wait_for_prev_handler_completion(prev_handler, datetime.datetime.utcnow()) + def wait_for_handler_successful_completion(prev_handler, wait_until): + return orig_wait_for_handler_successful_completion(prev_handler, datetime.datetime.utcnow() + datetime.timedelta(seconds=5)) - orig_wait_for_prev_handler_completion = handler.wait_for_prev_handler_completion - handler.wait_for_prev_handler_completion = wait_for_prev_handler_completion + orig_wait_for_handler_successful_completion = handler.wait_for_handler_successful_completion + handler.wait_for_handler_successful_completion = wait_for_handler_successful_completion return handler def _set_dependency_levels(self, dependency_levels, exthandlers_handler): + ''' + Creates extensions with the given dependencyLevel + ''' handler_map = dict() all_handlers = [] for h, level in dependency_levels: @@ -1149,61 +1161,78 @@ def _set_dependency_levels(self, dependency_levels, exthandlers_handler): for handler in all_handlers: exthandlers_handler.ext_handlers.extHandlers.append(handler) - def _validate_installed_extensions(self, expected_extensions, exthandlers_handler): + def _validate_extension_sequence(self, expected_sequence, exthandlers_handler): installed_extensions = [a[0].name for a, k in exthandlers_handler.handle_ext_handler.call_args_list] - self.assertListEqual(expected_extensions, installed_extensions, "Expected and actual list of extensions are not equal") + self.assertListEqual(expected_sequence, installed_extensions, "Expected and actual list of extensions are not equal") - def _run_test(self, extensions_to_be_failed, expected_extensions, exthandlers_handler): + def _run_test(self, extensions_to_be_failed, expected_sequence, exthandlers_handler): + ''' + Mocks get_ext_handling_status() to mimic error status for a given extension. + Calls ExtHandlersHandler.run() + Verifies if the ExtHandlersHandler.handle_ext_handler() was called with appropriate extensions + in the expected order. + ''' def get_ext_handling_status(ext): - status = ExtensionStatus(seq_no=0) - status.status = "error" if ext.name in extensions_to_be_failed else "success" + status = "error" if ext.name in extensions_to_be_failed else "success" return status ExtHandlerInstance.get_ext_handling_status = MagicMock(side_effect = get_ext_handling_status) exthandlers_handler.handle_ext_handler = MagicMock() exthandlers_handler.run() - self._validate_installed_extensions(expected_extensions, exthandlers_handler) + self._validate_extension_sequence(expected_sequence, exthandlers_handler) def test_handle_ext_handlers(self, *args): + ''' + Tests extension sequencing among multiple extensions with dependencies. + This test introduces failure in all possible levels and extensions. + Verifies that the sequencing is in the expected order and a failure in one extension + skips the rest of the extensions in the sequence. + ''' exthandlers_handler = self._create_mock(*args) self._set_dependency_levels([("A", 3), ("B", 2), ("C", 2), ("D", 1), ("E", 1), ("F", 1), ("G", 1)], exthandlers_handler) extensions_to_be_failed = [] - expected_extensions = ["D", "E", "F", "G", "B", "C", "A"] - self._run_test(extensions_to_be_failed, expected_extensions, exthandlers_handler) + expected_sequence = ["D", "E", "F", "G", "B", "C", "A"] + self._run_test(extensions_to_be_failed, expected_sequence, exthandlers_handler) extensions_to_be_failed = ["D"] - expected_extensions = ["D"] - self._run_test(extensions_to_be_failed, expected_extensions, exthandlers_handler) + expected_sequence = ["D"] + self._run_test(extensions_to_be_failed, expected_sequence, exthandlers_handler) extensions_to_be_failed = ["E"] - expected_extensions = ["D", "E"] - self._run_test(extensions_to_be_failed, expected_extensions, exthandlers_handler) + expected_sequence = ["D", "E"] + self._run_test(extensions_to_be_failed, expected_sequence, exthandlers_handler) extensions_to_be_failed = ["F"] - expected_extensions = ["D", "E", "F"] - self._run_test(extensions_to_be_failed, expected_extensions, exthandlers_handler) + expected_sequence = ["D", "E", "F"] + self._run_test(extensions_to_be_failed, expected_sequence, exthandlers_handler) extensions_to_be_failed = ["G"] - expected_extensions = ["D", "E", "F", "G"] - self._run_test(extensions_to_be_failed, expected_extensions, exthandlers_handler) + expected_sequence = ["D", "E", "F", "G"] + self._run_test(extensions_to_be_failed, expected_sequence, exthandlers_handler) extensions_to_be_failed = ["B"] - expected_extensions = ["D", "E", "F", "G", "B"] - self._run_test(extensions_to_be_failed, expected_extensions, exthandlers_handler) + expected_sequence = ["D", "E", "F", "G", "B"] + self._run_test(extensions_to_be_failed, expected_sequence, exthandlers_handler) extensions_to_be_failed = ["C"] - expected_extensions = ["D", "E", "F", "G", "B", "C"] - self._run_test(extensions_to_be_failed, expected_extensions, exthandlers_handler) + expected_sequence = ["D", "E", "F", "G", "B", "C"] + self._run_test(extensions_to_be_failed, expected_sequence, exthandlers_handler) extensions_to_be_failed = ["A"] - expected_extensions = ["D", "E", "F", "G", "B", "C", "A"] - self._run_test(extensions_to_be_failed, expected_extensions, exthandlers_handler) + expected_sequence = ["D", "E", "F", "G", "B", "C", "A"] + self._run_test(extensions_to_be_failed, expected_sequence, exthandlers_handler) def test_handle_ext_handlers_with_uninstallation(self, *args): + ''' + Tests extension sequencing among multiple extensions with dependencies when + some extension are to be uninstalled. + Verifies that the sequencing is in the expected order and the uninstallation takes place + prior to all the installation/enable. + ''' exthandlers_handler = self._create_mock(*args) # "A", "D" and "F" are marked as to be uninstalled @@ -1211,43 +1240,49 @@ def test_handle_ext_handlers_with_uninstallation(self, *args): exthandlers_handler) extensions_to_be_failed = [] - expected_extensions = ["A", "D", "F", "E", "G", "B", "C"] - self._run_test(extensions_to_be_failed, expected_extensions, exthandlers_handler) + expected_sequence = ["A", "D", "F", "E", "G", "B", "C"] + self._run_test(extensions_to_be_failed, expected_sequence, exthandlers_handler) def test_handle_ext_handlers_fallback(self, *args): + ''' + This test makes sure that the extension sequencing is applied only when the user specifies + dependency information in the extension. + When there is no dependency specified, the agent is expected to assign dependencyLevel=0 to all extension. + Also, it is expected to install all the extension no matter if there is any failure in any of the extensions. + ''' exthandlers_handler = self._create_mock(*args) self._set_dependency_levels([("A", 1), ("B", 1), ("C", 1), ("D", 1), ("E", 1), ("F", 1), ("G", 1)], exthandlers_handler) - # Make sure that the extension sequencing is not applied - self.assertFalse(exthandlers_handler.is_ext_seq_required()) - expected_extensions = ["A", "B", "C", "D", "E", "F", "G"] + # Expected sequence must contain all the extensions in the given order. + # The following test cases verfy against this same expected sequence no matter if any extension failed + expected_sequence = ["A", "B", "C", "D", "E", "F", "G"] # Make sure that failure in any extension does not prevent other extensions to be installed extensions_to_be_failed = [] - self._run_test(extensions_to_be_failed, expected_extensions, exthandlers_handler) + self._run_test(extensions_to_be_failed, expected_sequence, exthandlers_handler) extensions_to_be_failed = ["A"] - self._run_test(extensions_to_be_failed, expected_extensions, exthandlers_handler) + self._run_test(extensions_to_be_failed, expected_sequence, exthandlers_handler) extensions_to_be_failed = ["B"] - self._run_test(extensions_to_be_failed, expected_extensions, exthandlers_handler) + self._run_test(extensions_to_be_failed, expected_sequence, exthandlers_handler) extensions_to_be_failed = ["C"] - self._run_test(extensions_to_be_failed, expected_extensions, exthandlers_handler) + self._run_test(extensions_to_be_failed, expected_sequence, exthandlers_handler) extensions_to_be_failed = ["D"] - self._run_test(extensions_to_be_failed, expected_extensions, exthandlers_handler) + self._run_test(extensions_to_be_failed, expected_sequence, exthandlers_handler) extensions_to_be_failed = ["E"] - self._run_test(extensions_to_be_failed, expected_extensions, exthandlers_handler) + self._run_test(extensions_to_be_failed, expected_sequence, exthandlers_handler) extensions_to_be_failed = ["F"] - self._run_test(extensions_to_be_failed, expected_extensions, exthandlers_handler) + self._run_test(extensions_to_be_failed, expected_sequence, exthandlers_handler) extensions_to_be_failed = ["G"] - self._run_test(extensions_to_be_failed, expected_extensions, exthandlers_handler) + self._run_test(extensions_to_be_failed, expected_sequence, exthandlers_handler) if __name__ == '__main__': unittest.main()