From f95be90248ff64cb37cd0e2dbe220719c1df346c Mon Sep 17 00:00:00 2001 From: Julia Kreger Date: Wed, 20 Jul 2016 14:00:11 -0400 Subject: [PATCH] Bug fixes and doc updates for adoption 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 --- doc/source/deploy/adoption.rst | 14 +++++++++++++- ironic/drivers/modules/agent.py | 2 +- ironic/drivers/modules/iscsi_deploy.py | 2 +- .../tests/unit/drivers/modules/test_agent.py | 19 +++++++++++++++++++ .../unit/drivers/modules/test_iscsi_deploy.py | 14 ++++++++++++++ ...ption-feature-update-d2160954a2c36b0a.yaml | 11 +++++++++++ 6 files changed, 59 insertions(+), 3 deletions(-) create mode 100644 releasenotes/notes/adoption-feature-update-d2160954a2c36b0a.yaml diff --git a/doc/source/deploy/adoption.rst b/doc/source/deploy/adoption.rst index aed8bd284f..b3d769f0e5 100644 --- a/doc/source/deploy/adoption.rst +++ b/doc/source/deploy/adoption.rst @@ -132,7 +132,8 @@ from the ``manageable`` state to ``active`` state.:: ironic port-create --node -a 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 add instance_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 =============== diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py index 2cf56809f7..1e55a8495e 100644 --- a/ironic/drivers/modules/agent.py +++ b/ironic/drivers/modules/agent.py @@ -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: diff --git a/ironic/drivers/modules/iscsi_deploy.py b/ironic/drivers/modules/iscsi_deploy.py index 957368eb32..a049f3f2c8 100644 --- a/ironic/drivers/modules/iscsi_deploy.py +++ b/ironic/drivers/modules/iscsi_deploy.py @@ -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: diff --git a/ironic/tests/unit/drivers/modules/test_agent.py b/ironic/tests/unit/drivers/modules/test_agent.py index e2b647b89d..81e34448ce 100644 --- a/ironic/tests/unit/drivers/modules/test_agent.py +++ b/ironic/tests/unit/drivers/modules/test_agent.py @@ -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') diff --git a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py index a4f6d9991b..64b38e063e 100644 --- a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py +++ b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py @@ -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.' diff --git a/releasenotes/notes/adoption-feature-update-d2160954a2c36b0a.yaml b/releasenotes/notes/adoption-feature-update-d2160954a2c36b0a.yaml new file mode 100644 index 0000000000..bcc5247c90 --- /dev/null +++ b/releasenotes/notes/adoption-feature-update-d2160954a2c36b0a.yaml @@ -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.