From d2875b78b5746bfcb082a7c5385375d704518581 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Fri, 13 May 2016 22:28:24 -0400 Subject: [PATCH] ironic: fix call to _cleanup_deploy on config drive failure The call to _cleanup_deploy when config drive generation failed during spawn didn't match the method signature. This was missed in unit testing because the assertion on the mock of that method matched the actual call, but not the actual method signature. This fixes the call and also fixes the test by auto-spec'ing the _cleanup_deploy method in the mock so that it validates the actual function signature is called correctly. In order to use autospec properly here, the mock has to be on the driver object rather than the class. Change-Id: Ic2c096ef846f11f94aa828222c927ed7d03051c9 Closes-Bug: #1581246 --- nova/tests/unit/virt/ironic/test_driver.py | 12 ++++++------ nova/virt/ironic/driver.py | 3 +-- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/nova/tests/unit/virt/ironic/test_driver.py b/nova/tests/unit/virt/ironic/test_driver.py index f4a63ba2f368..b7ea2a822a65 100644 --- a/nova/tests/unit/virt/ironic/test_driver.py +++ b/nova/tests/unit/virt/ironic/test_driver.py @@ -1015,8 +1015,7 @@ class IronicDriverTestCase(test.NoDBTestCase): @mock.patch.object(ironic_driver.IronicDriver, '_generate_configdrive') @mock.patch.object(ironic_driver.IronicDriver, '_start_firewall') @mock.patch.object(ironic_driver.IronicDriver, '_plug_vifs') - @mock.patch.object(ironic_driver.IronicDriver, '_cleanup_deploy') - def test_spawn_node_configdrive_fail(self, mock_cleanup_deploy, + def test_spawn_node_configdrive_fail(self, mock_pvifs, mock_sf, mock_configdrive, mock_node, mock_save, mock_required_by): @@ -1034,14 +1033,15 @@ class IronicDriverTestCase(test.NoDBTestCase): pass mock_configdrive.side_effect = TestException() - self.assertRaises(TestException, self.driver.spawn, - self.ctx, instance, image_meta, [], None) + with mock.patch.object(self.driver, '_cleanup_deploy', + autospec=True) as mock_cleanup_deploy: + self.assertRaises(TestException, self.driver.spawn, + self.ctx, instance, image_meta, [], None) mock_node.get.assert_called_once_with( node_uuid, fields=ironic_driver._NODE_FIELDS) mock_node.validate.assert_called_once_with(node_uuid) - mock_cleanup_deploy.assert_called_with(self.ctx, node, instance, None, - flavor=flavor) + mock_cleanup_deploy.assert_called_with(node, instance, None) @mock.patch.object(configdrive, 'required_by') @mock.patch.object(FAKE_CLIENT, 'node') diff --git a/nova/virt/ironic/driver.py b/nova/virt/ironic/driver.py index ef0fffa70a11..fd2d050317c4 100644 --- a/nova/virt/ironic/driver.py +++ b/nova/virt/ironic/driver.py @@ -743,8 +743,7 @@ class IronicDriver(virt_driver.ComputeDriver): msg = (_LE("Failed to build configdrive: %s") % six.text_type(e)) LOG.error(msg, instance=instance) - self._cleanup_deploy(context, node, instance, network_info, - flavor=flavor) + self._cleanup_deploy(node, instance, network_info) LOG.info(_LI("Config drive for instance %(instance)s on " "baremetal node %(node)s created."),