Add option to sync node power state from DB
Previously, during _sync_node_power_state, when a node's power state did not match the expected state, the DB was always updated and a warning logged. This patch causes the node's power state to instead be set to the expected value, by default, and adds a CONF option to enable the old behavior. Implements bp:keep-powered-off-nodes-off Co-Authored-By: Michael Davies <michael@the-davies.net> Change-Id: I1f5af0061dbe8c57b8bd3f8a65b215aadd549551
This commit is contained in:
Michael Davies
Lucas Alvares Gomes
parent
5638531a91
commit
8ad764a9de
@ -497,6 +497,12 @@
|
||||
# 0 - unlimited. (integer value)
|
||||
#deploy_callback_timeout=1800
|
||||
|
||||
# During sync_power_state, should the hardware power state be
|
||||
# set to the state recorded in the database (True) or should
|
||||
# the database be updated based on the hardware state (False).
|
||||
# (boolean value)
|
||||
#force_power_state_during_sync=true
|
||||
|
||||
|
||||
[database]
|
||||
|
||||
|
||||
@ -93,12 +93,68 @@ conductor_opts = [
|
||||
default=1800,
|
||||
help='Timeout (seconds) for waiting callback from deploy '
|
||||
'ramdisk. 0 - unlimited.'),
|
||||
cfg.BoolOpt('force_power_state_during_sync',
|
||||
default=True,
|
||||
help='During sync_power_state, should the hardware power '
|
||||
'state be set to the state recorded in the database '
|
||||
'(True) or should the database be updated based on '
|
||||
'the hardware state (False).'),
|
||||
]
|
||||
|
||||
CONF = cfg.CONF
|
||||
CONF.register_opts(conductor_opts, 'conductor')
|
||||
|
||||
|
||||
def _do_sync_power_state(task):
|
||||
node = task.node
|
||||
|
||||
try:
|
||||
power_state = task.driver.power.get_power_state(task, node)
|
||||
except Exception as e:
|
||||
# TODO(rloo): change to IronicException, after
|
||||
# https://bugs.launchpad.net/ironic/+bug/1267693
|
||||
LOG.warning(_("During sync_power_state, could not get power state for "
|
||||
"node %(node)s. Error: %(err)s."),
|
||||
{'node': node.uuid, 'err': e})
|
||||
return
|
||||
|
||||
if node.power_state is None:
|
||||
LOG.info(_("During sync_power_state, node %(node)s has no previous "
|
||||
"known state. Recording current state '%(state)s'."),
|
||||
{'node': node.uuid, 'state': power_state})
|
||||
node.power_state = power_state
|
||||
node.save(task.context)
|
||||
|
||||
if power_state == node.power_state:
|
||||
return
|
||||
|
||||
if CONF.conductor.force_power_state_during_sync:
|
||||
LOG.warning(_("During sync_power_state, node %(node)s state "
|
||||
"'%(actual)s' does not match expected state. "
|
||||
"Changing hardware state to '%(state)s'."),
|
||||
{'node': node.uuid, 'actual': power_state,
|
||||
'state': node.power_state})
|
||||
try:
|
||||
# node_power_action will update the node record
|
||||
# so don't do that again here.
|
||||
utils.node_power_action(task, task.node,
|
||||
node.power_state)
|
||||
except Exception as e:
|
||||
# TODO(rloo): change to IronicException after
|
||||
# https://bugs.launchpad.net/ironic/+bug/1267693
|
||||
LOG.error(_("Failed to change power state of node %(node)s "
|
||||
"to '%(state)s'."), {'node': node.uuid,
|
||||
'state': node.power_state})
|
||||
else:
|
||||
LOG.warning(_("During sync_power_state, node %(node)s state "
|
||||
"does not match expected state '%(state)s'. "
|
||||
"Updating recorded state to '%(actual)s'."),
|
||||
{'node': node.uuid, 'actual': power_state,
|
||||
'state': node.power_state})
|
||||
node.power_state = power_state
|
||||
node.save(task.context)
|
||||
|
||||
|
||||
class ConductorManager(service.PeriodicService):
|
||||
"""Ironic Conductor service main class."""
|
||||
|
||||
@ -395,31 +451,7 @@ class ConductorManager(service.PeriodicService):
|
||||
continue
|
||||
|
||||
with task_manager.acquire(context, node_id) as task:
|
||||
node = task.node
|
||||
|
||||
try:
|
||||
power_state = task.driver.power.get_power_state(task,
|
||||
node)
|
||||
except Exception as e:
|
||||
#TODO(rloo): change to IronicException, after
|
||||
# https://bugs.launchpad.net/ironic/+bug/1267693
|
||||
LOG.debug(_("During sync_power_state, could not get "
|
||||
"power state for node %(node)s. Error: %(err)s.") %
|
||||
{'node': node.uuid, 'err': e})
|
||||
continue
|
||||
|
||||
if power_state != node['power_state']:
|
||||
# NOTE(deva): don't log a warning the first time we
|
||||
# sync a node's power state
|
||||
if node['power_state'] is not None:
|
||||
LOG.warning(_("During sync_power_state, node "
|
||||
"%(node)s out of sync. Expected: %(old)s. "
|
||||
"Actual: %(new)s. Updating DB.") %
|
||||
{'node': node['uuid'],
|
||||
'old': node['power_state'],
|
||||
'new': power_state})
|
||||
node['power_state'] = power_state
|
||||
node.save(context)
|
||||
_do_sync_power_state(task)
|
||||
|
||||
except exception.NodeNotFound:
|
||||
LOG.info(_("During sync_power_state, node %(node)s was not "
|
||||
|
||||
@ -98,9 +98,9 @@ class ManagerTestCase(base.DbTestCase):
|
||||
node = self.dbapi.get_node(n['id'])
|
||||
self.assertEqual(node['power_state'], 'fake-power')
|
||||
|
||||
def test__sync_power_state_do_sync(self):
|
||||
def test__sync_power_state_not_set(self):
|
||||
self.service.start()
|
||||
n = utils.get_test_node(driver='fake', power_state='fake-power')
|
||||
n = utils.get_test_node(driver='fake', power_state=None)
|
||||
self.dbapi.create_node(n)
|
||||
with mock.patch.object(self.driver.power,
|
||||
'get_power_state') as get_power_mock:
|
||||
@ -110,6 +110,68 @@ class ManagerTestCase(base.DbTestCase):
|
||||
node = self.dbapi.get_node(n['id'])
|
||||
self.assertEqual(node['power_state'], states.POWER_ON)
|
||||
|
||||
@mock.patch.object(objects.node.Node, 'save')
|
||||
def test__sync_power_state_unchanged(self, save_mock):
|
||||
self.service.start()
|
||||
n = utils.get_test_node(driver='fake', power_state=states.POWER_ON)
|
||||
self.dbapi.create_node(n)
|
||||
with mock.patch.object(self.driver.power,
|
||||
'get_power_state') as get_power_mock:
|
||||
get_power_mock.return_value = states.POWER_ON
|
||||
self.service._sync_power_states(self.context)
|
||||
get_power_mock.assert_called_once_with(mock.ANY, mock.ANY)
|
||||
self.assertFalse(save_mock.called)
|
||||
|
||||
@mock.patch.object(conductor_utils, 'node_power_action')
|
||||
@mock.patch.object(objects.node.Node, 'save')
|
||||
def test__sync_power_state_changed_sync(self, save_mock, npa_mock):
|
||||
self.service.start()
|
||||
self.config(force_power_state_during_sync=True, group='conductor')
|
||||
n = utils.get_test_node(driver='fake', power_state=states.POWER_ON)
|
||||
self.dbapi.create_node(n)
|
||||
with mock.patch.object(self.driver.power,
|
||||
'get_power_state') as get_power_mock:
|
||||
get_power_mock.return_value = states.POWER_OFF
|
||||
self.service._sync_power_states(self.context)
|
||||
get_power_mock.assert_called_once_with(mock.ANY, mock.ANY)
|
||||
npa_mock.assert_called_once_with(mock.ANY, mock.ANY,
|
||||
states.POWER_ON)
|
||||
self.assertFalse(save_mock.called)
|
||||
|
||||
@mock.patch.object(conductor_utils, 'node_power_action')
|
||||
@mock.patch.object(objects.node.Node, 'save')
|
||||
def test__sync_power_state_changed_failed(self, save_mock, npa_mock):
|
||||
self.service.start()
|
||||
self.config(force_power_state_during_sync=True, group='conductor')
|
||||
n = utils.get_test_node(driver='fake', power_state=states.POWER_ON)
|
||||
self.dbapi.create_node(n)
|
||||
with mock.patch.object(self.driver.power,
|
||||
'get_power_state') as get_power_mock:
|
||||
get_power_mock.return_value = states.POWER_OFF
|
||||
npa_mock.side_effect = exception.IronicException('test')
|
||||
# we are testing that this does NOT raise an exception
|
||||
# so don't assertRaises here
|
||||
self.service._sync_power_states(self.context)
|
||||
get_power_mock.assert_called_once_with(mock.ANY, mock.ANY)
|
||||
npa_mock.assert_called_once_with(mock.ANY, mock.ANY,
|
||||
states.POWER_ON)
|
||||
self.assertFalse(save_mock.called)
|
||||
|
||||
@mock.patch.object(conductor_utils, 'node_power_action')
|
||||
@mock.patch.object(objects.node.Node, 'save')
|
||||
def test__sync_power_state_changed_save(self, save_mock, npa_mock):
|
||||
self.service.start()
|
||||
self.config(force_power_state_during_sync=False, group='conductor')
|
||||
n = utils.get_test_node(driver='fake', power_state=states.POWER_OFF)
|
||||
self.dbapi.create_node(n)
|
||||
with mock.patch.object(self.driver.power,
|
||||
'get_power_state') as get_power_mock:
|
||||
get_power_mock.return_value = states.POWER_ON
|
||||
self.service._sync_power_states(self.context)
|
||||
get_power_mock.assert_called_once_with(mock.ANY, mock.ANY)
|
||||
self.assertFalse(npa_mock.called)
|
||||
self.assertTrue(save_mock.called)
|
||||
|
||||
def test__sync_power_state_node_locked(self):
|
||||
self.service.start()
|
||||
n = utils.get_test_node(driver='fake', power_state='fake-power')
|
||||
@ -124,6 +186,7 @@ class ManagerTestCase(base.DbTestCase):
|
||||
|
||||
def test__sync_power_state_multiple_nodes(self):
|
||||
self.service.start()
|
||||
self.config(force_power_state_during_sync=False, group='conductor')
|
||||
|
||||
# create three nodes
|
||||
nodes = []
|
||||
@ -156,6 +219,7 @@ class ManagerTestCase(base.DbTestCase):
|
||||
|
||||
def test__sync_power_state_node_no_power_state(self):
|
||||
self.service.start()
|
||||
self.config(force_power_state_during_sync=False, group='conductor')
|
||||
|
||||
# create three nodes
|
||||
nodes = []
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user