From 8b527f150c0eb70c64b343992563abc544e40e41 Mon Sep 17 00:00:00 2001 From: Hongbin Lu Date: Sat, 3 Feb 2018 23:31:56 +0000 Subject: [PATCH] Reraise exception on creating container It is desirable to raise exceptions to upper stack so that it can be captured and handled. For example, we are introducing action APIs that will record exception details if there is an exception that has been raised to the upper stack. Otherwise, the exception information will lose and the action will be wrongly recorded as success. Change-Id: I2737102510e04383c878c47441f46a86fed9d744 --- zun/compute/manager.py | 62 ++++++---------- .../unit/compute/test_compute_manager.py | 70 +++++++++++-------- 2 files changed, 64 insertions(+), 68 deletions(-) diff --git a/zun/compute/manager.py b/zun/compute/manager.py index cdfbaf53d..b8d20b188 100644 --- a/zun/compute/manager.py +++ b/zun/compute/manager.py @@ -198,12 +198,11 @@ class Manager(periodic_task.PeriodicTasks): def do_container_create(): self._wait_for_volumes_available(context, requested_volumes, container) - if not self._attach_volumes(context, container, requested_volumes): - return + self._attach_volumes(context, container, requested_volumes) created_container = self._do_container_create( context, container, requested_networks, requested_volumes, pci_requests, limits) - if run and created_container: + if run: self._do_container_start(context, created_container) utils.spawn_n(do_container_create) @@ -224,8 +223,8 @@ class Manager(periodic_task.PeriodicTasks): container.save(context) def _do_container_create_base(self, context, container, requested_networks, - requested_volumes, sandbox=None, limits=None, - reraise=False): + requested_volumes, sandbox=None, + limits=None): self._update_task_state(context, container, consts.IMAGE_PULLING) repo, tag = utils.parse_image_name(container.image) image_pull_policy = utils.get_image_pull_policy( @@ -238,25 +237,22 @@ class Manager(periodic_task.PeriodicTasks): if not image_loaded: self.driver.load_image(image['path']) except exception.ImageNotFound as e: - with excutils.save_and_reraise_exception(reraise=reraise): + with excutils.save_and_reraise_exception(): LOG.error(six.text_type(e)) self._do_sandbox_cleanup(context, container) self._fail_container(context, container, six.text_type(e)) - return except exception.DockerError as e: - with excutils.save_and_reraise_exception(reraise=reraise): + with excutils.save_and_reraise_exception(): LOG.error("Error occurred while calling Docker image API: %s", six.text_type(e)) self._do_sandbox_cleanup(context, container) self._fail_container(context, container, six.text_type(e)) - return except Exception as e: - with excutils.save_and_reraise_exception(reraise=reraise): + with excutils.save_and_reraise_exception(): LOG.exception("Unexpected exception: %s", six.text_type(e)) self._do_sandbox_cleanup(context, container) self._fail_container(context, container, six.text_type(e)) - return container.task_state = consts.CONTAINER_CREATING container.image_driver = image.get('driver') @@ -270,26 +266,24 @@ class Manager(periodic_task.PeriodicTasks): self._update_task_state(context, container, None) return container except exception.DockerError as e: - with excutils.save_and_reraise_exception(reraise=reraise): + with excutils.save_and_reraise_exception(): LOG.error("Error occurred while calling Docker create API: %s", six.text_type(e)) self._do_sandbox_cleanup(context, container) self._fail_container(context, container, six.text_type(e), unset_host=True) - return except Exception as e: - with excutils.save_and_reraise_exception(reraise=reraise): + with excutils.save_and_reraise_exception(): LOG.exception("Unexpected exception: %s", six.text_type(e)) self._do_sandbox_cleanup(context, container) self._fail_container(context, container, six.text_type(e), unset_host=True) - return @wrap_container_event(prefix='compute') def _do_container_create(self, context, container, requested_networks, requested_volumes, pci_requests=None, - limits=None, reraise=False): + limits=None): LOG.debug('Creating container: %s', container.uuid) try: @@ -300,31 +294,26 @@ class Manager(periodic_task.PeriodicTasks): sandbox = None if self.use_sandbox: sandbox = self._create_sandbox(context, container, - requested_networks, - reraise) - if sandbox is None: - return + requested_networks) created_container = self._do_container_create_base( context, container, requested_networks, requested_volumes, - sandbox, limits, reraise) + sandbox, limits) return created_container except exception.ResourcesUnavailable as e: - with excutils.save_and_reraise_exception(reraise=reraise): + with excutils.save_and_reraise_exception(): LOG.exception("Container resource claim failed: %s", six.text_type(e)) self._fail_container(context, container, six.text_type(e), unset_host=True) - return def _attach_volumes(self, context, container, volumes): try: for volume in volumes: volume.container_uuid = container.uuid self._attach_volume(context, volume) - return True except Exception as e: - with excutils.save_and_reraise_exception(reraise=False): + with excutils.save_and_reraise_exception(): self._fail_container(context, container, six.text_type(e), unset_host=True) @@ -377,8 +366,7 @@ class Manager(periodic_task.PeriodicTasks): {'use_sandbox': CONF.use_sandbox, 'driver': self.driver}) - def _create_sandbox(self, context, container, requested_networks, - reraise=False): + def _create_sandbox(self, context, container, requested_networks): self._update_task_state(context, container, consts.SANDBOX_CREATING) sandbox_image = CONF.sandbox_image sandbox_image_driver = CONF.sandbox_image_driver @@ -396,7 +384,7 @@ class Manager(periodic_task.PeriodicTasks): requested_volumes=[]) return sandbox_id except Exception as e: - with excutils.save_and_reraise_exception(reraise=reraise): + with excutils.save_and_reraise_exception(): LOG.exception("Unexpected exception: %s", six.text_type(e)) self._fail_container(context, container, six.text_type(e)) @@ -410,12 +398,12 @@ class Manager(periodic_task.PeriodicTasks): self._update_task_state(context, container, None) return container except exception.DockerError as e: - with excutils.save_and_reraise_exception(reraise=False): + with excutils.save_and_reraise_exception(): LOG.error("Error occurred while calling Docker start API: %s", six.text_type(e)) self._fail_container(context, container, six.text_type(e)) except Exception as e: - with excutils.save_and_reraise_exception(reraise=False): + with excutils.save_and_reraise_exception(): LOG.exception("Unexpected exception: %s", six.text_type(e)) self._fail_container(context, container, six.text_type(e)) @@ -960,7 +948,7 @@ class Manager(periodic_task.PeriodicTasks): def _do_capsule_create(self, context, capsule, requested_networks=None, requested_volumes=None, - limits=None, reraise=False): + limits=None): """Create capsule in the compute node :param context: security context @@ -969,7 +957,6 @@ class Manager(periodic_task.PeriodicTasks): connect :param requested_volumes: the volume that capsule need :param limits: no use field now. - :param reraise: flag of reraise the error, default is Falses """ capsule.containers[0].image = CONF.sandbox_image capsule.containers[0].image_driver = CONF.sandbox_image_driver @@ -978,8 +965,7 @@ class Manager(periodic_task.PeriodicTasks): capsule.containers[0].save(context) sandbox = self._create_sandbox(context, capsule.containers[0], - requested_networks, - reraise) + requested_networks) capsule.containers[0].task_state = None capsule.containers[0].status = consts.RUNNING sandbox_id = capsule.containers[0].get_sandbox_id() @@ -998,9 +984,8 @@ class Manager(periodic_task.PeriodicTasks): if volume.get(container_name, None): container_requested_volumes.append( volume.get(container_name)) - if not self._attach_volumes(context, capsule.containers[k], - container_requested_volumes): - return + self._attach_volumes(context, capsule.containers[k], + container_requested_volumes) # Add volume assignment created_container = \ self._do_container_create_base(context, @@ -1009,8 +994,7 @@ class Manager(periodic_task.PeriodicTasks): container_requested_volumes, sandbox=sandbox, limits=limits) - if created_container: - self._do_container_start(context, created_container) + self._do_container_start(context, created_container) # Save the volumes_info to capsule database for volumeapp in container_requested_volumes: diff --git a/zun/tests/unit/compute/test_compute_manager.py b/zun/tests/unit/compute/test_compute_manager.py index 7f8753999..2f4c0baa7 100644 --- a/zun/tests/unit/compute/test_compute_manager.py +++ b/zun/tests/unit/compute/test_compute_manager.py @@ -211,8 +211,9 @@ class TestManager(base.TestCase): mock_pull.side_effect = exception.DockerError("Pull Failed") networks = [] volumes = [] - self.compute_manager._do_container_create(self.context, container, - networks, volumes) + self.assertRaises(exception.DockerError, + self.compute_manager._do_container_create, + self.context, container, networks, volumes) mock_fail.assert_called_once_with(self.context, container, "Pull Failed") mock_event_start.assert_called_once() @@ -220,9 +221,8 @@ class TestManager(base.TestCase): self.assertEqual( (self.context, container.uuid, 'compute__do_container_create'), mock_event_finish.call_args[0]) - # TODO(hongbin): uncomment this after - # self.assertIsNotNone(mock_event_finish.call_args[1]['exc_val']) - # self.assertIsNotNone(mock_event_finish.call_args[1]['exc_tb']) + self.assertIsNotNone(mock_event_finish.call_args[1]['exc_val']) + self.assertIsNotNone(mock_event_finish.call_args[1]['exc_tb']) @mock.patch.object(ContainerActionEvent, 'event_start') @mock.patch.object(ContainerActionEvent, 'event_finish') @@ -236,8 +236,9 @@ class TestManager(base.TestCase): mock_pull.side_effect = exception.ImageNotFound("Image Not Found") networks = [] volumes = [] - self.compute_manager._do_container_create(self.context, container, - networks, volumes) + self.assertRaises(exception.ImageNotFound, + self.compute_manager._do_container_create, + self.context, container, networks, volumes) mock_fail.assert_called_once_with(self.context, container, "Image Not Found") mock_event_start.assert_called_once() @@ -245,9 +246,8 @@ class TestManager(base.TestCase): self.assertEqual( (self.context, container.uuid, 'compute__do_container_create'), mock_event_finish.call_args[0]) - # TODO(hongbin): uncomment this after - # self.assertIsNotNone(mock_event_finish.call_args[1]['exc_val']) - # self.assertIsNotNone(mock_event_finish.call_args[1]['exc_tb']) + self.assertIsNotNone(mock_event_finish.call_args[1]['exc_val']) + self.assertIsNotNone(mock_event_finish.call_args[1]['exc_tb']) @mock.patch.object(ContainerActionEvent, 'event_start') @mock.patch.object(ContainerActionEvent, 'event_finish') @@ -262,8 +262,10 @@ class TestManager(base.TestCase): message="Image Not Found") networks = [] volumes = [] - self.compute_manager._do_container_create(self.context, container, - networks, volumes) + self.assertRaises( + exception.ZunException, + self.compute_manager._do_container_create, + self.context, container, networks, volumes) mock_fail.assert_called_once_with(self.context, container, "Image Not Found") mock_event_start.assert_called_once() @@ -271,9 +273,8 @@ class TestManager(base.TestCase): self.assertEqual( (self.context, container.uuid, 'compute__do_container_create'), mock_event_finish.call_args[0]) - # TODO(hongbin): uncomment this after - # self.assertIsNotNone(mock_event_finish.call_args[1]['exc_val']) - # self.assertIsNotNone(mock_event_finish.call_args[1]['exc_tb']) + self.assertIsNotNone(mock_event_finish.call_args[1]['exc_val']) + self.assertIsNotNone(mock_event_finish.call_args[1]['exc_tb']) @mock.patch.object(ContainerActionEvent, 'event_start') @mock.patch.object(ContainerActionEvent, 'event_finish') @@ -292,8 +293,9 @@ class TestManager(base.TestCase): self.compute_manager._resource_tracker = FakeResourceTracker() networks = [] volumes = [] - self.compute_manager._do_container_create(self.context, container, - networks, volumes) + self.assertRaises(exception.DockerError, + self.compute_manager._do_container_create, + self.context, container, networks, volumes) mock_fail.assert_called_once_with( self.context, container, "Creation Failed", unset_host=True) mock_event_start.assert_called_once() @@ -301,9 +303,8 @@ class TestManager(base.TestCase): self.assertEqual( (self.context, container.uuid, 'compute__do_container_create'), mock_event_finish.call_args[0]) - # TODO(hongbin): uncomment this after - # self.assertIsNotNone(mock_event_finish.call_args[1]['exc_val']) - # self.assertIsNotNone(mock_event_finish.call_args[1]['exc_tb']) + self.assertIsNotNone(mock_event_finish.call_args[1]['exc_val']) + self.assertIsNotNone(mock_event_finish.call_args[1]['exc_tb']) @mock.patch.object(ContainerActionEvent, 'event_start') @mock.patch.object(ContainerActionEvent, 'event_finish') @@ -378,7 +379,9 @@ class TestManager(base.TestCase): self.compute_manager._resource_tracker = FakeResourceTracker() networks = [] volumes = [vol, vol2] - self.compute_manager.container_create( + self.assertRaises( + base.TestingException, + self.compute_manager.container_create, self.context, requested_networks=networks, requested_volumes=volumes, @@ -418,7 +421,9 @@ class TestManager(base.TestCase): mock_spawn_n.side_effect = lambda f, *x, **y: f(*x, **y) networks = [] volumes = [FakeVolumeMapping()] - self.compute_manager.container_create( + self.assertRaises( + exception.ImageNotFound, + self.compute_manager.container_create, self.context, requested_networks=networks, requested_volumes=volumes, @@ -458,7 +463,9 @@ class TestManager(base.TestCase): mock_spawn_n.side_effect = lambda f, *x, **y: f(*x, **y) networks = [] volumes = [FakeVolumeMapping()] - self.compute_manager.container_create( + self.assertRaises( + exception.ZunException, + self.compute_manager.container_create, self.context, requested_networks=networks, requested_volumes=volumes, @@ -498,7 +505,9 @@ class TestManager(base.TestCase): mock_spawn_n.side_effect = lambda f, *x, **y: f(*x, **y) networks = [] volumes = [FakeVolumeMapping()] - self.compute_manager.container_create( + self.assertRaises( + exception.DockerError, + self.compute_manager.container_create, self.context, requested_networks=networks, requested_volumes=volumes, @@ -540,7 +549,9 @@ class TestManager(base.TestCase): self.compute_manager._resource_tracker = FakeResourceTracker() networks = [] volumes = [FakeVolumeMapping()] - self.compute_manager.container_create( + self.assertRaises( + exception.DockerError, + self.compute_manager.container_create, self.context, requested_networks=networks, requested_volumes=volumes, @@ -792,7 +803,9 @@ class TestManager(base.TestCase): container = Container(self.context, **utils.get_test_container()) mock_start.side_effect = exception.DockerError( message="Docker Error occurred") - self.compute_manager._do_container_start(self.context, container) + self.assertRaises(exception.DockerError, + self.compute_manager._do_container_start, + self.context, container) mock_save.assert_called_with(self.context) mock_fail.assert_called_with(self.context, container, 'Docker Error occurred') @@ -801,9 +814,8 @@ class TestManager(base.TestCase): self.assertEqual( (self.context, container.uuid, 'compute__do_container_start'), mock_event_finish.call_args[0]) - # TODO(hongbin): uncomment this after - # self.assertIsNotNone(mock_event_finish.call_args[1]['exc_val']) - # self.assertIsNotNone(mock_event_finish.call_args[1]['exc_tb']) + self.assertIsNotNone(mock_event_finish.call_args[1]['exc_val']) + self.assertIsNotNone(mock_event_finish.call_args[1]['exc_tb']) @mock.patch.object(ContainerActionEvent, 'event_start') @mock.patch.object(ContainerActionEvent, 'event_finish')