From 29a7c9afce32e7acbd40b9e4a73140ef9e05e7f3 Mon Sep 17 00:00:00 2001
From: Stephen Finucane <sfinucan@redhat.com>
Date: Tue, 22 Dec 2020 16:46:33 +0000
Subject: [PATCH] image: Unset properties rather than setting to None

Currently, we attempt to unset an image property by setting it to None.
This doesn't work for known properties and is rightly rejected by the
Glance API with the following error:

  BadRequestException: 400: Client Error for url:
  http://172.20.4.87/image/v2/images/368c5751-2b0b-4a38-a255-fd146fe52d31,
  Bad Request

The solution is to actually unset the field by deleting it.

Change-Id: Ie156bedbe0f9244f82c81401679706f484caf9aa
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
Story: #2008463
Task: #41493
---
 openstackclient/image/v2/image.py             |  2 +-
 .../tests/functional/image/v2/test_image.py   |  2 ++
 .../tests/unit/image/v2/test_image.py         | 29 +++++++++----------
 3 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/openstackclient/image/v2/image.py b/openstackclient/image/v2/image.py
index 58d92f5165..6081152217 100644
--- a/openstackclient/image/v2/image.py
+++ b/openstackclient/image/v2/image.py
@@ -1166,7 +1166,7 @@ class UnsetImage(command.Command):
         if parsed_args.properties:
             for k in parsed_args.properties:
                 if k in image:
-                    kwargs[k] = None
+                    delattr(image, k)
                 elif k in image.properties:
                     # Since image is an "evil" object from SDK POV we need to
                     # pass modified properties object, so that SDK can figure
diff --git a/openstackclient/tests/functional/image/v2/test_image.py b/openstackclient/tests/functional/image/v2/test_image.py
index 264ba5199c..0a3a73602c 100644
--- a/openstackclient/tests/functional/image/v2/test_image.py
+++ b/openstackclient/tests/functional/image/v2/test_image.py
@@ -119,6 +119,7 @@ class ImageTests(base.BaseImageTests):
             'image set ' +
             '--property a=b ' +
             '--property c=d ' +
+            '--property hw_rng_model=virtio ' +
             '--public ' +
             self.name
         )
@@ -133,6 +134,7 @@ class ImageTests(base.BaseImageTests):
             'image unset ' +
             '--property a ' +
             '--property c ' +
+            '--property hw_rng_model ' +
             self.name
         )
         json_output = json.loads(self.openstack(
diff --git a/openstackclient/tests/unit/image/v2/test_image.py b/openstackclient/tests/unit/image/v2/test_image.py
index b72e983599..8088e15e41 100644
--- a/openstackclient/tests/unit/image/v2/test_image.py
+++ b/openstackclient/tests/unit/image/v2/test_image.py
@@ -1501,15 +1501,16 @@ class TestImageShow(TestImage):
 
 class TestImageUnset(TestImage):
 
-    attrs = {}
-    attrs['tags'] = ['test']
-    attrs['prop'] = 'test'
-    attrs['prop2'] = 'fake'
-    image = image_fakes.FakeImage.create_one_image(attrs)
-
     def setUp(self):
         super(TestImageUnset, self).setUp()
 
+        attrs = {}
+        attrs['tags'] = ['test']
+        attrs['hw_rng_model'] = 'virtio'
+        attrs['prop'] = 'test'
+        attrs['prop2'] = 'fake'
+        self.image = image_fakes.FakeImage.create_one_image(attrs)
+
         self.client.find_image.return_value = self.image
         self.client.remove_tag.return_value = self.image
         self.client.update_image.return_value = self.image
@@ -1552,22 +1553,20 @@ class TestImageUnset(TestImage):
     def test_image_unset_property_option(self):
 
         arglist = [
+            '--property', 'hw_rng_model',
             '--property', 'prop',
             self.image.id,
         ]
 
         verifylist = [
-            ('properties', ['prop']),
+            ('properties', ['hw_rng_model', 'prop']),
             ('image', self.image.id)
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
         result = self.cmd.take_action(parsed_args)
 
-        kwargs = {}
         self.client.update_image.assert_called_with(
-            self.image,
-            properties={'prop2': 'fake'},
-            **kwargs)
+            self.image, properties={'prop2': 'fake'})
 
         self.assertIsNone(result)
 
@@ -1575,23 +1574,21 @@ class TestImageUnset(TestImage):
 
         arglist = [
             '--tag', 'test',
+            '--property', 'hw_rng_model',
             '--property', 'prop',
             self.image.id,
         ]
 
         verifylist = [
             ('tags', ['test']),
-            ('properties', ['prop']),
+            ('properties', ['hw_rng_model', 'prop']),
             ('image', self.image.id)
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
         result = self.cmd.take_action(parsed_args)
 
-        kwargs = {}
         self.client.update_image.assert_called_with(
-            self.image,
-            properties={'prop2': 'fake'},
-            **kwargs)
+            self.image, properties={'prop2': 'fake'})
 
         self.client.remove_tag.assert_called_with(
             self.image.id, 'test'