openstack
/
nova
Code Issues Proposed changes

Remove warning & change @periodic_task behaviour 

During development of Juno, Ia227f4c4e69ecf361ab02d1d17a3010303650104
added a warning that the behaviour of periodic_tasks with an interval of
0 would change.  This patch changes the behaviour and removes the
warning.

Oslo (which provides the @periodic_task decorator) uses spacing=0 to
mean "run at the default interval of 60s", but several nova methods used
spacing=0 to mean "don't run".  These methods now behave consistently
with the Oslo code.

Due to an error in the original patch, some of the warnings refer to the
intended behaviour change as being in "the K release", and some to "Juno".
The warning about changing behaviour is first present in Juno, so this
patch should be merged in Kilo.

Change-Id: Ie5f2092a3b84e3674fe6a68ce511d2dc218bb625
Closes-bug: 1276203
This commit is contained in:
Matthew Gilliard 2014-07-15 14:37:13 +01:00
parent 98f4f345ef
commit 324d951667
5 changed files with 6 additions and 125 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
nova/compute/manager.py
View File

@ -132,16 +132,12 @@ interval_opts = [
default=600,
help='Interval to pull network bandwidth usage info. Not '
'supported on all hypervisors. Set to -1 to disable. '
'Setting this to 0 will disable, but this will change in '
'the K release to mean "run at the default rate".'),
# TODO(gilliard): Clean the above message after the K release
'Setting this to 0 will run at the default rate.'),
cfg.IntOpt('sync_power_state_interval',
default=600,
help='Interval to sync power states between the database and '
'the hypervisor. Set to -1 to disable. '
'Setting this to 0 will disable, but this will change in '
'Juno to mean "run at the default rate".'),
# TODO(gilliard): Clean the above message after the K release
'Setting this to 0 will run at the default rate.'),
cfg.IntOpt("heal_instance_info_cache_interval",
default=60,
help="Number of seconds between instance info_cache self "
@ -156,9 +152,7 @@ interval_opts = [
default=3600,
help='Interval in seconds for polling shelved instances to '
'offload. Set to -1 to disable.'
'Setting this to 0 will disable, but this will change in '
'Juno to mean "run at the default rate".'),
# TODO(gilliard): Clean the above message after the K release
'Setting this to 0 will run at the default rate.'),
cfg.IntOpt('shelved_offload_time',
default=0,
help='Time in seconds before a shelved instance is eligible '
@ -168,8 +162,7 @@ interval_opts = [
default=300,
help=('Interval in seconds for retrying failed instance file '
'deletes. Set to -1 to disable. '
'Setting this to 0 will disable, but this will change in '
'the K release to mean "run at the default rate".')),
'Setting this to 0 will run at the default rate.')),
cfg.IntOpt('block_device_allocate_retries_interval',
default=3,
help='Waiting time interval (seconds) between block'
@ -5469,11 +5462,8 @@ class ComputeManager(manager.Manager):
"Will retry later."),
e, instance=instance)
@compute_utils.periodic_task_spacing_warn("shelved_poll_interval")
@periodic_task.periodic_task(spacing=CONF.shelved_poll_interval)
def _poll_shelved_instances(self, context):
if CONF.shelved_offload_time <= 0:
return
filters = {'vm_state': vm_states.SHELVED,
'host': self.host}
@ -5551,11 +5541,10 @@ class ComputeManager(manager.Manager):
num_instances,
time.time() - start_time))
@compute_utils.periodic_task_spacing_warn("bandwidth_poll_interval")
@periodic_task.periodic_task(spacing=CONF.bandwidth_poll_interval)
def _poll_bandwidth_usage(self, context):
if (CONF.bandwidth_poll_interval <= 0 or not self._bw_usage_supported):
if not self._bw_usage_supported:
return
prev_time, start_time = utils.last_completed_audit_period()
@ -5687,7 +5676,6 @@ class ComputeManager(manager.Manager):
self._update_volume_usage_cache(context, vol_usages)
@compute_utils.periodic_task_spacing_warn("sync_power_state_interval")
@periodic_task.periodic_task(spacing=CONF.sync_power_state_interval,
run_immediately=True)
def _sync_power_states(self, context):
@ -6181,7 +6169,6 @@ class ComputeManager(manager.Manager):
else:
self._process_instance_event(instance, event)
@compute_utils.periodic_task_spacing_warn("image_cache_manager_interval")
@periodic_task.periodic_task(spacing=CONF.image_cache_manager_interval,
external_process_ok=True)
def _run_image_cache_manager_pass(self, context):
@ -6189,8 +6176,6 @@ class ComputeManager(manager.Manager):
if not self.driver.capabilities["has_imagecache"]:
return
if CONF.image_cache_manager_interval == 0:
return
# Determine what other nodes use this storage
storage_users.register_storage_use(CONF.instances_path, CONF.host)
@ -6209,13 +6194,9 @@ class ComputeManager(manager.Manager):
self.driver.manage_image_cache(context, filtered_instances)
@compute_utils.periodic_task_spacing_warn("instance_delete_interval")
@periodic_task.periodic_task(spacing=CONF.instance_delete_interval)
def _run_pending_deletes(self, context):
"""Retry any pending instance file deletes."""
if CONF.instance_delete_interval == 0:
return
LOG.debug('Cleaning up deleted instances')
filters = {'deleted': True,
'soft_deleted': False,

28
nova/compute/utils.py
View File

@ -466,31 +466,3 @@ class EventReporter(object):
self.context, uuid, self.event_name, exc_val=exc_val,
exc_tb=exc_tb, want_result=False)
return False
def periodic_task_spacing_warn(config_option_name):
"""Decorator to warn about an upcoming breaking change in methods which
use the @periodic_task decorator.
Some methods using the @periodic_task decorator specify spacing=0 or
None to mean "do not call this method", but the decorator itself uses
0/None to mean "call at the default rate".
Starting with the K release the Nova methods will be changed to conform
to the Oslo decorator. This decorator should be present wherever a
spacing value from user-supplied config is passed to @periodic_task, and
there is also a check to skip the method if the value is zero. It will
log a warning if the spacing value from config is 0/None.
"""
# TODO(gilliard) remove this decorator, its usages and the early returns
# near them after the K release.
def wrapper(f):
if (hasattr(f, "_periodic_spacing") and
(f._periodic_spacing == 0 or f._periodic_spacing is None)):
LOG.warning(_LW("Value of 0 or None specified for %s."
" This behaviour will change in meaning in the K release, to"
" mean 'call at the default rate' rather than 'do not call'."
" To keep the 'do not call' behaviour, use a negative value."),
config_option_name)
return f
return wrapper

10
nova/tests/compute/test_compute.py
View File

@ -667,16 +667,6 @@ class ComputeVolumeTestCase(BaseTestCase):
def test_boot_image_no_metadata(self):
self.test_boot_image_metadata(metadata=False)
def test_poll_bandwidth_usage_disabled(self):
ctxt = 'MockContext'
self.mox.StubOutWithMock(utils, 'last_completed_audit_period')
# None of the mocks should be called.
self.mox.ReplayAll()
self.flags(bandwidth_poll_interval=0)
self.compute._poll_bandwidth_usage(ctxt)
self.mox.UnsetStubs()
def test_poll_bandwidth_usage_not_implemented(self):
ctxt = context.get_admin_context()

60
nova/tests/compute/test_compute_utils.py
View File

@ -38,7 +38,6 @@ from nova.network import api as network_api
from nova import objects
from nova.objects import block_device as block_device_obj
from nova.objects import instance as instance_obj
from nova.openstack.common import periodic_task
from nova import rpc
from nova import test
from nova.tests import fake_block_device
@ -826,62 +825,3 @@ class ComputeUtilsGetRebootTypes(test.TestCase):
def test_get_reboot_not_running_hard(self):
reboot_type = compute_utils.get_reboot_type('foo', 'bar')
self.assertEqual(reboot_type, 'HARD')
class ComputeUtilsPeriodicTaskSpacingWarning(test.NoDBTestCase):
@mock.patch.object(compute_utils, 'LOG')
def test_periodic_task_spacing_warning_no_op(self, mock_log):
@compute_utils.periodic_task_spacing_warn("config_value")
def not_a_periodic_task():
return "something"
self.assertEqual("something", not_a_periodic_task())
self.assertFalse(mock_log.warning.called)
self.assertFalse(mock_log.warn.called)
@mock.patch.object(compute_utils, 'LOG')
def test_periodic_task_spacing_warning_nonzero_spacing(self, mock_log):
@compute_utils.periodic_task_spacing_warn("config_value")
@periodic_task.periodic_task(spacing=10)
def a_periodic_task():
return "something"
self.assertEqual("something", a_periodic_task())
self.assertFalse(mock_log.warning.called)
self.assertFalse(mock_log.warn.called)
@mock.patch.object(compute_utils, 'LOG')
def test_periodic_task_spacing_warning_zero_spacing(self, mock_log):
@compute_utils.periodic_task_spacing_warn("config_value")
@periodic_task.periodic_task(spacing=0)
def zero_spacing_periodic_task():
return "something"
self.assertEqual("something", zero_spacing_periodic_task())
mock_log.warning.assert_called_with(mock.ANY, "config_value")
@mock.patch.object(compute_utils, 'LOG')
def test_periodic_task_spacing_warning_none_spacing(self, mock_log):
@compute_utils.periodic_task_spacing_warn("config_value")
@periodic_task.periodic_task(spacing=None)
def none_spacing_periodic_task():
return "something"
self.assertEqual("something", none_spacing_periodic_task())
mock_log.warning.assert_called_with(mock.ANY, "config_value")
@mock.patch.object(compute_utils, 'LOG')
def test_periodic_task_spacing_warning_default_spacing(self, mock_log):
@compute_utils.periodic_task_spacing_warn("config_value")
@periodic_task.periodic_task
def default_spacing_periodic_task():
return "something"
self.assertEqual("something", default_spacing_periodic_task())
mock_log.warning.assert_called_with(mock.ANY, "config_value")

4
nova/virt/imagecache.py
View File

@ -23,9 +23,7 @@ imagecache_opts = [
default=2400,
help='Number of seconds to wait between runs of the image '
'cache manager. Set to -1 to disable. '
'Setting this to 0 will disable, but this will change in '
'the K release to mean "run at the default rate".'),
# TODO(gilliard): Clean the above message after the K release
'Setting this to 0 will run at the default rate.'),
cfg.StrOpt('image_cache_subdirectory_name',
default='_base',
help='Where cached images are stored under $instances_path. '