From a4c49201c0d6f4d821fcb21798fa28a2885262b4 Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Thu, 11 Aug 2016 23:54:31 +0200 Subject: [PATCH 1/9] Add jobs option for parallel execution, global explorers first. --- cdist/config.py | 7 +++--- cdist/core/explorer.py | 38 ++++++++++++++++++++++++++++----- cdist/test/explorer/__init__.py | 32 +++++++++++++++++++++++++++ scripts/cdist | 20 +++++++++++++++++ 4 files changed, 89 insertions(+), 8 deletions(-) diff --git a/cdist/config.py b/cdist/config.py index da560e91..34d647e1 100644 --- a/cdist/config.py +++ b/cdist/config.py @@ -70,15 +70,16 @@ def inspect_ssh_mux_opts(): class Config(object): """Cdist main class to hold arbitrary data""" - def __init__(self, local, remote, dry_run=False): + def __init__(self, local, remote, dry_run=False, jobs=None): self.local = local self.remote = remote self.log = logging.getLogger(self.local.target_host) self.dry_run = dry_run + self.jobs = jobs self.explorer = core.Explorer(self.local.target_host, self.local, - self.remote) + self.remote, jobs=self.jobs) self.manifest = core.Manifest(self.local.target_host, self.local) self.code = core.Code(self.local.target_host, self.local, self.remote) @@ -241,7 +242,7 @@ class Config(object): remote_exec=remote_exec, remote_copy=remote_copy) - c = cls(local, remote, dry_run=args.dry_run) + c = cls(local, remote, dry_run=args.dry_run, jobs=args.jobs) c.run() except cdist.Error as e: diff --git a/cdist/core/explorer.py b/cdist/core/explorer.py index 345f45ff..ac2a45ca 100644 --- a/cdist/core/explorer.py +++ b/cdist/core/explorer.py @@ -23,6 +23,7 @@ import logging import os import glob +import multiprocessing import cdist @@ -65,7 +66,7 @@ class Explorer(object): """Executes cdist explorers. """ - def __init__(self, target_host, local, remote): + def __init__(self, target_host, local, remote, jobs=None): self.target_host = target_host self.log = logging.getLogger(target_host) @@ -77,6 +78,7 @@ class Explorer(object): '__explorer': self.remote.global_explorer_path, } self._type_explorers_transferred = [] + self.jobs = jobs # global @@ -91,11 +93,37 @@ class Explorer(object): """ self.log.info("Running global explorers") self.transfer_global_explorers() + if self.jobs is None: + self._run_global_explorers_seq(out_path) + else: + self._run_global_explorers_parallel(out_path) + + def _run_global_explorer(self, explorer, out_path): + output = self.run_global_explorer(explorer) + path = os.path.join(out_path, explorer) + with open(path, 'w') as fd: + fd.write(output) + + def _run_global_explorers_seq(self, out_path): + self.log.info("Running global explorers sequentially") for explorer in self.list_global_explorer_names(): - output = self.run_global_explorer(explorer) - path = os.path.join(out_path, explorer) - with open(path, 'w') as fd: - fd.write(output) + self._run_global_explorer(explorer, out_path) + + def _run_global_explorers_parallel(self, out_path): + self.log.info("Running global explorers in {} parallel jobs".format( + self.jobs)) + self.log.info("Starting multiprocessing Pool") + with multiprocessing.Pool(self.jobs) as pool: + self.log.info("Starting async global explorer run") + results = [ + pool.apply_async(self._run_global_explorer, (e, out_path,)) + for e in self.list_global_explorer_names() + ] + self.log.info("Waiting async global explorer run results") + for r in results: + r.get() + self.log.info("Async global explorer run finished") + self.log.info("Multiprocessing Pool finished") def transfer_global_explorers(self): """Transfer the global explorers to the remote side.""" diff --git a/cdist/test/explorer/__init__.py b/cdist/test/explorer/__init__.py index 9a4555b8..27820292 100644 --- a/cdist/test/explorer/__init__.py +++ b/cdist/test/explorer/__init__.py @@ -23,6 +23,7 @@ import os import shutil import getpass +import multiprocessing import cdist from cdist import core @@ -168,3 +169,34 @@ class ExplorerClassTestCase(test.CdistTestCase): cdist_object.create() self.explorer.run_type_explorers(cdist_object) self.assertEqual(cdist_object.explorers, {'world': 'hello'}) + + def test_jobs_parameter(self): + self.assertIsNone(self.explorer.jobs) + expl = explorer.Explorer( + self.target_host, + self.local, + self.remote, + jobs=8) + self.assertEqual(expl.jobs, 8) + + def test_run_parallel_jobs(self): + expl = explorer.Explorer( + self.target_host, + self.local, + self.remote, + jobs=multiprocessing.cpu_count()) + self.assertIsNotNone(expl.jobs) + out_path = self.mkdtemp() + + expl.run_global_explorers(out_path) + names = sorted(expl.list_global_explorer_names()) + output = sorted(os.listdir(out_path)) + + self.assertEqual(names, output) + shutil.rmtree(out_path) + + +if __name__ == '__main__': + import unittest + + unittest.main() diff --git a/scripts/cdist b/scripts/cdist index 953cad78..b7e3a805 100755 --- a/scripts/cdist +++ b/scripts/cdist @@ -22,6 +22,20 @@ # +def check_positive_int(value): + import argparse + + try: + val = int(value) + except ValueError as e: + raise argparse.ArgumentTypeError( + "{} is invalid int value".format(value)) + if val <= 0: + raise argparse.ArgumentTypeError( + "{} is invalid positive int value".format(val)) + return val + + def commandline(): """Parse command line""" import argparse @@ -31,6 +45,7 @@ def commandline(): import cdist.shell import shutil import os + import multiprocessing # Construct parser others can reuse parser = {} @@ -105,6 +120,11 @@ def commandline(): '(should behave like ssh)'), action='store', dest='remote_exec', default=os.environ.get('CDIST_REMOTE_EXEC')) + parser['config'].add_argument( + '-j', '--jobs', nargs='?', type=check_positive_int, + help='Specify the maximum number of parallel jobs', + action='store', dest='jobs', + const=multiprocessing.cpu_count()) parser['config'].set_defaults(func=cdist.config.Config.commandline) # Shell From 5f436f21b8a33d56d9d7e5d2ea24ea76c73819d4 Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Fri, 12 Aug 2016 21:14:56 +0200 Subject: [PATCH 2/9] Transfer and run global explorers in parallel. --- cdist/core/explorer.py | 43 ++++++++++++++++++++------ cdist/exec/local.py | 12 ++++++++ cdist/exec/remote.py | 54 +++++++++++++++++++++++++++++++-- cdist/test/exec/remote.py | 17 +++++++++++ cdist/test/explorer/__init__.py | 14 +++++++++ docs/changelog | 4 +-- 6 files changed, 131 insertions(+), 13 deletions(-) diff --git a/cdist/core/explorer.py b/cdist/core/explorer.py index ac2a45ca..efffb6ed 100644 --- a/cdist/core/explorer.py +++ b/cdist/core/explorer.py @@ -69,7 +69,7 @@ class Explorer(object): def __init__(self, target_host, local, remote, jobs=None): self.target_host = target_host - self.log = logging.getLogger(target_host) + self._open_logger() self.local = local self.remote = remote @@ -80,6 +80,9 @@ class Explorer(object): self._type_explorers_transferred = [] self.jobs = jobs + def _open_logger(self): + self.log = logging.getLogger(self.target_host) + # global def list_global_explorer_names(self): @@ -112,24 +115,46 @@ class Explorer(object): def _run_global_explorers_parallel(self, out_path): self.log.info("Running global explorers in {} parallel jobs".format( self.jobs)) - self.log.info("Starting multiprocessing Pool") + self.log.debug("Multiprocessing start method is {}".format( + multiprocessing.get_start_method())) + self.log.info(("Starting multiprocessing Pool for global " + "explorers run")) with multiprocessing.Pool(self.jobs) as pool: - self.log.info("Starting async global explorer run") + self.log.info("Starting async for global explorer run") results = [ pool.apply_async(self._run_global_explorer, (e, out_path,)) for e in self.list_global_explorer_names() ] - self.log.info("Waiting async global explorer run results") + + self.log.info("Waiting async results for global explorer runs") for r in results: - r.get() - self.log.info("Async global explorer run finished") - self.log.info("Multiprocessing Pool finished") + r.get() # self._run_global_explorer returns None + self.log.info(("Multiprocessing run for global explorers " + "finished")) + + # logger is not pickable, so remove it when we pickle + def __getstate__(self): + state = self.__dict__.copy() + if 'log' in state: + del state['log'] + return state + + # recreate logger when we unpickle + def __setstate__(self, state): + self.__dict__.update(state) + self._open_logger() def transfer_global_explorers(self): """Transfer the global explorers to the remote side.""" self.remote.mkdir(self.remote.global_explorer_path) - self.remote.transfer(self.local.global_explorer_path, - self.remote.global_explorer_path) + if self.jobs is None: + self.remote.transfer(self.local.global_explorer_path, + self.remote.global_explorer_path) + else: + self.remote.transfer_dir_parallel( + self.local.global_explorer_path, + self.remote.global_explorer_path, + self.jobs) self.remote.run(["chmod", "0700", "%s/*" % (self.remote.global_explorer_path)]) diff --git a/cdist/exec/local.py b/cdist/exec/local.py index 4fdb5170..34826bfb 100644 --- a/cdist/exec/local.py +++ b/cdist/exec/local.py @@ -85,6 +85,18 @@ class Local(object): def _init_log(self): self.log = logging.getLogger(self.target_host) + # logger is not pickable, so remove it when we pickle + def __getstate__(self): + state = self.__dict__.copy() + if 'log' in state: + del state['log'] + return state + + # recreate logger when we unpickle + def __setstate__(self, state): + self.__dict__.update(state) + self._init_log() + def _init_permissions(self): # Setup file permissions using umask os.umask(0o077) diff --git a/cdist/exec/remote.py b/cdist/exec/remote.py index b6ff8c1f..389c0da1 100644 --- a/cdist/exec/remote.py +++ b/cdist/exec/remote.py @@ -26,9 +26,10 @@ import sys import glob import subprocess import logging -import cdist.exec.util as exec_util +import multiprocessing import cdist +import cdist.exec.util as exec_util class DecodeError(cdist.Error): @@ -66,10 +67,25 @@ class Remote(object): self.type_path = os.path.join(self.conf_path, "type") self.global_explorer_path = os.path.join(self.conf_path, "explorer") - self.log = logging.getLogger(self.target_host) + self._open_logger() self._init_env() + def _open_logger(self): + self.log = logging.getLogger(self.target_host) + + # logger is not pickable, so remove it when we pickle + def __getstate__(self): + state = self.__dict__.copy() + if 'log' in state: + del state['log'] + return state + + # recreate logger when we unpickle + def __setstate__(self, state): + self.__dict__.update(state) + self._open_logger() + def _init_env(self): """Setup environment for scripts - HERE????""" # FIXME: better do so in exec functions that require it! @@ -110,6 +126,40 @@ class Remote(object): self.target_host, destination)]) self._run_command(command) + def transfer_dir_parallel(self, source, destination, jobs): + """Transfer a directory to the remote side in parallel mode.""" + self.log.debug("Remote transfer: %s -> %s", source, destination) + self.rmdir(destination) + if os.path.isdir(source): + self.mkdir(destination) + self.log.info("Remote transfer in {} parallel jobs".format( + jobs)) + self.log.debug("Multiprocessing start method is {}".format( + multiprocessing.get_start_method())) + self.log.info(("Starting multiprocessing Pool for parallel " + "remote transfer")) + with multiprocessing.Pool(jobs) as pool: + self.log.info("Starting async for parallel transfer") + commands = [] + for f in glob.glob1(source, '*'): + command = self._copy.split() + path = os.path.join(source, f) + command.extend([path, '{0}:{1}'.format( + self.target_host, destination)]) + commands.append(command) + results = [ + pool.apply_async(self._run_command, (cmd,)) + for cmd in commands + ] + + self.log.info("Waiting async results for parallel transfer") + for r in results: + r.get() # self._run_command returns None + self.log.info(("Multiprocessing for parallel transfer " + "finished")) + else: + raise cdist.Error("Source {} is not a directory".format(source)) + def run_script(self, script, env=None, return_output=False): """Run the given script with the given environment on the remote side. Return the output as a string. diff --git a/cdist/test/exec/remote.py b/cdist/test/exec/remote.py index 121ec4ac..f498c285 100644 --- a/cdist/test/exec/remote.py +++ b/cdist/test/exec/remote.py @@ -24,6 +24,7 @@ import getpass import shutil import string import random +import multiprocessing import cdist from cdist import test @@ -121,6 +122,22 @@ class RemoteTestCase(test.CdistTestCase): # test if the payload file is in the target directory self.assertTrue(os.path.isfile(os.path.join(target, source_file_name))) + def test_transfer_dir_parallel(self): + source = self.mkdtemp(dir=self.temp_dir) + # put 8 files in the directory as payload + filenames = [] + for x in range(8): + handle, source_file = self.mkstemp(dir=source) + os.close(handle) + source_file_name = os.path.split(source_file)[-1] + filenames.append(source_file_name) + target = self.mkdtemp(dir=self.temp_dir) + self.remote.transfer_dir_parallel(source, target, + multiprocessing.cpu_count()) + # test if the payload files are in the target directory + for filename in filenames: + self.assertTrue(os.path.isfile(os.path.join(target, filename))) + def test_create_files_dirs(self): self.remote.create_files_dirs() self.assertTrue(os.path.isdir(self.remote.base_path)) diff --git a/cdist/test/explorer/__init__.py b/cdist/test/explorer/__init__.py index 27820292..8ac9975e 100644 --- a/cdist/test/explorer/__init__.py +++ b/cdist/test/explorer/__init__.py @@ -179,6 +179,20 @@ class ExplorerClassTestCase(test.CdistTestCase): jobs=8) self.assertEqual(expl.jobs, 8) + def test_transfer_global_explorers_parallel(self): + expl = explorer.Explorer( + self.target_host, + self.local, + self.remote, + jobs=multiprocessing.cpu_count()) + self.assertIsNotNone(expl.jobs) + + expl.transfer_global_explorers() + source = self.local.global_explorer_path + destination = self.remote.global_explorer_path + self.assertEqual(sorted(os.listdir(source)), + sorted(os.listdir(destination))) + def test_run_parallel_jobs(self): expl = explorer.Explorer( self.target_host, diff --git a/docs/changelog b/docs/changelog index 5dd49ef5..13cde84b 100644 --- a/docs/changelog +++ b/docs/changelog @@ -1,8 +1,8 @@ Changelog --------- next: - * New type __filesystem: manage filesystems on devices ( Daniel Heule ) - + * Core: Add -j, --jobs option for parallel execution and add parallel support for global explorers (Darko Poljak) + * New type __filesystem: manage filesystems on devices (Daniel Heule) * New type: __locale_system (Steven Armstrong, Carlos Ortigoza, Nico Schottelius) * New type: __sysctl (Steven Armstrong) From 55b134597a941f604334386bc4675bbcc50105ed Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Fri, 12 Aug 2016 21:20:37 +0200 Subject: [PATCH 3/9] Update cdist docs with -j, --jobs option. --- docs/src/man1/cdist.rst | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/docs/src/man1/cdist.rst b/docs/src/man1/cdist.rst index 5a4873f7..b4dd2c33 100644 --- a/docs/src/man1/cdist.rst +++ b/docs/src/man1/cdist.rst @@ -15,7 +15,10 @@ SYNOPSIS cdist banner [-h] [-d] [-v] - cdist config [-h] [-d] [-V] [-c CONF_DIR] [-f HOSTFILE] [-i MANIFEST] [-p] [-s] [host [host ...]] + config [-h] [-d] [-v] [-c CONF_DIR] [-f HOSTFILE] [-i MANIFEST] + [-n] [-o OUT_PATH] [-p] [-s] [--remote-copy REMOTE_COPY] + [--remote-exec REMOTE_EXEC] [-j [JOBS]] + [host [host ...]] cdist shell [-h] [-d] [-v] [-s SHELL] @@ -96,6 +99,11 @@ Configure one or more hosts. Command to use for remote execution (should behave like ssh) +.. option:: -j [JOBS], --jobs [JOBS] + + Specify the maximum number of parallel jobs; currently only + global explorers are supported + SHELL ----- This command allows you to spawn a shell that enables access From 90454c4e6baaad3d51b34a0add78cad4e7d35ec4 Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Fri, 12 Aug 2016 21:34:10 +0200 Subject: [PATCH 4/9] Update jobs option description. --- scripts/cdist | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/cdist b/scripts/cdist index b7e3a805..d6afbeab 100755 --- a/scripts/cdist +++ b/scripts/cdist @@ -122,7 +122,8 @@ def commandline(): default=os.environ.get('CDIST_REMOTE_EXEC')) parser['config'].add_argument( '-j', '--jobs', nargs='?', type=check_positive_int, - help='Specify the maximum number of parallel jobs', + help=('Specify the maximum number of parallel jobs; currently ' + 'only global explorers are supported'), action='store', dest='jobs', const=multiprocessing.cpu_count()) parser['config'].set_defaults(func=cdist.config.Config.commandline) From 51ffc0f0377278cb455ee037f8e8cc21d218f328 Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Sun, 14 Aug 2016 21:30:09 +0200 Subject: [PATCH 5/9] log.info -> log.debug for debug messages --- cdist/core/explorer.py | 8 ++++---- cdist/exec/remote.py | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/cdist/core/explorer.py b/cdist/core/explorer.py index efffb6ed..17d6f46e 100644 --- a/cdist/core/explorer.py +++ b/cdist/core/explorer.py @@ -117,19 +117,19 @@ class Explorer(object): self.jobs)) self.log.debug("Multiprocessing start method is {}".format( multiprocessing.get_start_method())) - self.log.info(("Starting multiprocessing Pool for global " + self.log.debug(("Starting multiprocessing Pool for global " "explorers run")) with multiprocessing.Pool(self.jobs) as pool: - self.log.info("Starting async for global explorer run") + self.log.debug("Starting async for global explorer run") results = [ pool.apply_async(self._run_global_explorer, (e, out_path,)) for e in self.list_global_explorer_names() ] - self.log.info("Waiting async results for global explorer runs") + self.log.debug("Waiting async results for global explorer runs") for r in results: r.get() # self._run_global_explorer returns None - self.log.info(("Multiprocessing run for global explorers " + self.log.debug(("Multiprocessing run for global explorers " "finished")) # logger is not pickable, so remove it when we pickle diff --git a/cdist/exec/remote.py b/cdist/exec/remote.py index 389c0da1..72dc5840 100644 --- a/cdist/exec/remote.py +++ b/cdist/exec/remote.py @@ -136,10 +136,10 @@ class Remote(object): jobs)) self.log.debug("Multiprocessing start method is {}".format( multiprocessing.get_start_method())) - self.log.info(("Starting multiprocessing Pool for parallel " + self.log.debug(("Starting multiprocessing Pool for parallel " "remote transfer")) with multiprocessing.Pool(jobs) as pool: - self.log.info("Starting async for parallel transfer") + self.log.debug("Starting async for parallel transfer") commands = [] for f in glob.glob1(source, '*'): command = self._copy.split() @@ -152,10 +152,10 @@ class Remote(object): for cmd in commands ] - self.log.info("Waiting async results for parallel transfer") + self.log.debug("Waiting async results for parallel transfer") for r in results: r.get() # self._run_command returns None - self.log.info(("Multiprocessing for parallel transfer " + self.log.debug(("Multiprocessing for parallel transfer " "finished")) else: raise cdist.Error("Source {} is not a directory".format(source)) From 1c07b63f1d13bd7bce508eddcac184e63a8be85e Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Mon, 15 Aug 2016 14:20:35 +0200 Subject: [PATCH 6/9] Add -b/--enable-beta option for enabling beta functionalities. --- cdist/config.py | 20 +++++++++++++++++++- docs/changelog | 3 ++- docs/src/man1/cdist.rst | 9 +++++++-- scripts/cdist | 9 +++++++-- 4 files changed, 35 insertions(+), 6 deletions(-) diff --git a/cdist/config.py b/cdist/config.py index 6f3fde9b..ad256b12 100644 --- a/cdist/config.py +++ b/cdist/config.py @@ -71,6 +71,8 @@ def inspect_ssh_mux_opts(): class Config(object): """Cdist main class to hold arbitrary data""" + BETA_ARGS = ['jobs'] + def __init__(self, local, remote, dry_run=False, jobs=None): self.local = local @@ -109,6 +111,19 @@ class Config(object): for host in source: yield host + @classmethod + def _check_beta(cls, args_dict): + if 'beta' not in args_dict: + args_dict['beta'] = False + # Check only if beta is not enabled: if beta option is specified then + # raise error. + if not args_dict['beta']: + err_msg = ("\'{}\' is beta, but beta is not enabled. If you want " + "to use it please enable beta functionalities.") + for arg in cls.BETA_ARGS: + if arg in args_dict: + raise cdist.Error(err_msg.format(arg)) + @classmethod def commandline(cls, args): """Configure remote system""" @@ -120,6 +135,10 @@ class Config(object): if args.manifest == '-' and args.hostfile == '-': raise cdist.Error(("Cannot read both, manifest and host file, " "from stdin")) + + args_dict = vars(args) + cls._check_beta(args_dict) + # if no host source is specified then read hosts from stdin if not (args.hostfile or args.host): args.hostfile = '-' @@ -148,7 +167,6 @@ class Config(object): args.remote_exec_pattern = None args.remote_copy_pattern = None - args_dict = vars(args) # if remote-exec and/or remote-copy args are None then user # didn't specify command line options nor env vars: # inspect multiplexing options for default cdist.REMOTE_COPY/EXEC diff --git a/docs/changelog b/docs/changelog index 21fee305..d0d2c157 100644 --- a/docs/changelog +++ b/docs/changelog @@ -1,7 +1,8 @@ Changelog --------- next: - * Core: Add -j, --jobs option for parallel execution and add parallel support for global explorers (Darko Poljak) + * Core: Add -b, --enable-beta option for enabling beta functionalities (Darko Poljak) + * Core: Add -j, --jobs option for parallel execution and add parallel support for global explorers (currently in beta) (Darko Poljak) * Core: Add derived env vars for target hostname and fqdn (Darko Poljak) * New type: __keyboard: Set keyboard layout (Carlos Ortigoza) * Documentation: Re-license types' man pages to GPLV3+ (Dmitry Bogatov, Darko Poljak) diff --git a/docs/src/man1/cdist.rst b/docs/src/man1/cdist.rst index 7cac0afe..1eb57c12 100644 --- a/docs/src/man1/cdist.rst +++ b/docs/src/man1/cdist.rst @@ -17,7 +17,7 @@ SYNOPSIS cdist config [-h] [-d] [-v] [-c CONF_DIR] [-f HOSTFILE] [-i MANIFEST] [-n] [-o OUT_PATH] [-p] [-s] [--remote-copy REMOTE_COPY] - [--remote-exec REMOTE_EXEC] [-j [JOBS]] + [--remote-exec REMOTE_EXEC] [-j [JOBS]] [-b] [host [host ...]] cdist shell [-h] [-d] [-v] [-s SHELL] @@ -110,7 +110,12 @@ Configure one or more hosts. .. option:: -j [JOBS], --jobs [JOBS] Specify the maximum number of parallel jobs; currently only - global explorers are supported + global explorers are supported (currently in beta) + +.. option:: -b, --enable-beta + + Enable beta functionalities. Beta functionalities include the + following options: -j/--jobs. SHELL ----- diff --git a/scripts/cdist b/scripts/cdist index 557bf2b2..6c60ccb3 100755 --- a/scripts/cdist +++ b/scripts/cdist @@ -122,10 +122,15 @@ def commandline(): default=os.environ.get('CDIST_REMOTE_EXEC')) parser['config'].add_argument( '-j', '--jobs', nargs='?', type=check_positive_int, - help=('Specify the maximum number of parallel jobs; currently ' - 'only global explorers are supported'), + help=('Specify the maximum number of parallel jobs, currently ' + 'only global explorers are supported (currently in beta'), action='store', dest='jobs', const=multiprocessing.cpu_count()) + parser['config'].add_argument( + '-b', '--enable-beta', + help=('Enable beta functionalities. Beta functionalities ' + 'include the following options: -j/--jobs.'), + action='store_true', dest='beta', default=False) parser['config'].set_defaults(func=cdist.config.Config.commandline) # Shell From fdf6a6570c4342807525bf3b7401d491034e31f6 Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Mon, 15 Aug 2016 16:01:39 +0200 Subject: [PATCH 7/9] Check for beta in scripts/cdist. --- cdist/__init__.py | 14 ++++++++++++++ cdist/config.py | 19 +------------------ scripts/cdist | 21 +++++++++++++++++++++ 3 files changed, 36 insertions(+), 18 deletions(-) diff --git a/cdist/__init__.py b/cdist/__init__.py index 74db1a13..d06ae28b 100644 --- a/cdist/__init__.py +++ b/cdist/__init__.py @@ -56,6 +56,20 @@ class UnresolvableRequirementsError(cdist.Error): pass +class CdistBetaRequired(cdist.Error): + """Beta functionality is used but beta is not enabled""" + + def __init__(self, command, arg): + self.command = command + self.arg = arg + + def __str__(self): + err_msg = ("\'{}\' argument of \'{}\' command is beta, but beta is " + "not enabled. If you want to use it please enable beta " + "functionalities.") + return err_msg.format(self.arg, self.command) + + class CdistObjectError(Error): """Something went wrong with an object""" diff --git a/cdist/config.py b/cdist/config.py index ad256b12..9d4c5c10 100644 --- a/cdist/config.py +++ b/cdist/config.py @@ -71,8 +71,6 @@ def inspect_ssh_mux_opts(): class Config(object): """Cdist main class to hold arbitrary data""" - BETA_ARGS = ['jobs'] - def __init__(self, local, remote, dry_run=False, jobs=None): self.local = local @@ -111,19 +109,6 @@ class Config(object): for host in source: yield host - @classmethod - def _check_beta(cls, args_dict): - if 'beta' not in args_dict: - args_dict['beta'] = False - # Check only if beta is not enabled: if beta option is specified then - # raise error. - if not args_dict['beta']: - err_msg = ("\'{}\' is beta, but beta is not enabled. If you want " - "to use it please enable beta functionalities.") - for arg in cls.BETA_ARGS: - if arg in args_dict: - raise cdist.Error(err_msg.format(arg)) - @classmethod def commandline(cls, args): """Configure remote system""" @@ -136,9 +121,6 @@ class Config(object): raise cdist.Error(("Cannot read both, manifest and host file, " "from stdin")) - args_dict = vars(args) - cls._check_beta(args_dict) - # if no host source is specified then read hosts from stdin if not (args.hostfile or args.host): args.hostfile = '-' @@ -167,6 +149,7 @@ class Config(object): args.remote_exec_pattern = None args.remote_copy_pattern = None + args_dict = vars(args) # if remote-exec and/or remote-copy args are None then user # didn't specify command line options nor env vars: # inspect multiplexing options for default cdist.REMOTE_COPY/EXEC diff --git a/scripts/cdist b/scripts/cdist index 6c60ccb3..a2f5fbfb 100755 --- a/scripts/cdist +++ b/scripts/cdist @@ -22,6 +22,12 @@ # +# list of beta arguments for sub-commands +BETA_ARGS = { + 'config': ['jobs', ], +} + + def check_positive_int(value): import argparse @@ -36,6 +42,20 @@ def check_positive_int(value): return val +def check_beta(args_dict): + if 'beta' not in args_dict: + args_dict['beta'] = False + # Check only if beta is not enabled: if beta option is specified then + # raise error. + if not args_dict['beta']: + err_msg = ("\'{}\' is beta, but beta is not enabled. If you want " + "to use it please enable beta functionalities.") + cmd = args_dict['command'] + for arg in BETA_ARGS[cmd]: + if arg in args_dict: + raise cdist.CdistBetaRequired(cmd, arg) + + def commandline(): """Parse command line""" import argparse @@ -172,6 +192,7 @@ def commandline(): parser['main'].print_help() sys.exit(0) + check_beta(vars(args)) args.func(args) if __name__ == "__main__": From 7cc7c18e77dfeafb018ba12b3969d3be4fd47c5a Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Mon, 15 Aug 2016 16:37:11 +0200 Subject: [PATCH 8/9] Add hint for command line flag for enable-beta. --- cdist/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cdist/__init__.py b/cdist/__init__.py index d06ae28b..9068ae69 100644 --- a/cdist/__init__.py +++ b/cdist/__init__.py @@ -66,7 +66,8 @@ class CdistBetaRequired(cdist.Error): def __str__(self): err_msg = ("\'{}\' argument of \'{}\' command is beta, but beta is " "not enabled. If you want to use it please enable beta " - "functionalities.") + "functionalities by using the -b/--enable-beta command " + "line flag.") return err_msg.format(self.arg, self.command) From adac0113c5690599ce2f247d7a6eceba103b22a3 Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Mon, 15 Aug 2016 16:37:38 +0200 Subject: [PATCH 9/9] Order options lexicographicaly. --- docs/src/man1/cdist.rst | 26 +++++++++++++------------- scripts/cdist | 24 +++++++++++------------- 2 files changed, 24 insertions(+), 26 deletions(-) diff --git a/docs/src/man1/cdist.rst b/docs/src/man1/cdist.rst index 1eb57c12..a00a3ec0 100644 --- a/docs/src/man1/cdist.rst +++ b/docs/src/man1/cdist.rst @@ -15,9 +15,9 @@ SYNOPSIS cdist banner [-h] [-d] [-v] - cdist config [-h] [-d] [-v] [-c CONF_DIR] [-f HOSTFILE] [-i MANIFEST] - [-n] [-o OUT_PATH] [-p] [-s] [--remote-copy REMOTE_COPY] - [--remote-exec REMOTE_EXEC] [-j [JOBS]] [-b] + cdist config [-h] [-d] [-v] [-b] [-c CONF_DIR] [-f HOSTFILE] + [-i MANIFEST] [-j [JOBS]] [-n] [-o OUT_PATH] [-p] [-s] + [--remote-copy REMOTE_COPY] [--remote-exec REMOTE_EXEC] [host [host ...]] cdist shell [-h] [-d] [-v] [-s SHELL] @@ -62,6 +62,11 @@ CONFIG ------ Configure one or more hosts. +.. option:: -b, --enable-beta + + Enable beta functionalities. Beta functionalities include the + following options: -j/--jobs. + .. option:: -c CONF_DIR, --conf-dir CONF_DIR Add a configuration directory. Can be specified multiple times. @@ -83,6 +88,11 @@ Configure one or more hosts. Path to a cdist manifest or - to read from stdin +.. option:: -j [JOBS], --jobs [JOBS] + + Specify the maximum number of parallel jobs; currently only + global explorers are supported (currently in beta) + .. option:: -n, --dry-run Do not execute code @@ -107,16 +117,6 @@ Configure one or more hosts. Command to use for remote execution (should behave like ssh) -.. option:: -j [JOBS], --jobs [JOBS] - - Specify the maximum number of parallel jobs; currently only - global explorers are supported (currently in beta) - -.. option:: -b, --enable-beta - - Enable beta functionalities. Beta functionalities include the - following options: -j/--jobs. - SHELL ----- This command allows you to spawn a shell that enables access diff --git a/scripts/cdist b/scripts/cdist index a2f5fbfb..d02f0a5f 100755 --- a/scripts/cdist +++ b/scripts/cdist @@ -48,8 +48,6 @@ def check_beta(args_dict): # Check only if beta is not enabled: if beta option is specified then # raise error. if not args_dict['beta']: - err_msg = ("\'{}\' is beta, but beta is not enabled. If you want " - "to use it please enable beta functionalities.") cmd = args_dict['command'] for arg in BETA_ARGS[cmd]: if arg in args_dict: @@ -97,6 +95,11 @@ def commandline(): 'config', parents=[parser['loglevel']]) parser['config'].add_argument( 'host', nargs='*', help='host(s) to operate on') + parser['config'].add_argument( + '-b', '--enable-beta', + help=('Enable beta functionalities. Beta functionalities ' + 'include the following options: -j/--jobs.'), + action='store_true', dest='beta', default=False) parser['config'].add_argument( '-c', '--conf-dir', help=('Add configuration directory (can be repeated, ' @@ -112,6 +115,12 @@ def commandline(): '-i', '--initial-manifest', help='Path to a cdist manifest or \'-\' to read from stdin.', dest='manifest', required=False) + parser['config'].add_argument( + '-j', '--jobs', nargs='?', type=check_positive_int, + help=('Specify the maximum number of parallel jobs, currently ' + 'only global explorers are supported (currently in beta'), + action='store', dest='jobs', + const=multiprocessing.cpu_count()) parser['config'].add_argument( '-n', '--dry-run', help='Do not execute code', action='store_true') @@ -140,17 +149,6 @@ def commandline(): '(should behave like ssh)'), action='store', dest='remote_exec', default=os.environ.get('CDIST_REMOTE_EXEC')) - parser['config'].add_argument( - '-j', '--jobs', nargs='?', type=check_positive_int, - help=('Specify the maximum number of parallel jobs, currently ' - 'only global explorers are supported (currently in beta'), - action='store', dest='jobs', - const=multiprocessing.cpu_count()) - parser['config'].add_argument( - '-b', '--enable-beta', - help=('Enable beta functionalities. Beta functionalities ' - 'include the following options: -j/--jobs.'), - action='store_true', dest='beta', default=False) parser['config'].set_defaults(func=cdist.config.Config.commandline) # Shell