From 878c44f0cff4f11b2a01e60019d79ccf5e5d707a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mathieu=20Gagne=CC=81?= Date: Thu, 7 Sep 2017 13:10:00 -0400 Subject: [PATCH] Regenerate and pass configdrive when rebuild Ironic nodes Previously, the configdrive could only be set when setting the node's provisioning state to "active". When rebuilding, the old configdrive was used and therefore was never updated with latest content. Since Ironic API microversion 1.35, it is now allowed to provide a configdrive when setting the node's provisioning state to "rebuild". Blueprint: rebuild-ironic-config-drive Related-bug: #1575935 Depends-On: I9a5529f9fa796c75621e9f4354886bf3032cc248 Change-Id: I1f9056f66519b9ca2f4e23143559735f2bff8943 --- .../unit/virt/ironic/test_client_wrapper.py | 10 +-- nova/tests/unit/virt/ironic/test_driver.py | 68 +++++++++++++++++-- nova/virt/ironic/client_wrapper.py | 2 +- nova/virt/ironic/driver.py | 28 +++++++- ...-ironic-config-drive-77bea47ad20c105b.yaml | 6 ++ 5 files changed, 101 insertions(+), 13 deletions(-) create mode 100644 releasenotes/notes/rebuild-ironic-config-drive-77bea47ad20c105b.yaml diff --git a/nova/tests/unit/virt/ironic/test_client_wrapper.py b/nova/tests/unit/virt/ironic/test_client_wrapper.py index a0d3cd90d83b..3b9a884ede82 100644 --- a/nova/tests/unit/virt/ironic/test_client_wrapper.py +++ b/nova/tests/unit/virt/ironic/test_client_wrapper.py @@ -81,12 +81,12 @@ class IronicClientWrapperTestCase(test.NoDBTestCase): # nova.utils.get_ksa_adapter().get_endpoint() self.get_ksa_adapter.assert_called_once_with( 'baremetal', ksa_auth=self.get_auth_plugin.return_value, - ksa_session='session', min_version=(1, 32), + ksa_session='session', min_version=(1, 35), max_version=(1, ksa_disc.LATEST)) expected = {'session': 'session', 'max_retries': CONF.ironic.api_max_retries, 'retry_interval': CONF.ironic.api_retry_interval, - 'os_ironic_api_version': '1.32', + 'os_ironic_api_version': '1.35', 'ironic_url': self.get_ksa_adapter.return_value.get_endpoint.return_value} mock_ir_cli.assert_called_once_with(1, **expected) @@ -106,13 +106,13 @@ class IronicClientWrapperTestCase(test.NoDBTestCase): # nova.utils.get_endpoint_data self.get_ksa_adapter.assert_called_once_with( 'baremetal', ksa_auth=self.get_auth_plugin.return_value, - ksa_session='session', min_version=(1, 32), + ksa_session='session', min_version=(1, 35), max_version=(1, ksa_disc.LATEST)) # When get_endpoint_data raises any ServiceNotFound, None is returned. expected = {'session': 'session', 'max_retries': CONF.ironic.api_max_retries, 'retry_interval': CONF.ironic.api_retry_interval, - 'os_ironic_api_version': '1.32', + 'os_ironic_api_version': '1.35', 'ironic_url': None} mock_ir_cli.assert_called_once_with(1, **expected) @@ -130,7 +130,7 @@ class IronicClientWrapperTestCase(test.NoDBTestCase): expected = {'session': 'session', 'max_retries': CONF.ironic.api_max_retries, 'retry_interval': CONF.ironic.api_retry_interval, - 'os_ironic_api_version': '1.32', + 'os_ironic_api_version': '1.35', 'ironic_url': endpoint} mock_ir_cli.assert_called_once_with(1, **expected) diff --git a/nova/tests/unit/virt/ironic/test_driver.py b/nova/tests/unit/virt/ironic/test_driver.py index 69a58a64ed40..351f0bb1d5dd 100644 --- a/nova/tests/unit/virt/ironic/test_driver.py +++ b/nova/tests/unit/virt/ironic/test_driver.py @@ -2027,30 +2027,90 @@ class IronicDriverTestCase(test.NoDBTestCase): test.MatchType(objects.ImageMeta), flavor, preserve) mock_set_pstate.assert_called_once_with(node_uuid, - ironic_states.REBUILD) + ironic_states.REBUILD, + configdrive=mock.ANY) mock_looping.assert_called_once_with(mock_wait_active, instance) fake_looping_call.start.assert_called_once_with( interval=CONF.ironic.api_retry_interval) fake_looping_call.wait.assert_called_once_with() - def test_rebuild_preserve_ephemeral(self): + @mock.patch.object(ironic_driver.IronicDriver, '_generate_configdrive') + @mock.patch.object(configdrive, 'required_by') + def test_rebuild_preserve_ephemeral(self, mock_required_by, + mock_configdrive): + mock_required_by.return_value = False self._test_rebuild(preserve=True) + # assert configdrive was not generated + mock_configdrive.assert_not_called() - def test_rebuild_no_preserve_ephemeral(self): + @mock.patch.object(ironic_driver.IronicDriver, '_generate_configdrive') + @mock.patch.object(configdrive, 'required_by') + def test_rebuild_no_preserve_ephemeral(self, mock_required_by, + mock_configdrive): + mock_required_by.return_value = False self._test_rebuild(preserve=False) + @mock.patch.object(ironic_driver.IronicDriver, '_generate_configdrive') + @mock.patch.object(configdrive, 'required_by') + def test_rebuild_with_configdrive(self, mock_required_by, + mock_configdrive): + mock_required_by.return_value = True + self._test_rebuild() + # assert configdrive was generated + mock_configdrive.assert_called_once_with( + self.ctx, mock.ANY, mock.ANY, mock.ANY, extra_md={}, files=None) + + @mock.patch.object(ironic_driver.IronicDriver, '_generate_configdrive') + @mock.patch.object(configdrive, 'required_by') + @mock.patch.object(ironic_driver.IronicDriver, + '_add_instance_info_to_node') + @mock.patch.object(FAKE_CLIENT.node, 'get') + @mock.patch.object(objects.Instance, 'save') + def test_rebuild_with_configdrive_failure(self, mock_save, mock_get, + mock_add_instance_info, + mock_required_by, + mock_configdrive): + node_uuid = uuidutils.generate_uuid() + node = ironic_utils.get_test_node(uuid=node_uuid, + instance_uuid=self.instance_uuid, + instance_type_id=5) + mock_get.return_value = node + mock_required_by.return_value = True + mock_configdrive.side_effect = exception.NovaException() + + image_meta = ironic_utils.get_test_image_meta() + flavor_id = 5 + flavor = objects.Flavor(flavor_id=flavor_id, name='baremetal') + + instance = fake_instance.fake_instance_obj(self.ctx, + uuid=self.instance_uuid, + node=node_uuid, + instance_type_id=flavor_id) + instance.flavor = flavor + + self.assertRaises(exception.InstanceDeployFailure, + self.driver.rebuild, + context=self.ctx, instance=instance, image_meta=image_meta, + injected_files=None, admin_password=None, allocations={}, + bdms=None, detach_block_devices=None, + attach_block_devices=None) + + @mock.patch.object(ironic_driver.IronicDriver, '_generate_configdrive') + @mock.patch.object(configdrive, 'required_by') @mock.patch.object(FAKE_CLIENT.node, 'set_provision_state') @mock.patch.object(ironic_driver.IronicDriver, '_add_instance_info_to_node') @mock.patch.object(FAKE_CLIENT.node, 'get') @mock.patch.object(objects.Instance, 'save') def test_rebuild_failures(self, mock_save, mock_get, - mock_add_instance_info, mock_set_pstate): + mock_add_instance_info, mock_set_pstate, + mock_required_by, mock_configdrive): node_uuid = uuidutils.generate_uuid() node = ironic_utils.get_test_node(uuid=node_uuid, instance_uuid=self.instance_uuid, instance_type_id=5) mock_get.return_value = node + mock_required_by.return_value = False image_meta = ironic_utils.get_test_image_meta() flavor_id = 5 diff --git a/nova/virt/ironic/client_wrapper.py b/nova/virt/ironic/client_wrapper.py index ad56969cd855..4190df658a59 100644 --- a/nova/virt/ironic/client_wrapper.py +++ b/nova/virt/ironic/client_wrapper.py @@ -32,7 +32,7 @@ ironic = None IRONIC_GROUP = nova.conf.ironic.ironic_group # The API version required by the Ironic driver -IRONIC_API_VERSION = (1, 32) +IRONIC_API_VERSION = (1, 35) class IronicClientWrapper(object): diff --git a/nova/virt/ironic/driver.py b/nova/virt/ironic/driver.py index c63519c71ae6..425a84461d93 100644 --- a/nova/virt/ironic/driver.py +++ b/nova/virt/ironic/driver.py @@ -1516,8 +1516,7 @@ class IronicDriver(virt_driver.ComputeDriver): :param image_meta: Image object returned by nova.image.glance that defines the image from which to boot this instance. Ignored by this driver. - :param injected_files: User files to inject into instance. Ignored - by this driver. + :param injected_files: User files to inject into instance. :param admin_password: Administrator password to set in instance. Ignored by this driver. :param allocations: Information about resources allocated to the @@ -1554,10 +1553,33 @@ class IronicDriver(virt_driver.ComputeDriver): self._add_instance_info_to_node(node, instance, image_meta, instance.flavor, preserve_ephemeral) + # Config drive + configdrive_value = None + if configdrive.required_by(instance): + extra_md = {} + if admin_password: + extra_md['admin_pass'] = admin_password + + try: + configdrive_value = self._generate_configdrive( + context, instance, node, network_info, extra_md=extra_md, + files=injected_files) + except Exception as e: + with excutils.save_and_reraise_exception(): + msg = ("Failed to build configdrive: %s" % + six.text_type(e)) + LOG.error(msg, instance=instance) + raise exception.InstanceDeployFailure(msg) + + LOG.info("Config drive for instance %(instance)s on " + "baremetal node %(node)s created.", + {'instance': instance['uuid'], 'node': node_uuid}) + # Trigger the node rebuild/redeploy. try: self.ironicclient.call("node.set_provision_state", - node_uuid, ironic_states.REBUILD) + node_uuid, ironic_states.REBUILD, + configdrive=configdrive_value) except (exception.NovaException, # Retry failed ironic.exc.InternalServerError, # Validations ironic.exc.BadRequest) as e: # Maintenance diff --git a/releasenotes/notes/rebuild-ironic-config-drive-77bea47ad20c105b.yaml b/releasenotes/notes/rebuild-ironic-config-drive-77bea47ad20c105b.yaml new file mode 100644 index 000000000000..163d35f196d6 --- /dev/null +++ b/releasenotes/notes/rebuild-ironic-config-drive-77bea47ad20c105b.yaml @@ -0,0 +1,6 @@ +--- +features: + - | + Now when you rebuild a baremetal instance, a new config drive + will be generated for the node based on the passed in personality files, + metadata, admin password, etc. This fix requires Ironic API 1.35.