Ramdisk: do not require image_source

Because of the way validation works, the ramdisk deploy interface
currently requires an image_source in addition to kernel/ramdisk.
After 1d6441cc34 it is no longer
necessary, and this change removes this requirement.

Change-Id: I59996fac059dade0ef186598be1e8971e073eb04

doc/source/admin/ramdisk-boot.rst

@@ -72,13 +72,13 @@ source, for example,
 
     baremetal node set <NODE> \
         --instance-info kernel=http://path/to/ramdisk.kernel \
-        --instance-info ramdisk=http://path/to/ramdisk.initramfs \
+        --instance-info ramdisk=http://path/to/ramdisk.initramfs
-        --instance-info image_source=http://path/to/ramdisk.initramfs
     baremetal node deploy <NODE>
 
 .. note::
-   The requirement to pass ``image_source`` is artificial and will be fixed
+   Before the Xena release, the ``image_source`` field was also required::
-   in a future version of the Bare Metal service.
+
+        --instance-info image_source=http://path/to/ramdisk.initramfs
 
 Booting an ISO
 --------------

ironic/drivers/modules/deploy_utils.py

@@ -565,17 +565,23 @@ def validate_image_properties(task, deploy_info):
         image_href = boot_iso
 
     properties = []
-    if (not boot_iso
+    if boot_iso or task.node.driver_internal_info.get('is_whole_disk_image'):
-            and not task.node.driver_internal_info.get('is_whole_disk_image')):
+        # No image properties are required in this case
-        if service_utils.is_glance_image(image_href):
+        return
-            properties = ['kernel_id', 'ramdisk_id']
-            boot_option = get_boot_option(task.node)
-            if boot_option == 'kickstart':
-                properties.append('squashfs_id')
-        else:
-            properties = ['kernel', 'ramdisk']
 
-    image_props = get_image_properties(task.context, image_href)
+    if service_utils.is_glance_image(image_href):
+        properties = ['kernel_id', 'ramdisk_id']
+        boot_option = get_boot_option(task.node)
+        if boot_option == 'kickstart':
+            properties.append('squashfs_id')
+    else:
+        properties = ['kernel', 'ramdisk']
+
+    if image_href:
+        image_props = get_image_properties(task.context, image_href)
+    else:
+        # Ramdisk deploy, no image_source is present
+        image_props = []
 
     missing_props = []
     for prop in properties:
@@ -783,17 +789,28 @@ def get_image_instance_info(node):
     # more explicit unit testing to exist.
     info = {}
 
-    is_whole_disk_image = node.driver_internal_info.get('is_whole_disk_image')
     boot_iso = node.instance_info.get('boot_iso')
-    if not boot_iso:
+    image_source = node.instance_info.get('image_source')
-        info['image_source'] = node.instance_info.get('image_source')
+    if boot_iso:
-    else:
+        if image_source:
+            raise exception.InvalidParameterValue(_(
+                "An 'image_source' and 'boot_iso' parameter may not be "
+                "specified at the same time."))
         info['boot_iso'] = boot_iso
-
+    else:
-    if not is_whole_disk_image and not boot_iso:
+        if get_boot_option(node) == 'ramdisk':
-        if not service_utils.is_glance_image(info['image_source']):
+            # Ramdisk deploy does not require an image
             info['kernel'] = node.instance_info.get('kernel')
             info['ramdisk'] = node.instance_info.get('ramdisk')
+        else:
+            info['image_source'] = image_source
+
+            is_whole_disk_image = node.driver_internal_info.get(
+                'is_whole_disk_image')
+            if (not is_whole_disk_image
+                    and not service_utils.is_glance_image(image_source)):
+                info['kernel'] = node.instance_info.get('kernel')
+                info['ramdisk'] = node.instance_info.get('ramdisk')
 
     error_msg = (_("Cannot validate image information for node %s because one "
                    "or more parameters are missing from its instance_info and "

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

@@ -1433,6 +1433,20 @@ class ValidateImagePropertiesTestCase(db_base.DbTestCase):
                                   instance_info)
         self.assertEqual(expected_error, str(error))
 
+    @mock.patch.object(utils, 'get_boot_option', autospec=True,
+                       return_value='ramdisk')
+    @mock.patch.object(image_service.HttpImageService, 'show', autospec=True)
+    def test_validate_image_properties_ramdisk_deploy(
+            self, image_service_show_mock, boot_options_mock):
+        instance_info = {
+            'kernel': 'file://kernel',
+            'ramdisk': 'file://initrd',
+        }
+        self.node.instance_info = instance_info
+        inst_info = utils.get_image_instance_info(self.node)
+        utils.validate_image_properties(self.task, inst_info)
+        image_service_show_mock.assert_not_called()
+
 
 class ValidateParametersTestCase(db_base.DbTestCase):
 
@@ -1449,12 +1463,11 @@ class ValidateParametersTestCase(db_base.DbTestCase):
             driver_internal_info=DRV_INTERNAL_INFO_DICT,
         )
 
-        info = utils.get_image_instance_info(node)
+        return utils.get_image_instance_info(node)
-        self.assertIsNotNone(info['image_source'])
-        return info
 
     def test__get_img_instance_info_good(self):
-        self._test__get_img_instance_info()
+        info = self._test__get_img_instance_info()
+        self.assertIsNotNone(info['image_source'])
 
     def test__get_img_instance_info_good_non_glance_image(self):
         instance_info = INST_INFO_DICT.copy()
@@ -1464,6 +1477,7 @@ class ValidateParametersTestCase(db_base.DbTestCase):
 
         info = self._test__get_img_instance_info(instance_info=instance_info)
 
+        self.assertIsNotNone(info['image_source'])
         self.assertIsNotNone(info['ramdisk'])
         self.assertIsNotNone(info['kernel'])
 
@@ -1500,8 +1514,31 @@ class ValidateParametersTestCase(db_base.DbTestCase):
         driver_internal_info = DRV_INTERNAL_INFO_DICT.copy()
         driver_internal_info['is_whole_disk_image'] = True
 
-        self._test__get_img_instance_info(
+        info = self._test__get_img_instance_info(
             driver_internal_info=driver_internal_info)
+        self.assertIsNotNone(info['image_source'])
+
+    def test__get_img_instance_info_boot_iso_only(self):
+        instance_info = {
+            'boot_iso': 'http://iso'
+        }
+
+        info = self._test__get_img_instance_info(instance_info=instance_info)
+        self.assertIsNotNone(info['boot_iso'])
+        self.assertNotIn('image_source', info)
+
+    @mock.patch.object(utils, 'get_boot_option', autospec=True,
+                       return_value='ramdisk')
+    def test__get_img_instance_info_ramdisk_deploy(self, mock_boot_opt):
+        instance_info = {
+            'kernel': 'http://kernel',
+            'ramdisk': 'http://ramdisk',
+        }
+
+        info = self._test__get_img_instance_info(instance_info=instance_info)
+        self.assertIsNotNone(info['kernel'])
+        self.assertIsNotNone(info['ramdisk'])
+        self.assertNotIn('image_source', info)
 
 
 class InstanceInfoTestCase(db_base.DbTestCase):

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

@@ -133,15 +133,17 @@ class iPXEBootTestCase(db_base.DbTestCase):
 
     @mock.patch.object(image_service.GlanceImageService, 'show', autospec=True)
     @mock.patch('ironic.drivers.modules.deploy_utils.get_boot_option',
-                return_value='ramdisk', autospec=True)
+                autospec=True)
     def test_validate_with_boot_iso(self, mock_boot_option, mock_glance):
-        i_info = self.node.driver_info
+        self.node.instance_info = {
-        i_info['boot_iso'] = "http://localhost:1234/boot.iso"
+            'boot_iso': "http://localhost:1234/boot.iso"
+        }
+        self.node.save()
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=True) as task:
             task.driver.boot.validate(task)
-            self.assertTrue(mock_boot_option.called)
+            mock_boot_option.assert_called_with(task.node)
-            self.assertTrue(mock_glance.called)
+            mock_glance.assert_not_called()
 
     @mock.patch('ironic.drivers.modules.deploy_utils.get_boot_option',
                 return_value='ramdisk', autospec=True)

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

@@ -853,7 +853,8 @@ class PXERamdiskDeployTestCase(db_base.DbTestCase):
                             'default_%s_interface' % iface: impl}
             self.config(**config_kwarg)
         self.config(enabled_hardware_types=['fake-hardware'])
-        instance_info = INST_INFO_DICT
+        instance_info = {'kernel': 'kernelUUID',
+                         'ramdisk': 'ramdiskUUID'}
         self.node = obj_utils.create_test_node(
             self.context,
             driver='fake-hardware',
@@ -963,8 +964,16 @@ class PXERamdiskDeployTestCase(db_base.DbTestCase):
                        autospec=True)
     def test_validate(self, mock_validate_img):
         with task_manager.acquire(self.context, self.node.uuid) as task:
-            task.node.instance_info['capabilities'] = {
+            task.driver.deploy.validate(task)
-                'boot_option': 'netboot'}
+        self.assertTrue(mock_validate_img.called)
+
+    @mock.patch.object(deploy_utils, 'validate_image_properties',
+                       autospec=True)
+    def test_validate_with_boot_iso(self, mock_validate_img):
+        with task_manager.acquire(self.context, self.node.uuid) as task:
+            task.node.instance_info = {
+                'boot_iso': 'isoUUID'
+            }
             task.driver.deploy.validate(task)
         self.assertTrue(mock_validate_img.called)
 

releasenotes/notes/ramdisk-validate-acbc4acdb71d10c2.yaml

@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    The ``ramdisk`` deploy interface no longer requires a fake ``image_source``
+    value to be provided when ``boot_iso`` is not used.