mirror of
https://github.com/valitydev/salt.git
synced 2024-11-07 17:09:03 +00:00
Merge pull request #45979 from isbm/isbm-parsers-loggerfix-unicode-rc1
Unicode/logger fix + bugfix: salt/utils/parsers
This commit is contained in:
Nicole Thomas
GitHub
commit
3e18604d75
@ -47,6 +47,8 @@ import salt.exceptions
|
||||
from salt.ext import six
|
||||
from salt.ext.six.moves import range # pylint: disable=import-error,redefined-builtin
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
def _sorted(mixins_or_funcs):
|
||||
return sorted(
|
||||
@ -64,8 +66,8 @@ class MixInMeta(type):
|
||||
instance = super(MixInMeta, mcs).__new__(mcs, name, bases, attrs)
|
||||
if not hasattr(instance, '_mixin_setup'):
|
||||
raise RuntimeError(
|
||||
'Don\'t subclass {0} in {1} if you\'re not going to use it '
|
||||
'as a salt parser mix-in.'.format(mcs.__name__, name)
|
||||
"Don't subclass {0} in {1} if you're not going "
|
||||
"to use it as a salt parser mix-in.".format(mcs.__name__, name)
|
||||
)
|
||||
return instance
|
||||
|
||||
@ -206,7 +208,7 @@ class OptionParser(optparse.OptionParser, object):
|
||||
try:
|
||||
process_option_func()
|
||||
except Exception as err: # pylint: disable=broad-except
|
||||
logging.getLogger(__name__).exception(err)
|
||||
logger.exception(err)
|
||||
self.error(
|
||||
'Error while processing {0}: {1}'.format(
|
||||
process_option_func, traceback.format_exc(err)
|
||||
@ -218,7 +220,7 @@ class OptionParser(optparse.OptionParser, object):
|
||||
try:
|
||||
mixin_after_parsed_func(self)
|
||||
except Exception as err: # pylint: disable=broad-except
|
||||
logging.getLogger(__name__).exception(err)
|
||||
logger.exception(err)
|
||||
self.error(
|
||||
'Error while processing {0}: {1}'.format(
|
||||
mixin_after_parsed_func, traceback.format_exc(err)
|
||||
@ -226,7 +228,7 @@ class OptionParser(optparse.OptionParser, object):
|
||||
)
|
||||
|
||||
if self.config.get('conf_file', None) is not None: # pylint: disable=no-member
|
||||
logging.getLogger(__name__).debug(
|
||||
logger.debug(
|
||||
'Configuration file path: %s',
|
||||
self.config['conf_file'] # pylint: disable=no-member
|
||||
)
|
||||
@ -259,12 +261,10 @@ class OptionParser(optparse.OptionParser, object):
|
||||
try:
|
||||
mixin_before_exit_func(self)
|
||||
except Exception as err: # pylint: disable=broad-except
|
||||
logger = logging.getLogger(__name__)
|
||||
logger.exception(err)
|
||||
logger.error(
|
||||
'Error while processing %s: %s',
|
||||
mixin_before_exit_func, traceback.format_exc(err)
|
||||
)
|
||||
logger.error('Error while processing %s: %s',
|
||||
six.text_type(mixin_before_exit_func),
|
||||
traceback.format_exc(err))
|
||||
if self._setup_mp_logging_listener_ is True:
|
||||
# Stop logging through the queue
|
||||
log.shutdown_multiprocessing_logging()
|
||||
@ -399,17 +399,13 @@ class SaltfileMixIn(six.with_metaclass(MixInMeta, object)):
|
||||
return
|
||||
|
||||
if not os.path.isfile(self.options.saltfile):
|
||||
self.error(
|
||||
'\'{0}\' file does not exist.\n'.format(self.options.saltfile)
|
||||
)
|
||||
self.error("'{0}' file does not exist.\n".format(self.options.saltfile))
|
||||
|
||||
# Make sure we have an absolute path
|
||||
self.options.saltfile = os.path.abspath(self.options.saltfile)
|
||||
|
||||
# Make sure we let the user know that we will be loading a Saltfile
|
||||
logging.getLogger(__name__).info(
|
||||
'Loading Saltfile from \'%s\'', self.options.saltfile
|
||||
)
|
||||
logger.info("Loading Saltfile from '%s'", six.text_type(self.options.saltfile))
|
||||
|
||||
try:
|
||||
saltfile_config = config._read_conf_file(saltfile)
|
||||
@ -488,8 +484,7 @@ class HardCrashMixin(six.with_metaclass(MixInMeta, object)):
|
||||
hard_crash = os.environ.get('SALT_HARD_CRASH', False)
|
||||
self.add_option(
|
||||
'--hard-crash', action='store_true', default=hard_crash,
|
||||
help=('Raise any original exception rather than exiting gracefully. '
|
||||
'Default: %default.')
|
||||
help='Raise any original exception rather than exiting gracefully. Default: %default.'
|
||||
)
|
||||
|
||||
|
||||
@ -500,9 +495,8 @@ class NoParseMixin(six.with_metaclass(MixInMeta, object)):
|
||||
no_parse = os.environ.get('SALT_NO_PARSE', '')
|
||||
self.add_option(
|
||||
'--no-parse', default=no_parse,
|
||||
help=('Comma-separated list of named CLI arguments (i.e. '
|
||||
'argname=value) which should not be parsed as Python '
|
||||
'data types'),
|
||||
help='Comma-separated list of named CLI arguments (i.e. argname=value) '
|
||||
'which should not be parsed as Python data types',
|
||||
metavar='argname1,argname2,...',
|
||||
)
|
||||
|
||||
@ -527,11 +521,10 @@ class ConfigDirMixIn(six.with_metaclass(MixInMeta, object)):
|
||||
config_dir = os.environ.get(self._default_config_dir_env_var_, None)
|
||||
if not config_dir:
|
||||
config_dir = self._default_config_dir_
|
||||
logging.getLogger(__name__).debug('SYSPATHS setup as: %s', syspaths.CONFIG_DIR)
|
||||
logger.debug('SYSPATHS setup as: %s', six.text_type(syspaths.CONFIG_DIR))
|
||||
self.add_option(
|
||||
'-c', '--config-dir', default=config_dir,
|
||||
help=('Pass in an alternative configuration directory. Default: '
|
||||
'\'%default\'.')
|
||||
help="Pass in an alternative configuration directory. Default: '%default'."
|
||||
)
|
||||
|
||||
def process_config_dir(self):
|
||||
@ -539,7 +532,7 @@ class ConfigDirMixIn(six.with_metaclass(MixInMeta, object)):
|
||||
if not os.path.isdir(self.options.config_dir):
|
||||
# No logging is configured yet
|
||||
sys.stderr.write(
|
||||
'WARNING: CONFIG \'{0}\' directory does not exist.\n'.format(
|
||||
"WARNING: CONFIG '{0}' directory does not exist.\n".format(
|
||||
self.options.config_dir
|
||||
)
|
||||
)
|
||||
@ -794,12 +787,11 @@ class LogLevelMixIn(six.with_metaclass(MixInMeta, object)):
|
||||
logfile_basename = os.path.basename(
|
||||
self._default_logging_logfile_
|
||||
)
|
||||
logging.getLogger(__name__).debug(
|
||||
'The user \'%s\' is not allowed to write to \'%s\'. '
|
||||
'The log file will be stored in '
|
||||
'\'~/.salt/\'%s\'.log\'',
|
||||
current_user, logfile, logfile_basename
|
||||
)
|
||||
logger.debug("The user '%s' is not allowed to write to '%s'. "
|
||||
"The log file will be stored in '~/.salt/'%s'.log'",
|
||||
six.text_type(current_user),
|
||||
six.text_type(logfile),
|
||||
six.text_type(logfile_basename))
|
||||
logfile = os.path.join(
|
||||
user_salt_dir, '{0}.log'.format(logfile_basename)
|
||||
)
|
||||
@ -814,10 +806,10 @@ class LogLevelMixIn(six.with_metaclass(MixInMeta, object)):
|
||||
# Not supported on platforms other than Windows.
|
||||
# Other platforms may use an external tool such as 'logrotate'
|
||||
if log_rotate_max_bytes != 0:
|
||||
logging.getLogger(__name__).warning('\'log_rotate_max_bytes\' is only supported on Windows')
|
||||
logger.warning("'log_rotate_max_bytes' is only supported on Windows")
|
||||
log_rotate_max_bytes = 0
|
||||
if log_rotate_backup_count != 0:
|
||||
logging.getLogger(__name__).warning('\'log_rotate_backup_count\' is only supported on Windows')
|
||||
logger.warning("'log_rotate_backup_count' is only supported on Windows")
|
||||
log_rotate_backup_count = 0
|
||||
|
||||
# Save the settings back to the configuration
|
||||
@ -962,22 +954,23 @@ class DaemonMixIn(six.with_metaclass(MixInMeta, object)):
|
||||
default=os.path.join(
|
||||
syspaths.PIDFILE_DIR, '{0}.pid'.format(self.get_prog_name())
|
||||
),
|
||||
help=('Specify the location of the pidfile. Default: \'%default\'.')
|
||||
help="Specify the location of the pidfile. Default: '%default'."
|
||||
)
|
||||
|
||||
def _mixin_before_exit(self):
|
||||
if hasattr(self, 'config') and self.config.get('pidfile', ''):
|
||||
if hasattr(self, 'config') and self.config.get('pidfile'):
|
||||
# We've loaded and merged options into the configuration, it's safe
|
||||
# to query about the pidfile
|
||||
if self.check_pidfile():
|
||||
try:
|
||||
os.unlink(self.config['pidfile'])
|
||||
except OSError as err:
|
||||
logging.getLogger(__name__).info(
|
||||
'PIDfile could not be deleted: {0}'.format(
|
||||
self.config['pidfile']
|
||||
)
|
||||
)
|
||||
# Log error only when running salt-master as a root user.
|
||||
# Otherwise this can be ignored, since salt-master is able to
|
||||
# overwrite the PIDfile on the next start.
|
||||
if not os.getuid():
|
||||
logger.info('PIDfile could not be deleted: %s', six.text_type(self.config['pidfile']))
|
||||
logger.debug(six.text_type(err))
|
||||
|
||||
def set_pidfile(self):
|
||||
from salt.utils.process import set_pidfile
|
||||
@ -2739,31 +2732,31 @@ class SaltCallOptionParser(six.with_metaclass(OptionParserMeta,
|
||||
|
||||
role = opts.get('id')
|
||||
if not role:
|
||||
emsg = ("Missing role required to setup RAET SaltCaller.")
|
||||
logging.getLogger(__name__).error(emsg + "\n")
|
||||
emsg = "Missing role required to setup RAET SaltCaller."
|
||||
logger.error(emsg)
|
||||
raise ValueError(emsg)
|
||||
|
||||
kind = opts.get('__role') # application kind 'master', 'minion', etc
|
||||
if kind not in kinds.APPL_KINDS:
|
||||
emsg = ("Invalid application kind = '{0}' for RAET SaltCaller.".format(kind))
|
||||
logging.getLogger(__name__).error(emsg + "\n")
|
||||
emsg = "Invalid application kind = '{0}' for RAET SaltCaller.".format(six.text_type(kind))
|
||||
logger.error(emsg)
|
||||
raise ValueError(emsg)
|
||||
|
||||
if kind in [kinds.APPL_KIND_NAMES[kinds.applKinds.minion],
|
||||
kinds.APPL_KIND_NAMES[kinds.applKinds.caller], ]:
|
||||
if kind in [kinds.APPL_KIND_NAMES[kinds.applKinds.minion], kinds.APPL_KIND_NAMES[kinds.applKinds.caller]]:
|
||||
lanename = "{0}_{1}".format(role, kind)
|
||||
else:
|
||||
emsg = ("Unsupported application kind '{0}' for RAET SaltCaller.".format(kind))
|
||||
logging.getLogger(__name__).error(emsg + '\n')
|
||||
emsg = "Unsupported application kind '{0}' for RAET SaltCaller.".format(six.text_type(kind))
|
||||
logger.error(emsg)
|
||||
raise ValueError(emsg)
|
||||
|
||||
if kind == kinds.APPL_KIND_NAMES[kinds.applKinds.minion]: # minion check
|
||||
try:
|
||||
from raet.lane.yarding import Yard # pylint: disable=3rd-party-module-not-gated
|
||||
ha, dirpath = Yard.computeHa(dirpath, lanename, yardname) # pylint: disable=invalid-name
|
||||
if (os.path.exists(ha) and
|
||||
not os.path.isfile(ha) and
|
||||
not os.path.isdir(ha)): # minion manor yard
|
||||
if os.path.exists(ha) and not os.path.isfile(ha) and not os.path.isdir(ha): # minion manor yard
|
||||
return True
|
||||
except ImportError as ex:
|
||||
logger.error("Error while importing Yard: %s", ex)
|
||||
return False
|
||||
|
||||
def process_module_dirs(self):
|
||||
@ -2822,13 +2815,13 @@ class SaltRunOptionParser(six.with_metaclass(OptionParserMeta,
|
||||
'--async',
|
||||
default=False,
|
||||
action='store_true',
|
||||
help=('Start the runner operation and immediately return control.')
|
||||
help='Start the runner operation and immediately return control.'
|
||||
)
|
||||
self.add_option(
|
||||
'--skip-grains',
|
||||
default=False,
|
||||
action='store_true',
|
||||
help=('Do not load grains.')
|
||||
help='Do not load grains.'
|
||||
)
|
||||
group = self.output_options_group = optparse.OptionGroup(
|
||||
self, 'Output Options', 'Configure your preferred output format.'
|
||||
@ -2946,14 +2939,13 @@ class SaltSSHOptionParser(six.with_metaclass(OptionParserMeta,
|
||||
'-v', '--verbose',
|
||||
default=False,
|
||||
action='store_true',
|
||||
help=('Turn on command verbosity, display jid.')
|
||||
help='Turn on command verbosity, display jid.'
|
||||
)
|
||||
self.add_option(
|
||||
'-s', '--static',
|
||||
default=False,
|
||||
action='store_true',
|
||||
help=('Return the data from minions as a group after they '
|
||||
'all return.')
|
||||
help='Return the data from minions as a group after they all return.'
|
||||
)
|
||||
self.add_option(
|
||||
'-w', '--wipe',
|
||||
|
||||
@ -5,9 +5,12 @@
|
||||
tests.support.mock
|
||||
~~~~~~~~~~~~~~~~~~
|
||||
|
||||
Helper module that wraps :mod:`mock <python3:unittest.mock>` and provides
|
||||
some fake objects in order to properly set the function/class decorators
|
||||
and yet skip the test cases execution.
|
||||
Helper module that wraps `mock` and provides some fake objects in order to
|
||||
properly set the function/class decorators and yet skip the test case's
|
||||
execution.
|
||||
|
||||
Note: mock >= 2.0.0 required since unittest.mock does not have
|
||||
MagicMock.assert_called in Python < 3.6.
|
||||
'''
|
||||
# pylint: disable=unused-import,function-redefined,blacklisted-module,blacklisted-external-module
|
||||
|
||||
@ -18,23 +21,6 @@ import sys
|
||||
from salt.ext import six
|
||||
|
||||
try:
|
||||
if sys.version_info >= (3,):
|
||||
# Python 3
|
||||
from unittest.mock import (
|
||||
Mock,
|
||||
MagicMock,
|
||||
patch,
|
||||
sentinel,
|
||||
DEFAULT,
|
||||
# ANY and call will be imported further down
|
||||
create_autospec,
|
||||
FILTER_DIR,
|
||||
NonCallableMock,
|
||||
NonCallableMagicMock,
|
||||
PropertyMock,
|
||||
__version__
|
||||
)
|
||||
else:
|
||||
from mock import (
|
||||
Mock,
|
||||
MagicMock,
|
||||
@ -99,10 +85,6 @@ except ImportError as exc:
|
||||
|
||||
if NO_MOCK is False:
|
||||
try:
|
||||
if sys.version_info >= (3,):
|
||||
# Python 3
|
||||
from unittest.mock import call, ANY
|
||||
else:
|
||||
from mock import call, ANY
|
||||
except ImportError:
|
||||
NO_MOCK = True
|
||||
|
||||
@ -5,7 +5,6 @@
|
||||
|
||||
# Import python libs
|
||||
from __future__ import absolute_import, print_function, unicode_literals
|
||||
import logging
|
||||
import os
|
||||
|
||||
# Import Salt Testing Libs
|
||||
@ -25,6 +24,11 @@ import salt.syspaths
|
||||
import salt.utils.parsers
|
||||
import salt.utils.platform
|
||||
|
||||
try:
|
||||
import pytest
|
||||
except ImportError:
|
||||
pytest = None
|
||||
|
||||
|
||||
class ErrorMock(object): # pylint: disable=too-few-public-methods
|
||||
'''
|
||||
@ -995,6 +999,7 @@ class SaltAPIParserTestCase(LogSettingsParserTests):
|
||||
self.addCleanup(delattr, self, 'parser')
|
||||
|
||||
|
||||
@skipIf(not pytest, False)
|
||||
@skipIf(NO_MOCK, NO_MOCK_REASON)
|
||||
class DaemonMixInTestCase(TestCase):
|
||||
'''
|
||||
@ -1005,39 +1010,57 @@ class DaemonMixInTestCase(TestCase):
|
||||
'''
|
||||
Setting up
|
||||
'''
|
||||
# Set PID
|
||||
self.pid = '/some/fake.pid'
|
||||
|
||||
# Setup mixin
|
||||
self.mixin = salt.utils.parsers.DaemonMixIn()
|
||||
self.mixin.config = {}
|
||||
self.mixin.config['pidfile'] = self.pid
|
||||
self.daemon_mixin = salt.utils.parsers.DaemonMixIn()
|
||||
self.daemon_mixin.config = {}
|
||||
self.daemon_mixin.config['pidfile'] = '/some/fake.pid'
|
||||
|
||||
# logger
|
||||
self.logger = logging.getLogger('salt.utils.parsers')
|
||||
def tearDown(self):
|
||||
'''
|
||||
Tear down test
|
||||
:return:
|
||||
'''
|
||||
del self.daemon_mixin
|
||||
|
||||
@patch('os.unlink', MagicMock())
|
||||
@patch('os.path.isfile', MagicMock(return_value=True))
|
||||
@patch('salt.utils.parsers.logger', MagicMock())
|
||||
def test_pid_file_deletion(self):
|
||||
'''
|
||||
PIDfile deletion without exception.
|
||||
'''
|
||||
with patch('os.unlink', MagicMock()) as os_unlink:
|
||||
with patch('os.path.isfile', MagicMock(return_value=True)):
|
||||
with patch.object(self.logger, 'info') as mock_logger:
|
||||
self.mixin._mixin_before_exit()
|
||||
assert mock_logger.call_count == 0
|
||||
assert os_unlink.call_count == 1
|
||||
self.daemon_mixin._mixin_before_exit()
|
||||
assert salt.utils.parsers.os.unlink.call_count == 1
|
||||
salt.utils.parsers.logger.info.assert_not_called()
|
||||
salt.utils.parsers.logger.debug.assert_not_called()
|
||||
|
||||
def test_pid_file_deletion_with_oserror(self):
|
||||
@patch('os.unlink', MagicMock(side_effect=OSError()))
|
||||
@patch('os.path.isfile', MagicMock(return_value=True))
|
||||
@patch('os.getuid', MagicMock(return_value=0))
|
||||
@patch('salt.utils.parsers.logger', MagicMock())
|
||||
def test_pid_deleted_oserror_as_root(self):
|
||||
'''
|
||||
PIDfile deletion with exception
|
||||
PIDfile deletion with exception, running as root.
|
||||
'''
|
||||
with patch('os.unlink', MagicMock(side_effect=OSError())) as os_unlink:
|
||||
with patch('os.path.isfile', MagicMock(return_value=True)):
|
||||
with patch.object(self.logger, 'info') as mock_logger:
|
||||
self.mixin._mixin_before_exit()
|
||||
assert os_unlink.call_count == 1
|
||||
mock_logger.assert_called_with(
|
||||
'PIDfile could not be deleted: {0}'.format(self.pid))
|
||||
self.daemon_mixin._mixin_before_exit()
|
||||
assert salt.utils.parsers.os.unlink.call_count == 1
|
||||
salt.utils.parsers.logger.info.assert_called_with('PIDfile could not be deleted: %s',
|
||||
format(self.daemon_mixin.config['pidfile']))
|
||||
salt.utils.parsers.logger.debug.assert_called()
|
||||
|
||||
@patch('os.unlink', MagicMock(side_effect=OSError()))
|
||||
@patch('os.path.isfile', MagicMock(return_value=True))
|
||||
@patch('os.getuid', MagicMock(return_value=1000))
|
||||
@patch('salt.utils.parsers.logger', MagicMock())
|
||||
def test_pid_deleted_oserror_as_non_root(self):
|
||||
'''
|
||||
PIDfile deletion with exception, running as non-root.
|
||||
'''
|
||||
self.daemon_mixin._mixin_before_exit()
|
||||
assert salt.utils.parsers.os.unlink.call_count == 1
|
||||
salt.utils.parsers.logger.info.assert_not_called()
|
||||
salt.utils.parsers.logger.debug.assert_not_called()
|
||||
|
||||
|
||||
# Hide the class from unittest framework when it searches for TestCase classes in the module
|
||||
del LogSettingsParserTests
|
||||
|
||||
Loading…
Reference in New Issue
Block a user