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

ensure prune rework #224

Merged
merged 2 commits into from
Nov 27, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def do_setup(self, context):
self.library.do_setup(context)

def check_for_setup_error(self):
self.library.check_for_setup_error()
self.library.check_for_setup_error(self.ensure)

def get_pool(self, share):
return self.library.get_pool(share)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def do_setup(self, context):
self.library.do_setup(context)

def check_for_setup_error(self):
self.library.check_for_setup_error()
self.library.check_for_setup_error(self.ensure)

def get_pool(self, share):
return self.library.get_pool(share)
Expand Down
27 changes: 10 additions & 17 deletions manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -226,8 +226,8 @@ def _set_cluster_info(self):
and self._client.features.FLEXVOL_ENCRYPTION)

@na_utils.trace
def check_for_setup_error(self):
self._start_periodic_tasks()
def check_for_setup_error(self, ensure=False):
self._start_periodic_tasks(ensure)

def _get_vserver(self, share_server=None):
raise NotImplementedError()
Expand Down Expand Up @@ -277,7 +277,7 @@ def _get_licenses(self):
return self._licenses

@na_utils.trace
def _start_periodic_tasks(self):
def _start_periodic_tasks(self, ensure):

# Run the task once in the current thread so prevent a race with
# the first invocation of get_share_stats.
Expand All @@ -295,12 +295,13 @@ def _start_periodic_tasks(self):
ems_periodic_task.start(interval=self.AUTOSUPPORT_INTERVAL_SECONDS,
initial_delay=0)

# Start the task that runs other housekeeping tasks, such as deletion
# of previously soft-deleted storage artifacts.
housekeeping_periodic_task = loopingcall.FixedIntervalLoopingCall(
self._handle_housekeeping_tasks)
housekeeping_periodic_task.start(
interval=self.HOUSEKEEPING_INTERVAL_SECONDS, initial_delay=0)
if ensure:
# Start the task that runs other housekeeping tasks, such as
# deletion of previously soft-deleted storage artifacts.
housekeeping_periodic_task = loopingcall.FixedIntervalLoopingCall(
self._handle_housekeeping_tasks)
housekeeping_periodic_task.start(
interval=self.HOUSEKEEPING_INTERVAL_SECONDS, initial_delay=0)

def _get_backend_share_name(self, share_id):
"""Get share name according to share name template."""
Expand Down Expand Up @@ -4220,14 +4221,6 @@ def ensure_shares(self, context, shares):
else:
LOG.warning(msg)

try:
self._client.prune_deleted_nfs_export_policies()
self._client.prune_deleted_snapshots()
self._client.prune_deleted_volumes()
except Exception as e:
LOG.warning("Failed to cleanup resources in ensure share: "
"Error - %s", e.message)

return updates

def ensure_share_server(self, context, share_server, network_info):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ class NetAppCmodeMultiSVMFileStorageLibrary(
lib_base.NetAppCmodeFileStorageLibrary):

@na_utils.trace
def check_for_setup_error(self):
def check_for_setup_error(self, ensure=False):

if self._have_cluster_creds:
if self.configuration.netapp_vserver:
Expand All @@ -88,7 +88,7 @@ def check_for_setup_error(self):
raise exception.NetAppException(msg)

(super(NetAppCmodeMultiSVMFileStorageLibrary, self).
check_for_setup_error())
check_for_setup_error(ensure))

@na_utils.trace
def _get_vserver(self, share_server=None, vserver_name=None,
Expand Down Expand Up @@ -134,6 +134,9 @@ def _get_ems_pool_info(self):
@na_utils.trace
def _handle_housekeeping_tasks(self):
"""Handle various cleanup activities."""
self._client.prune_deleted_nfs_export_policies()
self._client.prune_deleted_snapshots()
self._client.prune_deleted_volumes()
self._client.remove_unused_qos_policy_groups()

(super(NetAppCmodeMultiSVMFileStorageLibrary, self).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def __init__(self, driver_name, **kwargs):
self._vserver = self.configuration.netapp_vserver

@na_utils.trace
def check_for_setup_error(self):
def check_for_setup_error(self, ensure=False):

# Ensure vserver is specified in configuration.
if not self._vserver:
Expand Down Expand Up @@ -86,7 +86,7 @@ def check_for_setup_error(self):
LOG.info(msg, msg_args)

(super(NetAppCmodeSingleSVMFileStorageLibrary, self).
check_for_setup_error())
check_for_setup_error(ensure))

@na_utils.trace
def _get_vserver(self, share_server=None):
Expand Down Expand Up @@ -118,9 +118,12 @@ def _get_ems_pool_info(self):
@na_utils.trace
def _handle_housekeeping_tasks(self):
"""Handle various cleanup activities."""
vserver_client = self._get_api_client(vserver=self._vserver)
vserver_client.prune_deleted_nfs_export_policies()
vserver_client.prune_deleted_snapshots()
vserver_client.prune_deleted_volumes()

if self._have_cluster_creds:
vserver_client = self._get_api_client(vserver=self._vserver)
# Harvest soft-deleted QoS policy groups
vserver_client.remove_unused_qos_policy_groups()

Expand Down
3 changes: 2 additions & 1 deletion manila/share/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,7 @@ def init_host(self, service_id=None, reexport=False):
infinite=True, backoff_sleep_max=600)
def _driver_setup():
self.driver.initialized = False
self.driver.ensure = reexport
LOG.debug("Start initialization of driver: '%s'", driver_host_pair)
try:
self.driver.do_setup(ctxt)
Expand All @@ -403,7 +404,7 @@ def _driver_setup():
(self.driver.service_instance_manager.network_helper.
setup_connectivity_with_service_instances())

if reexport:
if self.driver.ensure:
# NOTE(chuan137) To be compatible with the old behavior, we run
# ensure_driver_resources only once when its interval is set to
# ngeative.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,12 +156,13 @@ def test_set_cluster_info(self):
self.assertTrue(self.library._cluster_info['nve_support'],
fake.CLUSTER_NODES)

def test_check_for_setup_error(self):
@ddt.data(False, True)
def test_check_for_setup_error(self, ensure):
mock_start_periodic_tasks = self.mock_object(self.library,
'_start_periodic_tasks')
self.library.check_for_setup_error()
self.library.check_for_setup_error(ensure)

mock_start_periodic_tasks.assert_called_once_with()
mock_start_periodic_tasks.assert_called_once_with(ensure)

def test_get_vserver(self):
self.assertRaises(NotImplementedError, self.library._get_vserver)
Expand Down Expand Up @@ -260,7 +261,8 @@ def test_get_licenses_no_cluster_creds(self):
self.assertListEqual([], result)
self.assertEqual(1, lib_base.LOG.debug.call_count)

def test_start_periodic_tasks(self):
@ddt.data(False, True)
def test_start_periodic_tasks(self, ensure):

mock_update_ssc_info = self.mock_object(self.library,
'_update_ssc_info')
Expand All @@ -278,18 +280,20 @@ def test_start_periodic_tasks(self):
mock_ems_periodic_task,
mock_housekeeping_periodic_task]))

self.library._start_periodic_tasks()
self.library._start_periodic_tasks(ensure)

self.assertTrue(mock_update_ssc_info.called)
self.assertFalse(mock_handle_ems_logging.called)
self.assertFalse(mock_housekeeping_periodic_task.called)
mock_loopingcall.assert_has_calls(
[mock.call(mock_update_ssc_info),
mock.call(mock_handle_ems_logging),
mock.call(mock_handle_housekeeping_tasks)])
loopingcalls = [
mock.call(mock_update_ssc_info),
mock.call(mock_handle_ems_logging)]
self.assertTrue(mock_ssc_periodic_task.start.called)
self.assertTrue(mock_ems_periodic_task.start.called)
self.assertTrue(mock_housekeeping_periodic_task.start.called)
if ensure:
self.assertTrue(mock_housekeeping_periodic_task.start.called)
loopingcalls.append(mock.call(mock_handle_housekeeping_tasks))
mock_loopingcall.assert_has_calls(loopingcalls)

def test_get_backend_share_name(self):

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ def test_check_for_setup_error_cluster_creds_no_vserver(self):
mock_init_flexgroup.assert_called_once_with(set(fake.AGGREGATES))
self.assertTrue(self.library.is_flexvol_pool_configured.called)
self.assertTrue(self.library._find_matching_aggregates.called)
mock_super.assert_called_once_with()
mock_super.assert_called_once_with(False)

def test_check_for_setup_error_cluster_creds_with_vserver(self):
self.library._have_cluster_creds = True
Expand All @@ -159,7 +159,7 @@ def test_check_for_setup_error_cluster_creds_with_vserver(self):

self.library.check_for_setup_error()

mock_super.assert_called_once_with()
mock_super.assert_called_once_with(False)
mock_list_non_root_aggregates.assert_called_once_with()
mock_init_flexgroup.assert_called_once_with(set(fake.AGGREGATES))
self.assertTrue(self.library.is_flexvol_pool_configured.called)
Expand Down Expand Up @@ -392,13 +392,15 @@ def test__verify_share_server_name(self, vserver_exists, identifier):

def test_handle_housekeeping_tasks(self):

self.mock_object(self.client, 'remove_unused_qos_policy_groups')
self.mock_object(self.client, 'prune_deleted_nfs_export_policies')
self.mock_object(self.client, 'prune_deleted_snapshots')
mock_super = self.mock_object(lib_base.NetAppCmodeFileStorageLibrary,
'_handle_housekeeping_tasks')

self.library._handle_housekeeping_tasks()

self.assertTrue(self.client.remove_unused_qos_policy_groups.called)
self.assertTrue(self.client.prune_deleted_nfs_export_policies.called)
self.assertTrue(self.client.prune_deleted_snapshots.called)
self.assertTrue(mock_super.called)

def test_find_matching_aggregates(self):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ def test_check_for_setup_error(self):
self.library.check_for_setup_error()

self.assertTrue(lib_single_svm.LOG.info.called)
mock_super.assert_called_once_with()
mock_super.assert_called_once_with(False)
mock_client.list_vserver_aggregates.assert_called_once_with()
self.assertTrue(self.library._get_api_client.called)
mock_init_flexgroup.assert_called_once_with(set(fake.AGGREGATES))
Expand Down Expand Up @@ -124,7 +124,7 @@ def test_check_for_setup_error_cluster_creds_vserver_match(self):

self.library.check_for_setup_error()

mock_super.assert_called_once_with()
mock_super.assert_called_once_with(False)
mock_client.list_vserver_aggregates.assert_called_once_with()
self.assertTrue(self.library._get_api_client.called)
mock_init_flexgroup.assert_called_once_with(set(fake.AGGREGATES))
Expand Down Expand Up @@ -239,6 +239,9 @@ def test_handle_housekeeping_tasks_with_cluster_creds(self, have_creds):

self.library._handle_housekeeping_tasks()

self.assertTrue(
mock_vserver_client.prune_deleted_nfs_export_policies.called)
self.assertTrue(mock_vserver_client.prune_deleted_snapshots.called)
self.assertIs(
have_creds,
mock_vserver_client.remove_unused_qos_policy_groups.called)
Expand Down