workload: fix custom_image cleanup

A VM ran by `custom_image` context is left running in case the script
failed. Fix this by moving cleanup to a try/finally block letting
the exception propagate upwards so no scenario is run when the context
have failed.

Change-Id: I90d783eeb50b1f0bfa77d59993ff116451d11878
Closes-Bug: #1502832

rally/plugins/openstack/context/vm/custom_image.py

@@ -163,16 +163,17 @@ class BaseCustomImageGenerator(context.Context):
             security_groups=[user["secgroup"]["name"]],
             **kwargs)
 
-        LOG.debug("Installing benchmark on %r %s", server, fip["ip"])
-        self.customize_image(server, fip, user)
+        try:
+            LOG.debug("Installing benchmark on %r %s", server, fip["ip"])
+            self.customize_image(server, fip, user)
 
-        LOG.debug("Stopping server %r", server)
-        vm_scenario._stop_server(server)
+            LOG.debug("Stopping server %r", server)
+            vm_scenario._stop_server(server)
 
-        LOG.debug("Creating snapshot for %r", server)
-        custom_image = vm_scenario._create_image(server).to_dict()
-
-        vm_scenario._delete_server_with_fip(server, fip)
+            LOG.debug("Creating snapshot for %r", server)
+            custom_image = vm_scenario._create_image(server).to_dict()
+        finally:
+            vm_scenario._delete_server_with_fip(server, fip)
 
         return custom_image
 

tests/unit/plugins/openstack/context/vm/test_custom_image.py

@@ -123,6 +123,48 @@ class BaseCustomImageContextVMTestCase(test.TestCase):
 
         self.assertEqual({"id": "image"}, custom_image)
 
+    @mock.patch("%s.vmtasks.VMTasks" % BASE)
+    @mock.patch("%s.osclients.Clients" % BASE)
+    @mock.patch("%s.types.ImageResourceType.transform" % BASE,
+                return_value="image")
+    @mock.patch("%s.types.FlavorResourceType.transform" % BASE,
+                return_value="flavor")
+    def test_create_one_image_cleanup(
+            self, mock_flavor_resource_type_transform,
+            mock_image_resource_type_transform, mock_clients, mock_vm_tasks):
+        ip = {"ip": "foo_ip", "id": "foo_id", "is_floating": True}
+        fake_server = mock.Mock()
+
+        fake_image = mock.MagicMock(
+            to_dict=mock.MagicMock(return_value={"id": "image"}))
+
+        mock_vm_scenario = mock_vm_tasks.return_value = mock.MagicMock(
+            _create_image=mock.MagicMock(return_value=fake_image),
+            _boot_server_with_fip=mock.MagicMock(
+                return_value=(fake_server, ip)),
+            _generate_random_name=mock.MagicMock(return_value="foo_name"),
+        )
+
+        generator_ctx = TestImageGenerator(self.context)
+        generator_ctx._customize_image = mock.MagicMock(
+            side_effect=ValueError())
+
+        user = {
+            "endpoint": "endpoint",
+            "keypair": {"name": "keypair_name"},
+            "secgroup": {"name": "secgroup_name"}
+        }
+
+        self.assertRaises(
+            ValueError,
+            generator_ctx.create_one_image, user, foo_arg="foo_value")
+
+        generator_ctx._customize_image.assert_called_once_with(
+            fake_server, ip, user)
+
+        mock_vm_scenario._delete_server_with_fip.assert_called_once_with(
+            fake_server, ip)
+
     @mock.patch("%s.osclients.Clients" % BASE)
     def test_make_image_public(self, mock_clients):
         fc = mock.MagicMock()