CRAYSAT-1551: Fix sorting of product versions in sat showrev --products

.gitignore

@@ -17,3 +17,4 @@ venv
 
 nosetests.xml
 .vscode/
+.idea

CHANGELOG.md

@@ -25,6 +25,12 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [3.32.12] - 2024-11-21
+
+### Fixed
+- Fix sorting by product version in `sat showrev --products` to sort by semantic
+  version rather than a simple lexicographic order.
+
 ## [3.32.11] - 2024-11-12
 
 ### Fixed
@@ -65,7 +71,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## [3.32.4] - 2024-09-27
 
 ### Added
-- Add jinja rendering of rootfs_provider_passthrough value for the boot_set to create session 
+- Add jinja rendering of rootfs_provider_passthrough value for the boot_set to create session
   template with iSCSI values.
 
 ## [3.32.3] - 2024-09-27
@@ -101,7 +107,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [3.31.0] - 2024-08-21
 
-### Fixed 
+### Fixed
 - Updating the cray-product-catalog & python-csm-api-client to latest versions.
 
 ## [3.30.2] - 2024-08-14
@@ -326,7 +332,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## [3.27.11] - 2024-02-28
 
 ### Fixed
-- Fixed for `sat showrev` to stop supporting `--release-files` option 
+- Fixed for `sat showrev` to stop supporting `--release-files` option
   and log a warning message indicating that this option is no longer supported.
 
 ## [3.27.10] - 2024-02-26
@@ -357,7 +363,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## [3.27.6] - 2024-02-16
 
 ### Fixed
-- Remove unnecessary queries to BOS to get the name of the session template for 
+- Remove unnecessary queries to BOS to get the name of the session template for
   every single node component in the output of `sat status`.
 
 ## [3.27.5] - 2024-02-07
@@ -371,7 +377,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Security
 - Update the version of jinja2 from 3.0.3 to 3.1.3 to address
   CVE-2024-22195
-  
+
 ## [3.27.3] - 2024-01-31
 
 ### Fixed

sat/cli/showrev/products.py

@@ -27,6 +27,7 @@
 import logging
 
 from cray_product_catalog.query import ProductCatalog, ProductCatalogError
+from sat.loose_version import LooseVersion
 
 
 LOGGER = logging.getLogger(__name__)
@@ -66,6 +67,6 @@ def get_product_versions():
     for product in product_catalog.products:
         images = '\n'.join(sorted(image['name'] for image in product.images)) or '-'
         recipes = '\n'.join(sorted(recipe['name'] for recipe in product.recipes)) or '-'
-        products.append([product.name, product.version, images, recipes])
+        products.append([product.name, LooseVersion(product.version), images, recipes])
 
     return headers, products

sat/loose_version.py

@@ -0,0 +1,98 @@
+#
+# MIT License
+#
+# (C) Copyright 2024 Hewlett Packard Enterprise Development LP
+#
+# Permission is hereby granted, free of charge, to any person obtaining a
+# copy of this software and associated documentation files (the "Software"),
+# to deal in the Software without restriction, including without limitation
+# the rights to use, copy, modify, merge, publish, distribute, sublicense,
+# and/or sell copies of the Software, and to permit persons to whom the
+# Software is furnished to do so, subject to the following conditions:
+#
+# The above copyright notice and this permission notice shall be included
+# in all copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
+# OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+# ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+# OTHER DEALINGS IN THE SOFTWARE.
+#
+"""
+Class for representing a semver version
+"""
+from semver import Version
+
+
+class LooseVersion:
+    """A LooseVersion representing a version that may be semver or may not.
+
+    This class is used to compare versions that may or may not comply with the
+    semver format. If the version does not comply with the semver format, then
+    the MAX_VERSION is used as the comparison version, which results in the
+    version being considered greater than any other version.
+
+    Args:
+        version_str (str): The string representation of the version.
+
+    Attributes:
+        version_str (str): The string representation of the version.
+        comparison_version (semver.version.Version): The semver.version.Version
+            object of either the version_str or the MAX_VERSION if semver fails to
+            parse the version
+    """
+
+    MAX_VERSION = "99999999999.99999999999.99999999999"
+
+    def __init__(self, version_str):
+        """Creates a new LooseVersion object from the given product_version string.
+
+        Args:
+            version_str (str): The string representation of the version.
+        """
+        self.version_str = version_str
+        self.comparison_version = self.parse_version(version_str)
+
+    def __str__(self):
+        return f'{self.version_str}'
+
+    def __repr__(self):
+        return f"{self.__class__.__name__}('{self.version_str}')"
+
+    def __lt__(self, other):
+        return self.comparison_version < other.comparison_version
+
+    def __le__(self, other):
+        return self.comparison_version <= other.comparison_version
+
+    def __eq__(self, other):
+        return (isinstance(self, type(other)) and
+                self.comparison_version == other.comparison_version)
+
+    def __gt__(self, other):
+        return self.comparison_version > other.comparison_version
+
+    def __ge__(self, other):
+        return self.comparison_version >= other.comparison_version
+
+    def parse_version(self, version_str):
+        """Parse the version string into a semver.version.Version object if possible.
+
+        Args:
+            version_str (str): The string representation of the version.
+
+        Returns:
+            semver.version.Version: The semver.version.Version object of either
+                the version_str or the MAX_VERSION if semver fails to parse the
+                version
+        """
+
+        try:
+            parsed_version = Version.parse(version_str)
+        except ValueError:
+            parsed_version = Version.parse(self.MAX_VERSION)
+
+        return parsed_version

sat/util.py

@@ -1,7 +1,7 @@
 #
 # MIT License
 #
-# (C) Copyright 2019-2023 Hewlett Packard Enterprise Development LP
+# (C) Copyright 2019-2024 Hewlett Packard Enterprise Development LP
 #
 # Permission is hereby granted, free of charge, to any person obtaining a
 # copy of this software and associated documentation files (the "Software"),
@@ -37,6 +37,7 @@
 import re
 import time
 
+
 # Logic borrowed from imps to get the most efficient YAML available
 try:
     from yaml import CSafeDumper as SafeDumper
@@ -49,6 +50,7 @@
 from prettytable import PrettyTable
 
 from sat.xname import XName
+from sat.loose_version import LooseVersion
 from sat.config import get_config_value, read_config_value_file
 
 
@@ -348,8 +350,15 @@ def _xname_representer(dumper, xname):
     )
 
 
+def _looseversion_representer(dumper, looseversion):
+    return dumper.represent_scalar(
+        BaseResolver.DEFAULT_SCALAR_TAG, looseversion.version_str
+    )
+
+
 SATDumper.add_representer(OrderedDict, _ordered_dict_representer)
 SATDumper.add_representer(XName, _xname_representer)
+SATDumper.add_representer(LooseVersion, _looseversion_representer)
 
 
 # A function to dump YAML to be used by all SAT code.
@@ -369,6 +378,8 @@ def default(self, o):
         """
         if isinstance(o, XName):
             return str(o)
+        if isinstance(o, LooseVersion):
+            return o.version_str
         return JSONEncoder.default(self, o)
 
 

tests/cli/showrev/test_products.py

@@ -33,6 +33,7 @@
 from cray_product_catalog.query import InstalledProductVersion, ProductCatalogError
 
 from sat.cli.showrev.products import get_product_versions
+from sat.loose_version import LooseVersion
 from tests.test_util import ExtendedTestCase
 
 SAMPLES_DIR = os.path.join(os.path.dirname(__file__), 'samples')
@@ -90,9 +91,9 @@ def tearDown(self):
     def test_get_product_versions(self):
         """Test a basic invocation of get_product_versions."""
         expected_fields = [
-            ['cos', '1.4.0', COS_IMAGE_NAME, COS_RECIPE_NAME],
-            ['uan', '2.0.0', UAN_IMAGE_NAME, UAN_RECIPE_NAME],
-            ['pbs', '0.1.0', '-', '-']
+            ['cos', LooseVersion('1.4.0'), COS_IMAGE_NAME, COS_RECIPE_NAME],
+            ['uan', LooseVersion('2.0.0'), UAN_IMAGE_NAME, UAN_RECIPE_NAME],
+            ['pbs', LooseVersion('0.1.0'), '-', '-']
         ]
         actual_headers, actual_fields = get_product_versions()
         self.mock_product_catalog_cls.assert_called_once_with()
@@ -108,10 +109,10 @@ def test_get_product_versions_multiple_versions(self):
         self.mock_product_catalog.products.insert(2, new_uan_product)
 
         expected_fields = [
-            ['cos', '1.4.0', COS_IMAGE_NAME, COS_RECIPE_NAME],
-            ['uan', '2.0.0', UAN_IMAGE_NAME, UAN_RECIPE_NAME],
-            ['uan', '2.0.1', new_uan_image_name, new_uan_recipe_name],
-            ['pbs', '0.1.0', '-', '-']
+            ['cos', LooseVersion('1.4.0'), COS_IMAGE_NAME, COS_RECIPE_NAME],
+            ['uan', LooseVersion('2.0.0'), UAN_IMAGE_NAME, UAN_RECIPE_NAME],
+            ['uan', LooseVersion('2.0.1'), new_uan_image_name, new_uan_recipe_name],
+            ['pbs', LooseVersion('0.1.0'), '-', '-']
         ]
         actual_headers, actual_fields = get_product_versions()
         self.mock_product_catalog_cls.assert_called_once_with()
@@ -125,9 +126,10 @@ def test_get_product_versions_multiple_images_recipes(self):
         self.mock_uan_product.recipes.append(get_fake_ims_data(other_uan_recipe))
 
         expected_fields = [
-            ['cos', '1.4.0', COS_IMAGE_NAME, COS_RECIPE_NAME],
-            ['uan', '2.0.0', f'{other_uan_image}\n{UAN_IMAGE_NAME}', f'{other_uan_recipe}\n{UAN_RECIPE_NAME}'],
-            ['pbs', '0.1.0', '-', '-']
+            ['cos', LooseVersion('1.4.0'), COS_IMAGE_NAME, COS_RECIPE_NAME],
+            ['uan', LooseVersion('2.0.0'), f'{other_uan_image}\n{UAN_IMAGE_NAME}',
+             f'{other_uan_recipe}\n{UAN_RECIPE_NAME}'],
+            ['pbs', LooseVersion('0.1.0'), '-', '-']
         ]
         actual_headers, actual_fields = get_product_versions()
         self.mock_product_catalog_cls.assert_called_once_with()
@@ -150,9 +152,9 @@ def test_get_product_versions_active_version(self):
         self.mock_uan_product.supports_active = True
         self.mock_uan_product.active = False
         expected_fields = [
-            ['cos', '1.4.0', COS_IMAGE_NAME, COS_RECIPE_NAME],
-            ['uan', '2.0.0', UAN_IMAGE_NAME, UAN_RECIPE_NAME],
-            ['pbs', '0.1.0', '-', '-']
+            ['cos', LooseVersion('1.4.0'), COS_IMAGE_NAME, COS_RECIPE_NAME],
+            ['uan', LooseVersion('2.0.0'), UAN_IMAGE_NAME, UAN_RECIPE_NAME],
+            ['pbs', LooseVersion('0.1.0'), '-', '-']
         ]
         actual_headers, actual_fields = get_product_versions()
         self.mock_product_catalog_cls.assert_called_once_with()

tests/test_loose_version.py

@@ -0,0 +1,91 @@
+#
+# MIT License
+#
+# (C) Copyright 2024 Hewlett Packard Enterprise Development LP
+#
+# Permission is hereby granted, free of charge, to any person obtaining a
+# copy of this software and associated documentation files (the "Software"),
+# to deal in the Software without restriction, including without limitation
+# the rights to use, copy, modify, merge, publish, distribute, sublicense,
+# and/or sell copies of the Software, and to permit persons to whom the
+# Software is furnished to do so, subject to the following conditions:
+#
+# The above copyright notice and this permission notice shall be included
+# in all copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
+# OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+# ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+# OTHER DEALINGS IN THE SOFTWARE.
+#
+"""
+Tests for the LooseVersion utility class.
+"""
+
+import unittest
+
+from sat.loose_version import LooseVersion
+
+
+class TestLooseVersion(unittest.TestCase):
+    """Tests for the XName class"""
+
+    def setUp(self):
+        self.examples = [
+            "1.0.0",
+            "1.0.0",
+            "1.0.2",
+            "2.1.3",
+            "20240129105721.3d243a4b5120",
+            "1.9.16-20240522115029.beaa1ce7a544",
+            "1.9.18-20240625143747.59e8b16343aa",
+            "2.1.1-64-cos-3.0-aarch64",
+            "23.10.30-20231027",
+            "2.2.0-57-cos-3.0-x86-64"
+        ]
+
+    def test_version_str(self):
+        """Test version_str property stores unmodified version string"""
+        for version_str in self.examples:
+            self.assertEqual(LooseVersion(version_str).version_str, version_str)
+
+    def test_bad_version_str(self):
+        """Test that bad version string evaluates to max value"""
+        bad_version_str = "1"
+        good_version_str = "2.0.0"
+        self.assertLess(LooseVersion(good_version_str), LooseVersion(bad_version_str))
+
+    def test_eq(self):
+        """Test __eq__ for LooseVersion."""
+        for version_str in self.examples:
+            self.assertEqual(LooseVersion(version_str), LooseVersion(version_str))
+
+    def test_lt(self):
+        """Test __lt__ for LooseVersion."""
+        self.assertLess(LooseVersion('1.0.0'), LooseVersion('1.0.1'))
+
+    def test_le(self):
+        """Test __le__ for LooseVersion."""
+        self.assertLessEqual(LooseVersion('1.0.0'), LooseVersion('1.0.1'))
+        self.assertLessEqual(LooseVersion('1.0.1'), LooseVersion('1.0.1'))
+
+    def test_gt(self):
+        """Test __gt__ for LooseVersion."""
+        self.assertGreater(LooseVersion('1.0.1'), LooseVersion('1.0.0'))
+
+    def test_ge(self):
+        """Test __ge__ for LooseVersion."""
+        self.assertGreaterEqual(LooseVersion('1.0.0'), LooseVersion('1.0.0'))
+        self.assertGreaterEqual(LooseVersion('1.0.1'), LooseVersion('1.0.0'))
+
+    def test_repr(self):
+        """Test __repr__ for LooseVersion."""
+        for version_str in self.examples:
+            self.assertEqual(repr(LooseVersion(version_str)), "LooseVersion('{}')".format(version_str))
+
+    def test_str(self):
+        for version_str in self.examples:
+            self.assertEqual(str(LooseVersion(version_str)), version_str)