From f5f02985f17b3cf5770843daf56445d863954718 Mon Sep 17 00:00:00 2001 From: Aaron Mildenstein Date: Mon, 12 Sep 2016 09:49:19 -0600 Subject: [PATCH 01/12] Fix error message for boolean values With `Any(bool, int)` in the Schema tests, it will properly report `expected bool` now instead of `expected int` if a non-boolean value was provided. fixes #757 --- curator/validators/config_file.py | 6 +++--- curator/validators/filter_elements.py | 4 ++-- curator/validators/options.py | 22 +++++++++++----------- docs/Changelog.rst | 9 +++++++++ 4 files changed, 25 insertions(+), 16 deletions(-) diff --git a/curator/validators/config_file.py b/curator/validators/config_file.py index e81d8e24..8c83dc64 100644 --- a/curator/validators/config_file.py +++ b/curator/validators/config_file.py @@ -8,7 +8,7 @@ def config_client(): None, All(Coerce(int), Range(min=1, max=65535)) ), Optional('url_prefix', default=''): Any(None, str), - Optional('use_ssl', default=False): All(Any(int, bool), Coerce(bool)), + Optional('use_ssl', default=False): All(Any(bool, int), Coerce(bool)), Optional('certificate', default=None): Any(None, str), Optional('client_cert', default=None): Any(None, str), Optional('client_key', default=None): Any(None, str), @@ -16,12 +16,12 @@ def config_client(): Optional('aws_secret_key', default=None): Any(None, str), Optional('aws_region', default=None): Any(None, str), Optional('ssl_no_validate', default=False): All( - Any(int, bool), Coerce(bool)), + Any(bool, int), Coerce(bool)), Optional('http_auth', default=None): Any(None, str), Optional('timeout', default=30): All( Coerce(int), Range(min=1, max=86400)), Optional('master_only', default=False): All( - Any(int, bool), Coerce(bool)), + Any(bool, int), Coerce(bool)), } # Configuration file: logging diff --git a/curator/validators/filter_elements.py b/curator/validators/filter_elements.py index e07d86b6..fb29be04 100644 --- a/curator/validators/filter_elements.py +++ b/curator/validators/filter_elements.py @@ -62,7 +62,7 @@ def reverse(**kwargs): # Only used with space filtertype # Should be ignored if `use_age` is True return { Optional('reverse', default=True): All( - Any(int, bool), Coerce(bool)) } + Any(bool, int), Coerce(bool)) } def source(**kwargs): # This setting is only used with the age filtertype, or with the space @@ -111,7 +111,7 @@ def unit_count(**kwargs): def use_age(**kwargs): # Use of this setting requires the additional setting, source. return { Optional('use_age', default=False): All( - Any(int, bool), Coerce(bool)) } + Any(bool, int), Coerce(bool)) } def value(**kwargs): # This setting is only used with the pattern filtertype and is a required diff --git a/curator/validators/options.py b/curator/validators/options.py index 3288f34d..35cfcf4f 100644 --- a/curator/validators/options.py +++ b/curator/validators/options.py @@ -9,7 +9,7 @@ def allocation_type(): def continue_if_exception(): return { Optional('continue_if_exception', default=False): All( - Any(int, bool), Coerce(bool)) } + Any(bool, int), Coerce(bool)) } def count(): return { Required('count'): All(Coerce(int), Range(min=0, max=10)) } @@ -23,30 +23,30 @@ def delay(): def delete_aliases(): return { Optional('delete_aliases', default=False): All( - Any(int, bool), Coerce(bool)) } + Any(bool, int), Coerce(bool)) } def disable_action(): return { Optional('disable_action', default=False): All( - Any(int, bool), Coerce(bool)) } + Any(bool, int), Coerce(bool)) } def extra_settings(): return { Optional('extra_settings', default={}): dict } def ignore_empty_list(): return { Optional('ignore_empty_list', default=False): All( - Any(int, bool), Coerce(bool)) } + Any(bool, int), Coerce(bool)) } def ignore_unavailable(): return { Optional('ignore_unavailable', default=False): All( - Any(int, bool), Coerce(bool)) } + Any(bool, int), Coerce(bool)) } def include_aliases(): return { Optional('include_aliases', default=False): All( - Any(int, bool), Coerce(bool)) } + Any(bool, int), Coerce(bool)) } def include_global_state(): return { Optional('include_global_state', default=True): All( - Any(int, bool), Coerce(bool)) } + Any(bool, int), Coerce(bool)) } def indices(): return { Optional('indices', default=None): Any(None, list) } @@ -69,7 +69,7 @@ def name(action): def partial(): return { Optional('partial', default=False): All( - Any(int, bool), Coerce(bool)) } + Any(bool, int), Coerce(bool)) } def rename_pattern(): return { Optional('rename_pattern'): str } @@ -96,7 +96,7 @@ def retry_interval(): def skip_repo_fs_check(): return { Optional('skip_repo_fs_check', default=False): All( - Any(int, bool), Coerce(bool)) } + Any(bool, int), Coerce(bool)) } def timeout_override(action): if action in ['forcemerge', 'restore', 'snapshot']: @@ -119,10 +119,10 @@ def value(): def wait_for_completion(action): if action in ['allocation', 'replicas']: return { Optional('wait_for_completion', default=False): All( - Any(int, bool), Coerce(bool)) } + Any(bool, int), Coerce(bool)) } elif action in ['restore', 'snapshot']: return { Optional('wait_for_completion', default=True): All( - Any(int, bool), Coerce(bool)) } + Any(bool, int), Coerce(bool)) } ## Methods for building the schema def action_specific(action): diff --git a/docs/Changelog.rst b/docs/Changelog.rst index 842de7d1..2c3dc9b1 100644 --- a/docs/Changelog.rst +++ b/docs/Changelog.rst @@ -3,6 +3,15 @@ Changelog ========= +4.1.1 (? ? ?) +------------- + +**Bug Fixes** + + * Have the boolean schema tests report ``expected bool`` instead of ``expected + int``. The result is the same, but the error message is misleading. + Reported in #757 (untergeek) + 4.1.0 (6 September 2016) ------------------------ From d73f8a02d3580cc4f15b8a51bd38e11b0be1d0a8 Mon Sep 17 00:00:00 2001 From: Aaron Mildenstein Date: Fri, 16 Sep 2016 14:24:18 -0600 Subject: [PATCH 02/12] use_age must be boolean interpretable This is for other tests to be better logged --- curator/validators/filters.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/curator/validators/filters.py b/curator/validators/filters.py index bf22221e..d1759fc4 100644 --- a/curator/validators/filters.py +++ b/curator/validators/filters.py @@ -37,7 +37,8 @@ def structure(): Optional('timestring'): Any(str, None), Optional('unit'): str, Optional('unit_count'): Coerce(int), - Optional('use_age'): Any(int, str, bool), + Optional('use_age'): All( + Any(bool, int), Coerce(bool)), Optional('value'): Any(int, float, str, bool), } retval.update(filtertype()) From 89dbf55effa849deec4b781744ae19f78f33d986 Mon Sep 17 00:00:00 2001 From: Aaron Mildenstein Date: Fri, 16 Sep 2016 14:25:09 -0600 Subject: [PATCH 03/12] 'source' is sometimes optional, sometimes required This should yield better error messages when 'source' is accidentally omitted when it shouldn't be --- curator/validators/filtertypes.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/curator/validators/filtertypes.py b/curator/validators/filtertypes.py index 5ff3a4d3..2132b0c6 100644 --- a/curator/validators/filtertypes.py +++ b/curator/validators/filtertypes.py @@ -8,11 +8,14 @@ def _age_elements(action, config): retval = [] - retval.append(filter_elements.source(action=action)) + is_req = True + if config['filtertype'] in ['count', 'space']: + is_req = True if 'use_age' in config and config['use_age'] else False + retval.append(filter_elements.source(action=action, required=is_req)) if action in settings.index_actions(): retval.append(filter_elements.stats_result()) - # This is a silly thing here, because the absence of 'source' will surely - # show up later in the schema check, but it keeps code from breaking... + # This is a silly thing here, because the absence of 'source' will + # show up in the actual schema check, but it keeps code from breaking here ts_req = False if 'source' in config: if config['source'] == 'name': @@ -24,6 +27,11 @@ def _age_elements(action, config): else: retval.append(filter_elements.field(required=False)) retval.append(filter_elements.timestring(required=ts_req)) + else: + # If source isn't in the config, then the other elements are not + # required, but should be Optional to prevent false positives + retval.append(filter_elements.field(required=False)) + retval.append(filter_elements.timestring(required=ts_req)) return retval ### Schema information ### @@ -37,7 +45,6 @@ def alias(action, config): def age(action, config): # Required & Optional retval = [ - filter_elements.source(action=action), filter_elements.direction(), filter_elements.unit(), filter_elements.unit_count(), From aa3a96331ee54f64c531d0b3be22846ca2a2729b Mon Sep 17 00:00:00 2001 From: Aaron Mildenstein Date: Fri, 16 Sep 2016 14:26:18 -0600 Subject: [PATCH 04/12] 'source' is sometimes required, sometimes optional This fixes the test for when it's optional and when it's required --- curator/validators/filter_elements.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/curator/validators/filter_elements.py b/curator/validators/filter_elements.py index fb29be04..aa44ead0 100644 --- a/curator/validators/filter_elements.py +++ b/curator/validators/filter_elements.py @@ -68,11 +68,14 @@ def source(**kwargs): # This setting is only used with the age filtertype, or with the space # filtertype when use_age is set to True. if 'action' in kwargs and kwargs['action'] in settings.snapshot_actions(): - return { Optional('source'): Any( - 'name', 'creation_date') } + valuelist = Any('name', 'creation_date') else: - return { Optional('source'): Any( - 'name', 'creation_date', 'field_stats') } + valuelist = Any('name', 'creation_date', 'field_stats') + + if 'required' in kwargs and kwargs['required']: + return { Required('source'): valuelist } + else: + return { Optional('source'): valuelist } def state(**kwargs): # This setting is only used with the state filtertype. From eb50206e5104e0bdb3ff791eb208cac00552c707 Mon Sep 17 00:00:00 2001 From: Aaron Mildenstein Date: Fri, 16 Sep 2016 16:04:10 -0600 Subject: [PATCH 05/12] Update repo_mgr to use curator.yml config file These changes also pave the way for other CLI scripts again, by reusing the client configuration information for client and logging (in config_utils.py). fixes feature/752 --- curator/cli.py | 47 +------- curator/config_utils.py | 49 ++++++++ curator/repomgrcli.py | 73 ++++------- test/integration/test_es_repo_mgr.py | 174 +++++++++++++++------------ test/integration/testvars.py | 9 ++ 5 files changed, 182 insertions(+), 170 deletions(-) create mode 100644 curator/config_utils.py diff --git a/curator/cli.py b/curator/cli.py index d2d95a33..7e80821e 100644 --- a/curator/cli.py +++ b/curator/cli.py @@ -4,23 +4,14 @@ import click from voluptuous import Schema from .defaults import settings -from .validators import SchemaCheck, config_file +from .validators import SchemaCheck +from .config_utils import process_config from .exceptions import * from .utils import * from .indexlist import IndexList from .snapshotlist import SnapshotList from .actions import * from ._version import __version__ -from .logtools import LogInfo, Whitelist, Blacklist - -try: - from logging import NullHandler -except ImportError: - from logging import Handler - - class NullHandler(Handler): - def emit(self, record): - pass CLASS_MAP = { 'alias' : Alias, @@ -68,9 +59,6 @@ def process_action(client, config, **kwargs): ### Update the defaults with whatever came with opts, minus any Nones mykwargs.update(prune_nones(opts)) logger.debug('Action kwargs: {0}'.format(mykwargs)) - # This is no longer necessary with the config schema validator - # # Verify the args we're going to pass match the action - # verify_args(action, mykwargs) ### Set up the action ### if action == 'alias': @@ -123,35 +111,8 @@ def cli(config, dry_run, action_file): See http://elastic.co/guide/en/elasticsearch/client/curator/current """ - # Get config from yaml file - yaml_config = get_yaml(config) - # if the file is empty, which is still valid yaml, set as an empty dict - yaml_config = {} if not yaml_config else prune_nones(yaml_config) - # Voluptuous can't verify the schema of a dict if it doesn't have keys, - # so make sure the keys are at least there and are dict() - for k in ['client', 'logging']: - if k not in yaml_config: - yaml_config[k] = {} - else: - yaml_config[k] = prune_nones(yaml_config[k]) - config_dict = SchemaCheck(yaml_config, config_file.client(), - 'Client Configuration', 'full configuration dictionary').result() - # Set up logging - log_opts = config_dict['logging'] - loginfo = LogInfo(log_opts) - logging.root.addHandler(loginfo.handler) - logging.root.setLevel(loginfo.numeric_log_level) - logger = logging.getLogger('curator.cli') - # Set up NullHandler() to handle nested elasticsearch.trace Logger - # instance in elasticsearch python client - logging.getLogger('elasticsearch.trace').addHandler(NullHandler()) - if log_opts['blacklist']: - for bl_entry in ensure_list(log_opts['blacklist']): - for handler in logging.root.handlers: - handler.addFilter(Blacklist(bl_entry)) - - client_args = config_dict['client'] - test_client_options(client_args) + client_args = process_config(config) + logger = logging.getLogger(__name__) logger.debug('Client and logging options validated.') # Extract this and save it for later, in case there's no timeout_override. diff --git a/curator/config_utils.py b/curator/config_utils.py new file mode 100644 index 00000000..29ac435b --- /dev/null +++ b/curator/config_utils.py @@ -0,0 +1,49 @@ +from voluptuous import Schema +# from .defaults import settings +from .validators import SchemaCheck, config_file +from .utils import * +from .logtools import LogInfo, Whitelist, Blacklist + +def test_config(config): + # Get config from yaml file + yaml_config = get_yaml(config) + # if the file is empty, which is still valid yaml, set as an empty dict + yaml_config = {} if not yaml_config else prune_nones(yaml_config) + # Voluptuous can't verify the schema of a dict if it doesn't have keys, + # so make sure the keys are at least there and are dict() + for k in ['client', 'logging']: + if k not in yaml_config: + yaml_config[k] = {} + else: + yaml_config[k] = prune_nones(yaml_config[k]) + return SchemaCheck(yaml_config, config_file.client(), + 'Client Configuration', 'full configuration dictionary').result() + +def set_logging(log_opts): + try: + from logging import NullHandler + except ImportError: + from logging import Handler + + class NullHandler(Handler): + def emit(self, record): + pass + + # Set up logging + loginfo = LogInfo(log_opts) + logging.root.addHandler(loginfo.handler) + logging.root.setLevel(loginfo.numeric_log_level) + logger = logging.getLogger('curator.cli') + # Set up NullHandler() to handle nested elasticsearch.trace Logger + # instance in elasticsearch python client + logging.getLogger('elasticsearch.trace').addHandler(NullHandler()) + if log_opts['blacklist']: + for bl_entry in ensure_list(log_opts['blacklist']): + for handler in logging.root.handlers: + handler.addFilter(Blacklist(bl_entry)) + +def process_config(yaml_file): + config = test_config(yaml_file) + set_logging(config['logging']) + test_client_options(config['client']) + return config['client'] diff --git a/curator/repomgrcli.py b/curator/repomgrcli.py index 5ced77e7..33c73b26 100644 --- a/curator/repomgrcli.py +++ b/curator/repomgrcli.py @@ -5,21 +5,12 @@ import logging from .defaults import settings from .exceptions import * +from .config_utils import process_config from .utils import * from ._version import __version__ -from .logtools import LogInfo logger = logging.getLogger('curator.repomgrcli') -try: - from logging import NullHandler -except ImportError: - from logging import Handler - - class NullHandler(Handler): - def emit(self, record): - pass - def delete_callback(ctx, param, value): if not value: ctx.abort() @@ -31,8 +22,15 @@ def show_repos(client): @click.command(short_help='Filesystem Repository') @click.option('--repository', required=True, type=str, help='Repository name') -@click.option('--location', required=True, type=str, - help='Shared file-system location. Must match remote path, & be accessible to all master & data nodes') +@click.option( + '--location', + required=True, + type=str, + help=( + 'Shared file-system location. ' + 'Must match remote path, & be accessible to all master & data nodes' + ) +) @click.option('--compression', type=bool, default=True, show_default=True, help='Enable/Disable metadata compression.') @click.option('--chunk_size', type=str, @@ -50,7 +48,8 @@ def fs( """ Create a filesystem repository. """ - client = get_client(**ctx.parent.parent.params) + logger = logging.getLogger('curator.repomgrcli.fs') + client = get_client(**ctx.obj['client_args']) try: create_repository(client, repo_type='fs', **ctx.params) except FailedExecution as e: @@ -85,7 +84,8 @@ def s3( """ Create an S3 repository. """ - client = get_client(**ctx.parent.parent.params) + logger = logging.getLogger('curator.repomgrcli.s3') + client = get_client(**ctx.obj['client_args']) try: create_repository(client, repo_type='s3', **ctx.params) except FailedExecution as e: @@ -95,41 +95,19 @@ def s3( @click.group() @click.option( - '--host', help='Elasticsearch host.', default='127.0.0.1') -@click.option( - '--url_prefix', help='Elasticsearch http url prefix.',default='') -@click.option('--port', help='Elasticsearch port.', default=9200, type=int) -@click.option('--use_ssl', help='Connect to Elasticsearch through SSL.', is_flag=True) -@click.option('--certificate', help='Path to certificate to use for SSL validation. (OPTIONAL)', type=str, default=None) -@click.option('--client-cert', help='Path to file containing SSL certificate for client auth. (OPTIONAL)', type=str, default=None) -@click.option('--client-key', help='Path to file containing SSL key for client auth. (OPTIONAL)', type=str, default=None) -@click.option('--ssl-no-validate', help='Do not validate server\'s SSL certificate', is_flag=True) -@click.option('--http_auth', help='Use Basic Authentication ex: user:pass', default='') -@click.option('--timeout', help='Connection timeout in seconds.', default=30, type=int) -@click.option('--master-only', is_flag=True, help='Only operate on elected master node.') -@click.option('--debug', is_flag=True, help='Debug mode') -@click.option('--loglevel', help='Log level', default='INFO') -@click.option('--logfile', help='log file', default=None) -@click.option('--logformat', help='Log output format [default|logstash].', default='default') -@click.version_option(version=__version__) + '--config', + help="Path to configuration file. Default: ~/.curator/curator.yml", + type=click.Path(exists=True), default=settings.config_file() +) @click.pass_context -def repo_mgr_cli( - ctx, host, url_prefix, port, use_ssl, certificate, client_cert, - client_key, ssl_no_validate, http_auth, timeout, master_only, debug, - loglevel, logfile, logformat): +def repo_mgr_cli(ctx, config): """ Repository manager for Elasticsearch Curator. """ - # Set up logging - if debug: - loglevel = 'DEBUG' - log_opts = {'loglevel':loglevel, 'logfile':logfile, 'logformat':logformat} - loginfo = LogInfo(log_opts) - logging.root.addHandler(loginfo.handler) - logging.root.setLevel(loginfo.numeric_log_level) - # Setting up NullHandler to handle nested elasticsearch.trace Logger - # instance in elasticsearch python client - logging.getLogger('elasticsearch.trace').addHandler(NullHandler()) + ctx.obj = {} + ctx.obj['client_args'] = process_config(config) + logger = logging.getLogger(__name__) + logger.debug('Client and logging options validated.') @repo_mgr_cli.group('create') @click.pass_context @@ -144,7 +122,7 @@ def show(ctx): """ Show all repositories """ - client = get_client(**ctx.parent.params) + client = get_client(**ctx.obj['client_args']) show_repos(client) @repo_mgr_cli.command('delete') @@ -155,11 +133,10 @@ def show(ctx): @click.pass_context def _delete(ctx, repository): """Delete an Elasticsearch repository""" - client = get_client(**ctx.parent.params) + client = get_client(**ctx.obj['client_args']) try: logger.info('Deleting repository {0}...'.format(repository)) client.snapshot.delete_repository(repository=repository) - # sys.exit(0) except elasticsearch.NotFoundError: logger.error( 'Unable to delete repository: {0} Not Found.'.format(repository)) diff --git a/test/integration/test_es_repo_mgr.py b/test/integration/test_es_repo_mgr.py index 8e6a36e0..508bb6ff 100644 --- a/test/integration/test_es_repo_mgr.py +++ b/test/integration/test_es_repo_mgr.py @@ -8,6 +8,7 @@ from mock import patch, Mock, MagicMock from . import CuratorTestCase +from . import testvars as testvars import logging logger = logging.getLogger(__name__) @@ -19,152 +20,167 @@ class TestLoggingModules(CuratorTestCase): def test_logger_without_null_handler(self): mock = Mock() modules = {'logger': mock, 'logger.NullHandler': mock.module} + self.write_config( + self.args['configfile'], + testvars.client_conf_logfile.format(host, port, os.devnull) + ) with patch.dict('sys.modules', modules): - self.create_repository() - test = clicktest.CliRunner() - result = test.invoke( - curator.repo_mgr_cli, - [ - '--logfile', os.devnull, - '--host', host, - '--port', str(port), - 'show' - ], - obj={"filters":[]}) + self.create_repository() + test = clicktest.CliRunner() + result = test.invoke( + curator.repo_mgr_cli, + [ + '--config', self.args['configfile'], + 'show' + ] + ) self.assertEqual(self.args['repository'], result.output.rstrip()) class TestCLIRepositoryCreate(CuratorTestCase): def test_create_fs_repository_success(self): + self.write_config( + self.args['configfile'], + testvars.client_conf_logfile.format(host, port, os.devnull) + ) test = clicktest.CliRunner() result = test.invoke( curator.repo_mgr_cli, [ - '--logfile', os.devnull, - '--host', host, - '--port', str(port), + '--config', self.args['configfile'], 'create', 'fs', '--repository', self.args['repository'], '--location', self.args['location'] - ], - obj={"filters":[]}) + ] + ) self.assertTrue(1, len(self.client.snapshot.get_repository(repository=self.args['repository']))) self.assertEqual(0, result.exit_code) def test_create_fs_repository_fail(self): + self.write_config( + self.args['configfile'], + testvars.client_conf_logfile.format(host, port, os.devnull) + ) test = clicktest.CliRunner() result = test.invoke( curator.repo_mgr_cli, [ - '--logfile', os.devnull, - '--host', host, - '--port', str(port), + '--config', self.args['configfile'], 'create', 'fs', '--repository', self.args['repository'], '--location', os.devnull - ], - obj={"filters":[]}) + ] + ) self.assertEqual(1, result.exit_code) def test_create_s3_repository_fail(self): + self.write_config( + self.args['configfile'], + testvars.client_conf_logfile.format(host, port, os.devnull) + ) test = clicktest.CliRunner() result = test.invoke( curator.repo_mgr_cli, [ - '--logfile', os.devnull, - '--host', host, - '--port', str(port), + '--config', self.args['configfile'], 'create', 's3', '--bucket', 'mybucket', '--repository', self.args['repository'], - ], - obj={"filters":[]}) + ] + ) self.assertEqual(1, result.exit_code) class TestCLIDeleteRepository(CuratorTestCase): def test_delete_repository_success(self): self.create_repository() + self.write_config( + self.args['configfile'], + testvars.client_conf_logfile.format(host, port, os.devnull) + ) test = clicktest.CliRunner() result = test.invoke( curator.repo_mgr_cli, [ - '--logfile', os.devnull, - '--host', host, - '--port', str(port), + '--config', self.args['configfile'], 'delete', '--yes', # This ensures no prompting will happen '--repository', self.args['repository'] - ], - obj={"filters":[]}) - self.assertFalse(curator.get_repository(self.client, self.args['repository'])) + ] + ) + self.assertFalse( + curator.get_repository(self.client, self.args['repository']) + ) def test_delete_repository_notfound(self): + self.write_config( + self.args['configfile'], + testvars.client_conf_logfile.format(host, port, os.devnull) + ) test = clicktest.CliRunner() result = test.invoke( curator.repo_mgr_cli, [ - '--logfile', os.devnull, - '--debug', - '--host', host, - '--port', str(port), + '--config', self.args['configfile'], 'delete', '--yes', # This ensures no prompting will happen '--repository', self.args['repository'] - ], - obj={"filters":[]}) + ] + ) self.assertEqual(1, result.exit_code) class TestCLIShowRepositories(CuratorTestCase): def test_show_repository(self): self.create_repository() + self.write_config( + self.args['configfile'], + testvars.client_conf_logfile.format(host, port, os.devnull) + ) test = clicktest.CliRunner() result = test.invoke( curator.repo_mgr_cli, [ - '--logfile', os.devnull, - '--host', host, - '--port', str(port), + '--config', self.args['configfile'], 'show' - ], - obj={"filters":[]}) + ] + ) self.assertEqual(self.args['repository'], result.output.rstrip()) -class TestRepoMGR_CLIOptions(CuratorTestCase): - def test_debug_logging(self): - dirname = ''.join(random.choice(string.ascii_uppercase + string.digits) for _ in range(8)) - logfile = tempfile.mkdtemp(suffix=dirname) + 'logfile' - self.create_repository() - test = clicktest.CliRunner() - result = test.invoke( - curator.repo_mgr_cli, - [ - '--logfile', logfile, - '--debug', - '--host', host, - '--port', str(port), - 'show' - ], - obj={"filters":[]}) - self.assertEqual(0, result.exit_code) - - def test_logstash_formatting(self): - dirname = ''.join(random.choice(string.ascii_uppercase + string.digits) for _ in range(8)) - logfile = tempfile.mkdtemp(suffix=dirname) + 'logfile' - self.create_repository() - test = clicktest.CliRunner() - result = test.invoke( - curator.repo_mgr_cli, - [ - '--logformat', 'logstash', - '--debug', - '--host', host, - '--port', str(port), - 'show' - ], - obj={"filters":[]}) - d = json.loads(result.output.splitlines()[:1][0]) - keys = sorted(list(d.keys())) - self.assertEqual(['@timestamp','function','linenum','loglevel','message','name'], keys) +# class TestRepoMGR_CLIOptions(CuratorTestCase): +# def test_debug_logging(self): +# dirname = ''.join(random.choice(string.ascii_uppercase + string.digits) for _ in range(8)) +# logfile = tempfile.mkdtemp(suffix=dirname) + 'logfile' +# self.create_repository() +# test = clicktest.CliRunner() +# result = test.invoke( +# curator.repo_mgr_cli, +# [ +# '--logfile', logfile, +# '--debug', +# '--host', host, +# '--port', str(port), +# 'show' +# ], +# obj={"filters":[]}) +# self.assertEqual(0, result.exit_code) +# +# def test_logstash_formatting(self): +# dirname = ''.join(random.choice(string.ascii_uppercase + string.digits) for _ in range(8)) +# logfile = tempfile.mkdtemp(suffix=dirname) + 'logfile' +# self.create_repository() +# test = clicktest.CliRunner() +# result = test.invoke( +# curator.repo_mgr_cli, +# [ +# '--logformat', 'logstash', +# '--debug', +# '--host', host, +# '--port', str(port), +# 'show' +# ], +# obj={"filters":[]}) +# d = json.loads(result.output.splitlines()[:1][0]) +# keys = sorted(list(d.keys())) +# self.assertEqual(['@timestamp','function','linenum','loglevel','message','name'], keys) diff --git a/test/integration/testvars.py b/test/integration/testvars.py index c2434841..f10aebe4 100644 --- a/test/integration/testvars.py +++ b/test/integration/testvars.py @@ -20,6 +20,15 @@ ' logformat: default\n' ' blacklist: []\n') +client_conf_logfile = ('---\n' +'client:\n' +' hosts: {0}\n' +' port: {1}\n' +'\n' +'logging:\n' +' loglevel: DEBUG\n' +' logfile: {2}\n') + client_config_envvars = ('---\n' 'client:\n' ' hosts: {0}\n' From b70dbbd69ea0232bf6019e296264fad3a2feac77 Mon Sep 17 00:00:00 2001 From: Aaron Mildenstein Date: Fri, 16 Sep 2016 16:09:52 -0600 Subject: [PATCH 06/12] Update Changelog fixes #752 --- docs/Changelog.rst | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/Changelog.rst b/docs/Changelog.rst index 2c3dc9b1..fcd68715 100644 --- a/docs/Changelog.rst +++ b/docs/Changelog.rst @@ -12,6 +12,17 @@ Changelog int``. The result is the same, but the error message is misleading. Reported in #757 (untergeek) +**General** + + * Update es_repo_mgr to use the same client/logging YAML config file. + Requested in #752 (untergeek) + +**Schema Validation** + + * Cases where ``source`` was not defined in a filter (but should have been) + were informing users that a `timestring` field was there that shouldn't have + been. This edge case has been corrected. + 4.1.0 (6 September 2016) ------------------------ From e8d56c8071c971ac17486b4af7cd0151953e3a22 Mon Sep 17 00:00:00 2001 From: Aaron Mildenstein Date: Fri, 16 Sep 2016 16:13:10 -0600 Subject: [PATCH 07/12] Remove unnecessary tests Logging and such are tested separately now. --- test/integration/test_es_repo_mgr.py | 37 ---------------------------- 1 file changed, 37 deletions(-) diff --git a/test/integration/test_es_repo_mgr.py b/test/integration/test_es_repo_mgr.py index 508bb6ff..b3d59037 100644 --- a/test/integration/test_es_repo_mgr.py +++ b/test/integration/test_es_repo_mgr.py @@ -147,40 +147,3 @@ def test_show_repository(self): ] ) self.assertEqual(self.args['repository'], result.output.rstrip()) - -# class TestRepoMGR_CLIOptions(CuratorTestCase): -# def test_debug_logging(self): -# dirname = ''.join(random.choice(string.ascii_uppercase + string.digits) for _ in range(8)) -# logfile = tempfile.mkdtemp(suffix=dirname) + 'logfile' -# self.create_repository() -# test = clicktest.CliRunner() -# result = test.invoke( -# curator.repo_mgr_cli, -# [ -# '--logfile', logfile, -# '--debug', -# '--host', host, -# '--port', str(port), -# 'show' -# ], -# obj={"filters":[]}) -# self.assertEqual(0, result.exit_code) -# -# def test_logstash_formatting(self): -# dirname = ''.join(random.choice(string.ascii_uppercase + string.digits) for _ in range(8)) -# logfile = tempfile.mkdtemp(suffix=dirname) + 'logfile' -# self.create_repository() -# test = clicktest.CliRunner() -# result = test.invoke( -# curator.repo_mgr_cli, -# [ -# '--logformat', 'logstash', -# '--debug', -# '--host', host, -# '--port', str(port), -# 'show' -# ], -# obj={"filters":[]}) -# d = json.loads(result.output.splitlines()[:1][0]) -# keys = sorted(list(d.keys())) -# self.assertEqual(['@timestamp','function','linenum','loglevel','message','name'], keys) From 898739e90f2a966a692a70405b0c12ef0d0b4d47 Mon Sep 17 00:00:00 2001 From: galkinrost Date: Wed, 21 Sep 2016 10:35:15 +0300 Subject: [PATCH 08/12] Fixes #762 --- curator/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/curator/utils.py b/curator/utils.py index b1a7a9c9..7f4e6d76 100644 --- a/curator/utils.py +++ b/curator/utils.py @@ -541,7 +541,7 @@ def get_client(**kwargs): else kwargs['aws_secret_key'] kwargs['aws_region'] = False if not 'aws_region' in kwargs \ else kwargs['aws_region'] - if kwargs['aws_key'] or kwargs['aws_secret_key'] or kwargs['region']: + if kwargs['aws_key'] or kwargs['aws_secret_key'] or kwargs['aws_region']: if not kwargs['aws_key'] and kwargs['aws_secret_key'] \ and kwargs['aws_region']: raise MissingArgument( From 896dfbe969b31929c6f32443d2096e57ef3f5633 Mon Sep 17 00:00:00 2001 From: Aaron Mildenstein Date: Mon, 26 Sep 2016 20:14:35 +0200 Subject: [PATCH 09/12] Use voluptuous' Boolean() method to coerce bools ...from strings, or numbers. fixes #765 --- curator/validators/config_file.py | 8 +++---- curator/validators/filter_elements.py | 9 +++----- curator/validators/filters.py | 3 +-- curator/validators/options.py | 33 +++++++++------------------ docs/Changelog.rst | 6 ++--- 5 files changed, 21 insertions(+), 38 deletions(-) diff --git a/curator/validators/config_file.py b/curator/validators/config_file.py index 8c83dc64..27bf0581 100644 --- a/curator/validators/config_file.py +++ b/curator/validators/config_file.py @@ -8,20 +8,18 @@ def config_client(): None, All(Coerce(int), Range(min=1, max=65535)) ), Optional('url_prefix', default=''): Any(None, str), - Optional('use_ssl', default=False): All(Any(bool, int), Coerce(bool)), + Optional('use_ssl', default=False): Boolean(), Optional('certificate', default=None): Any(None, str), Optional('client_cert', default=None): Any(None, str), Optional('client_key', default=None): Any(None, str), Optional('aws_key', default=None): Any(None, str), Optional('aws_secret_key', default=None): Any(None, str), Optional('aws_region', default=None): Any(None, str), - Optional('ssl_no_validate', default=False): All( - Any(bool, int), Coerce(bool)), + Optional('ssl_no_validate', default=False): Boolean(), Optional('http_auth', default=None): Any(None, str), Optional('timeout', default=30): All( Coerce(int), Range(min=1, max=86400)), - Optional('master_only', default=False): All( - Any(bool, int), Coerce(bool)), + Optional('master_only', default=False): Boolean(), } # Configuration file: logging diff --git a/curator/validators/filter_elements.py b/curator/validators/filter_elements.py index aa44ead0..67d9104b 100644 --- a/curator/validators/filter_elements.py +++ b/curator/validators/filter_elements.py @@ -33,8 +33,7 @@ def exclude(**kwargs): val = True else: # False by default val = False - return { Optional('exclude', default=val): All( - Any(bool, int), Coerce(bool)) } + return { Optional('exclude', default=val): Boolean() } def field(**kwargs): # This setting is only used with the age filtertype. @@ -61,8 +60,7 @@ def max_num_segments(**kwargs): def reverse(**kwargs): # Only used with space filtertype # Should be ignored if `use_age` is True - return { Optional('reverse', default=True): All( - Any(bool, int), Coerce(bool)) } + return { Optional('reverse', default=True): Boolean() } def source(**kwargs): # This setting is only used with the age filtertype, or with the space @@ -113,8 +111,7 @@ def unit_count(**kwargs): def use_age(**kwargs): # Use of this setting requires the additional setting, source. - return { Optional('use_age', default=False): All( - Any(bool, int), Coerce(bool)) } + return { Optional('use_age', default=False): Boolean() } def value(**kwargs): # This setting is only used with the pattern filtertype and is a required diff --git a/curator/validators/filters.py b/curator/validators/filters.py index d1759fc4..bbd80518 100644 --- a/curator/validators/filters.py +++ b/curator/validators/filters.py @@ -37,8 +37,7 @@ def structure(): Optional('timestring'): Any(str, None), Optional('unit'): str, Optional('unit_count'): Coerce(int), - Optional('use_age'): All( - Any(bool, int), Coerce(bool)), + Optional('use_age'): Boolean(), Optional('value'): Any(int, float, str, bool), } retval.update(filtertype()) diff --git a/curator/validators/options.py b/curator/validators/options.py index 35cfcf4f..67be9ca4 100644 --- a/curator/validators/options.py +++ b/curator/validators/options.py @@ -8,8 +8,7 @@ def allocation_type(): str, Any('require', 'include', 'exclude')) } def continue_if_exception(): - return { Optional('continue_if_exception', default=False): All( - Any(bool, int), Coerce(bool)) } + return { Optional('continue_if_exception', default=False): Boolean() } def count(): return { Required('count'): All(Coerce(int), Range(min=0, max=10)) } @@ -22,31 +21,25 @@ def delay(): } def delete_aliases(): - return { Optional('delete_aliases', default=False): All( - Any(bool, int), Coerce(bool)) } + return { Optional('delete_aliases', default=False): Boolean() } def disable_action(): - return { Optional('disable_action', default=False): All( - Any(bool, int), Coerce(bool)) } + return { Optional('disable_action', default=False): Boolean() } def extra_settings(): return { Optional('extra_settings', default={}): dict } def ignore_empty_list(): - return { Optional('ignore_empty_list', default=False): All( - Any(bool, int), Coerce(bool)) } + return { Optional('ignore_empty_list', default=False): Boolean() } def ignore_unavailable(): - return { Optional('ignore_unavailable', default=False): All( - Any(bool, int), Coerce(bool)) } + return { Optional('ignore_unavailable', default=False): Boolean() } def include_aliases(): - return { Optional('include_aliases', default=False): All( - Any(bool, int), Coerce(bool)) } + return { Optional('include_aliases', default=False): Boolean() } def include_global_state(): - return { Optional('include_global_state', default=True): All( - Any(bool, int), Coerce(bool)) } + return { Optional('include_global_state', default=True): Boolean() } def indices(): return { Optional('indices', default=None): Any(None, list) } @@ -68,8 +61,7 @@ def name(action): return { Optional('name'): str } def partial(): - return { Optional('partial', default=False): All( - Any(bool, int), Coerce(bool)) } + return { Optional('partial', default=False): Boolean() } def rename_pattern(): return { Optional('rename_pattern'): str } @@ -95,8 +87,7 @@ def retry_interval(): } def skip_repo_fs_check(): - return { Optional('skip_repo_fs_check', default=False): All( - Any(bool, int), Coerce(bool)) } + return { Optional('skip_repo_fs_check', default=False): Boolean() } def timeout_override(action): if action in ['forcemerge', 'restore', 'snapshot']: @@ -118,11 +109,9 @@ def value(): def wait_for_completion(action): if action in ['allocation', 'replicas']: - return { Optional('wait_for_completion', default=False): All( - Any(bool, int), Coerce(bool)) } + return { Optional('wait_for_completion', default=False): Boolean() } elif action in ['restore', 'snapshot']: - return { Optional('wait_for_completion', default=True): All( - Any(bool, int), Coerce(bool)) } + return { Optional('wait_for_completion', default=True): Boolean() } ## Methods for building the schema def action_specific(action): diff --git a/docs/Changelog.rst b/docs/Changelog.rst index fcd68715..09b02a6a 100644 --- a/docs/Changelog.rst +++ b/docs/Changelog.rst @@ -8,9 +8,9 @@ Changelog **Bug Fixes** - * Have the boolean schema tests report ``expected bool`` instead of ``expected - int``. The result is the same, but the error message is misleading. - Reported in #757 (untergeek) + * String-based booleans are now properly coerced. This fixes an issue where + `True`/`False` were used in environment variables, but not recognized. + #765 (untergeek) **General** From b30254e6278647555f32f4c02bd6266aac561cf6 Mon Sep 17 00:00:00 2001 From: Aaron Mildenstein Date: Mon, 26 Sep 2016 21:32:57 +0200 Subject: [PATCH 10/12] Document AWS IAM incompatibility fixes #769 --- curator/utils.py | 6 ++++++ docs/Changelog.rst | 4 ++++ docs/asciidoc/configuration.asciidoc | 14 +++++++------- docs/asciidoc/faq.asciidoc | 29 ++++++++++++++++++++++++++++ 4 files changed, 46 insertions(+), 7 deletions(-) diff --git a/curator/utils.py b/curator/utils.py index 7f4e6d76..ebf22afd 100644 --- a/curator/utils.py +++ b/curator/utils.py @@ -451,6 +451,12 @@ def check_master(client, master_only=False): def get_client(**kwargs): """ + NOTE: AWS IAM parameters `aws_key`, `aws_secret_key`, and `aws_region` are + provided for future compatibility, should AWS ES support the + ``/_cluster/state/metadata`` endpoint. So long as this endpoint does not + function in AWS ES, the client will not be able to use + :class:`curator.indexlist.IndexList`, which is the backbone of Curator 4 + Return an :class:`elasticsearch.Elasticsearch` client object using the provided parameters. Any of the keyword arguments the :class:`elasticsearch.Elasticsearch` client object can receive are valid, diff --git a/docs/Changelog.rst b/docs/Changelog.rst index 09b02a6a..b7fc9229 100644 --- a/docs/Changelog.rst +++ b/docs/Changelog.rst @@ -23,6 +23,10 @@ Changelog were informing users that a `timestring` field was there that shouldn't have been. This edge case has been corrected. +**Documentation** + + * Added notifications and FAQ entry to explain that AWS ES is not supported. + 4.1.0 (6 September 2016) ------------------------ diff --git a/docs/asciidoc/configuration.asciidoc b/docs/asciidoc/configuration.asciidoc index 4125e682..6848ed37 100644 --- a/docs/asciidoc/configuration.asciidoc +++ b/docs/asciidoc/configuration.asciidoc @@ -194,9 +194,6 @@ client: certificate: client_cert: client_key: - aws_key: - aws_secret_key: - aws_region: ssl_no_validate: False http_auth: timeout: 30 @@ -359,7 +356,8 @@ SSL key. The key file must be an unencrypted key in PEM format. [[aws_key]] === aws_key -IMPORTANT: This is an experimental feature and may not yet work as expected. +WARNING: This feature allows connection to AWS using IAM credentials, but + <>. WARNING: This setting will not work unless the `requests-aws4auth` Python module has been manually installed first. @@ -378,6 +376,9 @@ IMPORTANT: You must set your <> to the proper hostname _with_ port. [[aws_secret_key]] === aws_secret_key +WARNING: This feature allows connection to AWS using IAM credentials, but + <>. + WARNING: This setting will not work unless the `requests-aws4auth` Python module has been manually installed first. @@ -388,8 +389,6 @@ This should be an AWS IAM secret access key, or left empty. aws_secret_key: ----------- -IMPORTANT: This is an experimental feature and may not yet work as expected. - IMPORTANT: You must set your <> to the proper hostname _with_ port. It may not work setting <> and <> to only a host name due to the different connection module used. @@ -397,7 +396,8 @@ IMPORTANT: You must set your <> to the proper hostname _with_ port. [[aws_region]] === aws_region -IMPORTANT: This is an experimental feature and may not yet work as expected. +WARNING: This feature allows connection to AWS using IAM credentials, but + <>. WARNING: This setting will not work unless the `requests-aws4auth` Python module has been manually installed first. diff --git a/docs/asciidoc/faq.asciidoc b/docs/asciidoc/faq.asciidoc index 54ebc3c0..9c0623b0 100644 --- a/docs/asciidoc/faq.asciidoc +++ b/docs/asciidoc/faq.asciidoc @@ -11,6 +11,7 @@ This section will be updated as more frequently asked questions arise * <> * <> * <> +* <> -- [[faq_doc_error]] @@ -291,3 +292,31 @@ You can use the Sense plugin for Kibana to act on these indices manually, or your preference of API method. ''''' + +[[faq_aws_iam]] +== Q: Why doesn't Curator 4 work with AWS Elasticsearch? + +[float] +A: Because Curator 4 requires access to the `/_cluster/state/metadata` endpoint. +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +There is some confusion because Curator 3 supported AWS ES, but Curator 4 does +not. There are even some IAM credentials listed as options for client +connections. These are currently available, but not able to be used. This may +change at some point, so they remain at the ready until then. + +Curator 4 requires access to the `/_cluster/state/metadata` endpoint in order to +pull metadata at IndexList initialization time for _all_ indices. This metadata +is used to determine index routing information, index sizing, index state +(either `open` or `close`), aliases, and more. Curator 4 switched to doing this +in order to reduce the number of repetitive client calls that were made in the +previous versions. + +After Curator 4 was released, AWS announced it's 2.3 version of Elasticsearch. +Unfortunately, this version did not support the `/_cluster/state/metadata` +endpoint, which means that Curator cannot be used to manage indices in AWS. + +https://forums.aws.amazon.com/thread.jspa?threadID=236643&tstart=0[This thread] +(requires AWS login credentials) tracks the issue on the Amazon side. + +''''' From ea672eff6758688ab1ca61fa4e59dd593318eb21 Mon Sep 17 00:00:00 2001 From: Aaron Mildenstein Date: Mon, 26 Sep 2016 21:57:22 +0200 Subject: [PATCH 11/12] Fix omission of count in __map_method It was omitted by, well, accidental omission. fixes #766 --- curator/snapshotlist.py | 1 + docs/Changelog.rst | 3 +++ 2 files changed, 4 insertions(+) diff --git a/curator/snapshotlist.py b/curator/snapshotlist.py index b80fbbea..f32bd52e 100644 --- a/curator/snapshotlist.py +++ b/curator/snapshotlist.py @@ -85,6 +85,7 @@ def __get_snapshots(self): def __map_method(self, ft): methods = { 'age': self.filter_by_age, + 'count': self.filter_by_count, 'none': self.filter_none, 'pattern': self.filter_by_regex, 'state': self.filter_by_state, diff --git a/docs/Changelog.rst b/docs/Changelog.rst index b7fc9229..3e8f5f79 100644 --- a/docs/Changelog.rst +++ b/docs/Changelog.rst @@ -12,6 +12,9 @@ Changelog `True`/`False` were used in environment variables, but not recognized. #765 (untergeek) + * Fix missing `count` method in ``__map_method`` in SnapshotList. Reported in + #766 (untergeek) + **General** * Update es_repo_mgr to use the same client/logging YAML config file. From b597af76d7a10eca4e853007b8212dec6e99a449 Mon Sep 17 00:00:00 2001 From: Aaron Mildenstein Date: Mon, 26 Sep 2016 22:58:10 +0200 Subject: [PATCH 12/12] Prepare for 4.1.1 release And there was much rejoicing... --- curator/_version.py | 2 +- docs/Changelog.rst | 4 ++-- docs/asciidoc/index.asciidoc | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/curator/_version.py b/curator/_version.py index fa721b49..47cbba72 100644 --- a/curator/_version.py +++ b/curator/_version.py @@ -1 +1 @@ -__version__ = '4.1.0' +__version__ = '4.1.1' diff --git a/docs/Changelog.rst b/docs/Changelog.rst index 3e8f5f79..163b3506 100644 --- a/docs/Changelog.rst +++ b/docs/Changelog.rst @@ -3,8 +3,8 @@ Changelog ========= -4.1.1 (? ? ?) -------------- +4.1.1 (27 September 2016) +------------------------- **Bug Fixes** diff --git a/docs/asciidoc/index.asciidoc b/docs/asciidoc/index.asciidoc index 7b888d85..7d11de0c 100644 --- a/docs/asciidoc/index.asciidoc +++ b/docs/asciidoc/index.asciidoc @@ -1,4 +1,4 @@ -:curator_version: 4.1.0 +:curator_version: 4.1.1 :curator_major: 4 :es_py_version: 2.4.0 :ref: http://www.elastic.co/guide/en/elasticsearch/reference/current