From 363c0d148d743d5bcb304df8882ab47a2681b135 Mon Sep 17 00:00:00 2001 From: Lars Kellogg-Stedman Date: Mon, 1 Sep 2014 13:39:16 -0400 Subject: [PATCH] remove containers on delete This patch calls client.remove_container after a container has stopped running in response to client.kill. In the event that a container is created with an explict name, as in: docker_dbserver: type: "DockerInc::Docker::Container" properties: image: mysql name: dbserver Failure to remove the container on delete will prevent the stack from being re-deployed. Change-Id: Ic864d1a808b303fcb58815b87e55a0e7d1639bbb closes-bug: #1364019 --- .../heat_docker/resources/docker_container.py | 6 ++- .../heat_docker/tests/fake_docker_client.py | 29 +++++++++-- .../tests/test_docker_container.py | 51 ++++++++++--------- 3 files changed, 56 insertions(+), 30 deletions(-) diff --git a/contrib/heat_docker/heat_docker/resources/docker_container.py b/contrib/heat_docker/heat_docker/resources/docker_container.py index 1ba0c82ebf..9a9d52112c 100644 --- a/contrib/heat_docker/heat_docker/resources/docker_container.py +++ b/contrib/heat_docker/heat_docker/resources/docker_container.py @@ -495,11 +495,15 @@ class DockerContainer(resource.Resource): return True try: status = self._get_container_status(container_id) + if not status['Running']: + client = self.get_client() + client.remove_container(container_id) except docker.errors.APIError as ex: if ex.response.status_code == 404: return True raise - return (not status['Running']) + + return False def handle_suspend(self): if not self.resource_id: diff --git a/contrib/heat_docker/heat_docker/tests/fake_docker_client.py b/contrib/heat_docker/heat_docker/tests/fake_docker_client.py index 9c48e1e37b..98d7747d86 100644 --- a/contrib/heat_docker/heat_docker/tests/fake_docker_client.py +++ b/contrib/heat_docker/heat_docker/tests/fake_docker_client.py @@ -14,15 +14,29 @@ # License for the specific language governing permissions and limitations # under the License. +import mock import random import string class APIError(Exception): - pass + def __init__(self, content, response): + super(APIError, self).__init__(content) + self.response = response -class FakeDockerClient(object): +errors = mock.MagicMock() +errors.APIError = APIError + + +class FakeResponse (object): + def __init__(self, status_code=200, reason='OK'): + self.status_code = status_code + self.reason = reason + self.content = reason + + +class Client(object): def __init__(self, endpoint=None): self._endpoint = endpoint @@ -37,8 +51,11 @@ class FakeDockerClient(object): def _check_exists(self, container_id): if container_id not in self._containers: - raise APIError('404 Client Error: Not Found ("No such container: ' - '{0}")'.format(container_id)) + raise APIError( + '404 Client Error: Not Found ("No such container: ' + '{0}")'.format(container_id), + FakeResponse(status_code=404, + reason='No such container')) def _set_running(self, container_id, running): self._check_exists(container_id) @@ -77,6 +94,10 @@ class FakeDockerClient(object): self._set_running(container_id, False) return self.inspect_container(container_id) + def remove_container(self, container_id, **kwargs): + self._check_exists(container_id) + del self._containers[container_id] + def start(self, container_id, **kwargs): self.container_start.append(kwargs) self._set_running(container_id, True) diff --git a/contrib/heat_docker/heat_docker/tests/test_docker_container.py b/contrib/heat_docker/heat_docker/tests/test_docker_container.py index 5b3bd9ee7e..d5fd4d5b72 100644 --- a/contrib/heat_docker/heat_docker/tests/test_docker_container.py +++ b/contrib/heat_docker/heat_docker/tests/test_docker_container.py @@ -15,7 +15,6 @@ # under the License. import mock -from oslo_utils import importutils import six from heat.common import exception @@ -30,9 +29,9 @@ from heat.tests import utils import testtools from heat_docker.resources import docker_container -from heat_docker.tests import fake_docker_client as fakeclient +from heat_docker.tests import fake_docker_client as docker -docker = importutils.try_import('docker') +docker_container.docker = docker template = ''' @@ -72,7 +71,7 @@ class DockerContainerTest(common.HeatTestCase): self.stack) self.m.StubOutWithMock(resource, 'get_client') resource.get_client().MultipleTimes().AndReturn( - fakeclient.FakeDockerClient()) + docker.Client()) self.assertIsNone(resource.validate()) self.m.ReplayAll() scheduler.TaskRunner(resource.create)() @@ -102,7 +101,7 @@ class DockerContainerTest(common.HeatTestCase): 'Blog', definition, self.stack) self.m.StubOutWithMock(resource, 'get_client') resource.get_client().MultipleTimes().AndReturn( - fakeclient.FakeDockerClient()) + docker.Client()) self.assertIsNone(resource.validate()) self.m.ReplayAll() scheduler.TaskRunner(resource.create)() @@ -144,7 +143,7 @@ class DockerContainerTest(common.HeatTestCase): 'Blog', definition, self.stack) self.m.StubOutWithMock(resource, 'get_client') resource.get_client().MultipleTimes().AndReturn( - fakeclient.FakeDockerClient()) + docker.Client()) self.assertIsNone(resource.validate()) self.m.ReplayAll() scheduler.TaskRunner(resource.create)() @@ -171,21 +170,23 @@ class DockerContainerTest(common.HeatTestCase): self.assertRaises(exception.InvalidTemplateAttribute, container.FnGetAtt, 'invalid_attribute') + @testtools.skipIf(docker is None, 'docker-py not available') def test_resource_delete(self): container = self.create_container('Blog') scheduler.TaskRunner(container.delete)() self.assertEqual((container.DELETE, container.COMPLETE), container.state) - running = self.get_container_state(container)['Running'] - self.assertIs(False, running) - def test_resource_already_deleted(self): - container = self.create_container('Blog') - scheduler.TaskRunner(container.delete)() - running = self.get_container_state(container)['Running'] - self.assertIs(False, running) + exists = True + try: + self.get_container_state(container)['Running'] + except docker.errors.APIError as error: + if error.response.status_code == 404: + exists = False + else: + raise - scheduler.TaskRunner(container.delete)() + self.assertIs(False, exists) self.m.VerifyAll() @testtools.skipIf(docker is None, 'docker-py not available') @@ -232,7 +233,7 @@ class DockerContainerTest(common.HeatTestCase): resource = docker_container.DockerContainer( 'Blog', definition, self.stack) get_client_mock = self.patchobject(resource, 'get_client') - get_client_mock.return_value = fakeclient.FakeDockerClient() + get_client_mock.return_value = docker.Client() self.assertIsNone(resource.validate()) scheduler.TaskRunner(resource.create)() self.assertEqual((resource.CREATE, resource.COMPLETE), @@ -251,7 +252,7 @@ class DockerContainerTest(common.HeatTestCase): resource = docker_container.DockerContainer( 'Blog', definition, self.stack) get_client_mock = self.patchobject(resource, 'get_client') - get_client_mock.return_value = fakeclient.FakeDockerClient() + get_client_mock.return_value = docker.Client() self.assertIsNone(resource.validate()) scheduler.TaskRunner(resource.create)() self.assertEqual((resource.CREATE, resource.COMPLETE), @@ -270,7 +271,7 @@ class DockerContainerTest(common.HeatTestCase): resource = docker_container.DockerContainer( 'Blog', definition, self.stack) get_client_mock = self.patchobject(resource, 'get_client') - get_client_mock.return_value = fakeclient.FakeDockerClient() + get_client_mock.return_value = docker.Client() self.assertIsNone(resource.validate()) scheduler.TaskRunner(resource.create)() self.assertEqual((resource.CREATE, resource.COMPLETE), @@ -289,7 +290,7 @@ class DockerContainerTest(common.HeatTestCase): resource = docker_container.DockerContainer( 'Blog', definition, self.stack) get_client_mock = self.patchobject(resource, 'get_client') - get_client_mock.return_value = fakeclient.FakeDockerClient() + get_client_mock.return_value = docker.Client() self.assertIsNone(resource.validate()) scheduler.TaskRunner(resource.create)() self.assertEqual((resource.CREATE, resource.COMPLETE), @@ -307,7 +308,7 @@ class DockerContainerTest(common.HeatTestCase): resource = docker_container.DockerContainer( 'Blog', definition, self.stack) get_client_mock = self.patchobject(resource, 'get_client') - get_client_mock.return_value = fakeclient.FakeDockerClient() + get_client_mock.return_value = docker.Client() get_client_mock.return_value.set_api_version('1.17') self.assertIsNone(resource.validate()) scheduler.TaskRunner(resource.create)() @@ -325,7 +326,7 @@ class DockerContainerTest(common.HeatTestCase): my_resource = docker_container.DockerContainer( 'Blog', definition, self.stack) get_client_mock = self.patchobject(my_resource, 'get_client') - get_client_mock.return_value = fakeclient.FakeDockerClient() + get_client_mock.return_value = docker.Client() get_client_mock.return_value.set_api_version(low_version) msg = self.assertRaises(docker_container.InvalidArgForVersion, my_resource.validate) @@ -351,7 +352,7 @@ class DockerContainerTest(common.HeatTestCase): my_resource = docker_container.DockerContainer( 'Blog', definition, self.stack) get_client_mock = self.patchobject(my_resource, 'get_client') - get_client_mock.return_value = fakeclient.FakeDockerClient() + get_client_mock.return_value = docker.Client() self.assertIsNone(my_resource.validate()) scheduler.TaskRunner(my_resource.create)() self.assertEqual((my_resource.CREATE, my_resource.COMPLETE), @@ -377,7 +378,7 @@ class DockerContainerTest(common.HeatTestCase): my_resource = docker_container.DockerContainer( 'Blog', definition, self.stack) get_client_mock = self.patchobject(my_resource, 'get_client') - get_client_mock.return_value = fakeclient.FakeDockerClient() + get_client_mock.return_value = docker.Client() self.assertIsNone(my_resource.validate()) scheduler.TaskRunner(my_resource.create)() self.assertEqual((my_resource.CREATE, my_resource.COMPLETE), @@ -400,7 +401,7 @@ class DockerContainerTest(common.HeatTestCase): my_resource = docker_container.DockerContainer( 'Blog', definition, self.stack) get_client_mock = self.patchobject(my_resource, 'get_client') - get_client_mock.return_value = fakeclient.FakeDockerClient() + get_client_mock.return_value = docker.Client() self.assertIsNone(my_resource.validate()) scheduler.TaskRunner(my_resource.create)() self.assertEqual((my_resource.CREATE, my_resource.COMPLETE), @@ -425,7 +426,7 @@ class DockerContainerTest(common.HeatTestCase): my_resource = docker_container.DockerContainer( 'Blog', definition, self.stack) get_client_mock = self.patchobject(my_resource, 'get_client') - get_client_mock.return_value = fakeclient.FakeDockerClient() + get_client_mock.return_value = docker.Client() self.assertIsNone(my_resource.validate()) scheduler.TaskRunner(my_resource.create)() self.assertEqual((my_resource.CREATE, my_resource.COMPLETE), @@ -443,7 +444,7 @@ class DockerContainerTest(common.HeatTestCase): my_resource = docker_container.DockerContainer( 'Blog', definition, self.stack) get_client_mock = self.patchobject(my_resource, 'get_client') - get_client_mock.return_value = fakeclient.FakeDockerClient() + get_client_mock.return_value = docker.Client() self.assertIsNone(my_resource.validate()) scheduler.TaskRunner(my_resource.create)() self.assertEqual((my_resource.CREATE, my_resource.COMPLETE),