Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix delay clone split when creating share from snapshot #124

Merged
merged 3 commits into from Aug 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 1 addition & 4 deletions manila/share/drivers/netapp/dataontap/client/client_cmode.py
Expand Up @@ -3575,7 +3575,7 @@ def get_volume_to_manage(self, aggregate_name, volume_name):

@na_utils.trace
def create_volume_clone(self, volume_name, parent_volume_name,
parent_snapshot_name=None, split=False,
parent_snapshot_name=None,
qos_policy_group=None,
adaptive_qos_policy_group=None, **options):
"""Clones a volume."""
Expand All @@ -3591,9 +3591,6 @@ def create_volume_clone(self, volume_name, parent_volume_name,

self.send_request('volume-clone-create', api_args)

if split:
self.volume_clone_split_start(volume_name)

if adaptive_qos_policy_group is not None:
self.set_qos_adaptive_policy_group_for_volume(
volume_name, adaptive_qos_policy_group)
Expand Down
10 changes: 5 additions & 5 deletions manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py
Expand Up @@ -1713,16 +1713,16 @@ def _allocate_container_from_snapshot(

hide_snapdir = provisioning_options.pop('hide_snapdir')

# sapcc: Override split option from share type specs. Also clone split
# is postponed to last step, to avoid busy volume error.
split = provisioning_options.pop('split', split)
# split in args takes precedence over split in provisioning_options
if split is None:
split = provisioning_options.pop('split')

LOG.debug('Creating share from snapshot %s', snapshot['id'])
vserver_client.create_volume_clone(
share_name, parent_share_name, parent_snapshot_name, split=False,
share_name, parent_share_name, parent_snapshot_name,
**provisioning_options)

# sapcc: set share comment
# ccloud: set share comment
vserver_client.modify_volume(aggregate_name, share_name,
comment=share_comment,
**provisioning_options)
Expand Down
Expand Up @@ -4619,7 +4619,6 @@ def test_create_volume_clone(self, qos_policy_group_name,
adaptive_qos_policy_group_name):
self.client.features.add_feature('ADAPTIVE_QOS')
self.mock_object(self.client, 'send_request')
self.mock_object(self.client, 'volume_clone_split_start')
set_qos_adapt_mock = self.mock_object(
self.client,
'set_qos_adaptive_policy_group_for_volume')
Expand All @@ -4645,35 +4644,6 @@ def test_create_volume_clone(self, qos_policy_group_name,
set_qos_adapt_mock.assert_called_once_with(
fake.SHARE_NAME, fake.ADAPTIVE_QOS_POLICY_GROUP_NAME
)
self.client.send_request.assert_has_calls([
mock.call('volume-clone-create', volume_clone_create_args)])
self.assertFalse(self.client.volume_clone_split_start.called)

@ddt.data(True, False)
def test_create_volume_clone_split(self, split):

self.mock_object(self.client, 'send_request')
self.mock_object(self.client, 'volume_clone_split_start')

self.client.create_volume_clone(fake.SHARE_NAME,
fake.PARENT_SHARE_NAME,
fake.PARENT_SNAPSHOT_NAME,
split=split)

volume_clone_create_args = {
'volume': fake.SHARE_NAME,
'parent-volume': fake.PARENT_SHARE_NAME,
'parent-snapshot': fake.PARENT_SNAPSHOT_NAME,
'junction-path': '/%s' % fake.SHARE_NAME
}

self.client.send_request.assert_has_calls([
mock.call('volume-clone-create', volume_clone_create_args)])
if split:
self.client.volume_clone_split_start.assert_called_once_with(
fake.SHARE_NAME)
else:
self.assertFalse(self.client.volume_clone_split_start.called)

@ddt.data(None,
mock.Mock(side_effect=netapp_api.NaApiError(
Expand Down
Expand Up @@ -2074,6 +2074,7 @@ def test_allocate_container_from_snapshot(
fake_snapshot,
vserver,
vserver_client,
split=split,
create_fpolicy=create_fpolicy)

share_name = self.library._get_backend_share_name(
Expand All @@ -2086,7 +2087,7 @@ def test_allocate_container_from_snapshot(
fake_share_inst, fake.VSERVER1, vserver_client=vserver_client)
vserver_client.create_volume_clone.assert_called_once_with(
share_name, parent_share_name, parent_snapshot_name,
split=False, **provisioning_options)
**provisioning_options)
if size > original_snapshot_size:
vserver_client.set_volume_size.assert_called_once_with(
share_name, size, snapshot_reserve_percent=8,
Expand All @@ -2107,6 +2108,15 @@ def test_allocate_container_from_snapshot(
else:
mock_create_fpolicy.assert_not_called()

if split is None:
vserver_client.volume_clone_split_start.assert_called_once_with(
fake.SHARE_INSTANCE_NAME)
if split:
vserver_client.volume_clone_split_start.assert_called_once_with(
fake.SHARE_INSTANCE_NAME)
if split is False:
vserver_client.volume_clone_split_start.assert_not_called()

def test_share_exists(self):

vserver_client = mock.Mock()
Expand Down