openstack/ironic
Code Issues Proposed changes

Bug fixes and doc updates for adoption

Browse Source 
During the creation of a tempest test for the adoption feature,
some minor issues were identified with the adoption functionality.

Namely, the default logic was to create ramdisks, however that logic
path is more intended for deployment ramdisks. Logic was switched
to the instance preparation logic which is the default for nodes in
active state, which is realistically exactly what is desired.

Validation behavior ultimately remains unchanged and tests were
added to validate that the expected methods are called.

Additionally, it was identified that it would be ideal to
encourage the user to set the node to local boot, and as
such the documentation was updated as part of this change,
coupled with a note mentioning changes in API version 1.20
that a user may wish to leverage.

Change-Id: Id6053e0fa68deb431f4543005421982c795401f2
Closes-Bug: #1605239
This commit is contained in:
Julia Kreger 2016-07-20 14:00:11 -04:00
parent d52d23222e
commit f95be90248
6 changed files with 59 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
doc/source/deploy/adoption.rst
View File

@ -132,7 +132,8 @@ from the ``manageable`` state to ``active`` state.::
ironic port-create --node <node_uuid> -a <node_mac_address>
ironic node-update testnode add \
instance_info/image_source="http://localhost:8080/blankimage"
instance_info/image_source="http://localhost:8080/blankimage" \
instance_info/capabilities="{\"boot_option\": \"local\"}"
ironic node-set-provision-state testnode manage
@ -142,6 +143,11 @@ from the ``manageable`` state to ``active`` state.::
In the above example, the image_source setting must reference a valid
image or file, however that image or file can ultimately be empty.
.. NOTE::
The above example utilizes a capability that defines the boot operation
to be local. It is recommended to define the node as such unless network
booting is desired.
.. NOTE::
The above example will fail a re-deployment as a fake image is
defined and no instance_info/image_checksum value is defined.
@ -156,6 +162,12 @@ from the ``manageable`` state to ``active`` state.::
ironic node-update <node name or uuid> add instance_uuid=<uuid>
.. NOTE::
In Newton, coupled with API version 1.20, the concept of a
network_interface was introduced. A user of this feature may wish to
add new nodes with a network_interface of ``noop`` and then change
the interface at a later point and time.
Troubleshooting
===============

2
ironic/drivers/modules/agent.py
View File

@ -331,7 +331,7 @@ class AgentDeploy(base.DeployInterface):
# options get added for the provisioning port.
manager_utils.node_power_action(task, states.POWER_OFF)
task.driver.network.add_provisioning_network(task)
if node.provision_state != states.ACTIVE:
if node.provision_state not in [states.ACTIVE, states.ADOPTING]:
node.instance_info = build_instance_info_for_deploy(task)
node.save()
if CONF.agent.manage_agent_boot:

2
ironic/drivers/modules/iscsi_deploy.py
View File

@ -484,7 +484,7 @@ class ISCSIDeploy(base.DeployInterface):
:raises: any boot interface's prepare_ramdisk exceptions.
"""
node = task.node
if node.provision_state == states.ACTIVE:
if node.provision_state in [states.ACTIVE, states.ADOPTING]:
task.driver.boot.prepare_instance(task)
else:
if node.provision_state == states.DEPLOYING:

19
ironic/tests/unit/drivers/modules/test_agent.py
View File

@ -515,6 +515,25 @@ class TestAgentDeploy(db_base.DbTestCase):
self.assertFalse(pxe_prepare_ramdisk_mock.called)
self.assertFalse(add_provisioning_net_mock.called)
@mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.'
'add_provisioning_network', autospec=True)
@mock.patch.object(pxe.PXEBoot, 'prepare_ramdisk')
@mock.patch.object(deploy_utils, 'build_agent_options')
@mock.patch.object(agent, 'build_instance_info_for_deploy')
def test_prepare_adopting(
self, build_instance_info_mock, build_options_mock,
pxe_prepare_ramdisk_mock, add_provisioning_net_mock):
with task_manager.acquire(
self.context, self.node['uuid'], shared=False) as task:
task.node.provision_state = states.ADOPTING
self.driver.prepare(task)
self.assertFalse(build_instance_info_mock.called)
self.assertFalse(build_options_mock.called)
self.assertFalse(pxe_prepare_ramdisk_mock.called)
self.assertFalse(add_provisioning_net_mock.called)
@mock.patch('ironic.common.dhcp_factory.DHCPFactory._set_dhcp_provider')
@mock.patch('ironic.common.dhcp_factory.DHCPFactory.clean_dhcp')
@mock.patch.object(pxe.PXEBoot, 'clean_up_ramdisk')

14
ironic/tests/unit/drivers/modules/test_iscsi_deploy.py
View File

@ -541,6 +541,20 @@ class ISCSIDeployTestCase(db_base.DbTestCase):
task.driver.boot, task)
self.assertEqual(0, add_provisioning_net_mock.call_count)
@mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.'
'add_provisioning_network', spec_set=True, autospec=True)
@mock.patch.object(pxe.PXEBoot, 'prepare_instance', autospec=True)
def test_prepare_node_adopting(self, prepare_instance_mock,
add_provisioning_net_mock):
with task_manager.acquire(self.context, self.node.uuid) as task:
task.node.provision_state = states.ADOPTING
task.driver.deploy.prepare(task)
prepare_instance_mock.assert_called_once_with(
task.driver.boot, task)
self.assertEqual(0, add_provisioning_net_mock.call_count)
@mock.patch.object(deploy_utils, 'build_agent_options', autospec=True)
@mock.patch.object(pxe.PXEBoot, 'prepare_ramdisk', autospec=True)
@mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.'

11
releasenotes/notes/adoption-feature-update-d2160954a2c36b0a.yaml Normal file
View File

@ -0,0 +1,11 @@
---
fixes:
- Adoption feature logic was updated to prevent ramdisk
creation and default to instance creation where appropriate
based on the driver.
- Adoption documentation has been updated to note that the
boot_option should likely be defined for nodes by a user
leveraging the feature.
- Adoption documentation has been updated to note that a user
may wish to utilize the ``noop`` network interface that
arrived with API version 1.20.