From 07d6fc28ad07f278fa5f63fe445dc8b7948198b1 Mon Sep 17 00:00:00 2001 From: Matt Phillips Date: Tue, 30 Jul 2019 14:24:08 -0400 Subject: [PATCH 1/8] improve timeout handling the timeout kwarg previously was only being used for polling timeout. we can also pass it as a low kwarg and use it for urllib connection timeout as well. --- pepper/cli.py | 3 ++- pepper/libpepper.py | 13 +++++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/pepper/cli.py b/pepper/cli.py index 1446333..7e48c8b 100644 --- a/pepper/cli.py +++ b/pepper/cli.py @@ -663,7 +663,8 @@ def run(self): api = pepper.Pepper( self.parse_url(), debug_http=self.options.debug_http, - ignore_ssl_errors=self.options.ignore_ssl_certificate_errors) + ignore_ssl_errors=self.options.ignore_ssl_certificate_errors, + timeout=self.options.timeout) self.login(api) diff --git a/pepper/libpepper.py b/pepper/libpepper.py index c5cecf6..ebb5486 100644 --- a/pepper/libpepper.py +++ b/pepper/libpepper.py @@ -57,7 +57,7 @@ class Pepper(object): u'ms-4': True}]} ''' - def __init__(self, api_url='https://localhost:8000', debug_http=False, ignore_ssl_errors=False): + def __init__(self, api_url='https://localhost:8000', debug_http=False, ignore_ssl_errors=False, timeout=None): ''' Initialize the class with the URL of the API @@ -81,6 +81,7 @@ def __init__(self, api_url='https://localhost:8000', debug_http=False, ignore_ss self._ssl_verify = not ignore_ssl_errors self.auth = {} self.salt_version = None + self.timeout = timeout def req_stream(self, path): ''' @@ -224,11 +225,13 @@ def req(self, path, data=None): # Send request try: + con_kwargs = {} if not (self._ssl_verify): con = ssl.SSLContext(ssl.PROTOCOL_SSLv23) - f = urlopen(req, context=con) - else: - f = urlopen(req) + con_kwargs['context'] = con + if self.timeout: + con_kwargs['timeout'] = self.timeout + f = urlopen(req, **con_kwargs) content = f.read().decode('utf-8') if (self.debug_http): logger.debug('Response: %s', content) @@ -283,6 +286,8 @@ def req_requests(self, path, data=None): 'auth': auth, 'data': json.dumps(data), } + if self.timeout: + params['timeout'] = self.timeout logger.debug('postdata {0}'.format(params)) resp = requests.post(**params) if resp.status_code == 401: From 16f3cd887e1e46af3a171d8ad5041dc1d4027180 Mon Sep 17 00:00:00 2001 From: Matt Phillips Date: Tue, 30 Jul 2019 16:27:22 -0400 Subject: [PATCH 2/8] emit pepper-runtets.log for debug --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 1c69fd3..a138f85 100644 --- a/Makefile +++ b/Makefile @@ -8,4 +8,4 @@ install: pip install tox test: install - tox -e flake8,$(PYVERSION)-$(BACKEND)-$(SALT) + tox -e flake8,$(PYVERSION)-$(BACKEND)-$(SALT) || cat /tmp/pepper-runtests.log From aca237c1086373cbcd0abde2f7022c3ddc4954ca Mon Sep 17 00:00:00 2001 From: Matt Phillips Date: Tue, 30 Jul 2019 20:40:54 -0400 Subject: [PATCH 3/8] revert previous change, add a buffer on local timeout since these timeout kwargs should coincide with upstream response, we give a bit of buffer to let salt-api time out gracefully before forcing it first. --- Makefile | 2 +- pepper/libpepper.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index a138f85..1c69fd3 100644 --- a/Makefile +++ b/Makefile @@ -8,4 +8,4 @@ install: pip install tox test: install - tox -e flake8,$(PYVERSION)-$(BACKEND)-$(SALT) || cat /tmp/pepper-runtests.log + tox -e flake8,$(PYVERSION)-$(BACKEND)-$(SALT) diff --git a/pepper/libpepper.py b/pepper/libpepper.py index ebb5486..4fdb802 100644 --- a/pepper/libpepper.py +++ b/pepper/libpepper.py @@ -230,7 +230,7 @@ def req(self, path, data=None): con = ssl.SSLContext(ssl.PROTOCOL_SSLv23) con_kwargs['context'] = con if self.timeout: - con_kwargs['timeout'] = self.timeout + con_kwargs['timeout'] = self.timeout + 5 # throw a bit of buffer for upstream lag f = urlopen(req, **con_kwargs) content = f.read().decode('utf-8') if (self.debug_http): @@ -287,7 +287,7 @@ def req_requests(self, path, data=None): 'data': json.dumps(data), } if self.timeout: - params['timeout'] = self.timeout + params['timeout'] = self.timeout + 5 logger.debug('postdata {0}'.format(params)) resp = requests.post(**params) if resp.status_code == 401: From e79765824d28160b8d1559e66361d5d8e99e21b6 Mon Sep 17 00:00:00 2001 From: Matt Phillips Date: Wed, 31 Jul 2019 13:23:45 -0400 Subject: [PATCH 4/8] temporarily disable to see what travis does --- pepper/libpepper.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pepper/libpepper.py b/pepper/libpepper.py index 4fdb802..abadae2 100644 --- a/pepper/libpepper.py +++ b/pepper/libpepper.py @@ -229,8 +229,8 @@ def req(self, path, data=None): if not (self._ssl_verify): con = ssl.SSLContext(ssl.PROTOCOL_SSLv23) con_kwargs['context'] = con - if self.timeout: - con_kwargs['timeout'] = self.timeout + 5 # throw a bit of buffer for upstream lag + #if self.timeout: + # con_kwargs['timeout'] = self.timeout + 5 # throw a bit of buffer for upstream lag f = urlopen(req, **con_kwargs) content = f.read().decode('utf-8') if (self.debug_http): @@ -286,8 +286,8 @@ def req_requests(self, path, data=None): 'auth': auth, 'data': json.dumps(data), } - if self.timeout: - params['timeout'] = self.timeout + 5 + #if self.timeout: + # params['timeout'] = self.timeout + 5 logger.debug('postdata {0}'.format(params)) resp = requests.post(**params) if resp.status_code == 401: From 14144a0cc5cc33283f676891dbacfac067d28257 Mon Sep 17 00:00:00 2001 From: Matt Phillips Date: Fri, 4 Oct 2019 17:22:35 -0400 Subject: [PATCH 5/8] improve guessing game for runner outputter --- pepper/script.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pepper/script.py b/pepper/script.py index 21afe63..9d4cc63 100755 --- a/pepper/script.py +++ b/pepper/script.py @@ -87,9 +87,11 @@ def __call__(self): ) elif 'data' in ret: # unfold runners - outputter = ret.get('outputter', 'nested') + outputter = ret.get('outputter') or ret.get('data', {}).get('outputter') or ret.get('data', {}).get('return', {}).get('outputter', 'nested') + if isinstance(ret['data'], dict) and 'return' in ret['data']: ret = ret['data']['return'] + salt.output.display_output( ret, self.cli.options.output or outputter, From ceaf98006dfcf1687a443895ee6ed473e6f1e21a Mon Sep 17 00:00:00 2001 From: Matt Phillips Date: Fri, 4 Oct 2019 17:23:17 -0400 Subject: [PATCH 6/8] make logging verbosity actually work --- pepper/cli.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/pepper/cli.py b/pepper/cli.py index 7e48c8b..420764c 100644 --- a/pepper/cli.py +++ b/pepper/cli.py @@ -139,6 +139,14 @@ def parse(self): self.options, self.args = self.parser.parse_args() + # set up logging + rootLogger = logging.getLogger(name=None) + rootLogger.setLevel(max(logging.ERROR - (self.options.verbose * 10), 1)) + formatter = logging.Formatter('%(levelname)s %(asctime)s %(module)s: %(message)s') + console = logging.StreamHandler() + console.setFormatter(formatter) + rootLogger.addHandler(console) + option_names = ["fail_any", "fail_any_none", "fail_all", "fail_all_none"] toggled_options = [name for name in option_names if getattr(self.options, name)] if len(toggled_options) > 1: @@ -655,11 +663,6 @@ def run(self): ''' Parse all arguments and call salt-api ''' - # set up logging - rootLogger = logging.getLogger(name=None) - rootLogger.addHandler(logging.StreamHandler()) - rootLogger.setLevel(max(logging.ERROR - (self.options.verbose * 10), 1)) - api = pepper.Pepper( self.parse_url(), debug_http=self.options.debug_http, From 176b30b71d0d245a68453566462362dfd2b8b354 Mon Sep 17 00:00:00 2001 From: Matt Phillips Date: Fri, 4 Oct 2019 17:23:38 -0400 Subject: [PATCH 7/8] forced full_return: move to parse_cmd to restore --json behavior --json was broken with the introduction of this because client isnt set / shortcircuited when json is used. We can add the equivalent logic in parse_cmd instead. --- pepper/cli.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/pepper/cli.py b/pepper/cli.py index 420764c..4e8d7e5 100644 --- a/pepper/cli.py +++ b/pepper/cli.py @@ -506,6 +506,7 @@ def parse_cmd(self, api): if len(args) < 2: self.parser.error("Command or target not specified") + low['full_return'] = True low['tgt_type'] = self.options.expr_form low['tgt'] = args.pop(0) low['fun'] = args.pop(0) @@ -513,6 +514,7 @@ def parse_cmd(self, api): low['arg'] = args elif client.startswith('runner'): low['fun'] = args.pop(0) + low['full_return'] = True # post https://github.com/saltstack/salt/pull/50124, kwargs can be # passed as is in foo=bar form, splitting and deserializing will # happen in salt-api. additionally, the presence of salt-version header @@ -673,10 +675,6 @@ def run(self): load = self.parse_cmd(api) - for entry in load: - if not entry.get('client', '').startswith('wheel'): - entry['full_return'] = True - if self.options.fail_if_minions_dont_respond: for exit_code, ret in self.poll_for_returns(api, load): # pragma: no cover yield exit_code, json.dumps(ret, sort_keys=True, indent=4) From c0cbfa6385c0bd6b36f4a352eb482bce87d817c5 Mon Sep 17 00:00:00 2001 From: Matt Phillips Date: Fri, 4 Oct 2019 17:24:54 -0400 Subject: [PATCH 8/8] runners: use arg/kwarg instead of popping things into low data mixing low data with kwargs has been deprecated forever, it should be safe to change this now. --- pepper/cli.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pepper/cli.py b/pepper/cli.py index 4e8d7e5..7e1663b 100644 --- a/pepper/cli.py +++ b/pepper/cli.py @@ -539,11 +539,12 @@ def parse_cmd(self, api): else: for arg in args: if '=' in arg: + low.setdefault('kwarg', {}) key, value = arg.split('=', 1) try: - low[key] = json.loads(value) + low['kwarg'][key] = json.loads(value) except JSONDecodeError: - low[key] = value + low['kwarg'][key] = value else: low.setdefault('arg', []).append(arg) elif client.startswith('ssh'):