From 4dc1920154214be361c6964e4c840200365ffeaf Mon Sep 17 00:00:00 2001
From: Dmitry Tantsur <dtantsur@protonmail.com>
Date: Thu, 24 Jun 2021 12:43:07 +0200
Subject: [PATCH] Refactor: untie IloVendor from validate_image_properties

The function does more than just validating that the image exists.
This change simplifies further refactoring of image validation.

Change-Id: I9a3dc7ddabde38faa77a2c4bc7bf5db750a7f4d8
---
 ironic/drivers/modules/deploy_utils.py        | 40 ++++++++++++-------
 ironic/drivers/modules/ilo/vendor.py          |  7 ++--
 .../unit/drivers/modules/ilo/test_vendor.py   |  8 ++--
 3 files changed, 34 insertions(+), 21 deletions(-)

diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py
index dc57c85a42..115a1f8edb 100644
--- a/ironic/drivers/modules/deploy_utils.py
+++ b/ironic/drivers/modules/deploy_utils.py
@@ -513,6 +513,30 @@ def validate_capabilities(node):
                  'value': value, 'valid_values': ', '.join(valid_values)})
 
 
+def get_image_properties(ctx, image_href):
+    """Get properties of the image.
+
+    :param ctx: security context
+    :param image_href: reference to the image
+    :return: properties as a dictionary
+    :raises: InvalidParameterValue if the image cannot be accessed
+    """
+    try:
+        img_service = image_service.get_image_service(image_href, context=ctx)
+        return img_service.show(image_href)['properties']
+    except (exception.GlanceConnectionFailed,
+            exception.ImageNotAuthorized,
+            exception.Invalid):
+        raise exception.InvalidParameterValue(_(
+            "Failed to connect to Glance to get the properties "
+            "of the image %s") % image_href)
+    except exception.ImageNotFound:
+        raise exception.InvalidParameterValue(_(
+            "Image %s can not be found.") % image_href)
+    except exception.ImageRefValidationFailed as e:
+        raise exception.InvalidParameterValue(err=e)
+
+
 def validate_image_properties(ctx, deploy_info, properties):
     """Validate the image.
 
@@ -540,20 +564,8 @@ def validate_image_properties(ctx, deploy_info, properties):
             "specified at the same time."))
     if not image_href:
         image_href = boot_iso
-    try:
-        img_service = image_service.get_image_service(image_href, context=ctx)
-        image_props = img_service.show(image_href)['properties']
-    except (exception.GlanceConnectionFailed,
-            exception.ImageNotAuthorized,
-            exception.Invalid):
-        raise exception.InvalidParameterValue(_(
-            "Failed to connect to Glance to get the properties "
-            "of the image %s") % image_href)
-    except exception.ImageNotFound:
-        raise exception.InvalidParameterValue(_(
-            "Image %s can not be found.") % image_href)
-    except exception.ImageRefValidationFailed as e:
-        raise exception.InvalidParameterValue(err=e)
+
+    image_props = get_image_properties(ctx, image_href)
 
     missing_props = []
     for prop in properties:
diff --git a/ironic/drivers/modules/ilo/vendor.py b/ironic/drivers/modules/ilo/vendor.py
index cbfa308b04..2f4986a2fd 100644
--- a/ironic/drivers/modules/ilo/vendor.py
+++ b/ironic/drivers/modules/ilo/vendor.py
@@ -66,12 +66,13 @@ class VendorPassthru(base.VendorInterface):
                    {'node_uuid': task.node.uuid,
                     'state': states.MANAGEABLE})
             raise exception.InvalidStateRequested(msg)
-        d_info = {'boot_iso_href': kwargs.get('boot_iso_href')}
+        boot_iso = kwargs.get('boot_iso_href')
+        d_info = {'boot_iso_href': boot_iso}
         error_msg = _("Error validating input for boot_into_iso vendor "
                       "passthru. Some parameters were not provided: ")
         deploy_utils.check_for_missing_params(d_info, error_msg)
-        deploy_utils.validate_image_properties(
-            task.context, {'image_source': kwargs.get('boot_iso_href')}, [])
+        # Validate that the image exists
+        deploy_utils.get_image_properties(task.context, boot_iso)
 
     @METRICS.timer('IloVendorPassthru.boot_into_iso')
     @base.passthru(['POST'],
diff --git a/ironic/tests/unit/drivers/modules/ilo/test_vendor.py b/ironic/tests/unit/drivers/modules/ilo/test_vendor.py
index 05a735d213..f3114826e3 100644
--- a/ironic/tests/unit/drivers/modules/ilo/test_vendor.py
+++ b/ironic/tests/unit/drivers/modules/ilo/test_vendor.py
@@ -71,7 +71,7 @@ class VendorPassthruTestCase(test_common.BaseIloTest):
                 task.driver.vendor._validate_boot_into_iso,
                 task, {})
 
-    @mock.patch.object(deploy_utils, 'validate_image_properties',
+    @mock.patch.object(deploy_utils, 'get_image_properties',
                        spec_set=True, autospec=True)
     def test__validate_boot_into_iso_manage(self, validate_image_prop_mock):
         with task_manager.acquire(self.context, self.node.uuid,
@@ -81,9 +81,9 @@ class VendorPassthruTestCase(test_common.BaseIloTest):
             task.driver.vendor._validate_boot_into_iso(
                 task, info)
             validate_image_prop_mock.assert_called_once_with(
-                task.context, {'image_source': 'foo'}, [])
+                task.context, 'foo')
 
-    @mock.patch.object(deploy_utils, 'validate_image_properties',
+    @mock.patch.object(deploy_utils, 'get_image_properties',
                        spec_set=True, autospec=True)
     def test__validate_boot_into_iso_maintenance(
             self, validate_image_prop_mock):
@@ -94,4 +94,4 @@ class VendorPassthruTestCase(test_common.BaseIloTest):
             task.driver.vendor._validate_boot_into_iso(
                 task, info)
             validate_image_prop_mock.assert_called_once_with(
-                task.context, {'image_source': 'foo'}, [])
+                task.context, 'foo')