From 56b5d3f0bc8ca25b6601a22508e3e173dc46e853 Mon Sep 17 00:00:00 2001
From: Jim Rollenhagen <jim@jimrollenhagen.com>
Date: Tue, 6 Nov 2018 18:28:17 -0500
Subject: 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.

Conflicts:
	ironic/drivers/modules/deploy_utils.py
Due to class InstanceImageCache being in iscsi_deploy.py instead of
deploy_utils.py.

Change-Id: I1bb62d55e3e18272fd5da355d63fd2c11a033acd
Story: 2004279
Task: 27829
(cherry picked from commit 0498e2982901a586f19c9c8412c596bad0cf8004)
---
 ironic/conf/pxe.py                                 |  2 +-
 ironic/drivers/modules/iscsi_deploy.py             |  3 ++-
 .../unit/drivers/modules/test_iscsi_deploy.py      | 26 ++++++++++++++++++++++
 ...stance-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 229d4e7eb..45be1564c 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/iscsi_deploy.py b/ironic/drivers/modules/iscsi_deploy.py
index 392837022..f7e3b6e9f 100644
--- a/ironic/drivers/modules/iscsi_deploy.py
+++ b/ironic/drivers/modules/iscsi_deploy.py
@@ -47,8 +47,9 @@ DISK_LAYOUT_PARAMS = ('root_gb', 'swap_mb', 'ephemeral_gb')
 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_iscsi_deploy.py b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py
index ba3479352..a96979eae 100644
--- a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py
+++ b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py
@@ -1018,3 +1018,29 @@ class CleanUpFullFlowTestCase(db_base.DbTestCase):
                      + self.files):
             self.assertFalse(os.path.exists(path),
                              '%s is not expected to exist' % path)
+
+
+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 = iscsi_deploy.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 = iscsi_deploy.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 000000000..4184a4cd7
--- /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.
-- 
cgit v1.2.1