From 3813fc7f2bd5768248f7c450d707a13c33f19fc3 Mon Sep 17 00:00:00 2001
From: Bogdan Dobrelya <bdobreli@redhat.com>
Date: Thu, 16 Jan 2020 16:41:59 +0100
Subject: [PATCH] Do not force remove containers

Paunch does docker/podman rm -f, when removes containers. It seems it
returns too early, having some leftovers (like docker service
endpoints) behind or pending for it to be removed later, in case of
big fat deamons.

Long story short, don't do rm -f and allow it to do its job gracefully
and without a hurry.

Change-Id: I346c49cb204f273bd7077ca5153412cda9846534
Closes-Bug: #1860004
Co-authored-by: Sergii Golovatiuk <sgolovat@redhat.com>
Signed-off-by: Bogdan Dobrelya <bdobreli@redhat.com>
---
 paunch/runner.py                  |  3 ++-
 paunch/tests/test_builder_base.py | 15 ++++++++++++---
 paunch/tests/test_runner.py       |  2 +-
 3 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/paunch/runner.py b/paunch/runner.py
index 09f38d1..99a4a3a 100644
--- a/paunch/runner.py
+++ b/paunch/runner.py
@@ -283,7 +283,8 @@ class BaseRunner(object):
     def remove_container(self, container):
         if self.cont_cmd == 'podman':
             systemd.service_delete(container=container, log=self.log)
-        cmd = [self.cont_cmd, 'rm', '-f', container]
+        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)
         if returncode != 0:
             self.log.error('Error removing container: %s' % container)
diff --git a/paunch/tests/test_builder_base.py b/paunch/tests/test_builder_base.py
index f6761d6..40ee804 100644
--- a/paunch/tests/test_builder_base.py
+++ b/paunch/tests/test_builder_base.py
@@ -247,12 +247,18 @@ class TestBaseBuilder(base.TestCase):
 six six
 two-12345678 two
 three-12345678 three''', '', 0),
+            # stop five
+            ('', '', 0),
             # rm five
             ('', '', 0),
+            # stop six
+            ('', '', 0),
             # rm six
             ('', '', 0),
             # inspect two
             ('{"start_order": 1, "image": "centos:6"}', '', 0),
+            # stop two, changed config data
+            ('', '', 0),
             # rm two, changed config data
             ('', '', 0),
             # inspect three
@@ -298,13 +304,16 @@ three-12345678 three''', '', 0),
                 mock.ANY
             ),
             # rm containers not in config
-            mock.call(['docker', 'rm', '-f', 'five'], mock.ANY),
-            mock.call(['docker', 'rm', '-f', 'six'], mock.ANY),
+            mock.call(['docker', 'stop', 'five'], mock.ANY),
+            mock.call(['docker', 'rm', 'five'], mock.ANY),
+            mock.call(['docker', 'stop', 'six'], mock.ANY),
+            mock.call(['docker', 'rm', 'six'], mock.ANY),
             # rm two, changed config
             mock.call(['docker', 'inspect', '--type', 'container',
                        '--format', '{{index .Config.Labels "config_data"}}',
                        'two-12345678'], mock.ANY, False),
-            mock.call(['docker', 'rm', '-f', 'two-12345678'], mock.ANY),
+            mock.call(['docker', 'stop', 'two-12345678'], mock.ANY),
+            mock.call(['docker', 'rm', 'two-12345678'], mock.ANY),
             # check three, config hasn't changed
             mock.call(['docker', 'inspect', '--type', 'container',
                        '--format', '{{index .Config.Labels "config_data"}}',
diff --git a/paunch/tests/test_runner.py b/paunch/tests/test_runner.py
index 5c35333..a490eac 100644
--- a/paunch/tests/test_runner.py
+++ b/paunch/tests/test_runner.py
@@ -269,7 +269,7 @@ class TestBaseRunner(base.TestCase):
 
         self.runner.remove_container('one')
         self.assert_execute(
-            popen, ['docker', 'rm', '-f', 'one']
+            popen, ['docker', 'rm', 'one']
         )
 
     @mock.patch('subprocess.Popen')