From c6cd3a8190332b4b778e6ac63b2985eee72135e9 Mon Sep 17 00:00:00 2001 From: Lucas Alvares Gomes Date: Mon, 9 Mar 2015 12:23:12 +0000 Subject: [PATCH] Move _get_agent_params() to a common place The function _get_agent_params() parse the parameters passed to the agent via kernel cmdline or vmedia. Other parts of the code needs to access these parameters as well, so this patch is moving _get_agent_params() and the related functions to a common place (utils.py). Change-Id: I860f84d1d13511fff56d4aa56358ee597a9760d5 --- ironic_python_agent/cmd/agent.py | 105 +------------------- ironic_python_agent/netutils.py | 5 + ironic_python_agent/tests/agent.py | 148 ---------------------------- ironic_python_agent/tests/utils.py | 149 +++++++++++++++++++++++++++++ ironic_python_agent/utils.py | 101 +++++++++++++++++++ 5 files changed, 256 insertions(+), 252 deletions(-) diff --git a/ironic_python_agent/cmd/agent.py b/ironic_python_agent/cmd/agent.py index 77c3b4b10..4befd5d7c 100644 --- a/ironic_python_agent/cmd/agent.py +++ b/ironic_python_agent/cmd/agent.py @@ -12,119 +12,16 @@ # See the License for the specific language governing permissions and # limitations under the License. -import glob -import os - -from oslo_concurrency import processutils from oslo_config import cfg from ironic_python_agent import agent -from ironic_python_agent import errors from ironic_python_agent.openstack.common import log from ironic_python_agent import utils CONF = cfg.CONF -def _read_params_from_file(filepath): - """Extract key=value pairs from a file. - - :param filepath: path to a file containing key=value pairs separated by - whitespace or newlines. - :returns: a dictionary representing the content of the file - """ - with open(filepath) as f: - cmdline = f.read() - - options = cmdline.split() - params = {} - for option in options: - if '=' not in option: - continue - k, v = option.split('=', 1) - params[k] = v - - return params - - -def _get_agent_params(): - """Gets parameters passed to the agent via kernel cmdline or vmedia. - - Parameters can be passed using either the kernel commandline or through - virtual media. If boot_method is vmedia, merge params provided via vmedia - with those read from the kernel command line. - - Although it should never happen, if a variable is both set by vmedia and - kernel command line, the setting in vmedia will take precedence. - - :returns: a dict of potential configuration parameters for the agent - """ - params = _read_params_from_file('/proc/cmdline') - - # If the node booted over virtual media, the parameters are passed - # in a text file within the virtual media floppy. - if params.get('boot_method', None) == 'vmedia': - vmedia_params = _get_vmedia_params() - params.update(vmedia_params) - - return params - - -def _get_vmedia_device(): - """Finds the device filename of the virtual media device using sysfs. - - :returns: a string containing the filename of the virtual media device - """ - sysfs_device_models = glob.glob("/sys/class/block/*/device/model") - vmedia_device_model = "virtual media" - for model_file in sysfs_device_models: - try: - with open(model_file) as model_file_fobj: - if vmedia_device_model in model_file_fobj.read().lower(): - vmedia_device = model_file.split('/')[4] - return vmedia_device - except Exception: - pass - - -def _get_vmedia_params(): - """This method returns the parameters passed to the agent through virtual - media floppy. - - :returns: a partial dict of potential agent configuration parameters - :raises: VirtualMediaBootError when it cannot find the virtual media device - """ - vmedia_mount_point = "/vmedia_mnt" - parameters_file = "parameters.txt" - - vmedia_device = _get_vmedia_device() - if not vmedia_device: - msg = "Unable to find virtual media device" - raise errors.VirtualMediaBootError(msg) - - vmedia_device_file = os.path.join("/dev", vmedia_device) - os.mkdir(vmedia_mount_point) - - try: - stdout, stderr = utils.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 = utils.execute("umount", vmedia_mount_point) - except processutils.ProcessExecutionError as e: - pass - - return params - - -APARAMS = _get_agent_params() +APARAMS = utils.get_agent_params() cli_opts = [ cfg.StrOpt('api_url', diff --git a/ironic_python_agent/netutils.py b/ironic_python_agent/netutils.py index 299b474b3..8b005eba4 100644 --- a/ironic_python_agent/netutils.py +++ b/ironic_python_agent/netutils.py @@ -22,6 +22,11 @@ import sys from oslo_config import cfg +# FIXME(lucasagomes): If you don't import the agent module the tests in +# this file will fail, it was working before because the agent module was +# being imported at tests/agent.py +from ironic_python_agent.cmd import agent # noqa + LOG = logging.getLogger(__name__) CONF = cfg.CONF diff --git a/ironic_python_agent/tests/agent.py b/ironic_python_agent/tests/agent.py index 63ef31b9c..a6e2cd7a4 100644 --- a/ironic_python_agent/tests/agent.py +++ b/ironic_python_agent/tests/agent.py @@ -12,34 +12,23 @@ # See the License for the specific language governing permissions and # limitations under the License. -import glob import json -import os import time import mock -from oslo_concurrency import processutils from oslotest import base as test_base import pkg_resources -import six from stevedore import extension from wsgiref import simple_server from ironic_python_agent import agent -from ironic_python_agent.cmd import agent as agent_cmd from ironic_python_agent import encoding from ironic_python_agent import errors from ironic_python_agent.extensions import base from ironic_python_agent import hardware -from ironic_python_agent import utils EXPECTED_ERROR = RuntimeError('command execution failed') -if six.PY2: - OPEN_FUNCTION_NAME = '__builtin__.open' -else: - OPEN_FUNCTION_NAME = 'builtins.open' - def foo_execute(*args, **kwargs): if kwargs['fail']: @@ -352,140 +341,3 @@ class TestAgentStandalone(test_base.BaseTestCase): self.assertFalse(self.agent.heartbeater.called) self.assertFalse(self.agent.api_client.lookup_node.called) - - -class TestAgentCmd(test_base.BaseTestCase): - @mock.patch('ironic_python_agent.openstack.common.log.getLogger') - @mock.patch(OPEN_FUNCTION_NAME) - def test__read_params_from_file_fail(self, logger_mock, open_mock): - open_mock.side_effect = Exception - params = agent_cmd._read_params_from_file('file-path') - self.assertEqual(params, {}) - - @mock.patch(OPEN_FUNCTION_NAME) - def test__read_params_from_file(self, open_mock): - kernel_line = 'api-url=http://localhost:9999 baz foo=bar\n' - open_mock.return_value.__enter__ = lambda s: s - open_mock.return_value.__exit__ = mock.Mock() - read_mock = open_mock.return_value.read - read_mock.return_value = kernel_line - params = agent_cmd._read_params_from_file('file-path') - open_mock.assert_called_once_with('file-path') - read_mock.assert_called_once_with() - self.assertEqual(params['api-url'], 'http://localhost:9999') - self.assertEqual(params['foo'], 'bar') - self.assertFalse('baz' in params) - - @mock.patch.object(agent_cmd, '_read_params_from_file') - def test__get_agent_params_kernel_cmdline(self, read_params_mock): - - expected_params = {'a': 'b'} - read_params_mock.return_value = expected_params - returned_params = agent_cmd._get_agent_params() - read_params_mock.assert_called_once_with('/proc/cmdline') - self.assertEqual(expected_params, returned_params) - - @mock.patch.object(agent_cmd, '_get_vmedia_params') - @mock.patch.object(agent_cmd, '_read_params_from_file') - def test__get_agent_params_vmedia(self, read_params_mock, - get_vmedia_params_mock): - - kernel_params = {'boot_method': 'vmedia'} - vmedia_params = {'a': 'b'} - expected_params = dict(kernel_params.items() + - vmedia_params.items()) - read_params_mock.return_value = kernel_params - get_vmedia_params_mock.return_value = vmedia_params - - returned_params = agent_cmd._get_agent_params() - read_params_mock.assert_called_once_with('/proc/cmdline') - self.assertEqual(expected_params, returned_params) - - @mock.patch(OPEN_FUNCTION_NAME) - @mock.patch.object(glob, 'glob') - def test__get_vmedia_device(self, glob_mock, open_mock): - - glob_mock.return_value = ['/sys/class/block/sda/device/model', - '/sys/class/block/sdb/device/model', - '/sys/class/block/sdc/device/model'] - fobj_mock = mock.MagicMock() - mock_file_handle = mock.MagicMock(spec=file) - mock_file_handle.__enter__.return_value = fobj_mock - open_mock.return_value = mock_file_handle - - fobj_mock.read.side_effect = ['scsi disk', Exception, 'Virtual Media'] - vmedia_device_returned = agent_cmd._get_vmedia_device() - self.assertEqual('sdc', vmedia_device_returned) - - @mock.patch.object(agent_cmd, '_get_vmedia_device') - @mock.patch.object(agent_cmd, '_read_params_from_file') - @mock.patch.object(os, 'mkdir') - @mock.patch.object(utils, 'execute') - def test__get_vmedia_params(self, execute_mock, mkdir_mock, - read_params_mock, get_device_mock): - vmedia_mount_point = "/vmedia_mnt" - - null_output = ["", ""] - expected_params = {'a': 'b'} - read_params_mock.return_value = expected_params - execute_mock.side_effect = [null_output, null_output] - get_device_mock.return_value = "sda" - - returned_params = agent_cmd._get_vmedia_params() - - mkdir_mock.assert_called_once_with(vmedia_mount_point) - execute_mock.assert_any_call('mount', "/dev/sda", vmedia_mount_point) - read_params_mock.assert_called_once_with("/vmedia_mnt/parameters.txt") - execute_mock.assert_any_call('umount', vmedia_mount_point) - self.assertEqual(expected_params, returned_params) - - @mock.patch.object(agent_cmd, '_get_vmedia_device') - def test__get_vmedia_params_cannot_find_dev(self, get_device_mock): - get_device_mock.return_value = None - self.assertRaises(errors.VirtualMediaBootError, - agent_cmd._get_vmedia_params) - - @mock.patch.object(agent_cmd, '_get_vmedia_device') - @mock.patch.object(agent_cmd, '_read_params_from_file') - @mock.patch.object(os, 'mkdir') - @mock.patch.object(utils, 'execute') - def test__get_vmedia_params_mount_fails(self, execute_mock, - mkdir_mock, read_params_mock, - get_device_mock): - vmedia_mount_point = "/vmedia_mnt" - - expected_params = {'a': 'b'} - read_params_mock.return_value = expected_params - get_device_mock.return_value = "sda" - - execute_mock.side_effect = processutils.ProcessExecutionError() - - self.assertRaises(errors.VirtualMediaBootError, - agent_cmd._get_vmedia_params) - - mkdir_mock.assert_called_once_with(vmedia_mount_point) - execute_mock.assert_any_call('mount', "/dev/sda", vmedia_mount_point) - - @mock.patch.object(agent_cmd, '_get_vmedia_device') - @mock.patch.object(agent_cmd, '_read_params_from_file') - @mock.patch.object(os, 'mkdir') - @mock.patch.object(utils, 'execute') - def test__get_vmedia_params_umount_fails(self, execute_mock, mkdir_mock, - read_params_mock, get_device_mock): - vmedia_mount_point = "/vmedia_mnt" - - null_output = ["", ""] - expected_params = {'a': 'b'} - read_params_mock.return_value = expected_params - get_device_mock.return_value = "sda" - - execute_mock.side_effect = [null_output, - processutils.ProcessExecutionError()] - - returned_params = agent_cmd._get_vmedia_params() - - mkdir_mock.assert_called_once_with(vmedia_mount_point) - execute_mock.assert_any_call('mount', "/dev/sda", vmedia_mount_point) - read_params_mock.assert_called_once_with("/vmedia_mnt/parameters.txt") - execute_mock.assert_any_call('umount', vmedia_mount_point) - self.assertEqual(expected_params, returned_params) diff --git a/ironic_python_agent/tests/utils.py b/ironic_python_agent/tests/utils.py index 2d86b65aa..e47760c71 100644 --- a/ironic_python_agent/tests/utils.py +++ b/ironic_python_agent/tests/utils.py @@ -14,15 +14,26 @@ # under the License. import errno +import glob import os import tempfile import testtools +import mock from oslo_concurrency import processutils +from oslotest import base as test_base +import six +from ironic_python_agent import errors from ironic_python_agent import utils +if six.PY2: + OPEN_FUNCTION_NAME = '__builtin__.open' +else: + OPEN_FUNCTION_NAME = 'builtins.open' + + class ExecuteTestCase(testtools.TestCase): """This class is a copy of the same class in openstack/ironic.""" @@ -119,3 +130,141 @@ grep foo finally: os.unlink(tmpfilename) os.unlink(tmpfilename2) + + +class GetAgentParamsTestCase(test_base.BaseTestCase): + + @mock.patch('ironic_python_agent.openstack.common.log.getLogger') + @mock.patch(OPEN_FUNCTION_NAME) + def test__read_params_from_file_fail(self, logger_mock, open_mock): + open_mock.side_effect = Exception + params = utils._read_params_from_file('file-path') + self.assertEqual(params, {}) + + @mock.patch(OPEN_FUNCTION_NAME) + def test__read_params_from_file(self, open_mock): + kernel_line = 'api-url=http://localhost:9999 baz foo=bar\n' + open_mock.return_value.__enter__ = lambda s: s + open_mock.return_value.__exit__ = mock.Mock() + read_mock = open_mock.return_value.read + read_mock.return_value = kernel_line + params = utils._read_params_from_file('file-path') + open_mock.assert_called_once_with('file-path') + read_mock.assert_called_once_with() + self.assertEqual(params['api-url'], 'http://localhost:9999') + self.assertEqual(params['foo'], 'bar') + self.assertFalse('baz' in params) + + @mock.patch.object(utils, '_read_params_from_file') + def test_get_agent_params_kernel_cmdline(self, read_params_mock): + + expected_params = {'a': 'b'} + read_params_mock.return_value = expected_params + returned_params = utils.get_agent_params() + read_params_mock.assert_called_once_with('/proc/cmdline') + self.assertEqual(expected_params, returned_params) + + @mock.patch.object(utils, '_get_vmedia_params') + @mock.patch.object(utils, '_read_params_from_file') + def test_get_agent_params_vmedia(self, read_params_mock, + get_vmedia_params_mock): + + kernel_params = {'boot_method': 'vmedia'} + vmedia_params = {'a': 'b'} + expected_params = dict(kernel_params.items() + + vmedia_params.items()) + read_params_mock.return_value = kernel_params + get_vmedia_params_mock.return_value = vmedia_params + + returned_params = utils.get_agent_params() + read_params_mock.assert_called_once_with('/proc/cmdline') + self.assertEqual(expected_params, returned_params) + + @mock.patch(OPEN_FUNCTION_NAME) + @mock.patch.object(glob, 'glob') + def test__get_vmedia_device(self, glob_mock, open_mock): + + glob_mock.return_value = ['/sys/class/block/sda/device/model', + '/sys/class/block/sdb/device/model', + '/sys/class/block/sdc/device/model'] + fobj_mock = mock.MagicMock() + mock_file_handle = mock.MagicMock(spec=file) + mock_file_handle.__enter__.return_value = fobj_mock + open_mock.return_value = mock_file_handle + + fobj_mock.read.side_effect = ['scsi disk', Exception, 'Virtual Media'] + vmedia_device_returned = utils._get_vmedia_device() + self.assertEqual('sdc', vmedia_device_returned) + + @mock.patch.object(utils, '_get_vmedia_device') + @mock.patch.object(utils, '_read_params_from_file') + @mock.patch.object(os, 'mkdir') + @mock.patch.object(utils, 'execute') + def test__get_vmedia_params(self, execute_mock, mkdir_mock, + read_params_mock, get_device_mock): + vmedia_mount_point = "/vmedia_mnt" + + null_output = ["", ""] + expected_params = {'a': 'b'} + read_params_mock.return_value = expected_params + execute_mock.side_effect = [null_output, null_output] + get_device_mock.return_value = "sda" + + returned_params = utils._get_vmedia_params() + + mkdir_mock.assert_called_once_with(vmedia_mount_point) + execute_mock.assert_any_call('mount', "/dev/sda", vmedia_mount_point) + read_params_mock.assert_called_once_with("/vmedia_mnt/parameters.txt") + execute_mock.assert_any_call('umount', vmedia_mount_point) + self.assertEqual(expected_params, returned_params) + + @mock.patch.object(utils, '_get_vmedia_device') + def test__get_vmedia_params_cannot_find_dev(self, get_device_mock): + get_device_mock.return_value = None + self.assertRaises(errors.VirtualMediaBootError, + utils._get_vmedia_params) + + @mock.patch.object(utils, '_get_vmedia_device') + @mock.patch.object(utils, '_read_params_from_file') + @mock.patch.object(os, 'mkdir') + @mock.patch.object(utils, 'execute') + def test__get_vmedia_params_mount_fails(self, execute_mock, + mkdir_mock, read_params_mock, + get_device_mock): + vmedia_mount_point = "/vmedia_mnt" + + expected_params = {'a': 'b'} + 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) + + mkdir_mock.assert_called_once_with(vmedia_mount_point) + execute_mock.assert_any_call('mount', "/dev/sda", vmedia_mount_point) + + @mock.patch.object(utils, '_get_vmedia_device') + @mock.patch.object(utils, '_read_params_from_file') + @mock.patch.object(os, 'mkdir') + @mock.patch.object(utils, 'execute') + def test__get_vmedia_params_umount_fails(self, execute_mock, mkdir_mock, + read_params_mock, get_device_mock): + vmedia_mount_point = "/vmedia_mnt" + + null_output = ["", ""] + expected_params = {'a': 'b'} + 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() + + mkdir_mock.assert_called_once_with(vmedia_mount_point) + execute_mock.assert_any_call('mount', "/dev/sda", vmedia_mount_point) + read_params_mock.assert_called_once_with("/vmedia_mnt/parameters.txt") + execute_mock.assert_any_call('umount', vmedia_mount_point) + self.assertEqual(expected_params, returned_params) diff --git a/ironic_python_agent/utils.py b/ironic_python_agent/utils.py index 195783c2e..174468819 100644 --- a/ironic_python_agent/utils.py +++ b/ironic_python_agent/utils.py @@ -13,9 +13,12 @@ # limitations under the License. import collections +import glob +import os from oslo_concurrency import processutils +from ironic_python_agent import errors from ironic_python_agent.openstack.common import _i18n as gtu from ironic_python_agent.openstack.common import log as logging @@ -39,3 +42,101 @@ def execute(*cmd, **kwargs): LOG.debug(gtu._('Command stdout is: "%s"') % result[0]) LOG.debug(gtu._('Command stderr is: "%s"') % result[1]) return result + + +def _read_params_from_file(filepath): + """Extract key=value pairs from a file. + + :param filepath: path to a file containing key=value pairs separated by + whitespace or newlines. + :returns: a dictionary representing the content of the file + """ + with open(filepath) as f: + cmdline = f.read() + + options = cmdline.split() + params = {} + for option in options: + if '=' not in option: + continue + k, v = option.split('=', 1) + params[k] = v + + return params + + +def _get_vmedia_device(): + """Finds the device filename of the virtual media device using sysfs. + + :returns: a string containing the filename of the virtual media device + """ + sysfs_device_models = glob.glob("/sys/class/block/*/device/model") + vmedia_device_model = "virtual media" + for model_file in sysfs_device_models: + try: + with open(model_file) as model_file_fobj: + if vmedia_device_model in model_file_fobj.read().lower(): + vmedia_device = model_file.split('/')[4] + return vmedia_device + except Exception: + pass + + +def _get_vmedia_params(): + """This method returns the parameters passed to the agent through virtual + media floppy. + + :returns: a partial dict of potential agent configuration parameters + :raises: VirtualMediaBootError when it cannot find the virtual media device + """ + vmedia_mount_point = "/vmedia_mnt" + parameters_file = "parameters.txt" + + vmedia_device = _get_vmedia_device() + if not vmedia_device: + msg = "Unable to find virtual media device" + raise errors.VirtualMediaBootError(msg) + + vmedia_device_file = os.path.join("/dev", vmedia_device) + os.mkdir(vmedia_mount_point) + + 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 as e: + pass + + return params + + +def get_agent_params(): + """Gets parameters passed to the agent via kernel cmdline or vmedia. + + Parameters can be passed using either the kernel commandline or through + virtual media. If boot_method is vmedia, merge params provided via vmedia + with those read from the kernel command line. + + Although it should never happen, if a variable is both set by vmedia and + kernel command line, the setting in vmedia will take precedence. + + :returns: a dict of potential configuration parameters for the agent + """ + params = _read_params_from_file('/proc/cmdline') + + # If the node booted over virtual media, the parameters are passed + # in a text file within the virtual media floppy. + if params.get('boot_method', None) == 'vmedia': + vmedia_params = _get_vmedia_params() + params.update(vmedia_params) + + return params