openstack
/
ironic-python-agent
Code Issues Proposed changes

Merge "Copy any configuration from the virtual media"

This commit is contained in:
Zuul 2020-12-14 18:01:06 +00:00 committed by Gerrit Code Review
parent 67ee66785c b9b67fad77
commit 13cfa68974
4 changed files with 225 additions and 107 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
ironic_python_agent/cmd/agent.py
View File

@ -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

211
ironic_python_agent/tests/unit/test_utils.py
View File

@ -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(
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 = [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",
"/tempdir")
execute_mock.assert_any_call('mount', "/dev/fake", "/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,
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))

111
ironic_python_agent/utils.py
View File

@ -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

5
releasenotes/notes/vmedia-copy-6a58f3183b166c42.yaml Normal file
View File

@ -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.