openstack/ironic
Code Issues Proposed changes

Don't power off non-deploying iLO nodes in takeover

Browse Source 
The iLO driver, when the conductor takeover logic was triggered
would power off the node if it was not in the ACTIVE state,
which could potentially be harmful to the hardware.

Presently, takeover is only called for active nodes, however in
the future, features like ADOPTION will leverage take_over and
evolution of redundancy support may drive additional states that
the node is in when takeover is called.

This changes the behavior and unit tests accordingly such
that only nodes in deploying state perform deployment related
method calls and thus prevent accdiental actions occuring.

Change-Id: I244c3f31d0ad26194887cfb9b79f96b5111296c6
Closes-Bug: #1563949
This commit is contained in:
Julia Kreger 2016-03-30 14:21:20 -04:00
parent 4482eaa59b
commit 624e927fcd
3 changed files with 71 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
ironic/drivers/modules/ilo/deploy.py
View File

@ -170,7 +170,7 @@ class IloVirtualMediaIscsiDeploy(iscsi_deploy.ISCSIDeploy):
:param task: a TaskManager instance containing the node to act on.
:raises: IloOperationError, if some operation on iLO failed.
"""
if task.node.provision_state != states.ACTIVE:
if task.node.provision_state == states.DEPLOYING:
_prepare_node_for_deploy(task)
super(IloVirtualMediaIscsiDeploy, self).prepare(task)
@ -218,7 +218,7 @@ class IloVirtualMediaAgentDeploy(agent.AgentDeploy):
:param task: a TaskManager instance.
:raises: IloOperationError, if some operation on iLO failed.
"""
if task.node.provision_state != states.ACTIVE:
if task.node.provision_state == states.DEPLOYING:
_prepare_node_for_deploy(task)
super(IloVirtualMediaAgentDeploy, self).prepare(task)
@ -275,7 +275,7 @@ class IloPXEDeploy(iscsi_deploy.ISCSIDeploy):
:raises: IloOperationError, if some operation on iLO failed.
:raises: InvalidParameterValue, if some information is invalid.
"""
if task.node.provision_state != states.ACTIVE:
if task.node.provision_state == states.DEPLOYING:
_prepare_node_for_deploy(task)
# Check if 'boot_option' is compatible with 'boot_mode' and image.

81
ironic/tests/unit/drivers/modules/ilo/test_deploy.py
View File

@ -263,6 +263,8 @@ class IloVirtualMediaIscsiDeployTestCase(db_base.DbTestCase):
autospec=True)
def test_prepare(self, func_prepare_node_for_deploy,
iscsi_deploy_prepare_mock):
self.node.provision_state = states.DEPLOYING
self.node.save()
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
task.driver.deploy.prepare(task)
@ -275,13 +277,22 @@ class IloVirtualMediaIscsiDeployTestCase(db_base.DbTestCase):
autospec=True)
def test_prepare_active_node(self, func_prepare_node_for_deploy,
iscsi_deploy_prepare_mock):
self.node.provision_state = states.ACTIVE
self.node.save()
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
task.driver.deploy.prepare(task)
self.assertFalse(func_prepare_node_for_deploy.called)
iscsi_deploy_prepare_mock.assert_called_once_with(mock.ANY, task)
"""Ensure nodes in running states are not inadvertently changed"""
test_states = list(states.STABLE_STATES)
test_states.extend([states.CLEANING,
states.CLEANWAIT,
states.INSPECTING])
for state in test_states:
self.node.provision_state = state
self.node.save()
func_prepare_node_for_deploy.reset_mock()
iscsi_deploy_prepare_mock.reset_mock()
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
task.driver.deploy.prepare(task)
self.assertFalse(func_prepare_node_for_deploy.called)
iscsi_deploy_prepare_mock.assert_called_once_with(
mock.ANY, task)
@mock.patch.object(iscsi_deploy.ISCSIDeploy, 'prepare_cleaning',
spec_set=True, autospec=True)
@ -360,6 +371,8 @@ class IloVirtualMediaAgentDeployTestCase(db_base.DbTestCase):
def test_prepare(self,
agent_prepare_mock,
func_prepare_node_for_deploy):
self.node.provision_state = states.DEPLOYING
self.node.save()
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
task.driver.deploy.prepare(task)
@ -373,12 +386,21 @@ class IloVirtualMediaAgentDeployTestCase(db_base.DbTestCase):
def test_prepare_active_node(self,
func_prepare_node_for_deploy,
agent_prepare_mock):
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
task.node.provision_state = states.ACTIVE
task.driver.deploy.prepare(task)
self.assertFalse(func_prepare_node_for_deploy.called)
agent_prepare_mock.assert_called_once_with(mock.ANY, task)
"""Ensure nodes in running states are not inadvertently changed"""
test_states = list(states.STABLE_STATES)
test_states.extend([states.CLEANING,
states.CLEANWAIT,
states.INSPECTING])
for state in test_states:
self.node.provision_state = state
self.node.save()
func_prepare_node_for_deploy.reset_mock()
agent_prepare_mock.reset_mock()
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
task.driver.deploy.prepare(task)
self.assertFalse(func_prepare_node_for_deploy.called)
agent_prepare_mock.assert_called_once_with(mock.ANY, task)
@mock.patch.object(deploy_utils, 'agent_get_clean_steps', spec_set=True,
autospec=True)
@ -467,13 +489,15 @@ class IloPXEDeployTestCase(db_base.DbTestCase):
@mock.patch.object(ilo_deploy, '_prepare_node_for_deploy', spec_set=True,
autospec=True)
def test_prepare(self,
prepare_node_mock,
prepare_node_for_deploy_mock,
pxe_prepare_mock):
self.node.provision_state = states.DEPLOYING
self.node.save()
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
task.node.properties['capabilities'] = 'boot_mode:uefi'
task.driver.deploy.prepare(task)
prepare_node_mock.assert_called_once_with(task)
prepare_node_for_deploy_mock.assert_called_once_with(task)
pxe_prepare_mock.assert_called_once_with(mock.ANY, task)
@mock.patch.object(iscsi_deploy.ISCSIDeploy, 'prepare', spec_set=True,
@ -481,15 +505,24 @@ class IloPXEDeployTestCase(db_base.DbTestCase):
@mock.patch.object(ilo_deploy, '_prepare_node_for_deploy', spec_set=True,
autospec=True)
def test_prepare_active_node(self,
prepare_node_mock,
prepare_node_for_deploy_mock,
pxe_prepare_mock):
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
task.node.provision_state = states.ACTIVE
task.node.properties['capabilities'] = 'boot_mode:uefi'
task.driver.deploy.prepare(task)
self.assertFalse(prepare_node_mock.called)
pxe_prepare_mock.assert_called_once_with(mock.ANY, task)
"""Ensure nodes in running states are not inadvertently changed"""
test_states = list(states.STABLE_STATES)
test_states.extend([states.CLEANING,
states.CLEANWAIT,
states.INSPECTING])
for state in test_states:
self.node.provision_state = state
self.node.save()
prepare_node_for_deploy_mock.reset_mock()
pxe_prepare_mock.reset_mock()
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
task.node.properties['capabilities'] = 'boot_mode:uefi'
task.driver.deploy.prepare(task)
self.assertFalse(prepare_node_for_deploy_mock.called)
pxe_prepare_mock.assert_called_once_with(mock.ANY, task)
@mock.patch.object(iscsi_deploy.ISCSIDeploy, 'prepare', spec_set=True,
autospec=True)
@ -498,6 +531,8 @@ class IloPXEDeployTestCase(db_base.DbTestCase):
def test_prepare_uefi_whole_disk_image_fail(self,
prepare_node_for_deploy_mock,
pxe_prepare_mock):
self.node.provision_state = states.DEPLOYING
self.node.save()
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
task.node.properties['capabilities'] = 'boot_mode:uefi'

10
releasenotes/notes/ilo-do-not-power-off-non-deploying-nodes-0a3aed7c8ea3940a.yaml Normal file
View File

@ -0,0 +1,10 @@
---
fixes:
- A bug was identified in the behavior of the ilo driver
where nodes that are not active but taking part of a
conductor takeover could be powered off.
In preparation for new features and functionality,
that risk encountering this bug, we are limiting the
deployment preparation steps to the ``deploying``
state to prevent nodes from being erroniuosly
powered off.