From 18683ead97cf8e72ccf7e53f12aa1fed741c47dc Mon Sep 17 00:00:00 2001 From: Feng Shengqin Date: Thu, 5 Jan 2017 20:55:30 +0800 Subject: [PATCH] Container-update can set the same name for two containers If we create two containers with the same name,we will get "ERROR: Conflict (HTTP 409)".But the command "zun container-update" can update two containers with the same name. Change-Id: I890e4dc1b69a483df532d7e4ed4ab7bb789caf16 Closes-Bug: #1654134 --- zun/api/controllers/v1/containers.py | 3 +- zun/db/etcd/api.py | 3 ++ zun/db/sqlalchemy/api.py | 3 ++ .../api/controllers/v1/test_containers.py | 4 +- zun/tests/unit/db/test_container.py | 37 +++++++++++++++++++ 5 files changed, 47 insertions(+), 3 deletions(-) diff --git a/zun/api/controllers/v1/containers.py b/zun/api/controllers/v1/containers.py index 99b3b0e09..12ec90d1c 100644 --- a/zun/api/controllers/v1/containers.py +++ b/zun/api/controllers/v1/containers.py @@ -298,6 +298,7 @@ class ContainersController(rest.RestController): :param patch: a json PATCH document to apply to this container. """ + context = pecan.request.context container = _get_container(container_id) check_policy_on_container(container.as_dict(), "container:update") try: @@ -318,7 +319,7 @@ class ContainersController(rest.RestController): if getattr(container, field) != patch_val: setattr(container, field, patch_val) - container.save() + container.save(context) return Container.convert_with_links(container.as_dict()) @pecan.expose('json') diff --git a/zun/db/etcd/api.py b/zun/db/etcd/api.py index 020550de1..f1af6548a 100644 --- a/zun/db/etcd/api.py +++ b/zun/db/etcd/api.py @@ -277,6 +277,9 @@ class EtcdAPI(object): msg = _("Cannot overwrite UUID for an existing Container.") raise exception.InvalidParameterValue(err=msg) + if 'name' in values: + self._validate_unique_container_name(context, values['name']) + try: target_uuid = self._get_container_by_ident( context, container_ident).uuid diff --git a/zun/db/sqlalchemy/api.py b/zun/db/sqlalchemy/api.py index 5277a0955..4be3da3d8 100644 --- a/zun/db/sqlalchemy/api.py +++ b/zun/db/sqlalchemy/api.py @@ -221,6 +221,9 @@ class Connection(api.Connection): msg = _("Cannot overwrite UUID for an existing Container.") raise exception.InvalidParameterValue(err=msg) + if 'name' in values: + self._validate_unique_container_name(context, values['name']) + return self._do_update_container(container_id, values) def _do_update_container(self, container_id, values): diff --git a/zun/tests/unit/api/controllers/v1/test_containers.py b/zun/tests/unit/api/controllers/v1/test_containers.py index c66d55490..904b6dd4e 100644 --- a/zun/tests/unit/api/controllers/v1/test_containers.py +++ b/zun/tests/unit/api/controllers/v1/test_containers.py @@ -391,7 +391,7 @@ class TestContainerController(api_base.FunctionalTest): '/v1/containers/%s/' % container_uuid, params=params) - mock_save.assert_called_once_with() + mock_save.assert_called_once() self.assertEqual(200, response.status_int) self.assertEqual('new_name', test_container_obj.name) @@ -410,7 +410,7 @@ class TestContainerController(api_base.FunctionalTest): '/v1/containers/%s/' % container_name, params=params) - mock_save.assert_called_once_with() + mock_save.assert_called_once() self.assertEqual(200, response.status_int) self.assertEqual('new_name', test_container_obj.name) diff --git a/zun/tests/unit/db/test_container.py b/zun/tests/unit/db/test_container.py index fc51fcce9..1142ff86c 100644 --- a/zun/tests/unit/db/test_container.py +++ b/zun/tests/unit/db/test_container.py @@ -210,6 +210,22 @@ class DbContainerTestCase(base.DbTestCase): {'image': new_image}) self.assertEqual(new_image, res.image) + def test_update_container_with_the_same_name(self): + container1 = utils.create_test_container( + name='container-one', + uuid=uuidutils.generate_uuid(), + context=self.context) + container2 = utils.create_test_container( + name='container-two', + uuid=uuidutils.generate_uuid(), + context=self.context) + new_name = 'new_name' + dbapi.Connection.update_container(self.context, container1.id, + {'name': new_name}) + self.assertRaises(exception.ContainerAlreadyExists, + dbapi.Connection.update_container, self.context, + container2.id, {'name': new_name}) + def test_update_container_not_found(self): container_uuid = uuidutils.generate_uuid() new_image = 'new-image' @@ -433,6 +449,27 @@ class EtcdDbContainerTestCase(base.DbTestCase): self.assertEqual(new_image, json.loads( mock_update.call_args_list[0][0][0].value)['image']) + @mock.patch.object(etcd_client, 'read') + @mock.patch.object(etcd_client, 'write') + @mock.patch.object(etcd_client, 'update') + def test_update_container_with_the_same_name(self, mock_update, + mock_write, mock_read): + mock_read.side_effect = etcd.EtcdKeyNotFound + container1 = utils.create_test_container( + name='container-one', + uuid=uuidutils.generate_uuid(), + context=self.context) + container2 = utils.create_test_container( + name='container-two', + uuid=uuidutils.generate_uuid(), + context=self.context) + + mock_read.side_effect = lambda *args: FakeEtcdMutlipleResult( + [container1.as_dict(), container2.as_dict()]) + self.assertRaises(exception.ContainerAlreadyExists, + dbapi.Connection.update_container, self.context, + container2.uuid, {'name': 'container-one'}) + @mock.patch.object(etcd_client, 'read') def test_update_container_not_found(self, mock_read): container_uuid = uuidutils.generate_uuid()