From cb281e2a6c0c36209010bf31ce15016d362d3bba Mon Sep 17 00:00:00 2001
From: Monty Taylor <mordred@inaugust.com>
Date: Mon, 21 Nov 2016 18:37:40 -0500
Subject: [PATCH] Allow server to be snapshot to be name, id or dict

In documenting this method, it became clear we weren't telling people
what value server should have. Our tests made it look like it could be a
server name - but in fact it could actually only be a server dict or a
server id.

Make it explicitly all three, and fix the tests to not test their own
mocks. Or, if not actually fix them, at least make their examples not
blatantly wrong.

Change-Id: I64361a7a26cfa5137f9e862624fe379219f1cbb1
---
 shade/openstackcloud.py                 | 10 +++++++++-
 shade/tests/unit/test_image_snapshot.py | 13 +++++++++++--
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/shade/openstackcloud.py b/shade/openstackcloud.py
index 6dd8b3f15..0519dcf3d 100644
--- a/shade/openstackcloud.py
+++ b/shade/openstackcloud.py
@@ -2869,7 +2869,8 @@ class OpenStackCloud(_normalize.Normalizer):
         """Create a glance image by snapshotting an existing server.
 
         :param name: Name of the image to be created
-        :param server: Server dict representing the server to be snapshotted
+        :param server: Server name or id or dict representing the server
+                       to be snapshotted
         :param wait: If true, waits for image to be created.
         :param timeout: Seconds to wait for image creation. None is forever.
         :param metadata: Metadata to give newly-created image entity
@@ -2878,6 +2879,13 @@ class OpenStackCloud(_normalize.Normalizer):
 
         :raises: OpenStackCloudException if there are problems uploading
         """
+        if not isinstance(server, dict):
+            server_obj = self.get_server(server)
+            if not server_obj:
+                raise OpenStackCloudException(
+                    "Server {server} could not be found and therefor"
+                    " could not be snapshotted.".format(server=server))
+            server = server_obj
         image_id = str(self.manager.submit_task(_tasks.ImageSnapshotCreate(
             image_name=name, server=server, metadata=metadata)))
         self.list_images.invalidate(self)
diff --git a/shade/tests/unit/test_image_snapshot.py b/shade/tests/unit/test_image_snapshot.py
index 3c106a770..07c4ea5f8 100644
--- a/shade/tests/unit/test_image_snapshot.py
+++ b/shade/tests/unit/test_image_snapshot.py
@@ -39,7 +39,7 @@ class TestImageSnapshot(base.TestCase):
         mock_get.return_value = {'status': 'saving', 'id': self.image_id}
         self.assertRaises(exc.OpenStackCloudTimeout,
                           self.cloud.create_image_snapshot,
-                          'test-snapshot', 'fake-server',
+                          'test-snapshot', dict(id='fake-server'),
                           wait=True, timeout=0.01)
 
     @mock.patch.object(shade.OpenStackCloud, 'nova_client')
@@ -51,5 +51,14 @@ class TestImageSnapshot(base.TestCase):
         }
         mock_get.return_value = {'status': 'active', 'id': self.image_id}
         image = self.cloud.create_image_snapshot(
-            'test-snapshot', 'fake-server', wait=True, timeout=2)
+            'test-snapshot', dict(id='fake-server'), wait=True, timeout=2)
         self.assertEqual(image['id'], self.image_id)
+
+    @mock.patch.object(shade.OpenStackCloud, 'get_server')
+    def test_create_image_snapshot_bad_name_exception(
+            self, mock_get_server):
+        mock_get_server.return_value = None
+        self.assertRaises(
+            exc.OpenStackCloudException,
+            self.cloud.create_image_snapshot,
+            'test-snapshot', 'missing-server')