Move server image create command to its own resource file.

Change-Id: If37e82072bd7a32b81bfb1a8bb048f018dd5b04f

7 files changed, 346 insertions, 224 deletions

diff --git a/doc/source/command-objects/server-image.rst b/doc/source/command-objects/server-image.rst
index 8b489342..eb44e47e 100644
--- a/doc/source/command-objects/server-image.rst
+++ b/doc/source/command-objects/server-image.rst
@@ -10,7 +10,7 @@ Compute v2
 server image create
 -------------------
 
-Create a new disk image from a running server
+Create a new server disk image from an existing server
 
 .. program:: server image create
 .. code:: bash
@@ -22,12 +22,12 @@ Create a new disk image from a running server
 
 .. option:: --name <image-name>
 
-    Name of new image (default is server name)
+    Name of new disk image (default: server name)
 
 .. option:: --wait
 
-    Wait for image create to complete
+    Wait for operation to complete
 
 .. describe:: <server>
 
-    Server (name or ID)
+    Server to create image (name or ID)
diff --git a/functional/tests/common/test_help.py b/functional/tests/common/test_help.py
index fcce5f99..7601c41b 100644
--- a/functional/tests/common/test_help.py
+++ b/functional/tests/common/test_help.py
@@ -19,11 +19,12 @@ class HelpTests(test.TestCase):
     SERVER_COMMANDS = [
         ('server add security group', 'Add security group to server'),
         ('server add volume', 'Add volume to server'),
+        ('server backup create', 'Create a server backup image'),
         ('server create', 'Create a new server'),
         ('server delete', 'Delete server(s)'),
         ('server dump create', 'Create a dump file in server(s)'),
         ('server image create',
-         'Create a new disk image from a running server'),
+         'Create a new server disk image from an existing server'),
         ('server list', 'List servers'),
         ('server lock',
          'Lock server(s). '
diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py
index 27abbe63..16384074 100644
--- a/openstackclient/compute/v2/server.py
+++ b/openstackclient/compute/v2/server.py
@@ -164,23 +164,6 @@ def _prep_server_detail(compute_client, server):
     return info
 
 
-def _prep_image_detail(image_client, image_id):
-    """Prepare the detailed image dict for printing
-
-    :param image_client: an image client instance
-    :param image_id: id of image created
-    :rtype: a dict of image details
-    """
-
-    info = utils.find_resource(
-        image_client.images,
-        image_id,
-    )
-    # Glance client V2 doesn't have _info attribute
-    # The following condition deals with it.
-    return getattr(info, "_info", info)
-
-
 def _show_progress(progress):
     if progress:
         sys.stdout.write('\rProgress: %s' % progress)
@@ -597,63 +580,6 @@ class CreateServerDump(command.Command):
             ).trigger_crash_dump()
 
 
-class CreateServerImage(command.ShowOne):
-    """Create a new disk image from a running server"""
-
-    def get_parser(self, prog_name):
-        parser = super(CreateServerImage, self).get_parser(prog_name)
-        parser.add_argument(
-            'server',
-            metavar='<server>',
-            help=_('Server (name or ID)'),
-        )
-        parser.add_argument(
-            '--name',
-            metavar='<image-name>',
-            help=_('Name of new image (default is server name)'),
-        )
-        parser.add_argument(
-            '--wait',
-            action='store_true',
-            help=_('Wait for image create to complete'),
-        )
-        return parser
-
-    def take_action(self, parsed_args):
-        compute_client = self.app.client_manager.compute
-        image_client = self.app.client_manager.image
-        server = utils.find_resource(
-            compute_client.servers,
-            parsed_args.server,
-        )
-        if parsed_args.name:
-            name = parsed_args.name
-        else:
-            name = server.name
-
-        image_id = compute_client.servers.create_image(
-            server,
-            name,
-        )
-
-        if parsed_args.wait:
-            if utils.wait_for_status(
-                image_client.images.get,
-                image_id,
-                callback=_show_progress,
-            ):
-                sys.stdout.write('\n')
-            else:
-                self.log.error(_('Error creating snapshot of server: %s'),
-                               parsed_args.server)
-                sys.stdout.write(_('Error creating server snapshot\n'))
-                raise SystemExit
-
-        image = _prep_image_detail(image_client, image_id)
-
-        return zip(*sorted(six.iteritems(image)))
-
-
 class DeleteServer(command.Command):
     """Delete server(s)"""
 
diff --git a/openstackclient/compute/v2/server_image.py b/openstackclient/compute/v2/server_image.py
new file mode 100644
index 00000000..85ee7f2d
--- /dev/null
+++ b/openstackclient/compute/v2/server_image.py
@@ -0,0 +1,111 @@
+#   Copyright 2012-2013 OpenStack Foundation
+#
+#   Licensed under the Apache License, Version 2.0 (the "License"); you may
+#   not use this file except in compliance with the License. You may obtain
+#   a copy of the License at
+#
+#        http://www.apache.org/licenses/LICENSE-2.0
+#
+#   Unless required by applicable law or agreed to in writing, software
+#   distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#   WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#   License for the specific language governing permissions and limitations
+#   under the License.
+#
+
+"""Compute v2 Server action implementations"""
+
+import sys
+
+from oslo_utils import importutils
+import six
+
+from openstackclient.common import command
+from openstackclient.common import exceptions
+from openstackclient.common import utils
+from openstackclient.i18n import _
+
+
+def _show_progress(progress):
+    if progress:
+        sys.stdout.write('\rProgress: %s' % progress)
+        sys.stdout.flush()
+
+
+class CreateServerImage(command.ShowOne):
+    """Create a new server disk image from an existing server"""
+
+    IMAGE_API_VERSIONS = {
+        "1": "openstackclient.image.v1.image",
+        "2": "openstackclient.image.v2.image",
+    }
+
+    def get_parser(self, prog_name):
+        parser = super(CreateServerImage, self).get_parser(prog_name)
+        parser.add_argument(
+            'server',
+            metavar='<server>',
+            help=_('Server to create image (name or ID)'),
+        )
+        parser.add_argument(
+            '--name',
+            metavar='<image-name>',
+            help=_('Name of new disk image (default: server name)'),
+        )
+        parser.add_argument(
+            '--wait',
+            action='store_true',
+            help=_('Wait for operation to complete'),
+        )
+        return parser
+
+    def take_action(self, parsed_args):
+        compute_client = self.app.client_manager.compute
+
+        server = utils.find_resource(
+            compute_client.servers,
+            parsed_args.server,
+        )
+        if parsed_args.name:
+            image_name = parsed_args.name
+        else:
+            image_name = server.name
+
+        image_id = compute_client.servers.create_image(
+            server.id,
+            image_name,
+        )
+
+        image_client = self.app.client_manager.image
+        image = utils.find_resource(
+            image_client.images,
+            image_id,
+        )
+
+        if parsed_args.wait:
+            if utils.wait_for_status(
+                image_client.images.get,
+                image_id,
+                callback=_show_progress,
+            ):
+                sys.stdout.write('\n')
+            else:
+                self.log.error(
+                    _('Error creating server image: %s') %
+                    parsed_args.server,
+                )
+                raise exceptions.CommandError
+
+        if self.app.client_manager._api_version['image'] == '1':
+            info = {}
+            info.update(image._info)
+            info['properties'] = utils.format_dict(info.get('properties', {}))
+        else:
+            # Get the right image module to format the output
+            image_module = importutils.import_module(
+                self.IMAGE_API_VERSIONS[
+                    self.app.client_manager._api_version['image']
+                ]
+            )
+            info = image_module._format_image(image)
+        return zip(*sorted(six.iteritems(info)))
diff --git a/openstackclient/tests/compute/v2/test_server.py b/openstackclient/tests/compute/v2/test_server.py
index 2dfdb68a..01aa9fc1 100644
--- a/openstackclient/tests/compute/v2/test_server.py
+++ b/openstackclient/tests/compute/v2/test_server.py
@@ -509,150 +509,6 @@ class TestServerDumpCreate(TestServer):
         self.run_method_with_servers('trigger_crash_dump', 3)
 
 
-class TestServerImageCreate(TestServer):
-
-    columns = (
-        'id',
-        'name',
-        'owner',
-        'protected',
-        'tags',
-        'visibility',
-    )
-
-    def datalist(self):
-        datalist = (
-            self.image.id,
-            self.image.name,
-            self.image.owner,
-            self.image.protected,
-            self.image.tags,
-            self.image.visibility,
-        )
-        return datalist
-
-    def setUp(self):
-        super(TestServerImageCreate, self).setUp()
-
-        self.server = compute_fakes.FakeServer.create_one_server()
-
-        # This is the return value for utils.find_resource()
-        self.servers_mock.get.return_value = self.server
-
-        self.image = image_fakes.FakeImage.create_one_image()
-        self.images_mock.get.return_value = self.image
-        self.servers_mock.create_image.return_value = self.image.id
-
-        # Get the command object to test
-        self.cmd = server.CreateServerImage(self.app, None)
-
-    def test_server_image_create_no_options(self):
-        arglist = [
-            self.server.id,
-        ]
-        verifylist = [
-            ('server', self.server.id),
-        ]
-        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
-
-        # In base command class ShowOne in cliff, abstract method take_action()
-        # returns a two-part tuple with a tuple of column names and a tuple of
-        # data to be shown.
-        columns, data = self.cmd.take_action(parsed_args)
-
-        # ServerManager.create_image(server, image_name, metadata=)
-        self.servers_mock.create_image.assert_called_with(
-            self.servers_mock.get.return_value,
-            self.server.name,
-        )
-
-        self.assertEqual(self.columns, columns)
-        self.assertEqual(self.datalist(), data)
-
-    def test_server_image_create_name(self):
-        arglist = [
-            '--name', 'img-nam',
-            self.server.id,
-        ]
-        verifylist = [
-            ('name', 'img-nam'),
-            ('server', self.server.id),
-        ]
-        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
-
-        # In base command class ShowOne in cliff, abstract method take_action()
-        # returns a two-part tuple with a tuple of column names and a tuple of
-        # data to be shown.
-        columns, data = self.cmd.take_action(parsed_args)
-
-        # ServerManager.create_image(server, image_name, metadata=)
-        self.servers_mock.create_image.assert_called_with(
-            self.servers_mock.get.return_value,
-            'img-nam',
-        )
-
-        self.assertEqual(self.columns, columns)
-        self.assertEqual(self.datalist(), data)
-
-    @mock.patch.object(common_utils, 'wait_for_status', return_value=False)
-    def test_server_create_image_with_wait_fails(self, mock_wait_for_status):
-        arglist = [
-            '--wait',
-            self.server.id,
-        ]
-        verifylist = [
-            ('wait', True),
-            ('server', self.server.id),
-        ]
-        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
-
-        self.assertRaises(SystemExit, self.cmd.take_action, parsed_args)
-
-        mock_wait_for_status.assert_called_once_with(
-            self.images_mock.get,
-            self.image.id,
-            callback=server._show_progress
-        )
-
-        # ServerManager.create_image(server, image_name, metadata=)
-        self.servers_mock.create_image.assert_called_with(
-            self.servers_mock.get.return_value,
-            self.server.name,
-        )
-
-    @mock.patch.object(common_utils, 'wait_for_status', return_value=True)
-    def test_server_create_image_with_wait_ok(self, mock_wait_for_status):
-        arglist = [
-            '--wait',
-            self.server.id,
-        ]
-        verifylist = [
-            ('wait', True),
-            ('server', self.server.id),
-        ]
-        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
-
-        # In base command class ShowOne in cliff, abstract method take_action()
-        # returns a two-part tuple with a tuple of column names and a tuple of
-        # data to be shown.
-        columns, data = self.cmd.take_action(parsed_args)
-
-        # ServerManager.create_image(server, image_name, metadata=)
-        self.servers_mock.create_image.assert_called_with(
-            self.servers_mock.get.return_value,
-            self.server.name,
-        )
-
-        mock_wait_for_status.assert_called_once_with(
-            self.images_mock.get,
-            self.image.id,
-            callback=server._show_progress
-        )
-
-        self.assertEqual(self.columns, columns)
-        self.assertEqual(self.datalist(), data)
-
-
 class TestServerList(TestServer):
 
     # Columns to be listed up.
diff --git a/openstackclient/tests/compute/v2/test_server_image.py b/openstackclient/tests/compute/v2/test_server_image.py
new file mode 100644
index 00000000..660e9817
--- /dev/null
+++ b/openstackclient/tests/compute/v2/test_server_image.py
@@ -0,0 +1,227 @@
+#   Licensed under the Apache License, Version 2.0 (the "License"); you may
+#   not use this file except in compliance with the License. You may obtain
+#   a copy of the License at
+#
+#        http://www.apache.org/licenses/LICENSE-2.0
+#
+#   Unless required by applicable law or agreed to in writing, software
+#   distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#   WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#   License for the specific language governing permissions and limitations
+#   under the License.
+#
+import mock
+
+from openstackclient.common import exceptions
+from openstackclient.common import utils as common_utils
+from openstackclient.compute.v2 import server_image
+from openstackclient.tests.compute.v2 import fakes as compute_fakes
+from openstackclient.tests.image.v2 import fakes as image_fakes
+
+
+class TestServerImage(compute_fakes.TestComputev2):
+
+    def setUp(self):
+        super(TestServerImage, self).setUp()
+
+        # Get a shortcut to the compute client ServerManager Mock
+        self.servers_mock = self.app.client_manager.compute.servers
+        self.servers_mock.reset_mock()
+
+        # Get a shortcut to the image client ImageManager Mock
+        self.images_mock = self.app.client_manager.image.images
+        self.images_mock.reset_mock()
+
+        # Set object attributes to be tested. Could be overwriten in subclass.
+        self.attrs = {}
+
+        # Set object methods to be tested. Could be overwriten in subclass.
+        self.methods = {}
+
+    def setup_servers_mock(self, count):
+        servers = compute_fakes.FakeServer.create_servers(
+            attrs=self.attrs,
+            methods=self.methods,
+            count=count,
+        )
+
+        # This is the return value for utils.find_resource()
+        self.servers_mock.get = compute_fakes.FakeServer.get_servers(
+            servers,
+            0,
+        )
+        return servers
+
+
+class TestServerImageCreate(TestServerImage):
+
+    def image_columns(self, image):
+        columnlist = tuple(sorted(image.keys()))
+        return columnlist
+
+    def image_data(self, image):
+        datalist = (
+            image['id'],
+            image['name'],
+            image['owner'],
+            image['protected'],
+            'active',
+            common_utils.format_list(image.get('tags')),
+            image['visibility'],
+        )
+        return datalist
+
+    def setUp(self):
+        super(TestServerImageCreate, self).setUp()
+
+        # Get the command object to test
+        self.cmd = server_image.CreateServerImage(self.app, None)
+
+        self.methods = {
+            'create_image': None,
+        }
+
+    def setup_images_mock(self, count, servers=None):
+        if servers:
+            images = image_fakes.FakeImage.create_images(
+                attrs={
+                    'name': servers[0].name,
+                    'status': 'active',
+                },
+                count=count,
+            )
+        else:
+            images = image_fakes.FakeImage.create_images(
+                attrs={
+                    'status': 'active',
+                },
+                count=count,
+            )
+
+        self.images_mock.get = mock.MagicMock(side_effect=images)
+        self.servers_mock.create_image = mock.MagicMock(
+            return_value=images[0].id,
+        )
+        return images
+
+    def test_server_image_create_defaults(self):
+        servers = self.setup_servers_mock(count=1)
+        images = self.setup_images_mock(count=1, servers=servers)
+
+        arglist = [
+            servers[0].id,
+        ]
+        verifylist = [
+            ('server', servers[0].id),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        # In base command class ShowOne in cliff, abstract method take_action()
+        # returns a two-part tuple with a tuple of column names and a tuple of
+        # data to be shown.
+        columns, data = self.cmd.take_action(parsed_args)
+
+        # ServerManager.create_image(server, image_name, metadata=)
+        self.servers_mock.create_image.assert_called_with(
+            servers[0].id,
+            servers[0].name,
+        )
+
+        self.assertEqual(self.image_columns(images[0]), columns)
+        self.assertEqual(self.image_data(images[0]), data)
+
+    def test_server_image_create_options(self):
+        servers = self.setup_servers_mock(count=1)
+        images = self.setup_images_mock(count=1, servers=servers)
+
+        arglist = [
+            '--name', 'img-nam',
+            servers[0].id,
+        ]
+        verifylist = [
+            ('name', 'img-nam'),
+            ('server', servers[0].id),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        # In base command class ShowOne in cliff, abstract method take_action()
+        # returns a two-part tuple with a tuple of column names and a tuple of
+        # data to be shown.
+        columns, data = self.cmd.take_action(parsed_args)
+
+        # ServerManager.create_image(server, image_name, metadata=)
+        self.servers_mock.create_image.assert_called_with(
+            servers[0].id,
+            'img-nam',
+        )
+
+        self.assertEqual(self.image_columns(images[0]), columns)
+        self.assertEqual(self.image_data(images[0]), data)
+
+    @mock.patch.object(common_utils, 'wait_for_status', return_value=False)
+    def test_server_create_image_wait_fail(self, mock_wait_for_status):
+        servers = self.setup_servers_mock(count=1)
+        images = self.setup_images_mock(count=1, servers=servers)
+
+        arglist = [
+            '--wait',
+            servers[0].id,
+        ]
+        verifylist = [
+            ('wait', True),
+            ('server', servers[0].id),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        self.assertRaises(
+            exceptions.CommandError,
+            self.cmd.take_action,
+            parsed_args,
+        )
+
+        # ServerManager.create_image(server, image_name, metadata=)
+        self.servers_mock.create_image.assert_called_with(
+            servers[0].id,
+            servers[0].name,
+        )
+
+        mock_wait_for_status.assert_called_once_with(
+            self.images_mock.get,
+            images[0].id,
+            callback=mock.ANY
+        )
+
+    @mock.patch.object(common_utils, 'wait_for_status', return_value=True)
+    def test_server_create_image_wait_ok(self, mock_wait_for_status):
+        servers = self.setup_servers_mock(count=1)
+        images = self.setup_images_mock(count=1, servers=servers)
+
+        arglist = [
+            '--wait',
+            servers[0].id,
+        ]
+        verifylist = [
+            ('wait', True),
+            ('server', servers[0].id),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        # In base command class ShowOne in cliff, abstract method take_action()
+        # returns a two-part tuple with a tuple of column names and a tuple of
+        # data to be shown.
+        columns, data = self.cmd.take_action(parsed_args)
+
+        # ServerManager.create_image(server, image_name, metadata=)
+        self.servers_mock.create_image.assert_called_with(
+            servers[0].id,
+            servers[0].name,
+        )
+
+        mock_wait_for_status.assert_called_once_with(
+            self.images_mock.get,
+            images[0].id,
+            callback=mock.ANY
+        )
+
+        self.assertEqual(self.image_columns(images[0]), columns)
+        self.assertEqual(self.image_data(images[0]), data)
diff --git a/setup.cfg b/setup.cfg
index 78450c00..3a554228 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -104,7 +104,6 @@ openstack.compute.v2 =
     server_add_volume = openstackclient.compute.v2.server:AddServerVolume
     server_create = openstackclient.compute.v2.server:CreateServer
     server_delete = openstackclient.compute.v2.server:DeleteServer
-    server_image_create = openstackclient.compute.v2.server:CreateServerImage
     server_list = openstackclient.compute.v2.server:ListServer
     server_lock = openstackclient.compute.v2.server:LockServer
     server_migrate = openstackclient.compute.v2.server:MigrateServer
@@ -138,6 +137,8 @@ openstack.compute.v2 =
     server_group_list = openstackclient.compute.v2.server_group:ListServerGroup
     server_group_show = openstackclient.compute.v2.server_group:ShowServerGroup
 
+    server_image_create = openstackclient.compute.v2.server_image:CreateServerImage
+
     usage_list = openstackclient.compute.v2.usage:ListUsage
     usage_show = openstackclient.compute.v2.usage:ShowUsage