openstack
/
paunch
Code Issues Proposed changes

Actually fail if a stop or remove call fails 

As part of the container rename actions, attempt to stop and remove
containers without actually checking if they exist. Additionally we
don't actually fail if these command fail which leads to other errors
later that may be unrelated to the core issue. This change adds
additional checks around the stop and remove container actions to ensure
that the container is running before stopping and exists before
removing.

Backport note:
This change includes the subsequent change[1] to fix the wrong method
name used.

[1] Ie0462cd89f0beb2a2adc82768dea7d42dc343192

Change-Id: I82172f5a2d4683509c802c0f14d571a40e21e6cc
Closes-Bug: #1907833
(cherry picked from commit 523465fe8c)
This commit is contained in:
Alex Schultz 2020-12-11 10:27:21 -07:00 committed by Takashi Kajinami
parent 7e49691145
commit f1a0879fea
2 changed files with 69 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
paunch/runner.py
View File

@ -294,8 +294,6 @@ class BaseRunner(object):
self.remove_container(container)
def remove_container(self, container):
if self.cont_cmd == 'podman':
systemd.service_delete(container=container, log=self.log)
self.execute([self.cont_cmd, 'stop', container], self.log)
cmd = [self.cont_cmd, 'rm', container]
cmd_stdout, cmd_stderr, returncode = self.execute(cmd, self.log)
@ -308,6 +306,7 @@ class BaseRunner(object):
if returncode != 0:
self.log.error('Error removing container: %s' % container)
self.log.error(cmd_stderr)
raise Exception('Unable to remove container: %s' % container)
def stop_container(self, container, cont_cmd=None, quiet=False):
cont_cmd = cont_cmd or self.cont_cmd
@ -316,6 +315,7 @@ class BaseRunner(object):
if returncode != 0 and not quiet:
self.log.error('Error stopping container: %s' % container)
self.log.error(cmd_stderr)
raise Exception('Unable to stop container: %s' % container)
def rename_containers(self):
current_containers = []
@ -451,10 +451,9 @@ class PodmanRunner(BaseRunner):
self.log.debug("Renaming a container known as %s into %s, "
"via re-applying its original config" %
(container, name))
self.log.debug("Removing the destination container %s" % name)
# destination container
self.stop_container(name)
self.remove_container(name)
self.log.debug("Removing a container known as %s" % container)
self.stop_container(container)
self.remove_container(container)
builder = podman.PodmanBuilder(
@ -468,6 +467,23 @@ class PodmanRunner(BaseRunner):
)
builder.apply()
def stop_container(self, container, cont_cmd=None, quiet=False):
if not self.container_running(container):
self.log.debug('%s not running, skipping stop' % container)
return
self.log.debug("Stopping container: %s" % container)
return super(PodmanRunner, self).stop_container(container,
cont_cmd,
quiet)
def remove_container(self, container):
if not self.container_exist(container):
self.log.debug('%s does not exist, skipping remove' % container)
return
self.log.debug("Removing container: %s" % container)
systemd.service_delete(container=container, log=self.log)
return super(PodmanRunner, self).remove_container(container)
def image_exist(self, name, quiet=False):
cmd = ['podman', 'image', 'exists', name]
(_, _, returncode) = self.execute(cmd, self.log, quiet, True)
@ -479,6 +495,10 @@ class PodmanRunner(BaseRunner):
return returncode == 0
def container_running(self, container):
if not self.container_exist(container):
self.log.debug('%s is not running because it does not exist' %
container)
return False
service_name = 'tripleo_' + container + '.service'
try:
systemctl.is_active(service_name)

45
paunch/tests/test_runner.py
View File

@ -458,3 +458,48 @@ class PodmanRunner(TestBaseRunner):
self.assert_execute(
popen, ['podman', 'container', 'exists', 'one']
)
@mock.patch('paunch.runner.BaseRunner.stop_container')
def test_stop_container(self, mock_stop):
mock_running = mock.MagicMock()
mock_running.return_value = True
self.runner = runner.PodmanRunner('tester')
self.runner.container_running = mock_running
self.runner.stop_container('foo')
mock_running.assert_called_once_with('foo')
mock_stop.assert_called_once_with('foo', None, False)
@mock.patch('paunch.runner.BaseRunner.stop_container')
def test_stop_container_missing(self, mock_stop):
mock_running = mock.MagicMock()
mock_running.return_value = False
self.runner = runner.PodmanRunner('tester')
self.runner.container_running = mock_running
self.runner.stop_container('foo')
mock_running.assert_called_once_with('foo')
mock_stop.assert_not_called()
@mock.patch('paunch.utils.systemd.service_delete')
@mock.patch('paunch.runner.BaseRunner.remove_container')
def test_remove_container(self, mock_remove, mock_systemd):
mock_exists = mock.MagicMock()
mock_exists.return_value = True
self.runner = runner.PodmanRunner('tester')
self.runner.container_exist = mock_exists
self.runner.remove_container('foo')
mock_exists.assert_called_once_with('foo')
mock_systemd.assert_called_once_with(container='foo',
log=self.runner.log)
mock_remove.assert_called_once_with('foo')
@mock.patch('paunch.utils.systemd.service_delete')
@mock.patch('paunch.runner.BaseRunner.remove_container')
def test_remove_container_missing(self, mock_remove, mock_systemd):
mock_exists = mock.MagicMock()
mock_exists.return_value = False
self.runner = runner.PodmanRunner('tester')
self.runner.container_exist = mock_exists
self.runner.remove_container('foo')
mock_exists.assert_called_once_with('foo')
mock_systemd.assert_not_called()
mock_remove.assert_not_called()