Version Comparator Comments as Code on PR Branch (#296)

Author: kjohn-msft

src/core/src/bootstrap/Constants.py

@@ -382,7 +382,3 @@ class BufferMessage(EnumBackport):
         TRUE = 0
         FALSE = 1
         FLUSH = 2
-
-    class VersionComparator(EnumBackport):
-        LINUXPATCHEXTENSION = "LinuxPatchExtension"
-

src/core/src/core_logic/VersionComparator.py

@@ -13,15 +13,12 @@
 # limitations under the License.
 #
 # Requires Python 2.7+
-import os.path
 import re
 
-from core.src.bootstrap.Constants import Constants
-
 
 class VersionComparator(object):
 
-    def compare_version_nums(self, version_a, version_b):
+    def compare_versions(self, version_a, version_b):
         # type (str, str) -> int
         """ Compare two versions with handling numeric and string parts, return -1 (less), +1 (greater), 0 (equal) """
 
@@ -38,28 +35,8 @@ def compare_version_nums(self, version_a, version_b):
         # If equal 27.13.4 vs 27.13.4, return 0
         return (len(parse_version_a) > len(parse_version_b)) - (len(parse_version_a) < len(parse_version_b))
 
-    def extract_lpe_path_version_num(self, lpe_path):
-        # type (str) -> str
-        """
-        Extract the version part from a given lpe path.
-        Input	                                                                          Extracted Version
-        /var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.2.25                  "1.2.25"
-        /var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.2.250                 "1.2.250"
-        /var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.21.2501               "1.21.2501"
-        /var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.2.25.                 "1.2.25"
-        /var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.2.25..                "1.2.25"
-        /var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.2.25abc               "1.2.25"
-        /var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.2.25.abc              "1.2.25"
-        /var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.2.25+abc.123          "1.2.25"
-        /var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.2.25-abc+def.123      "1.2.25"
-        /var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-a.b.c                     ""
-        """
-
-        lpe_filename = os.path.basename(lpe_path)  # Microsoft.CPlat.Core.LinuxPatchExtension-x.x.xx
-        lpe_version = re.search(r'(\d+(?:\.\d+)*)', lpe_filename)  # extract numbers with optional dot-separated parts
-        return lpe_version.group(1).rstrip('.') if lpe_version else ""
-
-    def extract_os_version_nums(self, os_version):
+    @staticmethod
+    def extract_version_from_os_version_nums(os_version):
         # type (str) -> str
         """
         Extract the version part from a given os version.
@@ -82,59 +59,22 @@ def extract_os_version_nums(self, os_version):
         version_num = re.search(r'(\d+(?:\.\d+)*)', os_version)  # extract numbers with optional dot-separated parts
         return version_num.group(1) if version_num else str()
 
-    def sort_versions_desc_order(self, version_list):
-        # type (list[str]) -> list[str]
-        """
-        Sort paths based on version numbers extracted from paths.
-        Lpe input:
-            ["/var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.21.1001",
-            "/var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.6.100",
-            "/var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.21.100"]
-        Return:
-            ["/var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.21.1001",
-            "/var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.21.100",
-            "/var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.6.100"]
-
-        Os Version input:
-            ["32.101~18.01",
-            "32.101.15~18",
-            "34~18.04",
-            "32~18.04.01",
-            "32.1~18.04.01"]
-
-        return:
-            ["34~18.04",
-            "32.101.15~18",
-            "32.101~18.01",
-            "32.1~18.04.01",
-            "32~18.04.01"]
-        """
-        return sorted(version_list, key=self.__version_key, reverse=True)
-
     def __version_key(self, version_input):
         # type (str) -> (int)
         """ Extract version number from input and return int tuple.
-        Lpe input: "/var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.6.100"
-        Return: (1.6.100)
-
         os version input: "34~18.04"
         Return: (34)
         """
-        if Constants.VersionComparator.LINUXPATCHEXTENSION in version_input:
-            version_numbers = self.extract_lpe_path_version_num(lpe_path=version_input)
-        else:
-            version_numbers = self.extract_os_version_nums(os_version=version_input)
-
+        version_numbers = self.extract_version_from_os_version_nums(os_version=version_input)
         return tuple(map(int, version_numbers.split('.'))) if version_numbers else (0, 0, 0)
 
-    def __split_version_components(self, version):
-        # type (str) -> [any]
-        """ Split a version into numeric and non-numeric into components list: 27.13.4~18.04.1 -> [27][14][4]"""
-        return [int(x) if x.isdigit() else x for x in re.split(r'(\d+)', version) if x]
-
     def __parse_version(self, version_components):
         # type (str) -> [[any]]
-        """ Parse the split version list into list [27][14][4] -> [[27], [14], [4]]"""
+        """ Parse the split version list into list [27][14][4] -> [[27], [14], [4]] """
         return [self.__split_version_components(x) for x in version_components.split(".")]
 
-
+    @staticmethod
+    def __split_version_components(version):
+        # type (str) -> [any]
+        """ Splits a version into numeric and non-numeric into components list: 27.13.4~18.04.1 -> [27][14][4] """
+        return [int(x) if x.isdigit() else x for x in re.split(r'(\d+)', version) if x]

src/core/src/package_managers/UbuntuProClient.py

@@ -57,12 +57,12 @@ def is_pro_working(self):
             ubuntu_pro_client_version = version_result.installed_version
 
             # extract version from pro_client_verison 27.13.4~18.04.1 -> 27.13.4
-            extracted_ubuntu_pro_client_version = self.version_comparator.extract_os_version_nums(ubuntu_pro_client_version)
+            extracted_ubuntu_pro_client_version = self.version_comparator.extract_version_from_os_version_nums(ubuntu_pro_client_version)
 
             self.composite_logger.log_debug("Ubuntu Pro Client current version: [ClientVersion={0}]".format(str(extracted_ubuntu_pro_client_version)))
 
             # use custom comparator output 0 (equal), -1 (less), +1 (greater)
-            is_minimum_ubuntu_pro_version_installed = self.version_comparator.compare_version_nums(extracted_ubuntu_pro_client_version, Constants.UbuntuProClientSettings.MINIMUM_CLIENT_VERSION) >= 0
+            is_minimum_ubuntu_pro_version_installed = self.version_comparator.compare_versions(extracted_ubuntu_pro_client_version, Constants.UbuntuProClientSettings.MINIMUM_CLIENT_VERSION) >= 0
 
             if ubuntu_pro_client_version is not None and is_minimum_ubuntu_pro_version_installed:
                 is_ubuntu_pro_client_working = True

src/core/tests/Test_VersionComparator.py

@@ -24,108 +24,34 @@ class TestVersionComparator(unittest.TestCase):
     def setUp(self):
         self.version_comparator = VersionComparator()
 
-    def test_linux_extension_version_extract_comparator(self):
-        """ Test extract version logic on Extension package """
-        self.assertEqual(self.version_comparator.extract_lpe_path_version_num("/var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.2.25"), "1.2.25")
-        self.assertEqual(self.version_comparator.extract_lpe_path_version_num("/var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.2.250"), "1.2.250")
-        self.assertEqual(self.version_comparator.extract_lpe_path_version_num("/var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.21.2501"),
-            "1.21.2501")
-        self.assertEqual(self.version_comparator.extract_lpe_path_version_num("/var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.2.25."), "1.2.25")
-        self.assertEqual(self.version_comparator.extract_lpe_path_version_num("/var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.2.25.."), "1.2.25")
-        self.assertEqual(self.version_comparator.extract_lpe_path_version_num("/var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.2.25abc"), "1.2.25")
-        self.assertEqual(self.version_comparator.extract_lpe_path_version_num("/var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.2.25.abc"), "1.2.25")
-        self.assertEqual(self.version_comparator.extract_lpe_path_version_num("/var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.2.25+abc.123"),
-            "1.2.25")
-        self.assertEqual(self.version_comparator.extract_lpe_path_version_num("/var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.2.25-abc+def.123"),
-            "1.2.25")
-        self.assertEqual(self.version_comparator.extract_lpe_path_version_num("/var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-a.b.c"), "")
-
-    def test_linux_lpe_version_comparison(self):
-        """ Test compare versions logic lpe version with existing vm version """
-        test_extracted_good_version = self.version_comparator.extract_os_version_nums(
-            "/var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.2.25")  # return 1.2.25
-
-        self.assertEqual(self.version_comparator.compare_version_nums(test_extracted_good_version, "1.2.25"), 0)  # equal 1.2.25 == 1.2.25
-        self.assertEqual(self.version_comparator.compare_version_nums(test_extracted_good_version, "1.2.24"), 1)  # greater 1.2.25 > 1.2.24
-        self.assertEqual(self.version_comparator.compare_version_nums(test_extracted_good_version, "1.19.25"), -1)  # less 1.2.25 < 1.19.25, 19 > 2
-
-        test_extracted_bad_version = self.version_comparator.extract_os_version_nums(
-            "/var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-a.b.c")  # return ""
-        self.assertEqual(self.version_comparator.compare_version_nums(test_extracted_bad_version, "1.2.25"), -1)  # less  "" != 1.2.25
-
-    def test_linux_extension_sort_comparator(self):
-        """ Test sorting comparator on linux patch extension """
-        unsorted_lpe_versions = [
-            "/var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.2.25-abc+def.123",
-            "/var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.21.1001",
-            "/var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.6.100",
-            "/var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.6.99",
-            "/var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.21.100",
-            "/var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.2.25-abc",
-            "/var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.6.9",
-        ]
-
-        expected_sorted_lpe_versions = [
-            "/var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.21.1001",
-            "/var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.21.100",
-            "/var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.6.100",
-            "/var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.6.99",
-            "/var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.6.9",
-            "/var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.2.25-abc+def.123",
-            "/var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.2.25-abc"
-        ]
-
-        # validate sorted lpe versions
-        self.assertEqual(self.version_comparator.sort_versions_desc_order(unsorted_lpe_versions), expected_sorted_lpe_versions)
-
-    def test_linux_os_version_extract_comparator(self):
+    def test_extract_version_from_os_version_nums(self):
         """ Test extract version logic on Ubuntuproclient version """
-        self.assertEqual(self.version_comparator.extract_os_version_nums("34"), "34")
-        self.assertEqual(self.version_comparator.extract_os_version_nums("34~18"), "34")
-        self.assertEqual(self.version_comparator.extract_os_version_nums("34.~18.04"), "34")
-        self.assertEqual(self.version_comparator.extract_os_version_nums("34.a+18.04.1"), "34")
-        self.assertEqual(self.version_comparator.extract_os_version_nums("34abc-18.04"), "34")
-        self.assertEqual(self.version_comparator.extract_os_version_nums("abc34~18.04"), "34")
-        self.assertEqual(self.version_comparator.extract_os_version_nums("abc34~18.04.123"), "34")
-        self.assertEqual(self.version_comparator.extract_os_version_nums("34~25.1.2-18.04.1"), "34")
-
-        self.assertEqual(self.version_comparator.extract_os_version_nums("34.1~18.04.1"), "34.1")
-        self.assertEqual(self.version_comparator.extract_os_version_nums("34.13.4"), "34.13.4")
-        self.assertEqual(self.version_comparator.extract_os_version_nums("34.13.4~18.04.1"), "34.13.4")
-        self.assertEqual(self.version_comparator.extract_os_version_nums("34.13.4-ab+18.04.1"), "34.13.4")
-        self.assertEqual(self.version_comparator.extract_os_version_nums("34.13.4abc-18.04.1"), "34.13.4")
-        self.assertEqual(self.version_comparator.extract_os_version_nums("abc.34.13.4!@abc"), "34.13.4")
+        self.assertEqual(self.version_comparator.extract_version_from_os_version_nums("34"), "34")
+        self.assertEqual(self.version_comparator.extract_version_from_os_version_nums("34~18"), "34")
+        self.assertEqual(self.version_comparator.extract_version_from_os_version_nums("34.~18.04"), "34")
+        self.assertEqual(self.version_comparator.extract_version_from_os_version_nums("34.a+18.04.1"), "34")
+        self.assertEqual(self.version_comparator.extract_version_from_os_version_nums("34abc-18.04"), "34")
+        self.assertEqual(self.version_comparator.extract_version_from_os_version_nums("abc34~18.04"), "34")
+        self.assertEqual(self.version_comparator.extract_version_from_os_version_nums("abc34~18.04.123"), "34")
+        self.assertEqual(self.version_comparator.extract_version_from_os_version_nums("34~25.1.2-18.04.1"), "34")
+
+        self.assertEqual(self.version_comparator.extract_version_from_os_version_nums("34.1~18.04.1"), "34.1")
+        self.assertEqual(self.version_comparator.extract_version_from_os_version_nums("34.13.4"), "34.13.4")
+        self.assertEqual(self.version_comparator.extract_version_from_os_version_nums("34.13.4~18.04.1"), "34.13.4")
+        self.assertEqual(self.version_comparator.extract_version_from_os_version_nums("34.13.4-ab+18.04.1"), "34.13.4")
+        self.assertEqual(self.version_comparator.extract_version_from_os_version_nums("34.13.4abc-18.04.1"), "34.13.4")
+        self.assertEqual(self.version_comparator.extract_version_from_os_version_nums("abc.34.13.4!@abc"), "34.13.4")
 
     def test_linux_os_version_comparison(self):
         """ Test compare versions logic Ubuntuproclient version with existing vm version """
-        test_extracted_good_version = self.version_comparator.extract_os_version_nums("34.13.4~18.04.1")  # return 34
-
-        self.assertEqual(self.version_comparator.compare_version_nums(test_extracted_good_version, "34.13.4"), 0)  # equal  34.13.4 == 34.13.4
-        self.assertEqual(self.version_comparator.compare_version_nums(test_extracted_good_version, "34.13.3"), 1)  # greater 34.13.4 > 34.13.3
-        self.assertEqual(self.version_comparator.compare_version_nums(test_extracted_good_version, "34.13.5"), -1)  # less 34.13.4 < 34.13.5
-
-        test_extracted_bad_version = self.version_comparator.extract_os_version_nums("abc~18.04.1")  # return ""
-        self.assertEqual(self.version_comparator.compare_version_nums(test_extracted_bad_version, "34.13.4"), -1)  # less "" < 34.13.4
-
-    def test_os_version_sort_comparator(self):
-        """ Test sorting comparator on linux os version """
-        unsorted_os_versions = [
-            "32.101.~18.01",
-            "32.101.15~18",
-            "abc34~18.04",
-            "32~18.04.01",
-            "32.1~18.04.01"
-        ]
+        test_extracted_good_version = self.version_comparator.extract_version_from_os_version_nums("34.13.4~18.04.1")  # return 34
 
-        expected_sorted_os_versions = [
-            "abc34~18.04",
-            "32.101.15~18",
-            "32.101.~18.01",
-            "32.1~18.04.01",
-            "32~18.04.01"
-        ]
+        self.assertEqual(self.version_comparator.compare_versions(test_extracted_good_version, "34.13.4"), 0)  # equal  34.13.4 == 34.13.4
+        self.assertEqual(self.version_comparator.compare_versions(test_extracted_good_version, "34.13.3"), 1)  # greater 34.13.4 > 34.13.3
+        self.assertEqual(self.version_comparator.compare_versions(test_extracted_good_version, "34.13.5"), -1)  # less 34.13.4 < 34.13.5
 
-        self.assertEqual(self.version_comparator.sort_versions_desc_order(unsorted_os_versions), expected_sorted_os_versions)
+        test_extracted_bad_version = self.version_comparator.extract_version_from_os_version_nums("abc~18.04.1")  # return ""
+        self.assertEqual(self.version_comparator.compare_versions(test_extracted_bad_version, "34.13.4"), -1)  # less "" < 34.13.4
 
 
 if __name__ == '__main__':

src/extension/src/ActionHandler.py

@@ -23,8 +23,7 @@
 from extension.src.Constants import Constants
 from extension.src.EnableCommandHandler import EnableCommandHandler
 from extension.src.InstallCommandHandler import InstallCommandHandler
-from extension.src.Utility import Utility
-from extension.src.VersionComparatorHandler import VersionComparatorHandler
+from extension.src.ExtVersionComparator import ExtVersionComparator
 from extension.src.local_loggers.StdOutFileMirror import StdOutFileMirror
 
 
@@ -49,7 +48,7 @@ def __init__(self, logger, env_layer, telemetry_writer, utility, runtime_context
         self.file_logger = None
         self.operation_id_substitute_for_all_actions_in_telemetry = str((datetime.datetime.utcnow()).strftime(Constants.UTC_DATETIME_FORMAT))
         self.seq_no = self.ext_config_settings_handler.get_seq_no_from_env_var()
-        self.version_comparator_handler = VersionComparatorHandler()
+        self.ext_version_comparator = ExtVersionComparator()
 
     def determine_operation(self, command):
         switcher = {
@@ -228,7 +227,7 @@ def update(self):
             self.logger.log("Fetching the extension version preceding current from all available versions...")
 
             # use custom sort logic to sort path based on version numbers
-            sorted_versions = self.version_comparator_handler.sort_versions_desc_order(paths_to_all_versions)
+            sorted_versions = self.ext_version_comparator.sort_ext_paths_desc_order(paths_to_all_versions)
             self.logger.log_debug("List of extension versions in descending order: [SortedVersion={0}]".format(sorted_versions))
 
             preceding_version_path = sorted_versions[1]