From e500ba926220039bc8709778285ef16dabfe2b96 Mon Sep 17 00:00:00 2001 From: Emilien Macchi Date: Tue, 27 Aug 2019 14:12:02 +0000 Subject: [PATCH] Revert "Handle defined containers that are stopped." The proposed solution breaks pacemaker-managed containers, see bz#1745950. FWIW this only fix a corner case where the operators stopped the containers and want to run a tripleo operation e.g. major upgrade. We are proposing a revert to unblock downstream and we'll have to rework this patch. This reverts commit 85fb2ed4d3f7e42840842f2bdec617e33d6ba4ef. Change-Id: I8a27b92498d832698ba4d55bed1b2adcf2c1e3eb --- paunch/builder/base.py | 37 ++++-------------- paunch/tests/test_builder_base.py | 63 +++---------------------------- 2 files changed, 13 insertions(+), 87 deletions(-) diff --git a/paunch/builder/base.py b/paunch/builder/base.py index 45bec7b..54288e2 100644 --- a/paunch/builder/base.py +++ b/paunch/builder/base.py @@ -40,18 +40,19 @@ class BaseBuilder(object): stdout = [] stderr = [] pull_returncode = self.pull_missing_images(stdout, stderr) - if pull_returncode != 0: return stdout, stderr, pull_returncode self.delete_missing_and_updated() + deploy_status_code = 0 key_fltr = lambda k: self.config[k].get('start_order', 0) + container_names = self.runner.container_names(self.config_id) desired_names = set([cn[-1] for cn in container_names]) for container in sorted(self.config, key=key_fltr): - self.log.debug("Apply for container {}.".format(container)) + self.log.debug("Running container: %s" % container) cconfig = self.config[container] action = cconfig.get('action', 'run') restart = cconfig.get('restart', 'none') @@ -61,7 +62,6 @@ class BaseBuilder(object): and self.runner.cont_cmd == 'podman' and action == 'run') start_cmd = 'create' if systemd_managed else 'run' - cmd = [] # When upgrading from Docker to Podman, we want to stop the # container that runs under Docker first before starting it with @@ -70,32 +70,11 @@ class BaseBuilder(object): if self.runner.cont_cmd == 'podman' and self.which('docker'): self.runner.stop_container(container, 'docker', quiet=True) - self.log.debug("Apply action {} for container {}.".format( - action, container)) - if action == 'run': - if container in desired_names: - discover_container = self.runner.discover_container_name( - container, self.config_id) - inspect_container = self.runner.inspect(discover_container) - - try: - self.log.debug("Container running state for %s: %s." % - (container, - inspect_container["State"]["Running"])) - except Exception: - self.log.warn("Unable to find the container state" + - " for %s." % container) - - if inspect_container["State"]["Running"]: - self.log.debug("Container %s is already running." % - container) - continue - else: - self.log.debug("Deleting stopped container %s." % - container) - self.runner.remove_container(container) + self.log.debug('Skipping existing container: %s' % + container) + continue cmd = [ self.runner.cont_cmd, @@ -103,8 +82,8 @@ class BaseBuilder(object): '--name', container_name ] + self.label_arguments(cmd, container) - self.log.debug("Start container {}.".format(container)) validations_passed = self.container_run_args(cmd, container, container_name) @@ -119,11 +98,11 @@ class BaseBuilder(object): (cmd_stdout, cmd_stderr, returncode) = self.runner.execute( cmd, self.log) - if cmd_stdout: stdout.append(cmd_stdout) if cmd_stderr: stderr.append(cmd_stderr) + if returncode not in exit_codes: self.log.error("Error running %s. [%s]\n" % (cmd, returncode)) self.log.error("stdout: %s" % cmd_stdout) diff --git a/paunch/tests/test_builder_base.py b/paunch/tests/test_builder_base.py index b724b43..618bd82 100644 --- a/paunch/tests/test_builder_base.py +++ b/paunch/tests/test_builder_base.py @@ -217,11 +217,11 @@ three-12345678 three''', '', 0), ('', '', 0), # rm six ('', '', 0), - # inspect config data for two, we made it changed + # inspect two ('{"start_order": 1, "image": "centos:6"}', '', 0), - # rm two + # rm two, changed config data ('', '', 0), - # inspect config data for three + # inspect three ('{"start_order": 2, "image": "centos:7"}', '', 0), # ps for after delete_missing_and_updated renames ('', '', 0), @@ -229,10 +229,8 @@ three-12345678 three''', '', 0), ('three-12345678 three', '', 0), ('Created one-12345678', '', 0), ('Created two-12345678', '', 0), - # inspect state for three - ('[{"State": {"Running": true}}]', '', 0), ('Created four-12345678', '', 0), - ('a\nb\nc', '', 0), + ('a\nb\nc', '', 0) ] r.discover_container_name = lambda n, c: '%s-12345678' % n r.unique_container_name = lambda n: '%s-12345678' % n @@ -248,6 +246,7 @@ three-12345678 three''', '', 0), 'a\nb\nc' ], stdout) self.assertEqual([], stderr) + exe.assert_has_calls([ # inspect image centos:7 mock.call( @@ -307,9 +306,6 @@ three-12345678 three''', '', 0), '--label', 'config_data=%s' % json.dumps(config['two']), '--detach=true', 'centos:7'], mock.ANY ), - # check container three status via inspect - mock.call(['docker', 'inspect', '--type', 'container', - 'three-12345678'], mock.ANY, False), # don't run three, its already running # run four mock.call( @@ -537,52 +533,3 @@ three-12345678 three''', '', 0), ['ls', '-l', '"/foo', 'bar"'], b.command_argument('ls -l "/foo bar"') ) - - @mock.patch('paunch.runner.DockerRunner.execute') - @mock.patch('paunch.runner.DockerRunner.remove_container') - @mock.patch('paunch.builder.base.BaseBuilder.pull_missing_images') - @mock.patch('paunch.builder.base.BaseBuilder.delete_missing_and_updated') - @mock.patch('paunch.runner.DockerRunner.container_names') - @mock.patch('paunch.runner.DockerRunner.unique_container_name') - @mock.patch('paunch.runner.DockerRunner.discover_container_name') - @mock.patch('paunch.runner.DockerRunner.inspect') - def test_recreate_stopped_container(self, inspect, discover_container_name, - unique_container_name, container_names, - delete_missing_and_updated, - pull_missing_images, remove_container, - execute): - orig_call = tenacity.wait.wait_random_exponential.__call__ - orig_argspec = inspect.getargspec(orig_call) - config = { - # not running yet - 'one': { - 'start_order': 0, - 'image': 'centos:7' - } - } - r = runner.DockerRunner(managed_by='tester', cont_cmd='docker') - - with mock.patch('tenacity.wait.wait_random_exponential.__call__') as f: - f.return_value = 0 - - with mock.patch('inspect.getargspec') as mock_args: - mock_args.return_value = orig_argspec - builder = compose1.ComposeV1Builder('foo', config, r) - - pull_missing_images.return_value = 0 - # No value is returned - delete_missing_and_updated.return_value = None - container_names.return_value = [['one', 'one']] - unique_container_name.return_value = 'one' - # This can be a no-op since inspect_container() uses this return value - # and that will also be overridden. - discover_container_name.return_value = None - inspect.return_value = {"State": {"Running": False}} - remove_container.return_value = ('stdout', 'stderr', 0) - execute.return_value = ('stdout', 'stderr', 0) - # Run the main apply() method for this test. - stdout, stderr, deploy_status_code = builder.apply() - pull_missing_images.assert_called_once() - container_names.assert_called_once() - unique_container_name.assert_called_once() - delete_missing_and_updated.assert_called()