Fix the version comparator bug for ubuntu pro client version extraction

src/core/src/bootstrap/Constants.py

@@ -382,4 +382,3 @@ class BufferMessage(EnumBackport):
         TRUE = 0
         FALSE = 1
         FLUSH = 2
-

src/core/src/core_logic/VersionComparator.py

@@ -18,7 +18,7 @@
 
 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) """
 
@@ -35,48 +35,46 @@
         # 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_version_nums(self, path):
+    @staticmethod
+    def extract_version_from_os_version_nums(os_version):
         # type (str) -> str
         """
-        Extract the version part from a given path.
-        Input: /var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.2.5/config
-        Return: "1.2.5"
+        Extract the version part from a given os version.
+        Input os version	                        Extracted Version
+        34                                          34
+        34~18                                       34
+        34.~18.04                                   34
+        34.a+18.04.1                                34
+        34abc-18.04                                 34
+        abc34~18.04                                 34
+        abc34~18.04.123                             34
+        34~25.1.2-18.04.1                           34
+        34.1~18.04.1                                34.1
+        34.13.4                                     34.13.4
+        34.13.4~18.04.1                             34.13.4
+        34.13.4-ab+18.04.1                          34.13.4
+        34.13.4abc-18.04.1                          34.13.4
+        abc.34.13.4!@abc                            34.13.4
         """
-        match = re.search(r'([\d]+\.[\d]+\.[\d]+)', path)
-        return match.group(1) if match else str()
+        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, paths):
-        # type (list[str]) -> list[str]
-        """
-        Sort paths based on version numbers extracted from paths.
-        Input:
-            ["Microsoft.CPlat.Core.LinuxPatchExtension-1.21.1001",
-            "Microsoft.CPlat.Core.LinuxPatchExtension-1.6.100",
-            "Microsoft.CPlat.Core.LinuxPatchExtension-1.21.100"]
-        Return:
-            ["Microsoft.CPlat.Core.LinuxPatchExtension-1.21.1001",
-            "Microsoft.CPlat.Core.LinuxPatchExtension-1.21.100",
-            "Microsoft.CPlat.Core.LinuxPatchExtension-1.6.100"]
-        """
-        return sorted(paths, key=self.__version_key, reverse=True)
-
-    def __version_key(self, path):
+    def __version_key(self, version_input):
         # type (str) -> (int)
         """ Extract version number from input and return int tuple.
-        Input: "Microsoft.CPlat.Core.LinuxPatchExtension-1.6.100"
-        Return: (1.6.100)
+        os version input: "34~18.04"
+        Return: (34)
         """
-        version_numbers = self.extract_version_nums(path)
+        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_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_UbuntuProClient.py

@@ -57,6 +57,9 @@ def mock_to_below_minimum_version(self):
     def mock_version_raise_exception(self):
         raise
 
+    def mock_pro_version(self):
+        return MockVersionResult("34~18.004.01")
+
     def mock_import_uaclient_version_module(self, mock_name, method_name):
         if sys.version_info[0] == 3:
             sys.modules['uaclient.api.u.pro.version.v1'] = types.ModuleType('version_module')
@@ -185,6 +188,15 @@ def test_is_pro_working_success(self):
 
         obj.mock_unimport_uaclient_version_module()
 
+    def test_is_actual_pro_version_working_success(self):
+        obj = MockVersionResult()
+        obj.mock_import_uaclient_version_module('version', 'mock_pro_version')
+
+        package_manager = self.container.get('package_manager')
+        self.assertTrue(package_manager.ubuntu_pro_client.is_pro_working())
+
+        obj.mock_unimport_uaclient_version_module()
+
     def test_is_pro_working_failure_when_minimum_version_required_is_false(self):
         obj = MockVersionResult()
         obj.mock_import_uaclient_version_module('version', 'mock_to_below_minimum_version')

src/core/tests/Test_VersionComparator.py

@@ -24,54 +24,35 @@ class TestVersionComparator(unittest.TestCase):
     def setUp(self):
         self.version_comparator = VersionComparator()
 
-    def test_linux_version_comparator(self):
-        # Test extract version logic
-        self.assertEqual(self.version_comparator.extract_version_nums("Microsoft.CPlat.Core.LinuxPatchExtension-1.2.25"), "1.2.25")
-        self.assertEqual(self.version_comparator.extract_version_nums("Microsoft.CPlat.Core.LinuxPatchExtension-1.2.25-abc"), "1.2.25")
-        self.assertEqual(self.version_comparator.extract_version_nums("Microsoft.CPlat.Core.LinuxPatchExtension-1.2.25+abc.123"), "1.2.25")
-        self.assertEqual(self.version_comparator.extract_version_nums("Microsoft.CPlat.Core.LinuxPatchExtension-1.2.25-abc+def.123"), "1.2.25")
-        self.assertEqual(self.version_comparator.extract_version_nums("Microsoft.CPlat.Core.LinuxPatchExtension-1.21.1001"), "1.21.1001")
-        self.assertEqual(self.version_comparator.extract_version_nums("Microsoft.CPlat.Core.LinuxPatchExtension-1.6.100"), "1.6.100")
-        self.assertEqual(self.version_comparator.extract_version_nums("Microsoft.CPlat.Core.LinuxPatchExtension-1.6.99"), "1.6.99")
-        self.assertEqual(self.version_comparator.extract_version_nums("Microsoft.CPlat.Core.LinuxPatchExtension-1.6."), "")
-        self.assertEqual(self.version_comparator.extract_version_nums("Microsoft.CPlat.Core.LinuxPatchExtension-a.b.c"), "")
+    def test_extract_version_from_os_version_nums(self):
+        """ Test extract version logic on Ubuntuproclient version """
+        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_version_from_os_version_nums("34.13.4~18.04.1")  # return 34
+
+        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
+
+        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
 
-        expected_extracted_version = "27.13.4"
-        test_extracted_v1 = self.version_comparator.extract_version_nums("27.13.4~18.04.1")
-        test_extracted_v2 = self.version_comparator.extract_version_nums("27.13.4+18.04.1")
-        test_extracted_v3 = self.version_comparator.extract_version_nums("27.13.4-18.04.1")
-
-        self.assertEqual(test_extracted_v1, expected_extracted_version)
-        self.assertEqual(test_extracted_v2, expected_extracted_version)
-        self.assertEqual(test_extracted_v3, expected_extracted_version)
-
-        # Test compare versions logic
-        self.assertEqual(self.version_comparator.compare_version_nums(test_extracted_v1, "27.13.4"), 0) # equal
-        self.assertEqual(self.version_comparator.compare_version_nums(test_extracted_v2, "27.13.3"), 1) # greater
-        self.assertEqual(self.version_comparator.compare_version_nums(test_extracted_v3, "27.13.5"), -1) # less
-
-        # Test sort versions logic
-        unsorted_path_versions = [
-            "Microsoft.CPlat.Core.LinuxPatchExtension-1.2.25-abc+def.123",
-            "Microsoft.CPlat.Core.LinuxPatchExtension-1.21.1001",
-            "Microsoft.CPlat.Core.LinuxPatchExtension-1.6.100",
-            "Microsoft.CPlat.Core.LinuxPatchExtension-1.6.99",
-            "Microsoft.CPlat.Core.LinuxPatchExtension-1.21.100",
-            "Microsoft.CPlat.Core.LinuxPatchExtension-1.2.25-abc",
-        ]
-
-        expected_sorted_path_versions = [
-            "Microsoft.CPlat.Core.LinuxPatchExtension-1.21.1001",
-            "Microsoft.CPlat.Core.LinuxPatchExtension-1.21.100",
-            "Microsoft.CPlat.Core.LinuxPatchExtension-1.6.100",
-            "Microsoft.CPlat.Core.LinuxPatchExtension-1.6.99",
-            "Microsoft.CPlat.Core.LinuxPatchExtension-1.2.25-abc+def.123",
-            "Microsoft.CPlat.Core.LinuxPatchExtension-1.2.25-abc"
-        ]
-
-        # valid versions
-        self.assertEqual(self.version_comparator.sort_versions_desc_order(unsorted_path_versions), expected_sorted_path_versions)
 
 if __name__ == '__main__':
     unittest.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]

src/extension/src/ExtVersionComparator.py

@@ -0,0 +1,68 @@
+# Copyright 2024 Microsoft Corporation
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+# Requires Python 2.7+
+import os.path
+import re
+
+
+class ExtVersionComparator(object):
+
+    def sort_ext_paths_desc_order(self, ext_paths_with_versions):
+        # 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"]
+        """
+
+        return sorted(ext_paths_with_versions, key=self.__version_key, reverse=True)
+
+    @staticmethod
+    def __extract_lpe_path_version_num(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 __version_key(self, version_input):
+        # type (str) -> (int)
+        """ Extract version number from input and return int tuple.
+        Input: "/var/lib/waagent/Microsoft.CPlat.Core.LinuxPatchExtension-1.6.100"
+        Return: (1.6.100)
+        """
+
+        version_numbers = self.__extract_lpe_path_version_num(lpe_path=version_input)
+        return tuple(map(int, version_numbers.split('.'))) if version_numbers else (0, 0, 0)