Skip to content

Commit

Permalink
sapcc: move housekeeping tasks to reexport run mode
Browse files Browse the repository at this point in the history
Same like ensure, we only execute the housekeeping if we invoke the
service with the reexport flag.
We keep the original timing, i.e. run these every 600 seconds.

Change-Id: Ia58e3de56bc0e903a3d2886a2bdf52dc0fdf30c4
  • Loading branch information
Carthaca authored and chuan137 committed Nov 27, 2024
1 parent 950aa9a commit c3b6548
Show file tree
Hide file tree
Showing 9 changed files with 36 additions and 30 deletions.
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
19 changes: 10 additions & 9 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
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
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
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
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

0 comments on commit c3b6548

Please sign in to comment.