From fbd2c00b8999202917671bcdb39298eb39c3ad47 Mon Sep 17 00:00:00 2001
From: Myeongchul Chae <cocahack@naver.com>
Date: Sun, 16 Aug 2020 13:44:45 +0000
Subject: [PATCH] Fix --image-property option in 'create server'

There was a problem that the '-image-property' option, which can be used
to create an instance, did not work as intended.

I found that there were two problems with this option.

First, I cannot select an image as its metadata.

The second is that when there are multiple images available, the desired
image may not be selected depending on the situation.

This patch solves these two problems.

I wrote the test case with these two problems considered together.

Change-Id: Ib2745d7e067056ff4ca8bfaf6cff492d0dacb73a
story: #2007860
---
 openstackclient/compute/v2/server.py          | 14 ++++-
 .../tests/unit/compute/v2/test_server.py      | 59 +++++++++++++++++++
 ...-property-field.yaml-c51bf37c3106d6ff.yaml |  6 ++
 3 files changed, 76 insertions(+), 3 deletions(-)
 create mode 100644 releasenotes/notes/properties-with-image-property-field.yaml-c51bf37c3106d6ff.yaml

diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py
index 93e9f966ae..d7b01a0fb3 100644
--- a/openstackclient/compute/v2/server.py
+++ b/openstackclient/compute/v2/server.py
@@ -751,19 +751,27 @@ class CreateServer(command.ShowOne):
                 images_matched = []
                 for img in image_list:
                     img_dict = {}
+
                     # exclude any unhashable entries
-                    for key, value in img.items():
+                    img_dict_items = list(img.items())
+                    if img.properties:
+                        img_dict_items.extend(list(img.properties.items()))
+                    for key, value in img_dict_items:
                         try:
                             set([key, value])
                         except TypeError:
+                            if key != 'properties':
+                                LOG.debug('Skipped the \'%s\' attribute. '
+                                          'That cannot be compared. '
+                                          '(image: %s, value: %s)',
+                                          key, img.id, value)
                             pass
                         else:
                             img_dict[key] = value
+
                     if all(k in img_dict and img_dict[k] == v
                            for k, v in wanted_properties.items()):
                         images_matched.append(img)
-                    else:
-                        return []
                 return images_matched
 
             images = _match_image(image_client, parsed_args.image_property)
diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py
index 7e4c71c50c..6c9497b55c 100644
--- a/openstackclient/tests/unit/compute/v2/test_server.py
+++ b/openstackclient/tests/unit/compute/v2/test_server.py
@@ -2048,6 +2048,65 @@ class TestServerCreate(TestServer):
                           self.cmd.take_action,
                           parsed_args)
 
+    def test_server_create_image_property_with_image_list(self):
+        arglist = [
+            '--image-property',
+            'owner_specified.openstack.object=image/cirros',
+            '--flavor', 'flavor1',
+            '--nic', 'none',
+            self.new_server.name,
+        ]
+
+        verifylist = [
+            ('image_property',
+                {'owner_specified.openstack.object': 'image/cirros'}),
+            ('flavor', 'flavor1'),
+            ('nic', ['none']),
+            ('server_name', self.new_server.name),
+        ]
+        # create a image_info as the side_effect of the fake image_list()
+        image_info = {
+            'properties': {
+                'owner_specified.openstack.object': 'image/cirros'
+            }
+        }
+
+        target_image = image_fakes.FakeImage.create_one_image(image_info)
+        another_image = image_fakes.FakeImage.create_one_image({})
+        self.images_mock.return_value = [target_image, another_image]
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        columns, data = self.cmd.take_action(parsed_args)
+
+        # Set expected values
+        kwargs = dict(
+            files={},
+            reservation_id=None,
+            min_count=1,
+            max_count=1,
+            security_groups=[],
+            userdata=None,
+            key_name=None,
+            availability_zone=None,
+            block_device_mapping_v2=[],
+            nics='none',
+            meta=None,
+            scheduler_hints={},
+            config_drive=None,
+        )
+
+        # ServerManager.create(name, image, flavor, **kwargs)
+        self.servers_mock.create.assert_called_with(
+            self.new_server.name,
+            target_image,
+            self.flavor,
+            **kwargs
+        )
+
+        self.assertEqual(self.columns, columns)
+        self.assertEqual(self.datalist(), data)
+
     def test_server_create_invalid_hint(self):
         # Not a key-value pair
         arglist = [
diff --git a/releasenotes/notes/properties-with-image-property-field.yaml-c51bf37c3106d6ff.yaml b/releasenotes/notes/properties-with-image-property-field.yaml-c51bf37c3106d6ff.yaml
new file mode 100644
index 0000000000..cf082f45c8
--- /dev/null
+++ b/releasenotes/notes/properties-with-image-property-field.yaml-c51bf37c3106d6ff.yaml
@@ -0,0 +1,6 @@
+---
+features:
+  - Support for image search via properties of image. Currently
+    "openstack server create --image-property" only takes image property.
+    Now it can also search image via properties (user defined) too.
+    Story https://storyboard.openstack.org/#!/story/2007860.