From 861ed0612805c85fe84b9c836c32aaac3095ea38 Mon Sep 17 00:00:00 2001 From: Bogdan Dobrelya Date: Tue, 18 Aug 2020 11:37:11 +0200 Subject: [PATCH] [USSURY-Only] Log benign command errors as warns Errors logged when an image/container was not found are misleading and should become warnings instead. Only log errors for real execution failures. Change-Id: If23ee1b5b0211221c0dd547e95a0487ea8d04909 Signed-off-by: Bogdan Dobrelya --- paunch/runner.py | 32 +++++++++++++++++++------------ paunch/tests/test_builder_base.py | 20 +++++++++---------- 2 files changed, 30 insertions(+), 22 deletions(-) diff --git a/paunch/runner.py b/paunch/runner.py index 315b196..b922991 100644 --- a/paunch/runner.py +++ b/paunch/runner.py @@ -40,7 +40,7 @@ class BaseRunner(object): 'and will be removed in Train.') @staticmethod - def execute(cmd, log=None, quiet=False): + def execute(cmd, log=None, quiet=False, warn_only=False): if not log: log = common.configure_logging(__name__) if not quiet: @@ -49,8 +49,12 @@ class BaseRunner(object): stderr=subprocess.PIPE) cmd_stdout, cmd_stderr = subproc.communicate() if subproc.returncode != 0: - log.error('Error executing %s: returned %s' % (cmd, - subproc.returncode)) + if warn_only: + log.warning('Error executing %s: ' + 'returned %s' % (cmd, subproc.returncode)) + else: + log.error('Error executing %s: ' + 'returned %s' % (cmd, subproc.returncode)) if not quiet: log.debug(cmd_stdout) log.debug(cmd_stderr) @@ -78,7 +82,8 @@ class BaseRunner(object): '--filter', 'label=managed_by=%s' % self.managed_by, '--format', fmt ] - cmd_stdout, cmd_stderr, returncode = self.execute(cmd, self.log) + cmd_stdout, cmd_stderr, returncode = self.execute( + cmd, log=self.log, quiet=False, warn_only=True) results = cmd_stdout.split() if returncode != 0 or not results or results == ['']: # NOTE(bogdando): also look by the historically used to @@ -100,7 +105,8 @@ class BaseRunner(object): '--filter', 'label=managed_by=%s' % self.managed_by, '--filter', 'label=config_id=%s' % conf_id ] - cmd_stdout, cmd_stderr, returncode = self.execute(cmd, self.log) + cmd_stdout, cmd_stderr, returncode = self.execute( + cmd, log=self.log, quiet=False, warn_only=True) results = cmd_stdout.split() if returncode != 0 or not results or results == ['']: # NOTE(bogdando): also look by the historically used to @@ -131,7 +137,7 @@ class BaseRunner(object): cmd.append(output_format) cmd.append(name) (cmd_stdout, cmd_stderr, returncode) = self.execute( - cmd, self.log, quiet) + cmd, self.log, quiet, True) if returncode != 0: return try: @@ -172,7 +178,8 @@ class BaseRunner(object): '--format', '{{.Names}}' ] - (cmd_stdout, cmd_stderr, returncode) = self.execute(cmd, self.log) + (cmd_stdout, cmd_stderr, returncode) = self.execute( + cmd, log=self.log, quiet=False, warn_only=True) if returncode == 0: names = cmd_stdout.split() if names: @@ -255,7 +262,8 @@ class BaseRunner(object): cmd.extend(( '--format', '{{.Names}} %s' % fmt )) - cmd_stdout, cmd_stderr, returncode = self.execute(cmd, self.log) + cmd_stdout, cmd_stderr, returncode = self.execute( + cmd, log=self.log, quiet=False, warn_only=True) results = cmd_stdout.split("\n") if returncode != 0 or not results or results == ['']: # NOTE(bogdando): also look by the historically used to @@ -462,12 +470,12 @@ class PodmanRunner(BaseRunner): def image_exist(self, name, quiet=False): cmd = ['podman', 'image', 'exists', name] - (_, _, returncode) = self.execute(cmd, self.log, quiet) + (_, _, returncode) = self.execute(cmd, self.log, quiet, True) return returncode == 0 def container_exist(self, name, quiet=False): cmd = ['podman', 'container', 'exists', name] - (_, _, returncode) = self.execute(cmd, self.log, quiet) + (_, _, returncode) = self.execute(cmd, self.log, quiet, True) return returncode == 0 def container_running(self, container): @@ -494,8 +502,8 @@ class PodmanRunner(BaseRunner): # at the first retry, we will force a sync with the OCI runtime if self.cont_cmd == 'podman' and count == 2: chk_cmd.append('--sync') - (cmd_stdout, cmd_stderr, returncode) = self.execute(chk_cmd, - self.log) + (cmd_stdout, cmd_stderr, returncode) = self.execute( + chk_cmd, log=self.log, quiet=False, warn_only=True) if returncode != 0: self.log.warning('Attempt %i Error when running ' diff --git a/paunch/tests/test_builder_base.py b/paunch/tests/test_builder_base.py index e25e3d5..8beae64 100644 --- a/paunch/tests/test_builder_base.py +++ b/paunch/tests/test_builder_base.py @@ -118,12 +118,12 @@ three-12345678 three''', '', 0), # inspect existing image centos:6 mock.call( ['docker', 'inspect', '--type', 'image', - '--format', 'exists', 'centos:6'], mock.ANY, False + '--format', 'exists', 'centos:6'], mock.ANY, False, True ), # inspect and pull missing image centos:7 mock.call( ['docker', 'inspect', '--type', 'image', - '--format', 'exists', 'centos:7'], mock.ANY, False + '--format', 'exists', 'centos:7'], mock.ANY, False, True ), # first pull attempt fails mock.call( @@ -139,7 +139,7 @@ three-12345678 three''', '', 0), '--filter', 'label=managed_by=tester', '--filter', 'label=config_id=foo', '--format', '{{.Names}} {{.Label "container_name"}}'], - mock.ANY + log=mock.ANY, quiet=False, warn_only=True ), mock.call( ['docker', 'ps', '-a', @@ -158,7 +158,7 @@ three-12345678 three''', '', 0), ['docker', 'ps', '-a', '--filter', 'label=managed_by=tester', '--format', '{{.Names}} {{.Label "container_name"}}'], - mock.ANY + log=mock.ANY, quiet=False, warn_only=True ), # rename three from an ephemeral to the static name mock.call(['docker', 'rename', 'three-12345678', 'three'], @@ -169,7 +169,7 @@ three-12345678 three''', '', 0), '--filter', 'label=managed_by=tester', '--filter', 'label=config_id=foo', '--format', '{{.Names}} {{.Label "container_name"}}'], - mock.ANY + log=mock.ANY, quiet=False, warn_only=True ), # run one mock.call( @@ -295,7 +295,7 @@ three-12345678 three''', '', 0), # inspect image centos:7 mock.call( ['docker', 'inspect', '--type', 'image', - '--format', 'exists', 'centos:7'], mock.ANY, False + '--format', 'exists', 'centos:7'], mock.ANY, False, True ), # rm containers not in config mock.call(['docker', 'stop', 'five'], mock.ANY), @@ -306,7 +306,7 @@ three-12345678 three''', '', 0), # check the renamed one, config hasn't changed mock.call(['docker', 'inspect', '--type', 'container', '--format', '{{index .Config.Labels "config_data"}}', - 'one'], mock.ANY, False), + 'one'], mock.ANY, False, True), # don't run one, its already running # run two mock.call( @@ -321,7 +321,7 @@ three-12345678 three''', '', 0), # rm three, changed config mock.call(['docker', 'inspect', '--type', 'container', '--format', '{{index .Config.Labels "config_data"}}', - 'three'], mock.ANY, False), + 'three'], mock.ANY, False, True), mock.call(['docker', 'stop', 'three'], mock.ANY), mock.call(['docker', 'rm', 'three'], mock.ANY), # run three @@ -406,12 +406,12 @@ three-12345678 three''', '', 0), # inspect existing image centos:6 mock.call( ['docker', 'inspect', '--type', 'image', - '--format', 'exists', 'centos:6'], mock.ANY, False + '--format', 'exists', 'centos:6'], mock.ANY, False, True ), # inspect and pull missing image centos:7 mock.call( ['docker', 'inspect', '--type', 'image', - '--format', 'exists', 'centos:7'], mock.ANY, False + '--format', 'exists', 'centos:7'], mock.ANY, False, True ), mock.call( ['docker', 'pull', 'centos:7'], mock.ANY