From 0498e2982901a586f19c9c8412c596bad0cf8004 Mon Sep 17 00:00:00 2001
From: Jim Rollenhagen <jim@jimrollenhagen.com>
Date: Tue, 6 Nov 2018 18:28:17 -0500
Subject: [PATCH] Allow disabling instance image cache

We document that this can be disabled by setting the
instance_master_path config  to "<None>",
but don't actually support it in code. oslo.config doesn't actually
translate that value to the Python None as we expect.

Allow disabling the cache by setting the config to the empty string, as
in "instance_master_path=". This doesn't make sense as a directory to
use as a cache anyway, so it shouldn't break anyone.

Change-Id: I1bb62d55e3e18272fd5da355d63fd2c11a033acd
Story: 2004279
Task: 27829
---
 ironic/conf/pxe.py                            |  2 +-
 ironic/drivers/modules/deploy_utils.py        |  3 ++-
 .../unit/drivers/modules/test_deploy_utils.py | 26 +++++++++++++++++++
 ...e-master-path-config-fa524c907a7888e5.yaml |  6 +++++
 4 files changed, 35 insertions(+), 2 deletions(-)
 create mode 100644 releasenotes/notes/fix-instance-master-path-config-fa524c907a7888e5.yaml

diff --git a/ironic/conf/pxe.py b/ironic/conf/pxe.py
index 042046001d..f16aa94e04 100644
--- a/ironic/conf/pxe.py
+++ b/ironic/conf/pxe.py
@@ -36,7 +36,7 @@ opts = [
                default='/var/lib/ironic/master_images',
                help=_('On the ironic-conductor node, directory where master '
                       'instance images are stored on disk. '
-                      'Setting to <None> disables image caching.')),
+                      'Setting to the empty string disables image caching.')),
     cfg.IntOpt('image_cache_size',
                default=20480,
                help=_('Maximum size (in MiB) of cache for master images, '
diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py
index 25c4d381d9..c2d47edd9c 100644
--- a/ironic/drivers/modules/deploy_utils.py
+++ b/ironic/drivers/modules/deploy_utils.py
@@ -1113,8 +1113,9 @@ def direct_deploy_should_convert_raw_image(node):
 class InstanceImageCache(image_cache.ImageCache):
 
     def __init__(self):
+        master_path = CONF.pxe.instance_master_path or None
         super(self.__class__, self).__init__(
-            CONF.pxe.instance_master_path,
+            master_path,
             # MiB -> B
             cache_size=CONF.pxe.image_cache_size * 1024 * 1024,
             # min -> sec
diff --git a/ironic/tests/unit/drivers/modules/test_deploy_utils.py b/ironic/tests/unit/drivers/modules/test_deploy_utils.py
index a4086ee4fa..4a16a5d9d6 100644
--- a/ironic/tests/unit/drivers/modules/test_deploy_utils.py
+++ b/ironic/tests/unit/drivers/modules/test_deploy_utils.py
@@ -2761,3 +2761,29 @@ class TestStorageInterfaceUtils(db_base.DbTestCase):
         with task_manager.acquire(
                 self.context, self.node.uuid, shared=False) as task:
             self.assertFalse(utils.is_iscsi_boot(task))
+
+
+class InstanceImageCacheTestCase(db_base.DbTestCase):
+    @mock.patch.object(fileutils, 'ensure_tree')
+    def test_with_master_path(self, mock_ensure_tree):
+        self.config(instance_master_path='/fake/path', group='pxe')
+        self.config(image_cache_size=500, group='pxe')
+        self.config(image_cache_ttl=30, group='pxe')
+
+        cache = utils.InstanceImageCache()
+
+        mock_ensure_tree.assert_called_once_with('/fake/path')
+        self.assertEqual(500 * 1024 * 1024, cache._cache_size)
+        self.assertEqual(30 * 60, cache._cache_ttl)
+
+    @mock.patch.object(fileutils, 'ensure_tree')
+    def test_without_master_path(self, mock_ensure_tree):
+        self.config(instance_master_path='', group='pxe')
+        self.config(image_cache_size=500, group='pxe')
+        self.config(image_cache_ttl=30, group='pxe')
+
+        cache = utils.InstanceImageCache()
+
+        mock_ensure_tree.assert_not_called()
+        self.assertEqual(500 * 1024 * 1024, cache._cache_size)
+        self.assertEqual(30 * 60, cache._cache_ttl)
diff --git a/releasenotes/notes/fix-instance-master-path-config-fa524c907a7888e5.yaml b/releasenotes/notes/fix-instance-master-path-config-fa524c907a7888e5.yaml
new file mode 100644
index 0000000000..4184a4cd76
--- /dev/null
+++ b/releasenotes/notes/fix-instance-master-path-config-fa524c907a7888e5.yaml
@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    Fixes an issue where the master instance image cache could not be disabled.
+    The configuration option ``[pxe]/instance_master_path`` may now be set to
+    the empty string to disable the cache.