summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDmitry Tantsur <dtantsur@protonmail.com>2020-11-18 16:48:27 +0100
committerDmitry Tantsur <dtantsur@protonmail.com>2020-11-23 16:04:45 +0100
commitb9b67fad77bc03f1258db28ec20979e8cdaf2459 (patch)
treed92b9f85f969f7eca8ef0aed3ed4ab35ec5afb77
parent2c91494941c85306a74a331f2f22c4ecaa792bfb (diff)
downloadironic-python-agent-b9b67fad77bc03f1258db28ec20979e8cdaf2459.tar.gz
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
-rw-r--r--ironic_python_agent/cmd/agent.py5
-rw-r--r--ironic_python_agent/tests/unit/test_utils.py203
-rw-r--r--ironic_python_agent/utils.py111
-rw-r--r--releasenotes/notes/vmedia-copy-6a58f3183b166c42.yaml5
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.