Image create and set command updates and tests

Refactor image create and set commands to properly handle properties.
This is particularly tricky with exclusive booleans as in this case leaving
both choices off the command line should NOT assume a default value but
leave the existing value unchanged.

Properties were not being updated correctly in the 'image set' command.
Refactor it to use the same pattern as in other SetXxx commands.
Add tests for arg handling.

Change-Id: I123a64c9b4feecab25a3e2013cc047f55b1c9967

2 files changed, 382 insertions, 47 deletions

diff --git a/openstackclient/tests/image/v1/fakes.py b/openstackclient/tests/image/v1/fakes.py
index ea2af84c..972e6415 100644
--- a/openstackclient/tests/image/v1/fakes.py
+++ b/openstackclient/tests/image/v1/fakes.py
@@ -17,16 +17,38 @@ import mock
 
 from openstackclient.tests import fakes
 from openstackclient.tests import utils
+from openstackclient.tests.volume.v1 import fakes as volume_fakes
 
 
 image_id = 'im1'
 image_name = 'graven'
+image_owner = 'baal'
+image_protected = False
+image_public = True
+image_properties = {
+    'Alpha': 'a',
+    'Beta': 'b',
+    'Gamma': 'g',
+}
+image_properties_str = "{'Alpha': 'a', 'Beta': 'b', 'Gamma': 'g'}"
+image_data = 'line 1\nline 2\n'
 
 IMAGE = {
     'id': image_id,
-    'name': image_name
+    'name': image_name,
+    'container_format': '',
+    'disk_format': '',
+    'owner': image_owner,
+    'min_disk': 0,
+    'min_ram': 0,
+    'is_public': image_public,
+    'protected': image_protected,
+    'properties': image_properties,
 }
 
+IMAGE_columns = tuple(sorted(IMAGE))
+IMAGE_data = tuple((IMAGE[x] for x in sorted(IMAGE)))
+
 
 class FakeImagev1Client(object):
     def __init__(self, **kwargs):
@@ -44,3 +66,7 @@ class TestImagev1(utils.TestCommand):
             endpoint=fakes.AUTH_URL,
             token=fakes.AUTH_TOKEN,
         )
+        self.app.client_manager.volume = volume_fakes.FakeVolumev1Client(
+            endpoint=fakes.AUTH_URL,
+            token=fakes.AUTH_TOKEN,
+        )
diff --git a/openstackclient/tests/image/v1/test_image.py b/openstackclient/tests/image/v1/test_image.py
index d7547f76..b746a538 100644
--- a/openstackclient/tests/image/v1/test_image.py
+++ b/openstackclient/tests/image/v1/test_image.py
@@ -16,6 +16,7 @@
 import copy
 import mock
 
+from openstackclient.common import exceptions
 from openstackclient.image.v1 import image
 from openstackclient.tests import fakes
 from openstackclient.tests.image.v1 import fakes as image_fakes
@@ -35,75 +36,228 @@ class TestImageCreate(TestImage):
 
     def setUp(self):
         super(TestImageCreate, self).setUp()
+
+        self.images_mock.create.return_value = fakes.FakeResource(
+            None,
+            copy.deepcopy(image_fakes.IMAGE),
+            loaded=True,
+        )
+        # This is the return value for utils.find_resource()
         self.images_mock.get.return_value = fakes.FakeResource(
             None,
             copy.deepcopy(image_fakes.IMAGE),
             loaded=True,
         )
+        self.images_mock.update.return_value = fakes.FakeResource(
+            None,
+            copy.deepcopy(image_fakes.IMAGE),
+            loaded=True,
+        )
+
+        # Get the command object to test
         self.cmd = image.CreateImage(self.app, None)
 
-    def test_create_volume(self):
+    def test_image_reserve_no_options(self):
+        mock_exception = {
+            'find.side_effect': exceptions.CommandError('x'),
+            'get.side_effect': exceptions.CommandError('x'),
+        }
+        self.images_mock.configure_mock(**mock_exception)
         arglist = [
-            '--volume', 'volly',
             image_fakes.image_name,
         ]
         verifylist = [
-            ('volume', 'volly'),
+            ('container_format', image.DEFAULT_CONTAINER_FORMAT),
+            ('disk_format', image.DEFAULT_DISK_FORMAT),
             ('name', image_fakes.image_name),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
-        self.app.client_manager.volume = mock.Mock()
-        self.app.client_manager.volume.volumes = mock.Mock()
-        volumes = self.app.client_manager.volume.volumes
-        volumes.upload_to_image = mock.Mock()
-        response = {"id": 'volume_id',
-                    "updated_at": 'updated_at',
-                    "status": 'uploading',
-                    "display_description": 'desc',
-                    "size": 'size',
-                    "volume_type": 'volume_type',
-                    "image_id": 'image1',
-                    "container_format": parsed_args.container_format,
-                    "disk_format": parsed_args.disk_format,
-                    "image_name": parsed_args.name}
-        full_response = {"os-volume_upload_image": response}
-        volumes.upload_to_image.return_value = (201, full_response)
-        volume_resource = fakes.FakeResource(
+
+        # DisplayCommandBase.take_action() returns two tuples
+        columns, data = self.cmd.take_action(parsed_args)
+
+        # ImageManager.create(name=, **)
+        self.images_mock.create.assert_called_with(
+            name=image_fakes.image_name,
+            container_format=image.DEFAULT_CONTAINER_FORMAT,
+            disk_format=image.DEFAULT_DISK_FORMAT,
+            data=mock.ANY,
+        )
+
+        # Verify update() was not called, if it was show the args
+        self.assertEqual(self.images_mock.update.call_args_list, [])
+
+        self.assertEqual(image_fakes.IMAGE_columns, columns)
+        self.assertEqual(image_fakes.IMAGE_data, data)
+
+    def test_image_reserve_options(self):
+        mock_exception = {
+            'find.side_effect': exceptions.CommandError('x'),
+            'get.side_effect': exceptions.CommandError('x'),
+        }
+        self.images_mock.configure_mock(**mock_exception)
+        arglist = [
+            '--container-format', 'ovf',
+            '--disk-format', 'fs',
+            '--min-disk', '10',
+            '--min-ram', '4',
+            '--protected',
+            '--private',
+            image_fakes.image_name,
+        ]
+        verifylist = [
+            ('container_format', 'ovf'),
+            ('disk_format', 'fs'),
+            ('min_disk', 10),
+            ('min_ram', 4),
+            ('protected', True),
+            ('unprotected', False),
+            ('public', False),
+            ('private', True),
+            ('name', image_fakes.image_name),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        # DisplayCommandBase.take_action() returns two tuples
+        columns, data = self.cmd.take_action(parsed_args)
+
+        # ImageManager.create(name=, **)
+        self.images_mock.create.assert_called_with(
+            name=image_fakes.image_name,
+            container_format='ovf',
+            disk_format='fs',
+            min_disk=10,
+            min_ram=4,
+            protected=True,
+            is_public=False,
+            data=mock.ANY,
+        )
+
+        # Verify update() was not called, if it was show the args
+        self.assertEqual(self.images_mock.update.call_args_list, [])
+
+        self.assertEqual(image_fakes.IMAGE_columns, columns)
+        self.assertEqual(image_fakes.IMAGE_data, data)
+
+    @mock.patch('__builtin__.open')
+    def test_image_create_file(self, open_mock):
+        mock_exception = {
+            'find.side_effect': exceptions.CommandError('x'),
+            'get.side_effect': exceptions.CommandError('x'),
+        }
+        self.images_mock.configure_mock(**mock_exception)
+        open_mock.return_value = image_fakes.image_data
+        arglist = [
+            '--file', 'filer',
+            '--unprotected',
+            '--public',
+            '--property', 'Alpha=1',
+            '--property', 'Beta=2',
+            image_fakes.image_name,
+        ]
+        verifylist = [
+            ('file', 'filer'),
+            ('protected', False),
+            ('unprotected', True),
+            ('public', True),
+            ('private', False),
+            ('properties', {'Alpha': '1', 'Beta': '2'}),
+            ('name', image_fakes.image_name),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        # DisplayCommandBase.take_action() returns two tuples
+        columns, data = self.cmd.take_action(parsed_args)
+
+        open_mock.assert_called_with('filer', 'rb')
+
+        # ImageManager.get(name)
+        self.images_mock.get.assert_called_with(image_fakes.image_name)
+
+        # ImageManager.create(name=, **)
+        self.images_mock.create.assert_called_with(
+            name=image_fakes.image_name,
+            container_format=image.DEFAULT_CONTAINER_FORMAT,
+            disk_format=image.DEFAULT_DISK_FORMAT,
+            protected=False,
+            is_public=True,
+            properties={
+                'Alpha': '1',
+                'Beta': '2',
+            },
+            data=image_fakes.image_data,
+        )
+
+        # Verify update() was not called, if it was show the args
+        self.assertEqual(self.images_mock.update.call_args_list, [])
+
+        self.assertEqual(image_fakes.IMAGE_columns, columns)
+        self.assertEqual(image_fakes.IMAGE_data, data)
+
+    def test_image_create_volume(self):
+        # Set up VolumeManager Mock
+        volumes_mock = self.app.client_manager.volume.volumes
+        volumes_mock.reset_mock()
+        volumes_mock.get.return_value = fakes.FakeResource(
             None,
             copy.deepcopy({'id': 'vol1', 'name': 'volly'}),
             loaded=True,
         )
-        volumes.get.return_value = volume_resource
-        results = self.cmd.take_action(parsed_args)
-        volumes.upload_to_image.assert_called_with(
-            volume_resource,
+        response = {
+            "id": 'volume_id',
+            "updated_at": 'updated_at',
+            "status": 'uploading',
+            "display_description": 'desc',
+            "size": 'size',
+            "volume_type": 'volume_type',
+            "image_id": 'image1',
+            "container_format": image.DEFAULT_CONTAINER_FORMAT,
+            "disk_format": image.DEFAULT_DISK_FORMAT,
+            "image_name": image_fakes.image_name,
+        }
+        full_response = {"os-volume_upload_image": response}
+        volumes_mock.upload_to_image.return_value = (201, full_response)
+
+        arglist = [
+            '--volume', 'volly',
+            image_fakes.image_name,
+        ]
+        verifylist = [
+            ('private', False),
+            ('protected', False),
+            ('public', False),
+            ('unprotected', False),
+            ('volume', 'volly'),
+            ('force', False),
+            ('name', image_fakes.image_name),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        # DisplayCommandBase.take_action() returns two tuples
+        columns, data = self.cmd.take_action(parsed_args)
+
+        # VolumeManager.upload_to_image(volume, force, image_name,
+        #     container_format, disk_format)
+        volumes_mock.upload_to_image.assert_called_with(
+            'vol1',
             False,
             image_fakes.image_name,
             'bare',
             'raw',
         )
-        expects = [('container_format',
-                    'disk_format',
-                    'display_description',
-                    'id',
-                    'image_id',
-                    'image_name',
-                    'size',
-                    'status',
-                    'updated_at',
-                    'volume_type'),
-                   ('bare',
-                    'raw',
-                    'desc',
-                    'volume_id',
-                    'image1',
-                    'graven',
-                    'size',
-                    'uploading',
-                    'updated_at',
-                    'volume_type')]
-        for expected, result in zip(expects, results):
-            self.assertEqual(expected, result)
+
+        # ImageManager.update(image_id, remove_props=, **)
+        self.images_mock.update.assert_called_with(
+            image_fakes.image_id,
+            name=image_fakes.image_name,
+            container_format=image.DEFAULT_CONTAINER_FORMAT,
+            disk_format=image.DEFAULT_DISK_FORMAT,
+            properties=image_fakes.image_properties,
+            volume='volly',
+        )
+
+        self.assertEqual(image_fakes.IMAGE_columns, columns)
+        self.assertEqual(image_fakes.IMAGE_data, data)
 
 
 class TestImageDelete(TestImage):
@@ -137,3 +291,158 @@ class TestImageDelete(TestImage):
         self.images_mock.delete.assert_called_with(
             image_fakes.image_id,
         )
+
+
+class TestImageSet(TestImage):
+
+    def setUp(self):
+        super(TestImageSet, self).setUp()
+
+        # This is the return value for utils.find_resource()
+        self.images_mock.get.return_value = fakes.FakeResource(
+            None,
+            copy.deepcopy(image_fakes.IMAGE),
+            loaded=True,
+        )
+        self.images_mock.update.return_value = fakes.FakeResource(
+            None,
+            copy.deepcopy(image_fakes.IMAGE),
+            loaded=True,
+        )
+
+        # Get the command object to test
+        self.cmd = image.SetImage(self.app, None)
+
+    def test_image_set_no_options(self):
+        arglist = [
+            image_fakes.image_name,
+        ]
+        verifylist = [
+            ('image', image_fakes.image_name),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        # DisplayCommandBase.take_action() returns two tuples
+        self.cmd.take_action(parsed_args)
+
+        # Verify update() was not called, if it was show the args
+        self.assertEqual(self.images_mock.update.call_args_list, [])
+
+    def test_image_set_options(self):
+        arglist = [
+            '--name', 'new-name',
+            '--owner', 'new-owner',
+            '--min-disk', '2',
+            '--min-ram', '4',
+            image_fakes.image_name,
+        ]
+        verifylist = [
+            ('name', 'new-name'),
+            ('owner', 'new-owner'),
+            ('min_disk', 2),
+            ('min_ram', 4),
+            ('image', image_fakes.image_name),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        # DisplayCommandBase.take_action() returns two tuples
+        columns, data = self.cmd.take_action(parsed_args)
+
+        kwargs = {
+            'name': 'new-name',
+            'owner': 'new-owner',
+            'min_disk': 2,
+            'min_ram': 4,
+        }
+        # ImageManager.update(image, **kwargs)
+        self.images_mock.update.assert_called_with(
+            image_fakes.image_id,
+            **kwargs
+        )
+
+        self.assertEqual(image_fakes.IMAGE_columns, columns)
+        self.assertEqual(image_fakes.IMAGE_data, data)
+
+    def test_image_set_bools1(self):
+        arglist = [
+            '--protected',
+            '--private',
+            image_fakes.image_name,
+        ]
+        verifylist = [
+            ('protected', True),
+            ('unprotected', False),
+            ('public', False),
+            ('private', True),
+            ('image', image_fakes.image_name),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        # DisplayCommandBase.take_action() returns two tuples
+        self.cmd.take_action(parsed_args)
+
+        kwargs = {
+            'protected': True,
+            'is_public': False,
+        }
+        # ImageManager.update(image, **kwargs)
+        self.images_mock.update.assert_called_with(
+            image_fakes.image_id,
+            **kwargs
+        )
+
+    def test_image_set_bools2(self):
+        arglist = [
+            '--unprotected',
+            '--public',
+            image_fakes.image_name,
+        ]
+        verifylist = [
+            ('protected', False),
+            ('unprotected', True),
+            ('public', True),
+            ('private', False),
+            ('image', image_fakes.image_name),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        # DisplayCommandBase.take_action() returns two tuples
+        self.cmd.take_action(parsed_args)
+
+        kwargs = {
+            'protected': False,
+            'is_public': True,
+        }
+        # ImageManager.update(image, **kwargs)
+        self.images_mock.update.assert_called_with(
+            image_fakes.image_id,
+            **kwargs
+        )
+
+    def test_image_set_properties(self):
+        arglist = [
+            '--property', 'Alpha=1',
+            '--property', 'Beta=2',
+            image_fakes.image_name,
+        ]
+        verifylist = [
+            ('properties', {'Alpha': '1', 'Beta': '2'}),
+            ('image', image_fakes.image_name),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        # DisplayCommandBase.take_action() returns two tuples
+        self.cmd.take_action(parsed_args)
+
+        kwargs = {
+            'properties': {
+                'Alpha': '1',
+                'Beta': '2',
+                'Gamma': 'g',
+            },
+        }
+        # ImageManager.update(image, **kwargs)
+        self.images_mock.update.assert_called_with(
+            image_fakes.image_id,
+            **kwargs
+        )