Merge pull request #44 from YiHuangIX/upstream

Enhancement fix for #41 Cinder volume/snapshot cascade delete not successful

driver/ixsystems/common.py

@@ -13,6 +13,7 @@
 
 import urllib.parse
 import simplejson as json
+import time
 
 from cinder import exception
 from cinder.i18n import _
@@ -34,7 +35,7 @@ class TrueNASCommon(object):
     """ TrueNAS cinder driver helper class, contains reusable TrueNAS specific driver logic"""
     VERSION = "2.0.0"
     IGROUP_PREFIX = 'openstack-'
-
+    timeout = 600
     required_flags = ['ixsystems_transport_type',
                       'ixsystems_server_hostname',
                       'ixsystems_server_port',
@@ -52,6 +53,7 @@ def __init__(self, configuration=None):
         self.vendor_name = self.configuration.ixsystems_vendor_name
         self.storage_protocol = self.configuration.ixsystems_storage_protocol
         self.apikey = self.configuration.ixsystems_apikey
+        self.timeout = int(self.configuration.ixsystems_replication_timeout)
         self.stats = {}
 
     def _create_handle(self, **kwargs):
@@ -173,7 +175,7 @@ def _create_extent(self, name, volume_name, from_snapshot=False):
             ext_params['name'] = name
         vol_param = f'{self.configuration.ixsystems_dataset_path}/{volume_name}'
         ext_params['disk'] = 'zvol/' + vol_param
-            
+
         jext_params = json.dumps(ext_params)
         LOG.debug(f'_create_extent params : {jext_params}')
         jext_params = jext_params.encode('utf8')
@@ -600,3 +602,98 @@ def create_export(self, volume_name):
             f'{freenas_volume["target"]} {freenas_volume["iqn"]}'
         LOG.debug(f'provider_location: {handle}')
         return handle
+
+    def replicate_volume_from_snapshot(self, target_volume_name, snapshot_name, src_volume_name):
+        """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'
+        request_urn = f'{FreeNASServer.REST_API_REPLICATION}/'
+        params = {"target_dataset": target_name,
+                  "source_datasets": [source_name],"name":replication_name,
+                  "recursive": False,  "compression": None, "speed_limit":None,
+                  "enabled": True, "direction": "PUSH",  "transport": "LOCAL",  "auto":True,
+                  "schedule": {"minute": "*", "hour":"*", "dom":"*", "month":"*", "dow":"*",
+                               "begin":"00:00","end":"23:59"}, 
+                  "also_include_naming_schema": [naming_schema],"only_matching_schedule":True
+                  ,"retention_policy": "SOURCE","readonly":"IGNORE"}
+        jparams = json.dumps(params)
+        jparams = jparams.encode('utf8')
+        LOG.debug(f'replicate_volume_from_snapshot urn : {request_urn}')
+        try:
+            ret = self.handle.invoke_command(FreeNASServer.CREATE_COMMAND,
+                                             request_urn, jparams)
+            LOG.debug(f'{ret["response"]}')
+            if ret['status'] == FreeNASServer.STATUS_OK:
+                represult = json.loads(ret['response'])
+                self.replication_run(represult["id"])
+                starttime = int(time.time())
+                while (int(time.time()) - starttime) <= self.timeout:
+                    time.sleep(1)
+                    rs = self.replication_stats(represult["id"])
+                    LOG.debug(f'replication id {rs["id"]} has state {rs["state"]["state"] }')
+                    if rs['state']['state'] in ["PENDING","RUNNING"]: continue
+                    if rs['state']['state'] == "ERROR":
+                        # for whatever reason replication may fail and report back state ERROR
+                        # clean up replication generated snapshot and replication task
+                        self.delete_snapshot(snapshot_name, target_volume_name)
+                        self.replication_delete(represult["id"])
+                        msg = (f'Error replicate volume from snapshot: {ret["response"]}')
+                        raise FreeNASApiError('Unexpected error', msg)
+                    if rs['state']['state'] == "FINISHED":
+                        # replication api will create default snapshot, which need to be
+                        # removed to avoid future delete volume failed from openstack
+                        self.delete_snapshot(snapshot_name, target_volume_name)
+                        # delete replication record
+                        self.replication_delete(represult["id"])
+                        break
+            if ret['status'] != FreeNASServer.STATUS_OK:
+                msg = (f'Error replicate volume from snapshot: {ret["response"]}')
+                raise FreeNASApiError('Unexpected error', msg)
+        except FreeNASApiError as api_error:
+            raise FreeNASApiError('Unexpected error', api_error) from api_error
+
+    def replication_stats(self, id):
+        """ Use replication API /v2.0/replication/id/{id} to check status
+        """
+        LOG.debug('replication_status s')
+        request_urn = (f"/replication/id/{id}")
+        try:
+            rep = self.handle.invoke_command(
+                FreeNASServer.SELECT_COMMAND,
+                request_urn, None)
+            LOG.debug(f'replication_stats response: {rep["response"]}')
+            represult = json.loads(rep['response'])
+            LOG.debug(f'replication_stats response : {represult}')
+        except FreeNASApiError as api_error:
+            raise FreeNASApiError('Unexpected error', api_error) from api_error
+        finally:
+            return represult
+
+    def replication_delete(self, id):
+        """ Use replication API /v2.0/replication/id/{id} to delete replication
+        """
+        LOG.debug(f'replication_delete {id}')
+        request_urn = (f"/replication/id/{id}")
+        try:
+            rep = self.handle.invoke_command(
+                FreeNASServer.DELETE_COMMAND,
+                request_urn, None)
+            LOG.debug(f'replication_delete response: {rep}')
+        except FreeNASApiError as api_error:
+            raise FreeNASApiError('Unexpected error', api_error) from api_error
+
+    def replication_run(self, id):
+        """ Use replication API /v2.0/replication/id/{id} to run replication
+        """
+        LOG.debug(f'replication_run {id}')
+        request_urn = (f"/replication/id/{id}/run")
+        try:
+            rep = self.handle.invoke_command(
+                FreeNASServer.CREATE_COMMAND,
+                request_urn, None)
+            LOG.debug(f'replication_run response: {rep}')
+        except FreeNASApiError as api_error:
+            raise FreeNASApiError('Unexpected error', api_error) from api_error

driver/ixsystems/freenasapi.py

@@ -51,6 +51,7 @@ class FreeNASServer(object):
     REST_API_TARGET_TO_EXTENT = "/iscsi/targetextent"
     # REST_API_TARGET_GROUP = "/services/iscsi/targetgroup/"
     REST_API_SNAPSHOT = "/zfs/snapshot"
+    REST_API_REPLICATION = "/replication"
     ZVOLS = "zvols"
     TARGET_ID = -1  # We assume invalid id to begin with
     STORAGE_TABLE = "/storage"

driver/ixsystems/iscsi.py

@@ -17,7 +17,6 @@
 """
 
 import simplejson as json
-import re
 
 from cinder.volume import driver
 from cinder.volume.drivers.ixsystems import common
@@ -251,9 +250,6 @@ def delete_snapshot(self, snapshot):
 
     def create_volume_from_snapshot(self, volume, snapshot):
         """Creates a volume from snapshot."""
-        LOG.info('iXsystems Create Volume From Snapshot')
-        LOG.info(f'create_volume_from_snapshot {snapshot["name"]}')
-
         existing_vol = ix_utils.generate_freenas_volume_name(
             snapshot['volume_name'], self.configuration.ixsystems_iqn_prefix)
         freenas_snapshot = ix_utils.generate_freenas_snapshot_name(
@@ -263,24 +259,14 @@ def create_volume_from_snapshot(self, volume, snapshot):
         freenas_volume['size'] = volume['size']
         freenas_volume['target_size'] = volume['size']
 
-        self.common.create_volume_from_snapshot(freenas_volume['name'],
-                                                 freenas_snapshot['name'],
-                                                 existing_vol['name'])
+        LOG.info('iXsystems replicate Volume From Snapshot')
+        LOG.info(f'replicate_volume_from_snapshot {snapshot["name"]}')
+        self.common.replicate_volume_from_snapshot(freenas_volume['name'],
+                                                freenas_snapshot['name'],
+                                                existing_vol['name'])
         self.common.create_iscsitarget(freenas_volume['target'],
                                         freenas_volume['name'])
 
-        # Promote image cache volume created by cinder service account
-        # by checking project_id is cinder service project and display name match
-        # image-[a-zA-Z0-9]+-[a-z0-9]+-[a-z0-9]+-[a-z0-9]+-[a-z0-9]+ pattern
-        # This is required because image cache volume cloned from the snapshot of first volume
-        # provisioned by this image from upstream cinder flow code
-        # Without promoting image cache volume, the first volume created can no longer be deleted
-        if (self.configuration.safe_get('image_volume_cache_enabled')
-            and self.common.is_service_project(volume['project_id'])
-            and re.match(r"image-[a-zA-Z0-9]+-[a-z0-9]+-[a-z0-9]+-[a-z0-9]+-[a-z0-9]+",
-                         volume['display_name'])):
-            self.common.promote_volume(freenas_volume['name'])
-
     def get_volume_stats(self, refresh=False):
         """Get stats info from volume group / pool."""
         LOG.info('iXsystems Get Volume Status')
@@ -296,14 +282,13 @@ def create_cloned_volume(self, volume, src_vref):
 
         temp_snapshot = {'volume_name': src_vref['name'],
                          'name': f'name-{volume["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
+        # clone successful
         self.create_snapshot(temp_snapshot)
         self.create_volume_from_snapshot(volume, temp_snapshot)
-        # self.delete_snapshot(temp_snapshot)
-        # with API v2.0 this causes FreeNAS error
-        # "snapshot has dependent clones".  Cannot delete while volume is
-        # active.  Instead, added check and deletion of orphaned dependent
-        # clones in common._delete_volume()
+        self.delete_snapshot(temp_snapshot)
 
     def extend_volume(self, volume, new_size):
         """Driver entry point to extend an existing volumes size."""

driver/ixsystems/options.py

@@ -37,7 +37,7 @@
                help='vendor name on Storage controller'),
     cfg.StrOpt('ixsystems_storage_protocol',
                default='iscsi',
-               help='storage protocol on Storage controller'), ]
+               help='storage protocol on Storage controller'),]
 
 ixsystems_transport_opts = [
     cfg.StrOpt('ixsystems_transport_type',
@@ -77,4 +77,7 @@
                help='Storage controller iSCSI portal ID'),
     cfg.StrOpt('ixsystems_initiator_id',
                default=1,
-               help='Storage controller iSCSI Initiator ID'), ]
+               help='Storage controller iSCSI Initiator ID'),
+    cfg.StrOpt('ixsystems_replication_timeout',
+               default='600',
+               help='default 600 seconds for creating volume from snapshot by using replication API'),]

driver/ixsystems/utils.py

@@ -34,7 +34,7 @@ def generate_freenas_volume_name(name, iqn_prefix):
 
 def generate_freenas_snapshot_name(name, iqn_prefix):
     """Create FREENAS snapshot / iscsitarget name from Cinder name."""
-    backend_snap = 'snap-' + name.split('-')[1]
+    backend_snap = 'snap-' + name.split('-')[1] + '-1111-11-11-11-11'
     backend_target = 'target-' + name.split('-')[1]
     backend_iqn = iqn_prefix + backend_target
     return {'name': backend_snap,

test/test_common.py

@@ -297,6 +297,99 @@ def test_extend_volume(self, name, new_size, request_d, urlreadresult):
                 self.assertEqual(mock_request.method_calls[0][1][0],
                                  self.common.handle.get_url()+request_d)
 
+    @ddt.data(("volume-ba323557", "snap-ba323557", "volume-e66d45cd",
+               "/replication/",
+               b'{ "id": 1, "target_dataset": "pool/cinder/volume-ba323557'
+               b'", "recursive": false, "compression": null, "speed_limi'
+               b't": null, "enabled": true, "direction": "PUSH", "transp'
+               b'ort": "LOCAL", "netcat_active_side": null, "netcat_active'
+               b'_side_port_min": null, "netcat_active_side_port_max": null,'
+               b' "source_datasets": [  "pool/cinder/volume-e66d45cd" ],'
+               b' "exclude": [], "naming_schema": [], "auto": true, "o'
+               b'nly_matching_schedule": true, "readonly": "IGNORE", "allo'
+               b'w_from_scratch": false, "hold_pending_snapshots": false, '
+               b'"retention_policy": "SOURCE", "lifetime_unit": null, "lif'
+               b'etime_value": null, "large_block": true, "embed": false,'
+               b' "compressed": true, "retries": 5, "netcat_active_side'
+               b'_listen_address": null, "netcat_passive_side_connect_addre'
+               b'ss": null, "logging_level": null, "name": "Create volume'
+               b'volume-ba323557 from volume-e66d45cd@snap-ba323557", "stat'
+               b'e": {  "state": "FINISHED" }, "properties": true, "pr'
+               b'operties_exclude": [], "replicate": false, "encryption":'
+               b'false, "encryption_key": null, "encryption_key_format": '
+               b'null, "encryption_key_location": null, "ssh_credentials"'
+               b': null, "periodic_snapshot_tasks": [], "also_include_na'
+               b'ming_schema": [  "snap-ba323557-%Y-%m-%d-%H-%M" ], "sc'
+               b'hedule": {  "minute": "*",  "hour": "*",  "dom": "*",'
+               b'"month": "*",  "dow": "*",  "begin": "00:00",  "end": '
+               b'"23:59" }, "restrict_schedule": null, "job": null}',
+               {"id": 1,"state": {  "state": "FINISHED" }}
+               ))
+    @ddt.unpack
+    def test_replicate_volume_from_snapshot(self, target_volume_name,
+                                            snapshot_name, src_volume_name,
+                                            request_d, urlreadresult,
+                                            replicationstatresult):
+        urlrespond = MagicMock(name="urlrespond")
+        urlrespondcontext = MagicMock(name="urlrespondcontext")
+        urlrespond.__enter__.return_value = urlrespondcontext
+        urlrespondcontext.read.return_value = urlreadresult
+        self.common.replication_run = MagicMock()
+        self.common.replication_stats = MagicMock(return_value
+                                                  = replicationstatresult)
+        self.common.delete_snapshot = MagicMock()
+        with patch(request_patch) as mock_request:
+            with patch(open_patch, return_value=urlrespond):
+                self.common.replicate_volume_from_snapshot(target_volume_name,
+                                                           snapshot_name,
+                                                           src_volume_name)
+                self.assertEqual(mock_request.method_calls[0][1][0],
+                                 self.common.handle.get_url()+request_d)
+
+    @ddt.data(("1", "/replication/id/1",
+               b'{ "id": 1, "target_dataset": "pool/cinder/volume-ba323557",'
+               b'"state": {"state": "FINISHED"}}'))
+    @ddt.unpack
+    def test_replication_stats(self, repid, request_d, urlreadresult):
+        urlrespond = MagicMock(name="urlrespond")
+        urlrespondcontext = MagicMock(name="urlrespondcontext")
+        urlrespond.__enter__.return_value = urlrespondcontext
+        urlrespondcontext.read.return_value = urlreadresult
+        with patch(request_patch) as mock_request:
+            with patch(open_patch, return_value=urlrespond):
+                self.common.replication_stats(repid)
+                self.assertEqual(mock_request.method_calls[0][1][0],
+                                 self.common.handle.get_url()+request_d)
+
+    @ddt.data(("1", "/replication/id/1",
+               b'{ "id": 1, "target_dataset": "pool/cinder/volume-ba323557",'
+               b'true'))
+    @ddt.unpack
+    def test_replication_delete(self, repid, request_d, urlreadresult):
+        urlrespond = MagicMock(name="urlrespond")
+        urlrespondcontext = MagicMock(name="urlrespondcontext")
+        urlrespond.__enter__.return_value = urlrespondcontext
+        urlrespondcontext.read.return_value = urlreadresult
+        with patch(request_patch) as mock_request:
+            with patch(open_patch, return_value=urlrespond):
+                self.common.replication_delete(repid)
+                self.assertEqual(mock_request.method_calls[0][1][0],
+                                 self.common.handle.get_url()+request_d)
+
+    @ddt.data(("1", "/replication/id/1/run",
+               b'217682'))
+    @ddt.unpack
+    def test_replication_run(self, repid, request_d, urlreadresult):
+        urlrespond = MagicMock(name="urlrespond")
+        urlrespondcontext = MagicMock(name="urlrespondcontext")
+        urlrespond.__enter__.return_value = urlrespondcontext
+        urlrespondcontext.read.return_value = urlreadresult
+        with patch(request_patch) as mock_request:
+            with patch(open_patch, return_value=urlrespond):
+                self.common.replication_run(repid)
+                self.assertEqual(mock_request.method_calls[0][1][0],
+                                 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"))
@@ -325,7 +418,8 @@ def test_create_export(self, volume_name, expected):
     'ixsystems_storage_protocol': 'iscsi',
     'ixsystems_server_iscsi_port': 3260,
     'ixsystems_api_version': 'v2.0',
-    'ixsystems_reserved_percentage': 0
+    'ixsystems_reserved_percentage': 0,
+    'ixsystems_replication_timeout' : 600
     }