Agent driver fails without Ironic-managed TFTP

There are a lot of assumptions that Ironic has local PXE files.
This is untrue for deployments with separate DHCP servers and
agent images not hosted in Glance. Adds a new config option
'manage_tftp' in the DCHP section to disable TFTP management.

Closes-Bug: #1440896
Change-Id: Ibf5a415e3beee13d12e65e177816256df57cb423

3 files changed, 101 insertions, 18 deletions

diff --git a/etc/ironic/ironic.conf.sample b/etc/ironic/ironic.conf.sample
index bc38835b5..cf75c9b1f 100644
--- a/etc/ironic/ironic.conf.sample
+++ b/etc/ironic/ironic.conf.sample
@@ -360,6 +360,11 @@
 # set to 0, will not run during cleaning. (integer value)
 #agent_erase_devices_priority=<None>
 
+# Whether Ironic will manage TFTP files for the deploy
+# ramdisks. If set to False, you will need to configure your
+# own TFTP server that allows booting the deploy ramdisks.
+# (boolean value)
+#manage_tftp=true
 
 #
 # Options defined in ironic.drivers.modules.agent_base_vendor
diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py
index c257d2c77..6a590c8c4 100644
--- a/ironic/drivers/modules/agent.py
+++ b/ironic/drivers/modules/agent.py
@@ -57,7 +57,14 @@ agent_opts = [
                     'Python Agent ramdisk. If unset, will use the priority '
                     'set in the ramdisk (defaults to 10 for the '
                     'GenericHardwareManager). If set to 0, will not run '
-                    'during cleaning.')
+                    'during cleaning.'),
+    cfg.BoolOpt('manage_tftp',
+                default=True,
+                help='Whether Ironic will manage TFTP files for the deploy '
+                     'ramdisks. If set to False, you will need to configure '
+                     'your own TFTP server that allows booting the deploy '
+                     'ramdisks.'
+                ),
     ]
 
 CONF = cfg.CONF
@@ -196,12 +203,13 @@ def build_instance_info_for_deploy(task):
 
 def _prepare_pxe_boot(task):
     """Prepare the files required for PXE booting the agent."""
-    pxe_info = _get_tftp_image_info(task.node)
-    pxe_options = _build_pxe_config_options(task.node, pxe_info)
-    pxe_utils.create_pxe_config(task,
-                                pxe_options,
-                                CONF.agent.agent_pxe_config_template)
-    _cache_tftp_images(task.context, task.node, pxe_info)
+    if CONF.agent.manage_tftp:
+        pxe_info = _get_tftp_image_info(task.node)
+        pxe_options = _build_pxe_config_options(task.node, pxe_info)
+        pxe_utils.create_pxe_config(task,
+                                    pxe_options,
+                                    CONF.agent.agent_pxe_config_template)
+        _cache_tftp_images(task.context, task.node, pxe_info)
 
 
 def _do_pxe_boot(task, ports=None):
@@ -220,13 +228,13 @@ def _do_pxe_boot(task, ports=None):
 
 def _clean_up_pxe(task):
     """Clean up left over PXE and DHCP files."""
-    pxe_info = _get_tftp_image_info(task.node)
-    for label in pxe_info:
-        path = pxe_info[label][1]
-        utils.unlink_without_raise(path)
-    AgentTFTPImageCache().clean_up()
-
-    pxe_utils.clean_up_pxe_config(task)
+    if CONF.agent.manage_tftp:
+        pxe_info = _get_tftp_image_info(task.node)
+        for label in pxe_info:
+            path = pxe_info[label][1]
+            utils.unlink_without_raise(path)
+        AgentTFTPImageCache().clean_up()
+        pxe_utils.clean_up_pxe_config(task)
 
 
 class AgentDeploy(base.DeployInterface):
@@ -251,10 +259,11 @@ class AgentDeploy(base.DeployInterface):
         """
         node = task.node
         params = {}
-        params['driver_info.deploy_kernel'] = node.driver_info.get(
-                                                              'deploy_kernel')
-        params['driver_info.deploy_ramdisk'] = node.driver_info.get(
-                                                              'deploy_ramdisk')
+        if CONF.agent.manage_tftp:
+            params['driver_info.deploy_kernel'] = node.driver_info.get(
+                'deploy_kernel')
+            params['driver_info.deploy_ramdisk'] = node.driver_info.get(
+                'deploy_ramdisk')
         image_source = node.instance_info.get('image_source')
         params['instance_info.image_source'] = image_source
         error_msg = _('Node %s failed to validate deploy image info. Some '
diff --git a/ironic/tests/drivers/test_agent.py b/ironic/tests/drivers/test_agent.py
index b9ea638e0..bb9675f3f 100644
--- a/ironic/tests/drivers/test_agent.py
+++ b/ironic/tests/drivers/test_agent.py
@@ -164,6 +164,14 @@ class TestAgentDeploy(db_base.DbTestCase):
         self.assertIn('driver_info.deploy_ramdisk', str(e))
         self.assertIn('driver_info.deploy_kernel', str(e))
 
+    def test_validate_driver_info_manage_tftp_false(self):
+        self.config(manage_tftp=False, group='agent')
+        self.node.driver_info = {}
+        self.node.save()
+        with task_manager.acquire(
+                self.context, self.node['uuid'], shared=False) as task:
+            self.driver.validate(task)
+
     def test_validate_instance_info_missing_params(self):
         self.node.instance_info = {}
         self.node.save()
@@ -199,6 +207,37 @@ class TestAgentDeploy(db_base.DbTestCase):
             self.assertRaises(exception.InvalidParameterValue,
                               task.driver.deploy.validate, task)
 
+    @mock.patch.object(agent, '_cache_tftp_images')
+    @mock.patch.object(pxe_utils, 'create_pxe_config')
+    @mock.patch.object(agent, '_build_pxe_config_options')
+    @mock.patch.object(agent, '_get_tftp_image_info')
+    def test__prepare_pxe_boot(self, pxe_info_mock, options_mock,
+                               create_mock, cache_mock):
+        with task_manager.acquire(
+                self.context, self.node['uuid'], shared=False) as task:
+            agent._prepare_pxe_boot(task)
+            pxe_info_mock.assert_called_once_with(task.node)
+            options_mock.assert_called_once_with(task.node, mock.ANY)
+            create_mock.assert_called_once_with(
+                task, mock.ANY, CONF.agent.agent_pxe_config_template)
+            cache_mock.assert_called_once_with(task.context, task.node,
+                                               mock.ANY)
+
+    @mock.patch.object(agent, '_cache_tftp_images')
+    @mock.patch.object(pxe_utils, 'create_pxe_config')
+    @mock.patch.object(agent, '_build_pxe_config_options')
+    @mock.patch.object(agent, '_get_tftp_image_info')
+    def test__prepare_pxe_boot_manage_tftp_false(
+            self, pxe_info_mock, options_mock, create_mock, cache_mock):
+        self.config(manage_tftp=False, group='agent')
+        with task_manager.acquire(
+                self.context, self.node['uuid'], shared=False) as task:
+            agent._prepare_pxe_boot(task)
+        self.assertFalse(pxe_info_mock.called)
+        self.assertFalse(options_mock.called)
+        self.assertFalse(create_mock.called)
+        self.assertFalse(cache_mock.called)
+
     @mock.patch.object(dhcp_factory.DHCPFactory, 'update_dhcp')
     @mock.patch('ironic.conductor.utils.node_set_boot_device')
     @mock.patch('ironic.conductor.utils.node_power_action')
@@ -221,6 +260,36 @@ class TestAgentDeploy(db_base.DbTestCase):
             power_mock.assert_called_once_with(task, states.POWER_OFF)
             self.assertEqual(driver_return, states.DELETED)
 
+    @mock.patch.object(pxe_utils, 'clean_up_pxe_config')
+    @mock.patch.object(agent, 'AgentTFTPImageCache')
+    @mock.patch('ironic.common.utils.unlink_without_raise')
+    @mock.patch.object(agent, '_get_tftp_image_info')
+    def test__clean_up_pxe(self, info_mock, unlink_mock, cache_mock,
+                           clean_mock):
+        info_mock.return_value = {'label': ['fake1', 'fake2']}
+        with task_manager.acquire(
+                self.context, self.node['uuid'], shared=False) as task:
+            agent._clean_up_pxe(task)
+            info_mock.assert_called_once_with(task.node)
+            unlink_mock.assert_called_once_with('fake2')
+            clean_mock.assert_called_once_with(task)
+
+    @mock.patch.object(pxe_utils, 'clean_up_pxe_config')
+    @mock.patch.object(agent.AgentTFTPImageCache, 'clean_up')
+    @mock.patch('ironic.common.utils.unlink_without_raise')
+    @mock.patch.object(agent, '_get_tftp_image_info')
+    def test__clean_up_pxe_manage_tftp_false(
+            self, info_mock, unlink_mock, cache_mock, clean_mock):
+        self.config(manage_tftp=False, group='agent')
+        info_mock.return_value = {'label': ['fake1', 'fake2']}
+        with task_manager.acquire(
+                self.context, self.node['uuid'], shared=False) as task:
+            agent._clean_up_pxe(task)
+            self.assertFalse(info_mock.called)
+            self.assertFalse(unlink_mock.called)
+            self.assertFalse(cache_mock.called)
+            self.assertFalse(clean_mock.called)
+
     @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.delete_cleaning_ports')
     @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.create_cleaning_ports')
     @mock.patch('ironic.drivers.modules.agent._do_pxe_boot')