Merge "Copy any configuration from the virtual media"

4 files changed, 221 insertions, 103 deletions

diff --git a/ironic_python_agent/cmd/agent.py b/ironic_python_agent/cmd/agent.py
index 2d19fcf9..794f5d83 100644
--- a/ironic_python_agent/cmd/agent.py
+++ b/ironic_python_agent/cmd/agent.py
@@ -21,12 +21,17 @@ from oslo_utils import strutils
 
 from ironic_python_agent import agent
 from ironic_python_agent import config
+from ironic_python_agent import utils
 
 CONF = cfg.CONF
 
 
 def run():
     """Entrypoint for IronicPythonAgent."""
+    # NOTE(dtantsur): this must happen very early of the files from
+    # /etc/ironic-python-agent.d won't be loaded
+    utils.copy_config_from_vmedia()
+
     log.register_options(CONF)
     CONF(args=sys.argv[1:])
     # Debug option comes from oslo.log, allow overriding it via kernel cmdline
diff --git a/ironic_python_agent/tests/unit/test_utils.py b/ironic_python_agent/tests/unit/test_utils.py
index 21407292..f5972b5b 100644
--- a/ironic_python_agent/tests/unit/test_utils.py
+++ b/ironic_python_agent/tests/unit/test_utils.py
@@ -146,89 +146,90 @@ class GetAgentParamsTestCase(ironic_agent_base.IronicAgentTest):
         vmedia_device_returned = utils._get_vmedia_device()
         self.assertEqual('sdc', vmedia_device_returned)
 
-    @mock.patch.object(shutil, 'rmtree', autospec=True)
-    @mock.patch.object(tempfile, 'mkdtemp', autospec=True)
-    @mock.patch.object(utils, '_read_params_from_file', autospec=True)
-    @mock.patch.object(os.path, 'exists', autospec=True)
-    @mock.patch.object(os, 'mkdir', autospec=True)
     @mock.patch.object(utils, 'execute', autospec=True)
-    def test__get_vmedia_params_by_label_lower_case(
-            self, execute_mock, mkdir_mock, exists_mock, read_params_mock,
-            mkdtemp_mock, rmtree_mock):
-        mkdtemp_mock.return_value = "/tempdir"
-
-        null_output = ["", ""]
-        expected_params = {'a': 'b'}
-        read_params_mock.return_value = expected_params
-        exists_mock.side_effect = [True, False]
-        execute_mock.side_effect = [null_output, null_output]
+    def test__find_device_by_labels(self, execute_mock):
+        execute_mock.side_effect = [
+            processutils.ProcessExecutionError,
+            ('/dev/fake', ''),
+        ]
+        self.assertEqual('/dev/fake',
+                         utils._find_device_by_labels(['l1', 'l2']))
+        execute_mock.assert_has_calls([
+            mock.call('blkid', '-L', item)
+            for item in ('l1', 'l2')
+        ])
 
-        returned_params = utils._get_vmedia_params()
+    @mock.patch.object(utils, 'execute', autospec=True)
+    def test__find_device_by_labels_upper(self, execute_mock):
+        execute_mock.side_effect = [
+            processutils.ProcessExecutionError,
+            processutils.ProcessExecutionError,
+            ('/dev/fake', ''),
+        ]
+        self.assertEqual('/dev/fake',
+                         utils._find_device_by_labels(['l1', 'l2']))
+        execute_mock.assert_has_calls([
+            mock.call('blkid', '-L', item)
+            for item in ('l1', 'l2', 'L1')
+        ])
 
-        execute_mock.assert_any_call('mount', "/dev/disk/by-label/ir-vfd-dev",
-                                     "/tempdir")
-        read_params_mock.assert_called_once_with("/tempdir/parameters.txt")
-        exists_mock.assert_called_once_with("/dev/disk/by-label/ir-vfd-dev")
-        execute_mock.assert_any_call('umount', "/tempdir")
-        self.assertEqual(expected_params, returned_params)
-        mkdtemp_mock.assert_called_once_with()
-        rmtree_mock.assert_called_once_with("/tempdir")
+    @mock.patch.object(utils, 'execute', autospec=True)
+    def test__find_device_by_labels_not_found(self, execute_mock):
+        execute_mock.side_effect = processutils.ProcessExecutionError
+        self.assertIsNone(utils._find_device_by_labels(['l1', 'l2']))
+        execute_mock.assert_has_calls([
+            mock.call('blkid', '-L', item)
+            for item in ('l1', 'l2', 'L1', 'L2')
+        ])
 
+    @mock.patch.object(utils, '_find_device_by_labels', autospec=True)
     @mock.patch.object(shutil, 'rmtree', autospec=True)
     @mock.patch.object(tempfile, 'mkdtemp', autospec=True)
     @mock.patch.object(utils, '_read_params_from_file', autospec=True)
-    @mock.patch.object(os.path, 'exists', autospec=True)
     @mock.patch.object(os, 'mkdir', autospec=True)
     @mock.patch.object(utils, 'execute', autospec=True)
-    def test__get_vmedia_params_by_label_upper_case(
-            self, execute_mock, mkdir_mock, exists_mock, read_params_mock,
-            mkdtemp_mock, rmtree_mock):
+    def test__get_vmedia_params(
+            self, execute_mock, mkdir_mock, read_params_mock,
+            mkdtemp_mock, rmtree_mock, find_mock):
         mkdtemp_mock.return_value = "/tempdir"
+        find_mock.return_value = '/dev/fake'
 
         null_output = ["", ""]
         expected_params = {'a': 'b'}
         read_params_mock.return_value = expected_params
-        exists_mock.side_effect = [False, True]
         execute_mock.side_effect = [null_output, null_output]
 
         returned_params = utils._get_vmedia_params()
 
-        execute_mock.assert_any_call('mount', "/dev/disk/by-label/IR-VFD-DEV",
-                                     "/tempdir")
+        execute_mock.assert_any_call('mount', "/dev/fake", "/tempdir")
         read_params_mock.assert_called_once_with("/tempdir/parameters.txt")
-        exists_mock.assert_has_calls(
-            [mock.call("/dev/disk/by-label/ir-vfd-dev"),
-             mock.call("/dev/disk/by-label/IR-VFD-DEV")])
         execute_mock.assert_any_call('umount', "/tempdir")
         self.assertEqual(expected_params, returned_params)
         mkdtemp_mock.assert_called_once_with()
         rmtree_mock.assert_called_once_with("/tempdir")
 
+    @mock.patch.object(utils, '_find_device_by_labels', autospec=True)
     @mock.patch.object(shutil, 'rmtree', autospec=True)
     @mock.patch.object(tempfile, 'mkdtemp', autospec=True)
     @mock.patch.object(utils, '_get_vmedia_device', autospec=True)
     @mock.patch.object(utils, '_read_params_from_file', autospec=True)
-    @mock.patch.object(os.path, 'exists', autospec=True)
     @mock.patch.object(os, 'mkdir', autospec=True)
     @mock.patch.object(utils, 'execute', autospec=True)
     def test__get_vmedia_params_by_device(self, execute_mock, mkdir_mock,
-                                          exists_mock, read_params_mock,
-                                          get_device_mock, mkdtemp_mock,
-                                          rmtree_mock):
+                                          read_params_mock, get_device_mock,
+                                          mkdtemp_mock, rmtree_mock,
+                                          find_mock):
         mkdtemp_mock.return_value = "/tempdir"
+        find_mock.return_value = None
 
         null_output = ["", ""]
         expected_params = {'a': 'b'}
         read_params_mock.return_value = expected_params
-        exists_mock.side_effect = [False, False]
         execute_mock.side_effect = [null_output, null_output]
         get_device_mock.return_value = "sda"
 
         returned_params = utils._get_vmedia_params()
 
-        exists_mock.assert_has_calls(
-            [mock.call("/dev/disk/by-label/ir-vfd-dev"),
-             mock.call("/dev/disk/by-label/IR-VFD-DEV")])
         execute_mock.assert_any_call('mount', "/dev/sda",
                                      "/tempdir")
         read_params_mock.assert_called_once_with("/tempdir/parameters.txt")
@@ -237,102 +238,90 @@ class GetAgentParamsTestCase(ironic_agent_base.IronicAgentTest):
         mkdtemp_mock.assert_called_once_with()
         rmtree_mock.assert_called_once_with("/tempdir")
 
+    @mock.patch.object(utils, '_find_device_by_labels', autospec=True)
     @mock.patch.object(utils, '_get_vmedia_device', autospec=True)
-    @mock.patch.object(os.path, 'exists', autospec=True)
-    def test__get_vmedia_params_cannot_find_dev(self, exists_mock,
-                                                get_device_mock):
+    def test__get_vmedia_params_cannot_find_dev(self, get_device_mock,
+                                                find_mock):
+        find_mock.return_value = None
         get_device_mock.return_value = None
-        exists_mock.return_value = False
         self.assertRaises(errors.VirtualMediaBootError,
                           utils._get_vmedia_params)
 
+    @mock.patch.object(utils, '_find_device_by_labels', autospec=True)
     @mock.patch.object(shutil, 'rmtree', autospec=True)
     @mock.patch.object(tempfile, 'mkdtemp', autospec=True)
-    @mock.patch.object(utils, '_get_vmedia_device', autospec=True)
     @mock.patch.object(utils, '_read_params_from_file', autospec=True)
-    @mock.patch.object(os.path, 'exists', autospec=True)
     @mock.patch.object(os, 'mkdir', autospec=True)
     @mock.patch.object(utils, 'execute', autospec=True)
     def test__get_vmedia_params_mount_fails(self, execute_mock,
-                                            mkdir_mock, exists_mock,
-                                            read_params_mock,
-                                            get_device_mock, mkdtemp_mock,
-                                            rmtree_mock):
+                                            mkdir_mock, read_params_mock,
+                                            mkdtemp_mock, rmtree_mock,
+                                            find_mock):
+        find_mock.return_value = '/dev/fake'
         mkdtemp_mock.return_value = "/tempdir"
 
         expected_params = {'a': 'b'}
-        exists_mock.return_value = True
         read_params_mock.return_value = expected_params
-        get_device_mock.return_value = "sda"
 
         execute_mock.side_effect = processutils.ProcessExecutionError()
 
         self.assertRaises(errors.VirtualMediaBootError,
                           utils._get_vmedia_params)
 
-        execute_mock.assert_any_call('mount', "/dev/disk/by-label/ir-vfd-dev",
-                                     "/tempdir")
+        execute_mock.assert_any_call('mount', "/dev/fake", "/tempdir")
         mkdtemp_mock.assert_called_once_with()
         rmtree_mock.assert_called_once_with("/tempdir")
 
+    @mock.patch.object(utils, '_find_device_by_labels', autospec=True)
     @mock.patch.object(shutil, 'rmtree', autospec=True)
     @mock.patch.object(tempfile, 'mkdtemp', autospec=True)
-    @mock.patch.object(utils, '_get_vmedia_device', autospec=True)
     @mock.patch.object(utils, '_read_params_from_file', autospec=True)
-    @mock.patch.object(os.path, 'exists', autospec=True)
     @mock.patch.object(os, 'mkdir', autospec=True)
     @mock.patch.object(utils, 'execute', autospec=True)
     def test__get_vmedia_params_umount_fails(self, execute_mock, mkdir_mock,
-                                             exists_mock, read_params_mock,
-                                             get_device_mock, mkdtemp_mock,
-                                             rmtree_mock):
+                                             read_params_mock, mkdtemp_mock,
+                                             rmtree_mock, find_mock):
+        find_mock.return_value = '/dev/fake'
         mkdtemp_mock.return_value = "/tempdir"
 
         null_output = ["", ""]
         expected_params = {'a': 'b'}
-        exists_mock.return_value = True
         read_params_mock.return_value = expected_params
-        get_device_mock.return_value = "sda"
 
         execute_mock.side_effect = [null_output,
                                     processutils.ProcessExecutionError()]
 
         returned_params = utils._get_vmedia_params()
 
-        execute_mock.assert_any_call('mount', "/dev/disk/by-label/ir-vfd-dev",
-                                     "/tempdir")
+        execute_mock.assert_any_call('mount', "/dev/fake", "/tempdir")
         read_params_mock.assert_called_once_with("/tempdir/parameters.txt")
         execute_mock.assert_any_call('umount', "/tempdir")
         self.assertEqual(expected_params, returned_params)
         mkdtemp_mock.assert_called_once_with()
         rmtree_mock.assert_called_once_with("/tempdir")
 
+    @mock.patch.object(utils, '_find_device_by_labels', autospec=True)
     @mock.patch.object(shutil, 'rmtree', autospec=True)
     @mock.patch.object(tempfile, 'mkdtemp', autospec=True)
-    @mock.patch.object(utils, '_get_vmedia_device', autospec=True)
     @mock.patch.object(utils, '_read_params_from_file', autospec=True)
-    @mock.patch.object(os.path, 'exists', autospec=True)
     @mock.patch.object(os, 'mkdir', autospec=True)
     @mock.patch.object(utils, 'execute', autospec=True)
     def test__get_vmedia_params_rmtree_fails(self, execute_mock, mkdir_mock,
-                                             exists_mock, read_params_mock,
-                                             get_device_mock, mkdtemp_mock,
-                                             rmtree_mock):
+                                             read_params_mock, mkdtemp_mock,
+                                             rmtree_mock, find_mock):
+        find_mock.return_value = '/dev/fake'
         mkdtemp_mock.return_value = "/tempdir"
         rmtree_mock.side_effect = Exception
 
         null_output = ["", ""]
         expected_params = {'a': 'b'}
-        exists_mock.return_value = True
         read_params_mock.return_value = expected_params
-        get_device_mock.return_value = "sda"
 
         execute_mock.return_value = null_output
 
         returned_params = utils._get_vmedia_params()
 
-        execute_mock.assert_any_call('mount', "/dev/disk/by-label/ir-vfd-dev",
-                                     "/tempdir")
+        execute_mock.assert_any_call('mount', "/dev/fake", "/tempdir")
         read_params_mock.assert_called_once_with("/tempdir/parameters.txt")
         execute_mock.assert_any_call('umount', "/tempdir")
         self.assertEqual(expected_params, returned_params)
@@ -1025,3 +1014,69 @@ class TestGetEfiPart(testtools.TestCase):
             {'number': '14', 'flags': 'bios_grub'},
         ]
         self.assertIsNone(utils.get_efi_part_on_device('/dev/sda'))
+
+
+@mock.patch.object(utils, '_find_device_by_labels', autospec=True)
+@mock.patch.object(shutil, 'copy', autospec=True)
+@mock.patch.object(utils, 'execute', autospec=True)
+class TestCopyConfigFromVmedia(testtools.TestCase):
+
+    def test_no_vmedia(self, mock_execute, mock_copy, mock_find_device):
+        mock_find_device.return_value = None
+        utils.copy_config_from_vmedia()
+        mock_execute.assert_not_called()
+        mock_copy.assert_not_called()
+
+    def test_no_files(self, mock_execute, mock_copy, mock_find_device):
+        mock_find_device.return_value = '/dev/something'
+        utils.copy_config_from_vmedia()
+        mock_execute.assert_has_calls([
+            mock.call('mount', '/dev/something', mock.ANY),
+            mock.call('umount', mock.ANY),
+        ])
+        mock_copy.assert_not_called()
+
+    @mock.patch.object(os, 'makedirs', autospec=True)
+    def test_copy(self, mock_makedirs, mock_execute, mock_copy,
+                  mock_find_device):
+        mock_find_device.return_value = '/dev/something'
+        path = None
+
+        def _fake_exec(command, arg1, arg2=None):
+            nonlocal path
+            if command == 'mount':
+                path = arg2
+                self.assertTrue(os.path.isdir(path))
+                # NOTE(dtantsur): makedirs is mocked
+                os.mkdir(os.path.join(path, 'etc'))
+                os.mkdir(os.path.join(path, 'etc', 'ironic-python-agent'))
+                os.mkdir(os.path.join(path, 'etc', 'ironic-python-agent.d'))
+                with open(os.path.join(path, 'not copied'), 'wt') as fp:
+                    fp.write('not copied')
+                with open(os.path.join(path, 'etc', 'ironic-python-agent',
+                                       'ironic.crt'), 'wt') as fp:
+                    fp.write('I am a cert')
+                with open(os.path.join(path, 'etc', 'ironic-python-agent.d',
+                                       'ironic.conf'), 'wt') as fp:
+                    fp.write('I am a config')
+            else:
+                self.assertEqual('umount', command)
+
+        mock_find_device.return_value = '/dev/something'
+        mock_execute.side_effect = _fake_exec
+
+        utils.copy_config_from_vmedia()
+
+        mock_makedirs.assert_has_calls([
+            mock.call('/etc/ironic-python-agent', exist_ok=True),
+            mock.call('/etc/ironic-python-agent.d', exist_ok=True),
+        ], any_order=True)
+        mock_execute.assert_has_calls([
+            mock.call('mount', '/dev/something', mock.ANY),
+            mock.call('umount', mock.ANY),
+        ])
+        mock_copy.assert_has_calls([
+            mock.call(mock.ANY, '/etc/ironic-python-agent/ironic.crt'),
+            mock.call(mock.ANY, '/etc/ironic-python-agent.d/ironic.conf'),
+        ], any_order=True)
+        self.assertFalse(os.path.exists(path))
diff --git a/ironic_python_agent/utils.py b/ironic_python_agent/utils.py
index 637737dd..b2310d00 100644
--- a/ironic_python_agent/utils.py
+++ b/ironic_python_agent/utils.py
@@ -13,6 +13,7 @@
 # limitations under the License.
 
 from collections import abc
+import contextlib
 import copy
 import errno
 import glob
@@ -21,6 +22,7 @@ import os
 import re
 import shutil
 import subprocess
+import sys
 import tarfile
 import tempfile
 import time
@@ -142,6 +144,42 @@ def _get_vmedia_device():
             pass
 
 
+@contextlib.contextmanager
+def _mounted(source):
+    """A context manager for a temporary mount."""
+    dest = tempfile.mkdtemp()
+    try:
+        try:
+            execute("mount", source, dest)
+        except processutils.ProcessExecutionError as e:
+            msg = ("Unable to mount virtual media device %(device)s: "
+                   "%(error)s" % {'device': source, 'error': e})
+            raise errors.VirtualMediaBootError(msg)
+
+        yield dest
+    finally:
+        try:
+            execute("umount", dest)
+        except processutils.ProcessExecutionError:
+            pass
+
+        try:
+            shutil.rmtree(dest)
+        except Exception:
+            pass
+
+
+def _find_device_by_labels(labels):
+    """Find device matching any of the provided labels."""
+    for label in labels + [lbl.upper() for lbl in labels]:
+        try:
+            path, _e = execute('blkid', '-L', label)
+        except processutils.ProcessExecutionError:
+            pass
+        else:
+            return path.strip()
+
+
 def _get_vmedia_params():
     """This method returns the parameters passed through virtual media floppy.
 
@@ -149,15 +187,8 @@ def _get_vmedia_params():
     :raises: VirtualMediaBootError when it cannot find the virtual media device
     """
     parameters_file = "parameters.txt"
-
-    vmedia_device_file_lower_case = "/dev/disk/by-label/ir-vfd-dev"
-    vmedia_device_file_upper_case = "/dev/disk/by-label/IR-VFD-DEV"
-    if os.path.exists(vmedia_device_file_lower_case):
-        vmedia_device_file = vmedia_device_file_lower_case
-    elif os.path.exists(vmedia_device_file_upper_case):
-        vmedia_device_file = vmedia_device_file_upper_case
-    else:
-
+    vmedia_device_file = _find_device_by_labels(['ir-vfd-dev'])
+    if not vmedia_device_file:
         # TODO(rameshg87): This block of code is there only for compatibility
         # reasons (so that newer agent can work with older Ironic). Remove
         # this after Liberty release.
@@ -168,33 +199,55 @@ def _get_vmedia_params():
 
         vmedia_device_file = os.path.join("/dev", vmedia_device)
 
-    vmedia_mount_point = tempfile.mkdtemp()
-    try:
-        try:
-            stdout, stderr = execute("mount", vmedia_device_file,
-                                     vmedia_mount_point)
-        except processutils.ProcessExecutionError as e:
-            msg = ("Unable to mount virtual media device %(device)s: "
-                   "%(error)s" % {'device': vmedia_device_file, 'error': e})
-            raise errors.VirtualMediaBootError(msg)
-
+    with _mounted(vmedia_device_file) as vmedia_mount_point:
         parameters_file_path = os.path.join(vmedia_mount_point,
                                             parameters_file)
         params = _read_params_from_file(parameters_file_path)
 
-        try:
-            stdout, stderr = execute("umount", vmedia_mount_point)
-        except processutils.ProcessExecutionError:
-            pass
-    finally:
-        try:
-            shutil.rmtree(vmedia_mount_point)
-        except Exception:
-            pass
-
     return params
 
 
+def _early_log(msg, *args):
+    """Log via printing (before oslo.log is configured)."""
+    print('ironic-python-agent:', msg % args, file=sys.stderr)
+
+
+def copy_config_from_vmedia():
+    """Copies any configuration from a virtual media device.
+
+    Copies files under /etc/ironic-python-agent and /etc/ironic-python-agent.d.
+    """
+    vmedia_device_file = _find_device_by_labels(
+        ['config-2', 'vmedia_boot_iso'])
+    if not vmedia_device_file:
+        _early_log('No virtual media device detected')
+        return
+
+    with _mounted(vmedia_device_file) as vmedia_mount_point:
+        for ext in ('', '.d'):
+            src = os.path.join(vmedia_mount_point, 'etc',
+                               'ironic-python-agent%s' % ext)
+            if not os.path.isdir(src):
+                _early_log('%s not found', src)
+                continue
+
+            dest = '/etc/ironic-python-agent%s' % ext
+            _early_log('Copying configuration from %s to %s', src, dest)
+            try:
+                os.makedirs(dest, exist_ok=True)
+
+                # TODO(dtantsur): use shutil.copytree(.., dirs_exist_ok=True)
+                # when the minimum supported Python is 3.8.
+                for name in os.listdir(src):
+                    src_file = os.path.join(src, name)
+                    dst_file = os.path.join(dest, name)
+                    shutil.copy(src_file, dst_file)
+            except Exception as exc:
+                msg = ("Unable to copy vmedia configuration %s to %s: %s"
+                       % (src, dest, exc))
+                raise errors.VirtualMediaBootError(msg)
+
+
 def _get_cached_params():
     """Helper method to get cached params to ease unit testing."""
     return AGENT_PARAMS_CACHED
diff --git a/releasenotes/notes/vmedia-copy-6a58f3183b166c42.yaml b/releasenotes/notes/vmedia-copy-6a58f3183b166c42.yaml
new file mode 100644
index 00000000..0eda31d8
--- /dev/null
+++ b/releasenotes/notes/vmedia-copy-6a58f3183b166c42.yaml
@@ -0,0 +1,5 @@
+---
+other:
+  - |
+    Agent configuration files found on attached virtual media or config drive
+    devices are now copied to the ramdisk and loaded on start up.