lightos - bugfix compression stats should be True

the compression value in get_volume_stats should be ["True", "False"]
and not dependent on the
configuration: "lightos_default_compression_enabled"
if the flag is false, we can not create volume type with
compression=True. the backend is filtered in the scheduler.

Closes-Bug: #1962221

Signed-off-by: Yuval Brave  <yuval@lightbitslabs.com>
Change-Id: I09fe872eca1b112f67bad7297de5893696f83373

cinder/tests/unit/volume/drivers/lightos/test_lightos_storage.py

@@ -58,6 +58,7 @@ DEVICE_SCAN_ATTEMPTS_DEFAULT = 5
 LIGHTOS_API_SERVICE_TIMEOUT = 30
 VOLUME_BACKEND_NAME = "lightos_backend"
 RESERVED_PERCENTAGE = 30
+DEFAULT_COMPRESSION = False
 
 
 class InitiatorConnectorFactoryMocker:
@@ -260,7 +261,8 @@ class LightOSStorageVolumeDriverTest(test.TestCase):
         configuration.lightos_jwt = None
         configuration.lightos_snapshotname_prefix = 'openstack_'
         configuration.lightos_intermediate_snapshot_name_prefix = 'for_clone_'
-        configuration.lightos_default_compression_enabled = False
+        configuration.lightos_default_compression_enabled = (
+            DEFAULT_COMPRESSION)
         configuration.lightos_default_num_replicas = 3
         configuration.num_volume_device_scan_tries = (
             DEVICE_SCAN_ATTEMPTS_DEFAULT)
@@ -468,6 +470,73 @@ class LightOSStorageVolumeDriverTest(test.TestCase):
         self.driver.delete_volume(volume)
         db.volume_destroy(self.ctxt, volume.id)
 
+    def test_get_volume_specs_compression_True(self):
+        self.driver.do_setup(None)
+
+        vol_type = test_utils.create_volume_type(
+            self.ctxt, self,
+            extra_specs={'compression': 'True'},
+            name='my_vol_typ1')
+
+        vol_type2 = test_utils.create_volume_type(
+            self.ctxt, self,
+            extra_specs={'compression': '<is> True'},
+            name='my_vol_type2')
+
+        vol_type3 = test_utils.create_volume_type(
+            self.ctxt, self,
+            name='my_vol_type3')
+
+        volume1 = test_utils.create_volume(self.ctxt, size=4,
+                                           volume_type_id=vol_type.id)
+        volume2 = test_utils.create_volume(self.ctxt, size=4,
+                                           volume_type_id=vol_type2.id)
+        volume3 = test_utils.create_volume(self.ctxt, size=4,
+                                           volume_type_id=vol_type3.id)
+        compression, _, _ = self.driver._get_volume_specs(volume1)
+        self.assertTrue(compression == "True")
+        compression, _, _ = self.driver._get_volume_specs(volume2)
+        self.assertTrue(compression == "True")
+        compression, _, _ = self.driver._get_volume_specs(volume3)
+        self.assertTrue(compression == "False")
+
+        db.volume_destroy(self.ctxt, volume1.id)
+        db.volume_destroy(self.ctxt, volume2.id)
+        db.volume_destroy(self.ctxt, volume3.id)
+
+    def test_get_volume_specs_compression_False(self):
+        self.driver.do_setup(None)
+        self.driver.configuration.lightos_default_compression_enabled = True
+        vol_type = test_utils.create_volume_type(
+            self.ctxt, self,
+            extra_specs={'compression': 'False'},
+            name='my_vol_typ1')
+
+        vol_type2 = test_utils.create_volume_type(
+            self.ctxt, self,
+            extra_specs={'compression': '<is> False'},
+            name='my_vol_type2')
+
+        vol_type3 = test_utils.create_volume_type(
+            self.ctxt, self,
+            name='my_vol_type3')
+        volume1 = test_utils.create_volume(self.ctxt, size=4,
+                                           volume_type_id=vol_type.id)
+        volume2 = test_utils.create_volume(self.ctxt, size=4,
+                                           volume_type_id=vol_type2.id)
+        volume3 = test_utils.create_volume(self.ctxt, size=4,
+                                           volume_type_id=vol_type3.id)
+        compression, _, _ = self.driver._get_volume_specs(volume1)
+        self.assertTrue(compression == "False")
+        compression, _, _ = self.driver._get_volume_specs(volume2)
+        self.assertTrue(compression == "False")
+        compression, _, _ = self.driver._get_volume_specs(volume3)
+        self.assertTrue(compression == "True")
+
+        db.volume_destroy(self.ctxt, volume1.id)
+        db.volume_destroy(self.ctxt, volume2.id)
+        db.volume_destroy(self.ctxt, volume3.id)
+
     def test_extend_volume_should_fail_if_volume_does_not_exist(self):
         self.driver.do_setup(None)
 
@@ -695,8 +764,8 @@ class LightOSStorageVolumeDriverTest(test.TestCase):
         assert volumes_data['thin_provisioning_support'] is True, \
             "Expected True, received %s" % \
             volumes_data['thin_provisioning_support']
-        assert volumes_data['compression'] is False, \
+        assert volumes_data['compression'] == [True, False], \
-            "Expected False, received %s" % volumes_data['compression']
+            "Expected [True, False], received %s" % volumes_data['compression']
         assert volumes_data['multiattach'] is True, \
             "Expected True, received %s" % volumes_data['multiattach']
         assert volumes_data['free_capacity_gb'] == 'infinite', \

cinder/volume/drivers/lightos.py

@@ -69,8 +69,9 @@ lightos_opts = [
                     ' volume.'),
     cfg.BoolOpt('lightos_default_compression_enabled',
                 default=False,
-                help='The default compression enabled setting'
+                help='Set to True to create  new volumes compressed assuming'
-                     ' for new volumes.'),
+                     ' no other compression setting is specified via the'
+                     ' volumes type.'),
     cfg.IntOpt('lightos_api_service_timeout',
                default=30,
                help='The default amount of time (in seconds) to wait for'
@@ -553,17 +554,28 @@ class LightOSVolumeDriver(driver.VolumeDriver):
 
         return state
 
+    def _parse_extra_spec(self, extra_spec_value, default_value):
+        extra_spec_value = str(extra_spec_value)
+        extra_spec_value = extra_spec_value.casefold()
+        if "true" in extra_spec_value:
+            return "True"
+        elif "false" in extra_spec_value:
+            return "False"
+        return default_value
+
     def _get_volume_specs(self, volume):
-        compression = 'True' if self.configuration. \
+        default_compression = 'True' if self.configuration. \
             lightos_default_compression_enabled else 'False'
         num_replicas = str(self.configuration.lightos_default_num_replicas)
 
         if not volume.volume_type:
-            return (compression, num_replicas, LIGHTOS_DEFAULT_PROJECT_NAME)
+            return (default_compression, num_replicas,
+                    LIGHTOS_DEFAULT_PROJECT_NAME)
 
         specs = getattr(volume.volume_type, 'extra_specs', {})
-        compression = 'True' if specs.get('compression', None) \
+        type_compression = specs.get('compression', default_compression)
-            else compression
+        compression = self._parse_extra_spec(type_compression,
+                                             default_compression)
         num_replicas = str(specs.get('lightos:num_replicas', num_replicas))
         project_name = specs.get(
             'lightos:project_name',
@@ -968,8 +980,6 @@ class LightOSVolumeDriver(driver.VolumeDriver):
 
         backend_name = self.configuration.safe_get('volume_backend_name')
         res_percentage = self.configuration.safe_get('reserved_percentage')
-        compression = self.configuration.safe_get(
-            'lightos_default_compression_enabled')
         storage_protocol = 'lightos'
         # as a tenant we dont have access to cluster stats
         # in the future we might expose this per project via get_project API
@@ -984,7 +994,7 @@ class LightOSVolumeDriver(driver.VolumeDriver):
                 'QoS_support': False,
                 'online_extend_support': True,
                 'thin_provisioning_support': True,
-                'compression': compression,
+                'compression': [True, False],
                 'multiattach': True}
         # data['total_capacity_gb'] =
         # self.byte_to_gb(cluster_stats['effectivePhysicalStorage'])

doc/source/configuration/block-storage/drivers/lightbits-lightos-driver.rst

@@ -74,9 +74,12 @@ and
   ``~/lightos-default-admin-jwt``.
 - The default number of replicas for volumes is 3, and valid values
   for ``lightos_default_num_replicas`` are 1, 2, or 3.
-- The default compression setting is False (i.e., data is
+- The default compression setting is False (i.e., data is uncompressed).
-  uncompressed). The default compression setting can also be
+  The default compression setting can also be True to indicate that new
-  True. This can also be changed on a per-volume basis.
+  volumes should be created compressed, assuming no other compression
+  setting is specified via the volume type.
+  To control compression on a per-volume basis, create volume types for
+  compressed and uncompressed, and use them as appropriate.
 - The default time to wait for API service response is 30 seconds per
   API endpoint.