From b98d9e1d4f31417d277b996899aebfe758a9f40a Mon Sep 17 00:00:00 2001
From: Matt Riedemann <mriedem.os@gmail.com>
Date: Mon, 3 Apr 2017 18:04:31 -0400
Subject: [PATCH] Document and provide useful error message for volume-backed
 backup

The createBackup API does not support backing up volume-backed
instances. The error message the user gets is not useful about
why the request was invalid, and the API reference docs do not
mention the limitation. This change addresses both of those issues.

Change-Id: I04fd8ab4f8818d9d0ccccb6f6fcb34965b15b8f3
Partial-Bug: #1679314
---
 api-ref/source/servers-admin-action.inc       |  2 ++
 nova/compute/api.py                           |  3 ++-
 .../openstack/compute/test_create_backup.py   | 26 ++++++++++++-------
 3 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/api-ref/source/servers-admin-action.inc b/api-ref/source/servers-admin-action.inc
index 1110a49104f7..d69ba6b50e49 100644
--- a/api-ref/source/servers-admin-action.inc
+++ b/api-ref/source/servers-admin-action.inc
@@ -20,6 +20,8 @@ Create Server Back Up (createBackup Action)
 
 Creates a back up of a server.
 
+.. note:: This API is not supported for volume-backed instances.
+
 Specify the ``createBackup`` action in the request body.
 
 Policy defaults enable only users with the administrative role or the
diff --git a/nova/compute/api.py b/nova/compute/api.py
index 144fa792533a..23022734a78d 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -2788,7 +2788,8 @@ class API(base.Base):
         if compute_utils.is_volume_backed_instance(context, instance):
             LOG.info(_LI("It's not supported to backup volume backed "
                          "instance."), instance=instance)
-            raise exception.InvalidRequest()
+            raise exception.InvalidRequest(
+                _('Backup is not supported for volume-backed instances.'))
         else:
             image_meta = self._create_image(context, instance,
                                             name, 'backup',
diff --git a/nova/tests/unit/api/openstack/compute/test_create_backup.py b/nova/tests/unit/api/openstack/compute/test_create_backup.py
index b40bdc64dd92..0ebb3cdd576a 100644
--- a/nova/tests/unit/api/openstack/compute/test_create_backup.py
+++ b/nova/tests/unit/api/openstack/compute/test_create_backup.py
@@ -14,12 +14,15 @@
 #    under the License.
 
 import mock
+from oslo_utils import timeutils
+import six
 import webob
 
 from nova.api.openstack import common
 from nova.api.openstack.compute import create_backup \
         as create_backup_v21
 from nova.compute import api
+from nova.compute import utils as compute_utils
 from nova import exception
 from nova import test
 from nova.tests.unit.api.openstack.compute import admin_only_action_common
@@ -334,8 +337,9 @@ class CreateBackupTestsV21(admin_only_action_common.CommonMixin,
                           self.req, fakes.FAKE_UUID, body=body)
 
     @mock.patch.object(common, 'check_img_metadata_properties_quota')
-    @mock.patch.object(api.API, 'backup')
-    def test_backup_volume_backed_instance(self, mock_backup,
+    @mock.patch.object(compute_utils, 'is_volume_backed_instance',
+                       return_value=True)
+    def test_backup_volume_backed_instance(self, mock_is_volume_backed,
                                            mock_check_image):
         body = {
             'createBackup': {
@@ -345,18 +349,20 @@ class CreateBackupTestsV21(admin_only_action_common.CommonMixin,
             },
         }
 
-        instance = fake_instance.fake_instance_obj(self.context)
+        updates = {'vm_state': 'active',
+                   'task_state': None,
+                   'launched_at': timeutils.utcnow()}
+        instance = fake_instance.fake_instance_obj(self.context, **updates)
         instance.image_ref = None
         self.mock_get.return_value = instance
-        mock_backup.side_effect = exception.InvalidRequest()
 
-        self.assertRaises(webob.exc.HTTPBadRequest,
-                          self.controller._create_backup,
-                          self.req, instance['uuid'], body=body)
+        ex = self.assertRaises(webob.exc.HTTPBadRequest,
+                               self.controller._create_backup,
+                               self.req, instance['uuid'], body=body)
         mock_check_image.assert_called_once_with(self.context, {})
-        mock_backup.assert_called_once_with(self.context, instance, 'BackupMe',
-                                              'daily', 3,
-                                              extra_properties={})
+        mock_is_volume_backed.assert_called_once_with(self.context, instance)
+        self.assertIn('Backup is not supported for volume-backed instances',
+                      six.text_type(ex))
 
 
 class CreateBackupPolicyEnforcementv21(test.NoDBTestCase):