openstack
/
validations-libs
Code Issues Proposed changes

Improved log path handling 

Validations logging now has several documented fallbacks.
constants.py now includes constant default path,
in order to improve readability and provide reference.

utils.py contains new test function to detect and log
when validations logs are stored in volatile /tmp.

Tests are included.

Complementary patch for validations_commons callbacks
is optional, as the behavior should stay largely the same.

Depends-On: https://review.opendev.org/c/openstack/validations-common/+/789782

Signed-off-by: Jiri Podivin <jpodivin@redhat.com>
Change-Id: I02df49532974cd0bcb2df8176b19ffc1cd5a65d1
This commit is contained in:
Jiri Podivin 2021-05-03 11:36:29 +02:00
parent 36c9b8cbcb
commit 55d2382ac7
5 changed files with 189 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
validations_libs/constants.py
View File

@ -24,9 +24,7 @@ VALIDATION_GROUPS = ['no-op',
'prep', 'prep',
'post'] 'post']
VALIDATIONS_LOG_BASEDIR = ('/var/log/validations' VALIDATIONS_LOG_BASEDIR = os.path.join(os.environ.get('HOME'), 'validations')
if os.path.exists('/var/log/validations') else
os.getcwd())
VALIDATION_ANSIBLE_ARTIFACT_PATH = '{}/artifacts/'.format( VALIDATION_ANSIBLE_ARTIFACT_PATH = '{}/artifacts/'.format(
VALIDATIONS_LOG_BASEDIR) VALIDATIONS_LOG_BASEDIR)

69
validations_libs/tests/test_utils.py
View File

@ -47,17 +47,76 @@ class TestUtils(TestCase):
utils.get_validations_data, utils.get_validations_data,
validation) validation)
@mock.patch('validations_libs.utils.LOG.error')
@mock.patch('validations_libs.utils.LOG.warning')
@mock.patch('validations_libs.utils.os.path.exists',
return_value=True)
@mock.patch('validations_libs.utils.current_time', @mock.patch('validations_libs.utils.current_time',
return_value='2020-04-02T06:58:20.352272Z') return_value='2020-04-02T06:58:20.352272Z')
@mock.patch('os.makedirs') @mock.patch('os.makedirs')
@mock.patch('uuid.uuid4', return_value='1234') @mock.patch('uuid.uuid4', return_value='1234')
def test_create_artifacts_dir(self, mock_uuid, mock_makedirs, def test_create_artifacts_dir(self, mock_uuid, mock_makedirs,
mock_datetime): mock_datetime, mock_path_exists,
uuid, dir_path = utils.create_artifacts_dir(dir_path='/tmp/foo', mock_warning, mock_error):
prefix='ntp')
uuid, artifacts_dir = utils.get_artifacts_dir(
log_dir='/tmp/foo',
prefix='ntp')
self.assertEqual(uuid, '1234') self.assertEqual(uuid, '1234')
self.assertEqual(dir_path, self.assertEqual(artifacts_dir,
'/tmp/foo/1234_ntp_2020-04-02T06:58:20.352272Z') '/tmp/foo/artifacts/1234_ntp_2020-04-02T06:58:20.352272Z')
mock_error.assert_not_called()
mock_warning.assert_not_called()
@mock.patch('validations_libs.utils.LOG.error')
@mock.patch('validations_libs.utils.LOG.warning')
@mock.patch('validations_libs.utils.current_time',
return_value='2020-04-02T06:58:20.352272Z')
@mock.patch('validations_libs.utils.get_log_dir', return_value='/tmp/foo')
@mock.patch('validations_libs.utils.os.path.exists', return_value=False)
@mock.patch('os.makedirs', side_effect=[OSError, None])
@mock.patch('uuid.uuid4', return_value='1234')
def test_create_artifacts_dir_OSError(self, mock_uuid, mock_makedirs,
mock_get_log_dir,
mock_exists, mock_datetime,
mock_warning, mock_error):
uuid, artifacts_dir = utils.get_artifacts_dir(
log_dir='/tmp/foo',
prefix='ntp')
self.assertEqual(uuid, '1234')
self.assertNotEqual(
artifacts_dir,
'/tmp/foo/1234_ntp_2020-04-02T06:58:20.352272Z')
mock_error.assert_called()
mock_warning.assert_called()
@mock.patch('validations_libs.utils.LOG.error')
@mock.patch('validations_libs.utils.LOG.warning')
@mock.patch('validations_libs.utils.current_time',
return_value='2020-04-02T06:58:20.352272Z')
@mock.patch('validations_libs.utils.get_log_dir', return_value='/tmp/foo')
@mock.patch('validations_libs.utils.os.path.exists', return_value=False)
@mock.patch('os.makedirs', side_effect=[PermissionError, None])
@mock.patch('uuid.uuid4', return_value='1234')
def test_create_artifacts_dir_PermissionError(self, mock_uuid, mock_makedirs,
mock_get_log_dir,
mock_exists, mock_datetime,
mock_warning, mock_error):
uuid, artifacts_dir = utils.get_artifacts_dir(
log_dir='/tmp/foo',
prefix='ntp')
self.assertEqual(uuid, '1234')
self.assertNotEqual(
artifacts_dir,
'/tmp/foo/1234_ntp_2020-04-02T06:58:20.352272Z')
mock_error.assert_called()
mock_warning.assert_called()
@mock.patch('yaml.safe_load', return_value=fakes.FAKE_PLAYBOOK) @mock.patch('yaml.safe_load', return_value=fakes.FAKE_PLAYBOOK)
@mock.patch('six.moves.builtins.open') @mock.patch('six.moves.builtins.open')

30
validations_libs/tests/test_validation_actions.py
View File

@ -45,11 +45,14 @@ class TestValidationActions(TestCase):
'My Validation Two Name', 'My Validation Two Name',
['prep', 'pre-introspection'])])) ['prep', 'pre-introspection'])]))
@mock.patch('validations_libs.utils.get_log_dir',
return_value='/var/log/validations')
@mock.patch('validations_libs.utils.create_artifacts_dir', @mock.patch('validations_libs.utils.create_artifacts_dir',
return_value=('1234', '/tmp/')) return_value=('1234', '/tmp/'))
@mock.patch('validations_libs.utils.get_validations_playbook', @mock.patch('validations_libs.utils.get_validations_playbook',
return_value=['/tmp/foo/fake.yaml']) return_value=['/tmp/foo/fake.yaml'])
def test_validation_skip_validation(self, mock_validation_play, mock_tmp): def test_validation_skip_validation(self, mock_validation_play, mock_tmp,
mock_get_log_dir):
playbook = ['fake.yaml'] playbook = ['fake.yaml']
inventory = 'tmp/inventory.yaml' inventory = 'tmp/inventory.yaml'
@ -66,13 +69,16 @@ class TestValidationActions(TestCase):
limit_hosts=None) limit_hosts=None)
self.assertEqual(run_return, []) self.assertEqual(run_return, [])
@mock.patch('validations_libs.utils.get_log_dir',
return_value='/var/log/validations')
@mock.patch('validations_libs.utils.get_validations_playbook', @mock.patch('validations_libs.utils.get_validations_playbook',
return_value=['/tmp/foo/fake.yaml']) return_value=['/tmp/foo/fake.yaml'])
@mock.patch('validations_libs.ansible.Ansible.run') @mock.patch('validations_libs.ansible.Ansible.run')
@mock.patch('validations_libs.utils.create_artifacts_dir', @mock.patch('validations_libs.utils.create_artifacts_dir',
return_value=('1234', '/tmp/')) return_value=('1234', '/tmp/'))
def test_validation_skip_on_specific_host(self, mock_tmp, mock_ansible_run, def test_validation_skip_on_specific_host(self, mock_tmp, mock_ansible_run,
mock_validation_play): mock_validation_play,
mock_get_log_dir):
mock_ansible_run.return_value = ('fake.yaml', 0, 'successful') mock_ansible_run.return_value = ('fake.yaml', 0, 'successful')
run_called_args = { run_called_args = {
'workdir': '/tmp/', 'workdir': '/tmp/',
@ -90,7 +96,7 @@ class TestValidationActions(TestCase):
'extra_env_variables': None, 'extra_env_variables': None,
'ansible_cfg': None, 'ansible_cfg': None,
'gathering_policy': 'explicit', 'gathering_policy': 'explicit',
'log_path': None, 'log_path': '/var/log/validations',
'run_async': False, 'run_async': False,
'python_interpreter': None, 'python_interpreter': None,
'ssh_user': None 'ssh_user': None
@ -112,13 +118,16 @@ class TestValidationActions(TestCase):
) )
mock_ansible_run.assert_called_with(**run_called_args) mock_ansible_run.assert_called_with(**run_called_args)
@mock.patch('validations_libs.utils.get_log_dir',
return_value='/var/log/validations')
@mock.patch('validations_libs.utils.get_validations_playbook', @mock.patch('validations_libs.utils.get_validations_playbook',
return_value=['/tmp/foo/fake.yaml']) return_value=['/tmp/foo/fake.yaml'])
@mock.patch('validations_libs.ansible.Ansible.run') @mock.patch('validations_libs.ansible.Ansible.run')
@mock.patch('validations_libs.utils.create_artifacts_dir', @mock.patch('validations_libs.utils.create_artifacts_dir',
return_value=('1234', '/tmp/')) return_value=('1234', '/tmp/'))
def test_validation_skip_with_limit_host(self, mock_tmp, mock_ansible_run, def test_validation_skip_with_limit_host(self, mock_tmp, mock_ansible_run,
mock_validation_play): mock_validation_play,
mock_get_log_dir):
mock_ansible_run.return_value = ('fake.yaml', 0, 'successful') mock_ansible_run.return_value = ('fake.yaml', 0, 'successful')
run_called_args = { run_called_args = {
'workdir': '/tmp/', 'workdir': '/tmp/',
@ -136,7 +145,7 @@ class TestValidationActions(TestCase):
'ansible_cfg': None, 'ansible_cfg': None,
'gathering_policy': 'explicit', 'gathering_policy': 'explicit',
'ansible_artifact_path': '/tmp/', 'ansible_artifact_path': '/tmp/',
'log_path': None, 'log_path': '/var/log/validations',
'run_async': False, 'run_async': False,
'python_interpreter': None, 'python_interpreter': None,
'ssh_user': None 'ssh_user': None
@ -157,13 +166,16 @@ class TestValidationActions(TestCase):
limit_hosts='cloud,cloud1,!cloud2') limit_hosts='cloud,cloud1,!cloud2')
mock_ansible_run.assert_called_with(**run_called_args) mock_ansible_run.assert_called_with(**run_called_args)
@mock.patch('validations_libs.utils.get_log_dir',
return_value='/var/log/validations')
@mock.patch('validations_libs.validation_logs.ValidationLogs.get_results') @mock.patch('validations_libs.validation_logs.ValidationLogs.get_results')
@mock.patch('validations_libs.utils.parse_all_validations_on_disk') @mock.patch('validations_libs.utils.parse_all_validations_on_disk')
@mock.patch('validations_libs.ansible.Ansible.run') @mock.patch('validations_libs.ansible.Ansible.run')
@mock.patch('validations_libs.utils.create_artifacts_dir', @mock.patch('validations_libs.utils.create_artifacts_dir',
return_value=('1234', '/tmp/')) return_value=('1234', '/tmp/'))
def test_validation_run_success(self, mock_tmp, mock_ansible_run, def test_validation_run_success(self, mock_tmp, mock_ansible_run,
mock_validation_dir, mock_results): mock_validation_dir, mock_results,
mock_get_log_dir):
mock_validation_dir.return_value = [{ mock_validation_dir.return_value = [{
'description': 'My Validation One Description', 'description': 'My Validation One Description',
'groups': ['prep', 'pre-deployment'], 'groups': ['prep', 'pre-deployment'],
@ -206,13 +218,17 @@ class TestValidationActions(TestCase):
validations_dir='/tmp/foo' validations_dir='/tmp/foo'
) )
@mock.patch('validations_libs.utils.get_log_dir',
return_value='/var/log/validations')
@mock.patch('validations_libs.validation_logs.ValidationLogs.get_results') @mock.patch('validations_libs.validation_logs.ValidationLogs.get_results')
@mock.patch('validations_libs.utils.parse_all_validations_on_disk') @mock.patch('validations_libs.utils.parse_all_validations_on_disk')
@mock.patch('validations_libs.ansible.Ansible.run') @mock.patch('validations_libs.ansible.Ansible.run')
@mock.patch('validations_libs.utils.create_artifacts_dir', @mock.patch('validations_libs.utils.create_artifacts_dir',
return_value=('1234', '/tmp/')) return_value=('1234', '/tmp/'))
def test_validation_run_failed(self, mock_tmp, mock_ansible_run, def test_validation_run_failed(self, mock_tmp, mock_ansible_run,
mock_validation_dir, mock_results): mock_validation_dir, mock_results,
mock_get_log_dir):
mock_validation_dir.return_value = [{ mock_validation_dir.return_value = [{
'description': 'My Validation One Description', 'description': 'My Validation One Description',
'groups': ['prep', 'pre-deployment'], 'groups': ['prep', 'pre-deployment'],

104
validations_libs/utils.py
View File

@ -32,9 +32,80 @@ def current_time():
return '%sZ' % datetime.datetime.utcnow().isoformat() return '%sZ' % datetime.datetime.utcnow().isoformat()
def create_artifacts_dir(dir_path=None, prefix=None): def create_log_dir():
"""
"""
try:
log_path = "{}".format(
os.path.join(os.environ.get('HOME'), 'validations'))
os.makedirs(log_path)
except (OSError, PermissionError):
LOG.error(
(
"Error while creating the log directory. "
"Please check the access rights for: '{}'"
).format(log_path)
)
raise RuntimeError()
def get_log_dir(log_path):
"""Get validations log directory.
"""
if not os.path.exists(log_path):
LOG.warning(
(
"Requested log folder unavailable, defaulting to: '{}'"
).format(constants.VALIDATIONS_LOG_BASEDIR)
)
if os.path.exists(constants.VALIDATIONS_LOG_BASEDIR):
log_path = constants.VALIDATIONS_LOG_BASEDIR
else:
LOG.warning(
(
"Default log folder unavailable, creating: '{}'"
).format(log_path)
)
create_log_dir()
return log_path
def create_artifacts_dir(dir_path, prefix):
validation_uuid = str(uuid.uuid4())
log_sub_dir = "{}_{}_{}".format(
validation_uuid,
prefix,
current_time())
artifacts_dir = "{}/artifacts/{}".format(
dir_path,
log_sub_dir)
os.makedirs(artifacts_dir)
return validation_uuid, artifacts_dir
def get_artifacts_dir(log_dir, prefix=''):
"""Create Ansible artifacts directory """Create Ansible artifacts directory
This function contains additional checks for the log directory
access as well as possible fallbacks.
In order, following locations are considered for artifacts directory:
`dir_path` parameter
`validations_libs.constants.VALIDATION_ANSIBLE_ARTIFACT_PATH`
`$HOME/log/validations/artifacts`
`/tmp/log/validations/artifacts`
In the last two cases, an error and warning is logged,
to notify the user about remediation, and possible complexities
it may lead to.
:param dir_path: Directory asbolute path :param dir_path: Directory asbolute path
:type dir_path: `string` :type dir_path: `string`
:param prefix: Playbook name :param prefix: Playbook name
@ -42,22 +113,31 @@ def create_artifacts_dir(dir_path=None, prefix=None):
:return: The UUID of the validation and the absolute Path of the log file :return: The UUID of the validation and the absolute Path of the log file
:rtype: `string`, `string` :rtype: `string`, `string`
""" """
dir_path = (dir_path if dir_path else
constants.VALIDATION_ANSIBLE_ARTIFACT_PATH)
validation_uuid = str(uuid.uuid4())
log_dir = "{}/{}_{}_{}".format(dir_path, validation_uuid,
(prefix if prefix else ''), current_time())
try: try:
os.makedirs(log_dir) validation_uuid, artifacts_dir = create_artifacts_dir(
return validation_uuid, log_dir log_dir,
except OSError: prefix)
LOG.exception( except (OSError, PermissionError):
LOG.error(
( (
"Error while creating Ansible artifacts log file." "Error while creating Ansible artifacts directory. "
"Please check the access rights for {}" "Please check the access rights for: '{}'"
).format(log_dir) ).format(log_dir)
) )
validation_uuid, artifacts_dir = create_artifacts_dir(
constants.VALIDATIONS_LOG_BASEDIR,
prefix)
LOG.warning(
(
"Default artifact folder unavailable, defaulting to: '{}'"
).format(artifacts_dir)
)
return validation_uuid, artifacts_dir
def parse_all_validations_on_disk(path, groups=None): def parse_all_validations_on_disk(path, groups=None):
"""Return a list of validations metadata which can be sorted by Groups """Return a list of validations metadata which can be sorted by Groups

13
validations_libs/validation_actions.py
View File

@ -208,7 +208,8 @@ class ValidationActions(object):
extra_env_vars=None, ansible_cfg=None, quiet=True, extra_env_vars=None, ansible_cfg=None, quiet=True,
workdir=None, limit_hosts=None, run_async=False, workdir=None, limit_hosts=None, run_async=False,
base_dir=constants.DEFAULT_VALIDATIONS_BASEDIR, base_dir=constants.DEFAULT_VALIDATIONS_BASEDIR,
log_path=None, python_interpreter=None, log_path=constants.VALIDATIONS_LOG_BASEDIR,
python_interpreter=None,
skip_list=None, skip_list=None,
callback_whitelist=None, callback_whitelist=None,
output_callback='validation_stdout', output_callback='validation_stdout',
@ -248,6 +249,8 @@ class ValidationActions(object):
``constants.DEFAULT_VALIDATIONS_BASEDIR``) ``constants.DEFAULT_VALIDATIONS_BASEDIR``)
:type base_dir: ``string`` :type base_dir: ``string``
:param log_path: The absolute path of the validations logs directory :param log_path: The absolute path of the validations logs directory
(Defaults to
``constants.VALIDATIONS_LOG_BASEDIR``)
:type log_path: ``string`` :type log_path: ``string``
:param python_interpreter: Path to the Python interpreter to be :param python_interpreter: Path to the Python interpreter to be
used for module execution on remote targets, used for module execution on remote targets,
@ -333,6 +336,8 @@ class ValidationActions(object):
else: else:
raise RuntimeError("No validations found") raise RuntimeError("No validations found")
log_path = v_utils.get_log_dir(log_path)
self.log.debug('Running the validations with Ansible') self.log.debug('Running the validations with Ansible')
results = [] results = []
for playbook in playbooks: for playbook in playbooks:
@ -342,8 +347,8 @@ class ValidationActions(object):
play_name, play_name,
limit_hosts) limit_hosts)
if _play: if _play:
validation_uuid, artifacts_dir = v_utils.create_artifacts_dir( validation_uuid, artifacts_dir = v_utils.get_artifacts_dir(
dir_path=log_path, prefix=os.path.basename(playbook)) log_dir=log_path, prefix=os.path.basename(playbook))
run_ansible = v_ansible(validation_uuid) run_ansible = v_ansible(validation_uuid)
_playbook, _rc, _status = run_ansible.run( _playbook, _rc, _status = run_ansible.run(
workdir=artifacts_dir, workdir=artifacts_dir,
@ -378,7 +383,7 @@ class ValidationActions(object):
return results return results
# Return log results # Return log results
uuid = [id['UUID'] for id in results] uuid = [id['UUID'] for id in results]
vlog = ValidationLogs() vlog = ValidationLogs(log_path)
return vlog.get_results(uuid) return vlog.get_results(uuid)
def group_information(self, groups): def group_information(self, groups):