From 965e3cfecb5d20bf1929adbe3f5ef5adc069705f Mon Sep 17 00:00:00 2001 From: Bogdan Dobrelya Date: Thu, 28 Nov 2019 15:31:55 +0100 Subject: [PATCH] Fix action Apply ignoring managed-by arg From the very beginning (06036fd6dba3ba4b9a13052ea95a08ebcc97501e), the action apply was ignoring the passed --managed-by values and was always taking defaults ('paunch'). Fix this and provide no upgrade/update impact, which is for whatever --managed-by value given to paunch, perform all checks and searches also for that historically "wrong" value 'paunch': * if a container needs to be (re)started by a new 'managed-by', make sure it can be found by the default 'paunch' value as well, then reset its managed-by to the desired value. Closes-bug: #1853812 Change-Id: If129bbc1ff32941d06ff480f26870b10840591e0 Signed-off-by: Bogdan Dobrelya (cherry picked from commit 12ccf36b0e7f9b233b8b71a21fe24b241ecb639b) --- paunch/cmd.py | 2 +- paunch/runner.py | 57 ++++++++++++++++++++++----- paunch/tests/test_builder_compose1.py | 31 +++++++++++++++ 3 files changed, 80 insertions(+), 10 deletions(-) diff --git a/paunch/cmd.py b/paunch/cmd.py index d069230..c5d89d5 100644 --- a/paunch/cmd.py +++ b/paunch/cmd.py @@ -71,7 +71,7 @@ class Apply(command.Command): stdout, stderr, rc = paunch.apply( parsed_args.config_id, config, - managed_by='paunch', + managed_by=parsed_args.managed_by, labels=labels ) diff --git a/paunch/runner.py b/paunch/runner.py index 72025f7..2943766 100644 --- a/paunch/runner.py +++ b/paunch/runner.py @@ -53,9 +53,20 @@ class DockerRunner(object): '--format', '{{.Label "config_id"}}' ] cmd_stdout, cmd_stderr, returncode = self.execute(cmd) - if returncode != 0: - return set() - return set(cmd_stdout.split()) + results = cmd_stdout.split() + if returncode != 0 or not results or results == ['']: + # NOTE(bogdando): also look by the historically used to + # be always specified defaults, we must also identify such configs + cmd = [ + self.docker_cmd, 'ps', '-a', + '--filter', 'label=managed_by=paunch', + '--format', '{{.Label "config_id"}}' + ] + cmd_stdout, cmd_stderr, returncode = self.execute(cmd) + if returncode != 0: + return set() + results += cmd_stdout.split() + return set(results) def containers_in_config(self, conf_id): cmd = [ @@ -64,10 +75,21 @@ class DockerRunner(object): '--filter', 'label=config_id=%s' % conf_id ] cmd_stdout, cmd_stderr, returncode = self.execute(cmd) - if returncode != 0: - return [] + results = cmd_stdout.split() + if returncode != 0 or not results or results == ['']: + # NOTE(bogdando): also look by the historically used to + # be always specified defaults, we must also identify such configs + cmd = [ + self.docker_cmd, 'ps', '-q', '-a', + '--filter', 'label=managed_by=paunch', + '--filter', 'label=config_id=%s' % conf_id + ] + cmd_stdout, cmd_stderr, returncode = self.execute(cmd) + if returncode != 0: + return [] + results += cmd_stdout.split() - return [c for c in cmd_stdout.split()] + return [c for c in results] def remove_containers(self, conf_id): for container in self.containers_in_config(conf_id): @@ -94,9 +116,26 @@ class DockerRunner(object): '--format', '{{.Names}} {{.Label "container_name"}}' )) cmd_stdout, cmd_stderr, returncode = self.execute(cmd) - if returncode != 0: - return - for line in cmd_stdout.split("\n"): + results = cmd_stdout.split("\n") + if returncode != 0 or not results or results == ['']: + # NOTE(bogdando): also look by the historically used to + # be always specified defaults, we must also identify such configs + cmd = [ + self.docker_cmd, 'ps', '-a', + '--filter', 'label=managed_by=paunch' + ] + if conf_id: + cmd.extend(( + '--filter', 'label=config_id=%s' % conf_id + )) + cmd.extend(( + '--format', '{{.Names}} {{.Label "container_name"}}' + )) + cmd_stdout, cmd_stderr, returncode = self.execute(cmd) + if returncode != 0: + return + results += cmd_stdout.split("\n") + for line in results: if line: yield line.split() diff --git a/paunch/tests/test_builder_compose1.py b/paunch/tests/test_builder_compose1.py index a445b3a..399caf6 100644 --- a/paunch/tests/test_builder_compose1.py +++ b/paunch/tests/test_builder_compose1.py @@ -60,8 +60,11 @@ class TestComposeV1Builder(base.TestCase): ('Pulled centos:7', 'ouch', 1), # pull centos:6 fails ('Pulled centos:7', '', 0), # pull centos:6 succeeds ('', '', 0), # ps for delete_missing_and_updated container_names + ('', '', 0), # ps2 for delete_missing_and_updated container_names ('', '', 0), # ps for after delete_missing_and_updated renames + ('', '', 0), # ps2 for after delete_missing_and_updated renames ('', '', 0), # ps to only create containers which don't exist + ('', '', 0), # ps2 to only create containers which don't exist ('Created one-12345678', '', 0), ('Created two-12345678', '', 0), ('Created three-12345678', '', 0), @@ -116,12 +119,25 @@ class TestComposeV1Builder(base.TestCase): '--filter', 'label=config_id=foo', '--format', '{{.Names}} {{.Label "container_name"}}'] ), + # ps2 for delete_missing_and_updated container_names + mock.call( + ['docker', 'ps', '-a', + '--filter', 'label=managed_by=paunch', + '--filter', 'label=config_id=foo', + '--format', '{{.Names}} {{.Label "container_name"}}'] + ), # ps for after delete_missing_and_updated renames mock.call( ['docker', 'ps', '-a', '--filter', 'label=managed_by=tester', '--format', '{{.Names}} {{.Label "container_name"}}'] ), + # ps2 for after delete_missing_and_updated renames + mock.call( + ['docker', 'ps', '-a', + '--filter', 'label=managed_by=paunch', + '--format', '{{.Names}} {{.Label "container_name"}}'] + ), # ps to only create containers which don't exist mock.call( ['docker', 'ps', '-a', @@ -129,6 +145,13 @@ class TestComposeV1Builder(base.TestCase): '--filter', 'label=config_id=foo', '--format', '{{.Names}} {{.Label "container_name"}}'] ), + # ps2 to only create containers which don't exist + mock.call( + ['docker', 'ps', '-a', + '--filter', 'label=managed_by=paunch', + '--filter', 'label=config_id=foo', + '--format', '{{.Names}} {{.Label "container_name"}}'] + ), # run one mock.call( ['docker', 'run', '--name', 'one-12345678', @@ -222,6 +245,8 @@ three-12345678 three''', '', 0), ('{"start_order": 2, "image": "centos:7"}', '', 0), # ps for after delete_missing_and_updated renames ('', '', 0), + # ps2 for after delete_missing_and_updated renames + ('', '', 0), # ps to only create containers which don't exist ('three-12345678 three', '', 0), ('Created one-12345678', '', 0), @@ -275,6 +300,12 @@ three-12345678 three''', '', 0), '--filter', 'label=managed_by=tester', '--format', '{{.Names}} {{.Label "container_name"}}'] ), + # ps2 for after delete_missing_and_updated renames + mock.call( + ['docker', 'ps', '-a', + '--filter', 'label=managed_by=paunch', + '--format', '{{.Names}} {{.Label "container_name"}}'] + ), # ps to only create containers which don't exist mock.call( ['docker', 'ps', '-a',