diff --git a/nova/objects/image_meta.py b/nova/objects/image_meta.py index 15be3f1..83fc2fb 100644 --- a/nova/objects/image_meta.py +++ b/nova/objects/image_meta.py @@ -346,6 +346,7 @@ class ImageMetaProps(base.NovaObject): # is a fairly generic type. For a detailed type consider os_distro # instead 'os_type': fields.OSTypeField(), + 'deploy_config': fields.StringField(), } # The keys are the legacy property names and diff --git a/nova/tests/unit/objects/test_objects.py b/nova/tests/unit/objects/test_objects.py index 031555f..e0368b8 100644 --- a/nova/tests/unit/objects/test_objects.py +++ b/nova/tests/unit/objects/test_objects.py @@ -1180,7 +1180,7 @@ object_data = { 'HostMapping': '1.0-1a3390a696792a552ab7bd31a77ba9ac', 'HVSpec': '1.1-6b4f7c0f688cbd03e24142a44eb9010d', 'ImageMeta': '1.7-642d1b2eb3e880a367f37d72dd76162d', - 'ImageMetaProps': '1.7-f12fc4cf3e25d616f69a66fb9d2a7aa6', + 'ImageMetaProps': '1.7-716042e9e80ea16890f475200940d6f9', 'Instance': '2.0-ff56804dce87d81d9a04834d4bd1e3d2', # NOTE(danms): Reviewers: do not approve changes to the Instance1 # object schema. It is frozen for Liberty and will be removed in diff --git a/nova/tests/unit/virt/ironic/test_driver.py b/nova/tests/unit/virt/ironic/test_driver.py index a8c653a..940497c 100644 --- a/nova/tests/unit/virt/ironic/test_driver.py +++ b/nova/tests/unit/virt/ironic/test_driver.py @@ -799,6 +799,7 @@ class IronicDriverTestCase(test.NoDBTestCase): result = self.driver.macs_for_instance(instance) self.assertIsNone(result) + @mock.patch.object(ironic_driver.IronicDriver, '_get_switch_boot_options') @mock.patch.object(objects.Instance, 'save') @mock.patch.object(loopingcall, 'FixedIntervalLoopingCall') @mock.patch.object(FAKE_CLIENT, 'node') @@ -807,7 +808,7 @@ class IronicDriverTestCase(test.NoDBTestCase): @mock.patch.object(ironic_driver.IronicDriver, '_plug_vifs') @mock.patch.object(ironic_driver.IronicDriver, '_start_firewall') def _test_spawn(self, mock_sf, mock_pvifs, mock_adf, mock_wait_active, - mock_node, mock_looping, mock_save): + mock_node, mock_looping, mock_save, mock_sb_options): node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' node = ironic_utils.get_test_node(driver='fake', uuid=node_uuid) instance = fake_instance.fake_instance_obj(self.ctx, node=node_uuid) @@ -845,6 +846,7 @@ class IronicDriverTestCase(test.NoDBTestCase): fake_looping_call.start.assert_called_once_with( interval=CONF.ironic.api_retry_interval) fake_looping_call.wait.assert_called_once_with() + mock_sb_options.assert_called_once_with(self.ctx, instance, node_uuid) @mock.patch.object(ironic_driver.IronicDriver, '_generate_configdrive') @mock.patch.object(configdrive, 'required_by') @@ -897,14 +899,62 @@ class IronicDriverTestCase(test.NoDBTestCase): self.driver.spawn, self.ctx, instance, None, [], None) mock_destroy.assert_called_once_with(self.ctx, instance, None) + @mock.patch.object(FAKE_CLIENT, 'node') + def test__get_switch_boot_options(self, mock_node): + node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' + node = ironic_utils.get_test_node( + driver='fake', uuid=node_uuid, + instance_info={ + 'multiboot': True, + 'multiboot_info': {'elements': [ + {'image_name': "name_1"}, + {'image_name': "name_2"}, + ]}} + ) + mock_node.get.return_value = node + instance = fake_instance.fake_instance_obj( + self.ctx, node=node_uuid, expected_attrs=('metadata',)) + + self.driver._get_switch_boot_options(self.ctx, + instance, node_uuid) + + exp_meta = {'available_images': str(['name_1', 'name_2'])} + self.assertEqual(exp_meta, instance.metadata) + + @mock.patch.object(FAKE_CLIENT, 'node') + def test__get_switch_boot_options_not_multiboot(self, mock_node): + node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' + node = ironic_utils.get_test_node( + driver='fake', uuid=node_uuid, + instance_info={} + ) + mock_node.get.return_value = node + instance = fake_instance.fake_instance_obj( + self.ctx, node=node_uuid, expected_attrs=('metadata',)) + + self.driver._get_switch_boot_options(self.ctx, + instance, node_uuid) + + self.assertEqual({}, instance.metadata) + + @mock.patch.object(ironic_driver.IronicDriver, + "_get_deploy_config_options") @mock.patch.object(FAKE_CLIENT.node, 'update') - def test__add_driver_fields_good(self, mock_update): + def test__add_driver_fields_good(self, mock_update, + mock_get_depl_conf_opts): node = ironic_utils.get_test_node(driver='fake') - instance = fake_instance.fake_instance_obj(self.ctx, - node=node.uuid) - image_meta = ironic_utils.get_test_image_meta_object() + instance = fake_instance.fake_instance_obj( + self.ctx, + node=node.uuid, + expected_attrs=('metadata',)) flavor = ironic_utils.get_test_flavor() + + image_meta = ironic_utils.get_test_image_meta_object() + mock_get_depl_conf_opts.return_value = {'foo': 'bar123'} + instance['metadata']['driver_actions'] = 'test_driver_actions' + self.driver._add_driver_fields(node, instance, image_meta, flavor) + expected_patch = [{'path': '/instance_info/image_source', 'op': 'add', 'value': image_meta.id}, {'path': '/instance_info/root_gb', 'op': 'add', @@ -920,21 +970,96 @@ class IronicDriverTestCase(test.NoDBTestCase): {'path': '/instance_info/local_gb', 'op': 'add', 'value': str(node.properties.get('local_gb', 0))}, {'path': '/instance_uuid', 'op': 'add', - 'value': instance.uuid}] + 'value': instance.uuid}, + {'path': '/instance_info/deploy_config_options', + 'op': 'add', + 'value': {'foo': 'bar123'}}, + {'path': '/instance_info/driver_actions', + 'op': 'add', + 'value': 'test_driver_actions'}, + ] mock_update.assert_called_once_with(node.uuid, expected_patch) @mock.patch.object(FAKE_CLIENT.node, 'update') def test__add_driver_fields_fail(self, mock_update): mock_update.side_effect = ironic_exception.BadRequest() node = ironic_utils.get_test_node(driver='fake') - instance = fake_instance.fake_instance_obj(self.ctx, - node=node.uuid) + instance = fake_instance.fake_instance_obj( + self.ctx, + node=node.uuid, + expected_attrs=('metadata',)) image_meta = ironic_utils.get_test_image_meta_object() flavor = ironic_utils.get_test_flavor() self.assertRaises(exception.InstanceDeployFailure, self.driver._add_driver_fields, node, instance, image_meta, flavor) + def test__get_deploy_config_options_all_present(self): + node = ironic_utils.get_test_node( + driver='fake', driver_info={'deploy_config': "node-conf"}) + image_meta = ironic_utils.get_test_image_meta_object( + deploy_config="image-conf") + instance = fake_instance.fake_instance_obj( + self.ctx, node=node.uuid, expected_attrs=('metadata',), + metadata={'deploy_config': "instance-conf"}) + + opts = self.driver._get_deploy_config_options(node, instance, + image_meta) + + expected = {"node": "node-conf", + "image": "image-conf", + "instance": "instance-conf" + } + self.assertEqual(expected, opts) + + def test__get_deploy_config_options_on_node_rebuild(self): + # In case of rebuild a set of options is also present in the node + # already. We take them, override with the new ones, and pass back. + node = ironic_utils.get_test_node( + driver='fake', driver_info={'deploy_config': "node-conf"}, + instance_info={"deploy_config_options": { + "instance": "previous_inst_conf", + "image": "previous_image_conf", + }} + ) + image_meta = ironic_utils.get_test_image_meta_object( + deploy_config="image-conf") + instance = fake_instance.fake_instance_obj( + self.ctx, node=node.uuid, expected_attrs=('metadata',)) + opts = self.driver._get_deploy_config_options(node, instance, + image_meta) + + expected = {"node": "node-conf", + "image": "image-conf", + "instance": "previous_inst_conf" + } + self.assertEqual(expected, opts) + + def test__get_deploy_config_options_some_present(self): + node = ironic_utils.get_test_node(driver='fake') + image_meta = ironic_utils.get_test_image_meta_object() + instance = fake_instance.fake_instance_obj( + self.ctx, node=node.uuid, expected_attrs=('metadata',), + metadata={'deploy_config': "instance-conf"}) + + opts = self.driver._get_deploy_config_options(node, instance, + image_meta) + + expected = {"instance": "instance-conf"} + self.assertEqual(expected, opts) + + def test__get_deploy_config_options_none_present(self): + node = ironic_utils.get_test_node(driver='fake') + image_meta = ironic_utils.get_test_image_meta_object() + instance = fake_instance.fake_instance_obj( + self.ctx, node=node.uuid, expected_attrs=('metadata',)) + + opts = self.driver._get_deploy_config_options(node, instance, + image_meta) + + expected = {} + self.assertEqual(expected, opts) + @mock.patch.object(FAKE_CLIENT.node, 'update') def test__cleanup_deploy_good_with_flavor(self, mock_update): node = ironic_utils.get_test_node(driver='fake', @@ -983,8 +1108,10 @@ class IronicDriverTestCase(test.NoDBTestCase): node = ironic_utils.get_test_node(driver='fake', instance_uuid=self.instance_uuid) flavor = ironic_utils.get_test_flavor(extra_specs={}) - instance = fake_instance.fake_instance_obj(self.ctx, - node=node.uuid) + instance = fake_instance.fake_instance_obj( + self.ctx, + node=node.uuid, + expected_attrs=('metadata',)) instance.flavor = flavor self.assertRaises(exception.InstanceTerminationFailure, self.driver._cleanup_deploy, @@ -998,7 +1125,8 @@ class IronicDriverTestCase(test.NoDBTestCase): node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' node = ironic_utils.get_test_node(driver='fake', uuid=node_uuid) flavor = ironic_utils.get_test_flavor() - instance = fake_instance.fake_instance_obj(self.ctx, node=node_uuid) + instance = fake_instance.fake_instance_obj( + self.ctx, node=node_uuid, expected_attrs=('metadata',)) instance.flavor = flavor mock_node.validate.return_value = ironic_utils.get_test_validation( @@ -1023,7 +1151,8 @@ class IronicDriverTestCase(test.NoDBTestCase): node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' node = ironic_utils.get_test_node(driver='fake', uuid=node_uuid) flavor = ironic_utils.get_test_flavor() - instance = fake_instance.fake_instance_obj(self.ctx, node=node_uuid) + instance = fake_instance.fake_instance_obj( + self.ctx, node=node_uuid, expected_attrs=('metadata',)) instance.flavor = flavor mock_node.get.return_value = node mock_node.validate.return_value = ironic_utils.get_test_validation() @@ -1053,7 +1182,8 @@ class IronicDriverTestCase(test.NoDBTestCase): node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' node = ironic_utils.get_test_node(driver='fake', uuid=node_uuid) flavor = ironic_utils.get_test_flavor() - instance = fake_instance.fake_instance_obj(self.ctx, node=node_uuid) + instance = fake_instance.fake_instance_obj( + self.ctx, node=node_uuid, expected_attrs=('metadata',)) instance.flavor = flavor image_meta = ironic_utils.get_test_image_meta() @@ -1082,7 +1212,8 @@ class IronicDriverTestCase(test.NoDBTestCase): node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' node = ironic_utils.get_test_node(driver='fake', uuid=node_uuid) flavor = ironic_utils.get_test_flavor() - instance = fake_instance.fake_instance_obj(self.ctx, node=node_uuid) + instance = fake_instance.fake_instance_obj( + self.ctx, node=node_uuid, expected_attrs=('metadata',)) instance.flavor = flavor image_meta = ironic_utils.get_test_image_meta() @@ -1113,7 +1244,8 @@ class IronicDriverTestCase(test.NoDBTestCase): node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' node = ironic_utils.get_test_node(driver='fake', uuid=node_uuid) flavor = ironic_utils.get_test_flavor() - instance = fake_instance.fake_instance_obj(self.ctx, node=node_uuid) + instance = fake_instance.fake_instance_obj( + self.ctx, node=node_uuid, expected_attrs=('metadata',)) instance.flavor = flavor image_meta = ironic_utils.get_test_image_meta() @@ -1146,7 +1278,8 @@ class IronicDriverTestCase(test.NoDBTestCase): node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' node = ironic_utils.get_test_node(driver='fake', uuid=node_uuid) flavor = ironic_utils.get_test_flavor(ephemeral_gb=1) - instance = fake_instance.fake_instance_obj(self.ctx, node=node_uuid) + instance = fake_instance.fake_instance_obj( + self.ctx, node=node_uuid, expected_attrs=('metadata',)) instance.flavor = flavor mock_node.get_by_instance_uuid.return_value = node mock_node.set_provision_state.return_value = mock.MagicMock() @@ -1541,15 +1674,16 @@ class IronicDriverTestCase(test.NoDBTestCase): self.driver.refresh_instance_security_rules(fake_group) mock_risr.assert_called_once_with(fake_group) + @mock.patch.object(ironic_driver.IronicDriver, '_get_switch_boot_options') @mock.patch.object(ironic_driver.IronicDriver, '_wait_for_active') @mock.patch.object(loopingcall, 'FixedIntervalLoopingCall') @mock.patch.object(FAKE_CLIENT.node, 'set_provision_state') @mock.patch.object(ironic_driver.IronicDriver, '_add_driver_fields') - @mock.patch.object(FAKE_CLIENT.node, 'get') + @mock.patch.object(FAKE_CLIENT.node, 'get_by_instance_uuid') @mock.patch.object(objects.Instance, 'save') def _test_rebuild(self, mock_save, mock_get, mock_driver_fields, mock_set_pstate, mock_looping, mock_wait_active, - preserve=False): + mock_sb_options, preserve=False): node_uuid = uuidutils.generate_uuid() node = ironic_utils.get_test_node(uuid=node_uuid, instance_uuid=self.instance_uuid, @@ -1560,10 +1694,12 @@ class IronicDriverTestCase(test.NoDBTestCase): 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 = fake_instance.fake_instance_obj( + self.ctx, + uuid=self.instance_uuid, + node=node_uuid, + instance_type_id=flavor_id, + expected_attrs=('metadata',)) instance.flavor = flavor fake_looping_call = FakeLoopingCall() @@ -1589,6 +1725,7 @@ class IronicDriverTestCase(test.NoDBTestCase): fake_looping_call.start.assert_called_once_with( interval=CONF.ironic.api_retry_interval) fake_looping_call.wait.assert_called_once_with() + mock_sb_options.assert_called_once_with(self.ctx, instance, node_uuid) def test_rebuild_preserve_ephemeral(self): self._test_rebuild(preserve=True) @@ -1598,7 +1735,7 @@ class IronicDriverTestCase(test.NoDBTestCase): @mock.patch.object(FAKE_CLIENT.node, 'set_provision_state') @mock.patch.object(ironic_driver.IronicDriver, '_add_driver_fields') - @mock.patch.object(FAKE_CLIENT.node, 'get') + @mock.patch.object(FAKE_CLIENT.node, 'get_by_instance_uuid') @mock.patch.object(objects.Instance, 'save') def test_rebuild_failures(self, mock_save, mock_get, mock_driver_fields, mock_set_pstate): @@ -1612,10 +1749,12 @@ class IronicDriverTestCase(test.NoDBTestCase): 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 = fake_instance.fake_instance_obj( + self.ctx, + uuid=self.instance_uuid, + node=node_uuid, + instance_type_id=flavor_id, + expected_attrs=('metadata',)) instance.flavor = flavor exceptions = [ @@ -1631,6 +1770,316 @@ class IronicDriverTestCase(test.NoDBTestCase): injected_files=None, admin_password=None, bdms=None, detach_block_devices=None, attach_block_devices=None) + @mock.patch.object(ironic_driver.IronicDriver, '_do_rebuild') + @mock.patch.object(ironic_driver.IronicDriver, '_get_switch_boot_options') + @mock.patch.object(ironic_driver.IronicDriver, '_wait_for_active') + @mock.patch.object(loopingcall, 'FixedIntervalLoopingCall') + @mock.patch.object(FAKE_CLIENT.node, 'set_provision_state') + @mock.patch.object(ironic_driver.IronicDriver, '_add_driver_fields') + @mock.patch.object(FAKE_CLIENT.node, 'get_by_instance_uuid') + @mock.patch.object(objects.Instance, 'save') + def test_rebuild_multiboot_force_rebuild(self, mock_save, mock_get, + mock_driver_fields, + mock_set_pstate, mock_looping, + mock_wait_active, + mock_sb_options, rebuild_mock): + node_uuid = uuidutils.generate_uuid() + node = ironic_utils.get_test_node(uuid=node_uuid, + instance_uuid=self.instance_uuid, + instance_type_id=5, + instance_info={'multiboot': True}) + mock_get.return_value = node + + image_meta = ironic_utils.get_test_image_meta_object() + + 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, + expected_attrs=('metadata',), + metadata={'force_rebuild': True}) + instance.flavor = flavor + + fake_looping_call = FakeLoopingCall() + mock_looping.return_value = fake_looping_call + + with mock.patch.object(objects.ImageMeta, + 'from_dict') as mock_image_meta_from_dict: + mock_image_meta_from_dict.return_value = image_meta + self.driver.rebuild( + context=self.ctx, instance=instance, image_meta=image_meta, + injected_files=None, admin_password=None, bdms=None, + detach_block_devices=None, attach_block_devices=None, + preserve_ephemeral=False) + + rebuild_mock.assert_called_once_with( + self.ctx, FAKE_CLIENT_WRAPPER, node, instance, image_meta, + None, + None, None, None, + None, network_info=None, + recreate=False, + block_device_info=None, + preserve_ephemeral=False) + + @mock.patch.object(objects.ImageMeta, 'from_dict') + @mock.patch.object(FAKE_CLIENT.node, 'get_by_instance_uuid') + @mock.patch.object(FAKE_CLIENT.node, 'get') + @mock.patch.object(ironic_driver.IronicDriver, '_do_switch_boot_device') + @mock.patch.object(objects.Instance, 'save') + def test_rebuild_multiboot_switch_boot(self, mock_save, + mock_sb, mock_get, + mock_get_by_instance, + mock_image_meta_from_dict): + node_uuid = uuidutils.generate_uuid() + node = ironic_utils.get_test_node(uuid=node_uuid, + instance_uuid=self.instance_uuid, + instance_type_id=5, + instance_info={'multiboot': True}) + mock_get.return_value = mock_get_by_instance.return_value = node + + image_meta = ironic_utils.get_test_image_meta_object() + mock_image_meta_from_dict.return_value = image_meta + + flavor_id = 5 + instance = fake_instance.fake_instance_obj( + self.ctx, + uuid=self.instance_uuid, + node=node_uuid, + instance_type_id=flavor_id, + expected_attrs=('metadata',)) + + self.driver.rebuild( + context=self.ctx, instance=instance, image_meta=image_meta, + injected_files=None, admin_password=None, bdms=None, + detach_block_devices=None, attach_block_devices=None, + preserve_ephemeral=False) + + mock_sb.assert_called_once_with(self.ctx, FAKE_CLIENT_WRAPPER, node, + instance, image_meta) + + @mock.patch.object(FAKE_CLIENT.node, 'vendor_passthru') + @mock.patch.object(FAKE_CLIENT.node, 'set_power_state') + @mock.patch.object(FAKE_CLIENT.node, 'update') + @mock.patch.object(objects.Instance, 'save') + def test__do_switch_boot_device(self, mock_save, upd_mock, + sp_mock, vp_mock): + node_uuid = uuidutils.generate_uuid() + node = ironic_utils.get_test_node(uuid=node_uuid, + instance_uuid=self.instance_uuid, + instance_type_id=5, + instance_info={'multiboot': True}) + + image_meta = ironic_utils.get_test_image_meta_object() + flavor_id = 5 + instance = fake_instance.fake_instance_obj( + self.ctx, + uuid=self.instance_uuid, + node=node_uuid, + instance_type_id=flavor_id, + expected_attrs=('metadata',), + metadata={'sb_key': 'key1', 'sb_user': 'usr1', 'extras': '123'}) + + self.driver._do_switch_boot_device( + self.ctx, FAKE_CLIENT_WRAPPER, + node, instance, image_meta) + + vp_mock.assert_called_once_with(node_uuid, 'switch_boot', + {'image': image_meta.id, + 'ssh_user': 'usr1', + 'ssh_key': 'key1'}) + sp_mock.assert_called_once_with(node_uuid, 'reboot') + upd_mock.assert_called_once_with( + node_uuid, [{'path': '/instance_info/image_source', 'op': 'add', + 'value': image_meta.id}]) + + @mock.patch.object(FAKE_CLIENT.node, 'vendor_passthru') + @mock.patch.object(FAKE_CLIENT.node, 'set_power_state') + @mock.patch.object(FAKE_CLIENT.node, 'update') + @mock.patch.object(objects.Instance, 'save') + def test__do_switch_boot_device_no_key(self, mock_save, upd_mock, + sp_mock, vp_mock): + node_uuid = uuidutils.generate_uuid() + node = ironic_utils.get_test_node( + uuid=node_uuid, + instance_uuid=self.instance_uuid, + instance_type_id=5, + instance_info={'multiboot': True, + 'image_source': 'original_image', + }) + + image_meta = ironic_utils.get_test_image_meta() + flavor_id = 5 + instance = fake_instance.fake_instance_obj( + self.ctx, + uuid=self.instance_uuid, + node=node_uuid, + instance_type_id=flavor_id, + expected_attrs=('metadata',), + metadata={'sb_user': 'usr1'}) + + self.driver._do_switch_boot_device( + self.ctx, FAKE_CLIENT_WRAPPER, + node, instance, image_meta) + + self.assertEqual(instance.image_ref, 'original_image') + self.assertIn('Invalid metadata', + instance.metadata['switch_boot_error']) + mock_save.assert_called_once_with() + vp_mock.assert_has_calls([]) + sp_mock.assert_has_calls([]) + upd_mock.assert_has_calls([]) + + @mock.patch.object(FAKE_CLIENT.node, 'vendor_passthru') + @mock.patch.object(FAKE_CLIENT.node, 'set_power_state') + @mock.patch.object(FAKE_CLIENT.node, 'update') + @mock.patch.object(objects.Instance, 'save') + def test__do_switch_boot_device_not_supported_by_driver( + self, mock_save, upd_mock, sp_mock, vp_mock): + node_uuid = uuidutils.generate_uuid() + node = ironic_utils.get_test_node( + uuid=node_uuid, + instance_uuid=self.instance_uuid, + instance_type_id=5, + instance_info={'multiboot': True, + 'image_source': 'original_image', + }) + + image_meta = ironic_utils.get_test_image_meta_object() + flavor_id = 5 + instance = fake_instance.fake_instance_obj( + self.ctx, + uuid=self.instance_uuid, + node=node_uuid, + instance_type_id=flavor_id, + expected_attrs=('metadata',), + metadata={'sb_key': 'key1', 'sb_user': 'usr1'}) + + vp_mock.side_effect = ironic_exception.BadRequest() + + self.driver._do_switch_boot_device( + self.ctx, FAKE_CLIENT_WRAPPER, + node, instance, image_meta) + + self.assertEqual(instance.image_ref, 'original_image') + self.assertIn('Bad Request', + instance.metadata['switch_boot_error']) + mock_save.assert_called_once_with() + sp_mock.assert_has_calls([]) + upd_mock.assert_has_calls([]) + + @mock.patch.object(FAKE_CLIENT.node, 'vendor_passthru') + @mock.patch.object(FAKE_CLIENT.node, 'set_power_state') + @mock.patch.object(FAKE_CLIENT.node, 'update') + @mock.patch.object(objects.Instance, 'save') + def test__do_switch_boot_device_already_in_desired_device( + self, mock_save, upd_mock, sp_mock, vp_mock): + node_uuid = uuidutils.generate_uuid() + node = ironic_utils.get_test_node( + uuid=node_uuid, + instance_uuid=self.instance_uuid, + instance_type_id=5, + instance_info={'multiboot': True, + 'image_source': 'original_image', + }) + + image_meta = ironic_utils.get_test_image_meta_object() + flavor_id = 5 + instance = fake_instance.fake_instance_obj( + self.ctx, + uuid=self.instance_uuid, + node=node_uuid, + instance_type_id=flavor_id, + expected_attrs=('metadata',), + metadata={'sb_key': 'key1', 'sb_user': 'usr1'}) + + vp_mock.side_effect = ironic_exception.BadRequest( + message="Node 123 Already in desired boot device.") + + self.driver._do_switch_boot_device( + self.ctx, FAKE_CLIENT_WRAPPER, + node, instance, image_meta) + + mock_save.assert_has_calls([]) + sp_mock.assert_has_calls([]) + upd_mock.assert_has_calls([]) + + @mock.patch.object(FAKE_CLIENT.node, 'vendor_passthru') + @mock.patch.object(FAKE_CLIENT.node, 'set_power_state') + @mock.patch.object(FAKE_CLIENT.node, 'update') + @mock.patch.object(objects.Instance, 'save') + def test__do_switch_boot_device_reboot_error( + self, mock_save, upd_mock, sp_mock, vp_mock): + node_uuid = uuidutils.generate_uuid() + node = ironic_utils.get_test_node( + uuid=node_uuid, + instance_uuid=self.instance_uuid, + instance_type_id=5, + instance_info={'multiboot': True, + 'image_source': 'original_image', + }) + + image_meta = ironic_utils.get_test_image_meta_object() + flavor_id = 5 + instance = fake_instance.fake_instance_obj( + self.ctx, + uuid=self.instance_uuid, + node=node_uuid, + instance_type_id=flavor_id, + expected_attrs=('metadata',), + metadata={'sb_key': 'key1', 'sb_user': 'usr1'}) + + sp_mock.side_effect = ironic_exception.BadRequest() + + self.driver._do_switch_boot_device( + self.ctx, FAKE_CLIENT_WRAPPER, + node, instance, image_meta) + + self.assertEqual(instance.image_ref, 'original_image') + self.assertIn('Bad Request', + instance.metadata['switch_boot_error']) + mock_save.assert_called_once_with() + upd_mock.assert_has_calls([]) + + @mock.patch.object(FAKE_CLIENT.node, 'vendor_passthru') + @mock.patch.object(FAKE_CLIENT.node, 'set_power_state') + @mock.patch.object(FAKE_CLIENT.node, 'update') + @mock.patch.object(objects.Instance, 'save') + def test__do_switch_boot_device_update_node_error( + self, mock_save, upd_mock, sp_mock, vp_mock): + node_uuid = uuidutils.generate_uuid() + node = ironic_utils.get_test_node( + uuid=node_uuid, + instance_uuid=self.instance_uuid, + instance_type_id=5, + instance_info={'multiboot': True, + 'image_source': 'original_image', + }) + + image_meta = ironic_utils.get_test_image_meta_object() + flavor_id = 5 + instance = fake_instance.fake_instance_obj( + self.ctx, + uuid=self.instance_uuid, + node=node_uuid, + instance_type_id=flavor_id, + expected_attrs=('metadata',), + metadata={'sb_key': 'key1', 'sb_user': 'usr1'}) + + upd_mock.side_effect = ironic_exception.BadRequest() + + self.driver._do_switch_boot_device( + self.ctx, FAKE_CLIENT_WRAPPER, + node, instance, image_meta) + + self.assertEqual(instance.image_ref, 'original_image') + self.assertIn('Bad Request', + instance.metadata['switch_boot_error']) + mock_save.assert_called_once_with() + @mock.patch.object(instance_metadata, 'InstanceMetadata') @mock.patch.object(configdrive, 'ConfigDriveBuilder') diff --git a/nova/tests/unit/virt/ironic/utils.py b/nova/tests/unit/virt/ironic/utils.py index 0e67919..66eede3 100644 --- a/nova/tests/unit/virt/ironic/utils.py +++ b/nova/tests/unit/virt/ironic/utils.py @@ -39,7 +39,7 @@ def get_test_node(**kw): ironic_states.NOSTATE), 'last_error': kw.get('last_error'), 'instance_uuid': kw.get('instance_uuid'), - 'instance_info': kw.get('instance_info'), + 'instance_info': kw.get('instance_info', {}), 'driver': kw.get('driver', 'fake'), 'driver_info': kw.get('driver_info', {}), 'properties': kw.get('properties', {}), @@ -91,7 +91,11 @@ def get_test_flavor(**kw): def get_test_image_meta(**kw): - return {'id': kw.get('id', 'cccccccc-cccc-cccc-cccc-cccccccccccc')} + return {'id': kw.get('id', 'cccccccc-cccc-cccc-cccc-cccccccccccc'), + 'properties': { + 'deploy_config': kw.get('deploy_config', ''), + 'driver_actions': kw.get('driver_actions', ''), + }} def get_test_image_meta_object(**kw): @@ -134,6 +138,9 @@ class FakeNodeClient(object): def validate(self, node_uuid): pass + def vendor_passthru(self, node_uuid, method, args): + pass + class FakeClient(object): diff --git a/nova/virt/ironic/driver.py b/nova/virt/ironic/driver.py index 194221e..062f3d7 100644 --- a/nova/virt/ironic/driver.py +++ b/nova/virt/ironic/driver.py @@ -391,6 +391,17 @@ class IronicDriver(virt_driver.ComputeDriver): # Associate the node with an instance patch.append({'path': '/instance_uuid', 'op': 'add', 'value': instance.uuid}) + + deploy_config_options = self._get_deploy_config_options( + node, instance, image_meta) + patch.append( + {'path': '/instance_info/deploy_config_options', 'op': 'add', + 'value': deploy_config_options}) + + patch.append( + {'path': '/instance_info/driver_actions', 'op': 'add', + 'value': instance.metadata.get('driver_actions', '')}) + try: self.ironicclient.call('node.update', node.uuid, patch) except ironic.exc.BadRequest: @@ -400,6 +411,12 @@ class IronicDriver(virt_driver.ComputeDriver): LOG.error(msg) raise exception.InstanceDeployFailure(msg) + def _update_driver_fields_after_switch_boot(self, context, node, + instance, image_meta): + patch = [{'path': '/instance_info/image_source', 'op': 'add', + 'value': image_meta.id}] + self.ironicclient.call('node.update', node.uuid, patch) + def _cleanup_deploy(self, context, node, instance, network_info, flavor=None): if flavor is None: @@ -807,9 +824,19 @@ class IronicDriver(virt_driver.ComputeDriver): # trigger the node deploy try: - self.ironicclient.call("node.set_provision_state", node_uuid, - ironic_states.ACTIVE, - configdrive=configdrive_value) + # NOTE(lobur): set_provision_state to + # ACTIVE, REBUILD, and switch_boot_device are the only Ironic API + # calls where the user context needs to be passed to Ironic. This + # is needed to be able to fetch a tenant-owned resources for + # deployment, e.g. deploy_config stored in Swift. The user should + # have admin role, otherwise this context will be replaced by a + # standard Ironic context (admin tenant). It is also required to + # have a standalone instance of ironicclient to make sure + # no other calls use user context cached in the client. + ironicclient = client_wrapper.IronicClientWrapper() + ironicclient.call("node.set_provision_state", node_uuid, + ironic_states.ACTIVE, + configdrive=configdrive_value) except Exception as e: with excutils.save_and_reraise_exception(): msg = (_LE("Failed to request Ironic to provision instance " @@ -834,6 +861,17 @@ class IronicDriver(virt_driver.ComputeDriver): {'instance': instance.uuid, 'node': node_uuid}) self.destroy(context, instance, network_info) + else: + self._get_switch_boot_options(context, instance, node_uuid) + + def _get_switch_boot_options(self, context, instance, node_uuid): + # Reload node to see if multiboot flag appeared. + node = self.ironicclient.call("node.get", node_uuid) + if node.instance_info.get('multiboot'): + multiboot_meta = node.instance_info.get('multiboot_info', {}) + available_images = [img['image_name'] for img in + multiboot_meta.get('elements', [])] + instance.metadata['available_images'] = str(available_images) def _unprovision(self, ironicclient, instance, node): """This method is called from destroy() to unprovision @@ -1188,16 +1226,102 @@ class IronicDriver(virt_driver.ComputeDriver): instance.task_state = task_states.REBUILD_SPAWNING instance.save(expected_task_state=[task_states.REBUILDING]) - node_uuid = instance.node - node = self.ironicclient.call("node.get", node_uuid) + # NOTE(oberezovskyi): Required to get real node uuid assigned to nova + # instance. Workaround after + # Change-Id: I0233f964d8f294f0ffd9edcb16b1aaf93486177f + node = self.ironicclient.call("node.get_by_instance_uuid", + instance.uuid) + + # NOTE(lobur): set_provision_state to + # ACTIVE, REBUILD, and switch_boot_device are the only Ironic API + # calls where the user context needs to be passed to Ironic. This + # is needed to be able to fetch tenant-owned resources for + # deployment, e.g. deploy_config stored in Swift. The user should + # have admin role, otherwise this context will be replaced by a + # standard Ironic context (admin tenant). It is also required to + # have a standalone instance of ironicclient to make sure + # no other calls use user context cached in the client. + ironicclient = client_wrapper.IronicClientWrapper() + + # To get a multiboot node rebuilt through the standard flow we + # require a separate force_rebuild flag in meta. + forced_rebuild = instance.metadata.pop('force_rebuild', False) + + if node.instance_info.get('multiboot') and not forced_rebuild: + self._do_switch_boot_device( + context, ironicclient, node, instance, image_meta) + else: + self._do_rebuild( + context, ironicclient, node, instance, image_meta, + injected_files, + admin_password, bdms, detach_block_devices, + attach_block_devices, network_info=network_info, + recreate=recreate, + block_device_info=block_device_info, + preserve_ephemeral=preserve_ephemeral) - self._add_driver_fields(node, instance, image_meta, instance.flavor, - preserve_ephemeral) + self._get_switch_boot_options(context, instance, node.uuid) - # Trigger the node rebuild/redeploy. + def _do_switch_boot_device(self, context, ironicclient, node, instance, + image_meta): + old_image_ref = node.instance_info.get("image_source", "") try: - self.ironicclient.call("node.set_provision_state", - node_uuid, ironic_states.REBUILD) + sb_user, sb_key = self._get_switch_boot_user_key(instance.metadata) + args = dict(ssh_user=sb_user, + ssh_key=sb_key, + image=image_meta.id) + ironicclient.call("node.vendor_passthru", + node.uuid, "switch_boot", + args) + self.ironicclient.call("node.set_power_state", node.uuid, 'reboot') + self._update_driver_fields_after_switch_boot( + context, node, instance, image_meta) + except (exception.InvalidMetadata, # Bad Nova API call + exception.NovaException, # Retry failed + ironic.exc.InternalServerError, # Validations + ironic.exc.BadRequest) as e: # Maintenance or no such API + # Ironic Vendor API always return 200/400/500, so the only way + # to check the error is introspecting its message. + if "Already in desired boot device" in six.text_type(e): + msg = (_("Ironic node %(node)s already has desired " + "boot device set.") % {'node': node.uuid}) + LOG.warning(msg) + else: + # Rollback nova image ref + instance.image_ref = old_image_ref + # Cutting error msg to fit DB table. + instance.metadata['switch_boot_error'] = six.text_type(e)[:255] + instance.save() + msg = (_("Failed to switch Ironic %(node)s node boot " + "device: %(err)s") + % {'node': node.uuid, 'err': six.text_type(e)}) + LOG.error(msg) + + def _get_switch_boot_user_key(self, metadata): + sb_user = metadata.pop('sb_user', None) + sb_key = metadata.pop('sb_key', None) + if sb_user and sb_key: + return sb_user, sb_key + else: + raise exception.InvalidMetadata( + reason="To trigger switch boot device flow, both 'sb_user' " + "and 'sb_key' metadata params are required. To "s + "trigger a standard rebuild flow, use " + "force_rebuild=True metadata flag.") + + def _do_rebuild(self, context, ironicclient, node, instance, image_meta, + injected_files, + admin_password, bdms, detach_block_devices, + attach_block_devices, network_info=None, + recreate=False, block_device_info=None, + preserve_ephemeral=False): + + self._add_driver_fields(node, instance, image_meta, + instance.flavor, preserve_ephemeral) + try: + + ironicclient.call("node.set_provision_state", + node.uuid, ironic_states.REBUILD) except (exception.NovaException, # Retry failed ironic.exc.InternalServerError, # Validations ironic.exc.BadRequest) as e: # Maintenance @@ -1213,3 +1337,22 @@ class IronicDriver(virt_driver.ComputeDriver): instance) timer.start(interval=CONF.ironic.api_retry_interval).wait() LOG.info(_LI('Instance was successfully rebuilt'), instance=instance) + + def _get_deploy_config_options(self, node, instance, image_meta): + # Taking into account previous options, if any. This is to support + # rebuild flow where the user might or might not pass deploy_config + # reference. If no reference was passed, we'll take the option used for + # initial deployment. + res = node.instance_info.get('deploy_config_options', {}) + + curr_options = { + 'image': image_meta.properties.get('deploy_config', ''), + 'instance': instance.metadata.get('deploy_config', ''), + 'node': node.driver_info.get('deploy_config', ''), + } + # Filter out empty ones + curr_options = {key: value for key, value in + curr_options.items() if value} + # Override previous by current. + res.update(curr_options) + return res