Copy any configuration from the virtual media

For ramdisk TLS (and other potential future enhancements) we need
to be able to inject configuration and certificates into the ramdisk.
Since we cannot pass files through kernel parameters, we need to
put them on the generated ISO or (in the future) config drive.

This change detects IPA configuration and copies it into the ramdisk
early enough for any configuration files to get picked.

Changed /dev/disk/by-label to blkid since the former may not exist
on all ramdisks (e.g. tinyIPA).

Change-Id: Ic64d7842a59795bbf02f194221dedc07c6b56e8c

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

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(utils, 'execute', autospec=True)
+    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')
+        ])
+
+    @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')
+        ])
+
+    @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_lower_case(
+    def test__get_vmedia_params(
-            self, execute_mock, mkdir_mock, exists_mock, read_params_mock,
+            self, execute_mock, mkdir_mock, read_params_mock,
-            mkdtemp_mock, rmtree_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 = [True, False]
         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",
+        execute_mock.assert_any_call('mount', "/dev/fake", "/tempdir")
-                                     "/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(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):
-        mkdtemp_mock.return_value = "/tempdir"
-
-        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")
-        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,
+                                          read_params_mock, get_device_mock,
-                                          get_device_mock, mkdtemp_mock,
+                                          mkdtemp_mock, rmtree_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, get_device_mock,
-    def test__get_vmedia_params_cannot_find_dev(self, exists_mock,
+                                                find_mock):
-                                                get_device_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,
+                                            mkdir_mock, read_params_mock,
-                                            read_params_mock,
+                                            mkdtemp_mock, rmtree_mock,
-                                            get_device_mock, mkdtemp_mock,
+                                            find_mock):
-                                            rmtree_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",
+        execute_mock.assert_any_call('mount', "/dev/fake", "/tempdir")
-                                     "/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,
+                                             read_params_mock, mkdtemp_mock,
-                                             get_device_mock, mkdtemp_mock,
+                                             rmtree_mock, find_mock):
-                                             rmtree_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",
+        execute_mock.assert_any_call('mount', "/dev/fake", "/tempdir")
-                                     "/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,
+                                             read_params_mock, mkdtemp_mock,
-                                             get_device_mock, mkdtemp_mock,
+                                             rmtree_mock, find_mock):
-                                             rmtree_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",
+        execute_mock.assert_any_call('mount', "/dev/fake", "/tempdir")
-                                     "/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))

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 = _find_device_by_labels(['ir-vfd-dev'])
-    vmedia_device_file_lower_case = "/dev/disk/by-label/ir-vfd-dev"
+    if not vmedia_device_file:
-    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:
-
         # 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()
+    with _mounted(vmedia_device_file) as vmedia_mount_point:
-    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)
-
         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

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.