Allow setting image_download_source per node

Allows certain flexibility when it comes to low RAM vs high RAM nodes,
and large vs small images. Also deploy_interface is settable per node,
so this feature makes it easier to migrate from the iscsi deploy.

Story: #2008075
Task: #40766
Change-Id: Idf3bbc6d24042ce1d9a895095b5cb0979dd3183d

doc/source/admin/interfaces/deploy.rst

@@ -71,6 +71,15 @@ This configuration affects *glance* and ``file://`` images. If you want
    [agent]
    image_download_source = local
 
+.. note::
+   This option can also be set per node in ``driver_info``::
+
+    openstack baremetal node set <node> --driver-info image_download_source=local
+
+   or per instance in ``instance_info``::
+
+    openstack baremetal node set <node> --instance-info image_download_source=local
+
 You need to set up a workable HTTP server at each conductor node which with
 ``direct`` deploy interface enabled, and check http related options in the
 ironic configuration file to match the HTTP server configurations.

doc/source/install/standalone.rst

@@ -228,6 +228,13 @@ Populating instance_info
         --instance-info image_source=$IMG \
         --instance-info image_checksum=$MD5HASH
 
+#. When using low RAM nodes with ``http://`` images that are not in the RAW
+   format, you may want them cached locally, converted to raw and served from
+   the conductor's HTTP server::
+
+    openstack baremetal node set $NODE_UUID \
+        --instance-info image_download_source=local
+
 #. :ref:`Boot mode <boot_mode_support>` can be specified per instance::
 
     openstack baremetal node set $NODE_UUID \

ironic/drivers/modules/agent.py

@@ -57,6 +57,12 @@ OPTIONAL_PROPERTIES = {
                         'a dot to prefix the domain name. This value will be '
                         'ignored if ``image_http_proxy`` and '
                         '``image_https_proxy`` are not specified. Optional.'),
+    'image_download_source': _('Specifies whether direct deploy interface '
+                               'should try to use the image source directly '
+                               'or if ironic should cache the image on the '
+                               'conductor and serve it from ironic\'s own '
+                               'HTTP server. Accepted values are "swift", '
+                               '"http" and "local". Optional.'),
 }
 
 _RAID_APPLY_CONFIGURATION_ARGSINFO = {
@@ -168,13 +174,19 @@ def validate_http_provisioning_configuration(node):
     :raises: MissingParameterValue if required option(s) is not set.
     """
     image_source = node.instance_info.get('image_source')
+    image_download_source = deploy_utils.get_image_download_source(node)
+    if image_download_source not in ('swift', 'http', 'local'):
+        raise exception.InvalidParameterValue(
+            _('Invalid value for image_download_source: "%s". Valid values '
+              'are swift, http or local.') % image_download_source)
+
     # NOTE(dtantsur): local HTTP configuration is required in two cases:
     # 1. Glance images with image_download_source == http
     # 2. File images (since we need to serve them to IPA)
     if (not image_source.startswith('file://')
-            and CONF.agent.image_download_source != 'local'
+            and image_download_source != 'local'
             and (not service_utils.is_glance_image(image_source)
-                 or CONF.agent.image_download_source == 'swift')):
+                 or image_download_source == 'swift')):
         return
 
     params = {

ironic/drivers/modules/deploy_utils.py

@@ -1046,6 +1046,13 @@ def _cache_and_convert_image(task, instance_info, image_info=None):
     instance_info['image_url'] = http_image_url
 
 
+def get_image_download_source(node):
+    """Get the effective value of image_download_source for the node."""
+    return (node.instance_info.get('image_download_source')
+            or node.driver_info.get('image_download_source')
+            or CONF.agent.image_download_source)
+
+
 @METRICS.timer('build_instance_info_for_deploy')
 def build_instance_info_for_deploy(task):
     """Build instance_info necessary for deploying to a node.
@@ -1060,13 +1067,14 @@ def build_instance_info_for_deploy(task):
     instance_info = node.instance_info
     iwdi = node.driver_internal_info.get('is_whole_disk_image')
     image_source = instance_info['image_source']
+    image_download_source = get_image_download_source(node)
 
     if service_utils.is_glance_image(image_source):
         glance = image_service.GlanceImageService(context=task.context)
         image_info = glance.show(image_source)
         LOG.debug('Got image info: %(info)s for node %(node)s.',
                   {'info': image_info, 'node': node.uuid})
-        if CONF.agent.image_download_source == 'swift':
+        if image_download_source == 'swift':
             swift_temp_url = glance.swift_temp_url(image_info)
             _validate_image_url(node, swift_temp_url, secret=True)
             instance_info['image_url'] = swift_temp_url
@@ -1086,7 +1094,7 @@ def build_instance_info_for_deploy(task):
             instance_info['kernel'] = image_info['properties']['kernel_id']
             instance_info['ramdisk'] = image_info['properties']['ramdisk_id']
     elif (image_source.startswith('file://')
-          or CONF.agent.image_download_source == 'local'):
+          or image_download_source == 'local'):
         _cache_and_convert_image(task, instance_info)
     else:
         _validate_image_url(node, image_source)

ironic/tests/unit/drivers/modules/test_agent.py

@@ -213,6 +213,33 @@ class TestAgentMethods(db_base.DbTestCase):
                                agent.validate_http_provisioning_configuration,
                                self.node)
 
+    def test_validate_http_provisioning_missing_args_local_via_node(self):
+        CONF.set_override('http_url', None, group='deploy')
+        i_info = self.node.instance_info
+        i_info['image_source'] = 'http://image-ref'
+        i_info['image_download_source'] = 'local'
+        self.node.instance_info = i_info
+        self.assertRaisesRegex(exception.MissingParameterValue,
+                               'failed to validate http provisoning',
+                               agent.validate_http_provisioning_configuration,
+                               self.node)
+
+    def test_validate_http_provisioning_invalid_image_download_source(self):
+        CONF.set_override('http_url', None, group='deploy')
+        self.node.instance_info['image_source'] = 'http://image-ref'
+        self.node.instance_info['image_download_source'] = 'fridge'
+        self.assertRaisesRegex(exception.InvalidParameterValue, 'fridge',
+                               agent.validate_http_provisioning_configuration,
+                               self.node)
+
+    def test_validate_http_provisioning_invalid_image_download_source2(self):
+        CONF.set_override('http_url', None, group='deploy')
+        self.node.instance_info['image_source'] = 'http://image-ref'
+        self.node.driver_info['image_download_source'] = 'fridge'
+        self.assertRaisesRegex(exception.InvalidParameterValue, 'fridge',
+                               agent.validate_http_provisioning_configuration,
+                               self.node)
+
 
 class TestAgentDeploy(db_base.DbTestCase):
     def setUp(self):

ironic/tests/unit/drivers/modules/test_deploy_utils.py

@@ -2106,6 +2106,78 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase):
             validate_href_mock.assert_called_once_with(
                 mock.ANY, expected_url, False)
 
+    @mock.patch.object(image_service.HttpImageService, 'validate_href',
+                       autospec=True)
+    def test_build_instance_info_local_image_via_iinfo(self,
+                                                       validate_href_mock):
+        cfg.CONF.set_override('image_download_source', 'http', group='agent')
+        i_info = self.node.instance_info
+        driver_internal_info = self.node.driver_internal_info
+        i_info['image_source'] = 'http://image-ref'
+        i_info['image_checksum'] = 'aa'
+        i_info['root_gb'] = 10
+        i_info['image_checksum'] = 'aa'
+        i_info['image_download_source'] = 'local'
+        driver_internal_info['is_whole_disk_image'] = True
+        self.node.instance_info = i_info
+        self.node.driver_internal_info = driver_internal_info
+        self.node.save()
+
+        expected_url = (
+            'http://172.172.24.10:8080/agent_images/%s' % self.node.uuid)
+
+        with task_manager.acquire(
+                self.context, self.node.uuid, shared=False) as task:
+
+            info = utils.build_instance_info_for_deploy(task)
+
+            self.assertEqual(expected_url, info['image_url'])
+            self.assertEqual('sha256', info['image_os_hash_algo'])
+            self.assertEqual('fake-checksum', info['image_os_hash_value'])
+            self.cache_image_mock.assert_called_once_with(
+                task.context, task.node, force_raw=True)
+            self.checksum_mock.assert_called_once_with(
+                self.fake_path, algorithm='sha256')
+            validate_href_mock.assert_called_once_with(
+                mock.ANY, expected_url, False)
+
+    @mock.patch.object(image_service.HttpImageService, 'validate_href',
+                       autospec=True)
+    def test_build_instance_info_local_image_via_dinfo(self,
+                                                       validate_href_mock):
+        cfg.CONF.set_override('image_download_source', 'http', group='agent')
+        i_info = self.node.instance_info
+        d_info = self.node.driver_info
+        driver_internal_info = self.node.driver_internal_info
+        i_info['image_source'] = 'http://image-ref'
+        i_info['image_checksum'] = 'aa'
+        i_info['root_gb'] = 10
+        i_info['image_checksum'] = 'aa'
+        d_info['image_download_source'] = 'local'
+        driver_internal_info['is_whole_disk_image'] = True
+        self.node.instance_info = i_info
+        self.node.driver_info = d_info
+        self.node.driver_internal_info = driver_internal_info
+        self.node.save()
+
+        expected_url = (
+            'http://172.172.24.10:8080/agent_images/%s' % self.node.uuid)
+
+        with task_manager.acquire(
+                self.context, self.node.uuid, shared=False) as task:
+
+            info = utils.build_instance_info_for_deploy(task)
+
+            self.assertEqual(expected_url, info['image_url'])
+            self.assertEqual('sha256', info['image_os_hash_algo'])
+            self.assertEqual('fake-checksum', info['image_os_hash_value'])
+            self.cache_image_mock.assert_called_once_with(
+                task.context, task.node, force_raw=True)
+            self.checksum_mock.assert_called_once_with(
+                self.fake_path, algorithm='sha256')
+            validate_href_mock.assert_called_once_with(
+                mock.ANY, expected_url, False)
+
 
 class TestStorageInterfaceUtils(db_base.DbTestCase):
     def setUp(self):

releasenotes/notes/image_download_source-842282c70b226e93.yaml

@@ -0,0 +1,6 @@
+---
+features:
+  - |
+    The ``image_download_source`` configuration option can now also be set
+    per node in the ``instance_info`` or ``driver_info`` (the former having
+    the highest priority).