Merge pull request #45 from YiHuangIX/upstream

Enhancement fix for #42 adding manage/unmanage support for volume/snapshot

driver/ixsystems/common.py

@@ -607,9 +607,8 @@ def replicate_volume_from_snapshot(self, target_volume_name, snapshot_name, src_
         """Use replicate api (zfs send/recv) to create a volume from snapshot """
         source_name = self.configuration.ixsystems_dataset_path + "/" + src_volume_name
         target_name = self.configuration.ixsystems_dataset_path + "/" + target_volume_name
-        split_snapshot_name = snapshot_name.split("-")
-        replication_name = f'Create volume {target_volume_name} from {src_volume_name}@{split_snapshot_name[0]}-{split_snapshot_name[1]}'
-        naming_schema = f'{split_snapshot_name[0]}-{split_snapshot_name[1]}-%Y-%m-%d-%H-%M'
+        replication_name = f'Create volume {target_volume_name} from {src_volume_name}@{snapshot_name}'
+        naming_schema = f"{snapshot_name.replace('-1111-11-11-11-11','')}-%Y-%m-%d-%H-%M"
         request_urn = f'{FreeNASServer.REST_API_REPLICATION}/'
         params = {"target_dataset": target_name,
                   "source_datasets": [source_name],"name":replication_name,
@@ -697,3 +696,53 @@ def replication_run(self, id):
             LOG.debug(f'replication_run response: {rep}')
         except FreeNASApiError as api_error:
             raise FreeNASApiError('Unexpected error', api_error) from api_error
+
+    def get_volume(self, volume_id):
+        """ Use dataset API /v2.0/pool/dataset/id/{id} to get volume details
+            return None if volume not found        
+        """
+        LOG.debug(f'get_volume {volume_id}')
+        encoded_datapath = urllib.parse.quote_plus(
+                self.configuration.ixsystems_dataset_path + '/' + volume_id)        
+        request_urn = (f"/pool/dataset/id/{encoded_datapath}/")
+        try:
+            rep = self.handle.invoke_command(
+                FreeNASServer.SELECT_COMMAND,
+                request_urn, None)
+            LOG.debug(f'get_volume response: {rep}')
+            if rep['code'] == 404:
+                return None
+            if rep['status'] != FreeNASServer.STATUS_OK:
+                msg = (f'Error get volume: {rep["response"]}')
+                raise FreeNASApiError('Unexpected error', msg)          
+            if rep['status'] == FreeNASServer.STATUS_OK:
+                represult = json.loads(rep['response'])  
+                return represult
+        except FreeNASApiError as api_error:
+            raise FreeNASApiError('Unexpected error', api_error) from api_error
+
+    def get_snapshot(self, volume_name, snapshot_name):
+        """ Use dataset API /v2.0/zfs/snapshot/id/{id} to get snapshot details
+            return None if snapshot not found
+        """
+        LOG.debug(f'get_snapshot {volume_name}@{snapshot_name}')
+        encoded_datapath = urllib.parse.quote_plus(
+                self.configuration.ixsystems_dataset_path + '/' 
+                + volume_name) + '@' + snapshot_name
+        request_urn = (f"/zfs/snapshot/id/{encoded_datapath}/")
+        try:
+            rep = self.handle.invoke_command(
+                FreeNASServer.SELECT_COMMAND,
+                request_urn, None)
+            LOG.debug(f'get_snapshot response: {rep}')
+            if rep['code'] == 404:
+                return None
+            if rep['status'] != FreeNASServer.STATUS_OK:
+                msg = (f'Error get volume: {rep["response"]}')
+                raise FreeNASApiError('Unexpected error', msg)
+            if rep['status'] == FreeNASServer.STATUS_OK:
+                represult = json.loads(rep['response'])
+                return represult
+        except FreeNASApiError as api_error:
+            raise FreeNASApiError('Unexpected error', api_error) from api_error
+

driver/ixsystems/iscsi.py

@@ -87,7 +87,7 @@ def create_volume(self, volume):
         LOG.debug(f'create_volume : volume name :: {volume["name"]}')
 
         freenas_volume = ix_utils.generate_freenas_volume_name(
-            volume['name'],
+            volume['id'],
             self.configuration.ixsystems_iqn_prefix)
 
         LOG.debug(f'volume name after freenas generate : \
@@ -108,7 +108,7 @@ def delete_volume(self, volume):
         LOG.debug(f'delete_volume {volume["name"]}')
 
         freenas_volume = ix_utils.generate_freenas_volume_name(
-            volume['name'],
+            volume['id'],
             self.configuration.ixsystems_iqn_prefix)
 
         if freenas_volume['target']:
@@ -198,13 +198,13 @@ def initialize_connection(self, volume, connector):
         """Driver entry point to attach a volume to an instance."""
         LOG.info('iXsystems Initialise Connection')
         freenas_volume = ix_utils.generate_freenas_volume_name(
-            volume['name'],
+            volume['id'],
             self.configuration.ixsystems_iqn_prefix)
 
         if not freenas_volume['name']:
             # is this snapshot?
             freenas_volume = ix_utils.generate_freenas_snapshot_name(
-                volume['name'],
+                volume['id'],
                 self.configuration.ixsystems_iqn_prefix)
 
         properties = {}
@@ -224,25 +224,25 @@ def terminate_connection(self, volume, connector, **kwargs):
     def create_snapshot(self, snapshot):
         """Driver entry point for creating a snapshot."""
         LOG.info('iXsystems Create Snapshot')
-        LOG.debug(f'create_snapshot {snapshot["name"]}')
+        LOG.debug(f'create_snapshot {snapshot["id"]}')
 
         freenas_snapshot = ix_utils.generate_freenas_snapshot_name(
-            snapshot['name'], self.configuration.ixsystems_iqn_prefix)
+            snapshot['id'], self.configuration.ixsystems_iqn_prefix)
         freenas_volume = ix_utils.generate_freenas_volume_name(
-            snapshot['volume_name'], self.configuration.ixsystems_iqn_prefix)
+            snapshot['volume_id'], self.configuration.ixsystems_iqn_prefix)
 
         self.common.create_snapshot(freenas_snapshot['name'],
                                      freenas_volume['name'])
 
     def delete_snapshot(self, snapshot):
         """Driver entry point for deleting a snapshot."""
         LOG.info('iXsystems Delete Snapshot')
-        LOG.debug(f'delete_snapshot {snapshot["name"]}')
+        LOG.debug(f'delete_snapshot {snapshot["id"]}')
         freenas_snapshot = ix_utils.generate_freenas_snapshot_name(
-            snapshot['name'],
+            snapshot['id'],
             self.configuration.ixsystems_iqn_prefix)
         freenas_volume = ix_utils.generate_freenas_volume_name(
-            snapshot['volume_name'],
+            snapshot['volume_id'],
             self.configuration.ixsystems_iqn_prefix)
 
         self.common.delete_snapshot(freenas_snapshot['name'],
@@ -251,16 +251,16 @@ def delete_snapshot(self, snapshot):
     def create_volume_from_snapshot(self, volume, snapshot):
         """Creates a volume from snapshot."""
         existing_vol = ix_utils.generate_freenas_volume_name(
-            snapshot['volume_name'], self.configuration.ixsystems_iqn_prefix)
+            snapshot['volume_id'], self.configuration.ixsystems_iqn_prefix)
         freenas_snapshot = ix_utils.generate_freenas_snapshot_name(
-            snapshot['name'], self.configuration.ixsystems_iqn_prefix)
+            snapshot['id'], self.configuration.ixsystems_iqn_prefix)
         freenas_volume = ix_utils.generate_freenas_volume_name(
-            volume['name'], self.configuration.ixsystems_iqn_prefix)
+            volume['id'], self.configuration.ixsystems_iqn_prefix)
         freenas_volume['size'] = volume['size']
         freenas_volume['target_size'] = volume['size']
 
         LOG.info('iXsystems replicate Volume From Snapshot')
-        LOG.info(f'replicate_volume_from_snapshot {snapshot["name"]}')
+        LOG.info(f'replicate_volume_from_snapshot {snapshot["id"]}')
         self.common.replicate_volume_from_snapshot(freenas_volume['name'],
                                                 freenas_snapshot['name'],
                                                 existing_vol['name'])
@@ -278,10 +278,10 @@ def get_volume_stats(self, refresh=False):
     def create_cloned_volume(self, volume, src_vref):
         """Creates a volume from source volume."""
         LOG.info('iXsystems Create Cloned Volume')
-        LOG.info(f'create_cloned_volume: {volume["id"]}')
+        LOG.info(f'create_cloned_volume: {volume["id"]} from {src_vref["id"]}')
 
-        temp_snapshot = {'volume_name': src_vref['name'],
-                         'name': f'name-{volume["id"]}'}
+        # Create a temp snapshot for cloning, this snapshot will be removed
+        temp_snapshot = {'id': volume["id"], 'volume_id': src_vref["id"]}
         # New implementation uses replication api to create cloned volume
         # from snapshot, this removes the dependency between parent volume
         # snapshot and cloned volume. Temp snapshot need to be removed after
@@ -296,9 +296,158 @@ def extend_volume(self, volume, new_size):
         LOG.info(f'extend_volume { volume["name"]}')
 
         freenas_volume = ix_utils.generate_freenas_volume_name(
-            volume['name'], self.configuration.ixsystems_iqn_prefix)
+            volume['id'], self.configuration.ixsystems_iqn_prefix)
         freenas_new_size = new_size
 
         if volume['size'] != freenas_new_size:
             self.common.extend_volume(freenas_volume['name'],
                                        freenas_new_size)
+
+    def manage_existing(self, volume, existing_ref):
+        """Manages an existing Truenas volume zvol
+        - Existing volume must stays in ixsystems cinder driver managed pool
+          defined in cinder.conf ixsystems_dataset_path
+        - Truenas does not support rename existing zvol,
+        - driver will replicate to create a new zvol
+        """
+        volume_name = existing_ref['source-name']
+
+        # Check volume_name is not managed by cinder
+        if (len(cinderapi.CONF.list_all_sections()) > 0):
+            ctx = context.get_admin_context()
+            vols = cinderapi.volume_get_all(ctx)
+            find_matched_volume = 0
+            find_matched_volume = len(
+                [vol for vol in vols
+                    if vol.name and vol.name.find(volume_name) == 0])
+            if find_matched_volume == 1:
+                # volume_name is managed by cinder already
+                return
+
+        # Implementation of rename existing volume to new volume
+        # However truenas does not expose volume rename api
+        # Use replication api instead
+        volume_id = ix_utils.generate_volume_id_from_freenas_volume_name(volume_name)
+        temp_snapshot = {'volume_name': volume_name,
+                         'id': volume_id,
+                         'name': f'snap-{volume_id}',
+                         'volume_id': volume_id}
+        self.create_snapshot(temp_snapshot)
+        self.create_volume_from_snapshot(volume, temp_snapshot)
+        self.delete_snapshot(temp_snapshot)
+
+    def manage_existing_object_get_size(self, existing_object, existing_ref,
+                                        object_type):
+        """Return size of an existing manage existing volume or snapshot in GB
+
+        existing_ref is a dictionary of the form:
+        {'source-name': <name of volume or snapshot>}
+        """
+        if object_type == "volume":
+            volume_name = existing_ref['source-name']
+            volume_detail = self.common.get_volume(volume_name)
+            if volume_detail:
+                return ix_utils.get_size_in_gb(int(volume_detail['used']['rawvalue']))
+            else:
+                LOG.error(f"Volume {volume_name} does not exist.")
+                exception = FreeNASApiError(f"Volume {volume_name} does not exist.")
+                raise exception                  
+        if object_type == "snapshot":
+            snapshot_name = ix_utils.generate_freenas_snapshot_name(
+                existing_ref['source-name'], '')['name']
+            volume_name = ix_utils.generate_freenas_volume_name(
+                existing_object["volume_id"],
+                self.configuration.ixsystems_iqn_prefix)['name']
+            snapshot_detail = self.common.get_snapshot(volume_name, snapshot_name)
+            if snapshot_detail:
+                return ix_utils.get_size_in_gb(
+                    int(snapshot_detail['properties']['volsize']['rawvalue']))
+            else:
+                LOG.error(f"Snapshot {existing_ref['source-name']} does not exist.")
+                exception = FreeNASApiError(f"Snapshot {existing_ref['source-name']}"\
+                    " does not exist.")
+                raise exception
+
+    def manage_existing_get_size(self, volume, existing_ref):
+        """
+        Returns the size of the existing volume to be managed.
+
+        :param volume: The volume object being managed.
+        :param existing_ref: A dictionary containing the reference to the
+                     existing volume.
+        :return: The size of the existing volume in GB.
+        """
+        return self.manage_existing_object_get_size(volume, existing_ref,
+                                "volume")
+
+    def manage_existing_snapshot_get_size(self, snapshot, existing_ref):
+        """
+        Returns the size of an existing snapshot that is being managed.
+
+        :param snapshot: The snapshot object being managed.
+        :param existing_ref: A reference to the existing snapshot.
+        :return: The size of the existing snapshot in GB.
+        """
+        return self.manage_existing_object_get_size(snapshot, existing_ref,
+                                "snapshot")
+
+    def manage_existing_snapshot(self, snapshot, existing_ref):
+        """
+        Manages an existing snapshot.
+
+        Args:
+            snapshot: The snapshot object to be managed.
+            existing_ref: A dictionary containing the reference to the existing snapshot.
+
+        Raises:
+            FreeNASApiError: If the existing_ref is a snapshot of a non-cinder managed volume.
+
+        Returns:
+            None
+        """
+        snapshot_name = ix_utils.generate_freenas_snapshot_name(
+            existing_ref['source-name'], '')['name']
+        snapshot_id = ix_utils.generate_snapshot_id_from_freenas_snapshot_name(
+            snapshot_name)
+        volume_name = ix_utils.generate_freenas_volume_name(
+            snapshot["volume_id"],
+            self.configuration.ixsystems_iqn_prefix)['name']
+        # check snapshot_id is not a cinder managed snapshot
+        if (len(cinderapi.CONF.list_all_sections()) > 0):
+            ctx = context.get_admin_context()
+            snaps = cinderapi.snapshot_get_all(ctx)
+            find_matched_snapshot = 0
+            find_matched_snapshot = len(
+                [snap for snap in snaps
+                    if snap.name and snap.name.find(snapshot_id) == 0])
+            if find_matched_snapshot == 1:
+                # existing_ref is managed by cinder already
+                return
+
+        # Ensure volume_name is a cinder managed volume
+        if (len(cinderapi.CONF.list_all_sections()) > 0):
+            ctx = context.get_admin_context()
+            vols = cinderapi.volume_get_all(ctx)
+            find_matched_volume = 0
+            find_matched_volume = len(
+                [vol for vol in vols
+                    if vol.name and vol.name.find(volume_name) == 0])
+            if find_matched_volume != 1:
+                # existing_ref is a cinder managed volume snapshot
+                # It is not possible to manage a snapshot of a non-cinder managed volume
+                exception = FreeNASApiError('It is not possible to manage a '\
+                    'snapshot does not originate from a cinder managed volume')
+                raise exception
+        # Implementation of rename existing snapshot to new openstack snapshot id
+        # However truenas does not expose snapshot rename api 
+        # Leave for future implementation
+
+    def get_manageable_volumes(self, cinder_volumes, marker, limit, offset,
+                               sort_keys, sort_dirs):
+        # Return empty list for now, leave for future implement
+        return []
+
+    def get_manageable_snapshots(self, cinder_snapshots, marker, limit, offset,
+                                 sort_keys, sort_dirs):
+        # Return empty list for now, leave for future implement                        :
+        return []

driver/ixsystems/utils.py

@@ -25,17 +25,28 @@ def get_bytes_from_gb(size_in_gb):
 
 def generate_freenas_volume_name(name, iqn_prefix):
     """Create FREENAS volume / iscsitarget name from Cinder name."""
-    backend_volume = 'volume-' + name.split('-')[1]
-    backend_target = 'target-' + name.split('-')[1]
+    backend_volume = 'volume-' + name
+    backend_target = 'target-' + name
     backend_iqn = iqn_prefix + backend_target
     return {'name': backend_volume,
             'target': backend_target, 'iqn': backend_iqn}
 
+def generate_volume_id_from_freenas_volume_name(name):
+    """Create Cinder volume id from FREENAS volume name."""
+    return name.replace('volume-', '')
+
+def generate_snapshot_id_from_freenas_snapshot_name(name):
+    """Create Cinder snapshot id from FREENAS snapshot name."""
+    return name.replace('snap-', '').replace('-1111-11-11-11-11', '')
 
 def generate_freenas_snapshot_name(name, iqn_prefix):
     """Create FREENAS snapshot / iscsitarget name from Cinder name."""
-    backend_snap = 'snap-' + name.split('-')[1] + '-1111-11-11-11-11'
-    backend_target = 'target-' + name.split('-')[1]
+    backend_snap = name
+    if not backend_snap.startswith('snap-'):
+        backend_snap = f"snap-{backend_snap}"
+    if not backend_snap.endswith('-1111-11-11-11-11'):
+        backend_snap = f"{backend_snap}-1111-11-11-11-11"
+    backend_target = 'target-' + name
     backend_iqn = iqn_prefix + backend_target
     return {'name': backend_snap,
             'target': backend_target, 'iqn': backend_iqn}

test/test_common.py

@@ -391,8 +391,8 @@ def test_replication_run(self, repid, request_d, urlreadresult):
                                  self.common.handle.get_url()+request_d)
 
     @ddt.data(("f9ecfc53-2b12-4bfb-abe1-694970cc1341",
-               "10.3.1.81:3260,target-2b12 iqn.2005-10.org."
-               "freenas.ctltarget-2b12"))
+               "10.3.1.81:3260,target-f9ecfc53-2b12-4bfb-abe1-694970cc1341 iqn.2005-10.org."
+               "freenas.ctltarget-f9ecfc53-2b12-4bfb-abe1-694970cc1341"))
     @ddt.unpack
     def test_create_export(self, volume_name, expected):
         handle = self.common.create_export(volume_name)

test/test_iscsi.py

@@ -249,7 +249,7 @@ def create_export(self, volume_name):
                  }
 
 
-FakeSnapshot = {'name': "snap-fakeid-1111-11-11-11-11", 'volume_name': 'fake-volumeid'}
+FakeSnapshot = {'name': "snap-fakeid-1111-11-11-11-11", 'volume_name': 'fake-volumeid', 'id': 'fakeid', 'volume_id': 'fake-volumeid'}
 
 
 fake_config_dict = {