summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorZuul <zuul@review.openstack.org>2018-11-09 18:37:25 +0000
committerGerrit Code Review <review@openstack.org>2018-11-09 18:37:25 +0000
commita85218e765b5ca048967b1264627586b04b480a7 (patch)
treef0f1a4d13b520ceecc44cb5da57133bdfe905e99
parent881cafbfcbf2a12223f8cc18f98d793ef45e65c9 (diff)
parent3a0f26c822912dbaad7c8a1d6a3c599025afab37 (diff)
Merge "conductor: Recreate volume attachments during a reschedule" into stable/queensstable/queens
-rw-r--r--nova/conductor/manager.py25
-rw-r--r--nova/tests/functional/regressions/test_bug_1784353.py14
-rw-r--r--nova/tests/unit/conductor/test_conductor.py84
3 files changed, 117 insertions, 6 deletions
diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py
index b6ee6aa..ebff9a2 100644
--- a/nova/conductor/manager.py
+++ b/nova/conductor/manager.py
@@ -52,6 +52,7 @@ from nova.scheduler import client as scheduler_client
52from nova.scheduler import utils as scheduler_utils 52from nova.scheduler import utils as scheduler_utils
53from nova import servicegroup 53from nova import servicegroup
54from nova import utils 54from nova import utils
55from nova.volume import cinder
55 56
56LOG = logging.getLogger(__name__) 57LOG = logging.getLogger(__name__)
57CONF = cfg.CONF 58CONF = cfg.CONF
@@ -225,6 +226,7 @@ class ComputeTaskManager(base.Base):
225 def __init__(self): 226 def __init__(self):
226 super(ComputeTaskManager, self).__init__() 227 super(ComputeTaskManager, self).__init__()
227 self.compute_rpcapi = compute_rpcapi.ComputeAPI() 228 self.compute_rpcapi = compute_rpcapi.ComputeAPI()
229 self.volume_api = cinder.API()
228 self.image_api = image.API() 230 self.image_api = image.API()
229 self.network_api = network.API() 231 self.network_api = network.API()
230 self.servicegroup_api = servicegroup.API() 232 self.servicegroup_api = servicegroup.API()
@@ -514,6 +516,24 @@ class ComputeTaskManager(base.Base):
514 inst_mapping.save() 516 inst_mapping.save()
515 return inst_mapping 517 return inst_mapping
516 518
519 def _validate_existing_attachment_ids(self, context, instance, bdms):
520 """Ensure any attachment ids referenced by the bdms exist.
521
522 New attachments will only be created if the attachment ids referenced
523 by the bdms no longer exist. This can happen when an instance is
524 rescheduled after a failure to spawn as cleanup code on the previous
525 host will delete attachments before rescheduling.
526 """
527 for bdm in bdms:
528 if bdm.is_volume and bdm.attachment_id:
529 try:
530 self.volume_api.attachment_get(context, bdm.attachment_id)
531 except exception.VolumeAttachmentNotFound:
532 attachment = self.volume_api.attachment_create(
533 context, bdm.volume_id, instance.uuid)
534 bdm.attachment_id = attachment['id']
535 bdm.save()
536
517 # NOTE(danms): This is never cell-targeted because it is only used for 537 # NOTE(danms): This is never cell-targeted because it is only used for
518 # cellsv1 (which does not target cells directly) and n-cpu reschedules 538 # cellsv1 (which does not target cells directly) and n-cpu reschedules
519 # (which go to the cell conductor and thus are always cell-specific). 539 # (which go to the cell conductor and thus are always cell-specific).
@@ -693,6 +713,11 @@ class ComputeTaskManager(base.Base):
693 if inst_mapping: 713 if inst_mapping:
694 inst_mapping.destroy() 714 inst_mapping.destroy()
695 return 715 return
716 else:
717 # NOTE(lyarwood): If this is a reschedule then recreate any
718 # attachments that were previously removed when cleaning up
719 # after failures to spawn etc.
720 self._validate_existing_attachment_ids(context, instance, bdms)
696 721
697 alts = [(alt.service_host, alt.nodename) for alt in host_list] 722 alts = [(alt.service_host, alt.nodename) for alt in host_list]
698 LOG.debug("Selected host: %s; Selected node: %s; Alternates: %s", 723 LOG.debug("Selected host: %s; Selected node: %s; Alternates: %s",
diff --git a/nova/tests/functional/regressions/test_bug_1784353.py b/nova/tests/functional/regressions/test_bug_1784353.py
index 16c4ce4..837c31e 100644
--- a/nova/tests/functional/regressions/test_bug_1784353.py
+++ b/nova/tests/functional/regressions/test_bug_1784353.py
@@ -80,9 +80,11 @@ class TestRescheduleWithVolumesAttached(
80 server_response = self.api.post_server({'server': server_request}) 80 server_response = self.api.post_server({'server': server_request})
81 server_id = server_response['id'] 81 server_id = server_response['id']
82 82
83 # FIXME(lyarwood): Assert that the instance ends up in an ERROR state 83 self._wait_for_state_change(self.api, server_response, 'ACTIVE')
84 # with no attachments present in the fixture. The instance should be 84 attached_volume_ids = self.cinder.volume_ids_for_instance(server_id)
85 # ACTIVE with a single attachment associated to the volume. 85 self.assertIn(volume_id, attached_volume_ids)
86 self._wait_for_state_change(self.api, server_response, 'ERROR') 86 self.assertEqual(1, len(self.cinder.volume_to_attachment))
87 self.assertNotIn(volume_id, 87 # There should only be one attachment record for the volume and
88 self.cinder.volume_ids_for_instance(server_id)) 88 # instance because the original would have been deleted before
89 # rescheduling off the first host.
90 self.assertEqual(1, len(self.cinder.volume_to_attachment[volume_id]))
diff --git a/nova/tests/unit/conductor/test_conductor.py b/nova/tests/unit/conductor/test_conductor.py
index a720223..4a4d11f 100644
--- a/nova/tests/unit/conductor/test_conductor.py
+++ b/nova/tests/unit/conductor/test_conductor.py
@@ -44,6 +44,7 @@ from nova import exception as exc
44from nova.image import api as image_api 44from nova.image import api as image_api
45from nova import objects 45from nova import objects
46from nova.objects import base as obj_base 46from nova.objects import base as obj_base
47from nova.objects import block_device as block_device_obj
47from nova.objects import fields 48from nova.objects import fields
48from nova import rpc 49from nova import rpc
49from nova.scheduler import client as scheduler_client 50from nova.scheduler import client as scheduler_client
@@ -61,6 +62,7 @@ from nova.tests.unit import fake_server_actions
61from nova.tests.unit import utils as test_utils 62from nova.tests.unit import utils as test_utils
62from nova.tests import uuidsentinel as uuids 63from nova.tests import uuidsentinel as uuids
63from nova import utils 64from nova import utils
65from nova.volume import cinder
64 66
65CONF = conf.CONF 67CONF = conf.CONF
66 68
@@ -961,6 +963,88 @@ class _BaseTaskTestCase(object):
961 963
962 do_test() 964 do_test()
963 965
966 @mock.patch.object(cinder.API, 'attachment_get')
967 @mock.patch.object(cinder.API, 'attachment_create')
968 @mock.patch.object(block_device_obj.BlockDeviceMapping, 'save')
969 def test_validate_existing_attachment_ids_with_missing_attachments(self,
970 mock_bdm_save, mock_attachment_create, mock_attachment_get):
971 instance = self._create_fake_instance_obj()
972 bdms = [
973 block_device.BlockDeviceDict({
974 'boot_index': 0,
975 'guest_format': None,
976 'connection_info': None,
977 'device_type': u'disk',
978 'source_type': 'image',
979 'destination_type': 'volume',
980 'volume_size': 1,
981 'image_id': 1,
982 'device_name': '/dev/vdb',
983 'attachment_id': uuids.attachment,
984 'volume_id': uuids.volume
985 })]
986 bdms = block_device_obj.block_device_make_list_from_dicts(
987 self.context, bdms)
988 mock_attachment_get.side_effect = exc.VolumeAttachmentNotFound(
989 attachment_id=uuids.attachment)
990 mock_attachment_create.return_value = {'id': uuids.new_attachment}
991
992 self.assertEqual(uuids.attachment, bdms[0].attachment_id)
993 self.conductor_manager._validate_existing_attachment_ids(self.context,
994 instance,
995 bdms)
996 mock_attachment_get.assert_called_once_with(self.context,
997 uuids.attachment)
998 mock_attachment_create.assert_called_once_with(self.context,
999 uuids.volume,
1000 instance.uuid)
1001 mock_bdm_save.assert_called_once()
1002 self.assertEqual(uuids.new_attachment, bdms[0].attachment_id)
1003
1004 @mock.patch.object(cinder.API, 'attachment_get')
1005 @mock.patch.object(cinder.API, 'attachment_create')
1006 @mock.patch.object(block_device_obj.BlockDeviceMapping, 'save')
1007 def test_validate_existing_attachment_ids_with_attachments_present(self,
1008 mock_bdm_save, mock_attachment_create, mock_attachment_get):
1009 instance = self._create_fake_instance_obj()
1010 bdms = [
1011 block_device.BlockDeviceDict({
1012 'boot_index': 0,
1013 'guest_format': None,
1014 'connection_info': None,
1015 'device_type': u'disk',
1016 'source_type': 'image',
1017 'destination_type': 'volume',
1018 'volume_size': 1,
1019 'image_id': 1,
1020 'device_name': '/dev/vdb',
1021 'attachment_id': uuids.attachment,
1022 'volume_id': uuids.volume
1023 })]
1024 bdms = block_device_obj.block_device_make_list_from_dicts(
1025 self.context, bdms)
1026 mock_attachment_get.return_value = {
1027 "attachment": {
1028 "status": "attaching",
1029 "detached_at": "2015-09-16T09:28:52.000000",
1030 "connection_info": {},
1031 "attached_at": "2015-09-16T09:28:52.000000",
1032 "attach_mode": "ro",
1033 "instance": instance.uuid,
1034 "volume_id": uuids.volume,
1035 "id": uuids.attachment
1036 }}
1037
1038 self.assertEqual(uuids.attachment, bdms[0].attachment_id)
1039 self.conductor_manager._validate_existing_attachment_ids(self.context,
1040 instance,
1041 bdms)
1042 mock_attachment_get.assert_called_once_with(self.context,
1043 uuids.attachment)
1044 mock_attachment_create.assert_not_called()
1045 mock_bdm_save.assert_not_called()
1046 self.assertEqual(uuids.attachment, bdms[0].attachment_id)
1047
964 def test_unshelve_instance_on_host(self): 1048 def test_unshelve_instance_on_host(self):
965 instance = self._create_fake_instance_obj() 1049 instance = self._create_fake_instance_obj()
966 instance.vm_state = vm_states.SHELVED 1050 instance.vm_state = vm_states.SHELVED