From 4ff152785bd09772c2069be1bd4602159a74a0cc Mon Sep 17 00:00:00 2001 From: Madhuri Kumari Date: Thu, 27 Oct 2016 11:20:33 +0000 Subject: [PATCH] Add container-update API Allow updating the cpu/memory of a running container Co-Authored-By: Hongbin Lu Change-Id: Id9356c88f995fad6aed33bc21681ee58b2da8ac1 Implements: blueprint container-update --- etc/zun/policy.json | 1 + zun/api/controllers/v1/containers.py | 29 ++++------- zun/api/controllers/v1/schemas/containers.py | 11 ++++ zun/common/utils.py | 2 + zun/compute/api.py | 3 ++ zun/compute/manager.py | 18 +++++++ zun/compute/rpcapi.py | 4 ++ zun/conf/docker.py | 2 +- zun/container/docker/driver.py | 20 ++++++++ zun/container/driver.py | 3 ++ zun/tests/tempest/api/clients.py | 5 ++ zun/tests/tempest/api/common/datagen.py | 12 +++++ .../tempest/api/models/container_model.py | 11 ++++ zun/tests/tempest/api/test_containers.py | 27 ++++++++++ .../api/controllers/v1/test_containers.py | 51 ++++++++----------- .../unit/compute/test_compute_manager.py | 17 +++++++ zun/tests/unit/container/fake_driver.py | 4 ++ 17 files changed, 170 insertions(+), 50 deletions(-) diff --git a/etc/zun/policy.json b/etc/zun/policy.json index b00925515..9253bb0f1 100644 --- a/etc/zun/policy.json +++ b/etc/zun/policy.json @@ -18,6 +18,7 @@ "container:logs": "rule:admin_or_user", "container:execute": "rule:admin_or_user", "container:kill": "rule:admin_or_user", + "container:update": "rule:admin_or_user", "container:rename": "rule:admin_or_user", "image:pull": "rule:default", diff --git a/zun/api/controllers/v1/containers.py b/zun/api/controllers/v1/containers.py index 1c69cc359..c687b2336 100644 --- a/zun/api/controllers/v1/containers.py +++ b/zun/api/controllers/v1/containers.py @@ -226,7 +226,8 @@ class ContainersController(rest.RestController): @pecan.expose('json') @exception.wrap_pecan_controller_exception - def patch(self, container_id, **kwargs): + @validation.validated(schema.container_update) + def patch(self, container_id, **patch): """Update an existing container. :param patch: a json PATCH document to apply to this container. @@ -234,25 +235,13 @@ class ContainersController(rest.RestController): context = pecan.request.context container = _get_container(container_id) check_policy_on_container(container.as_dict(), "container:update") - try: - patch = kwargs.get('patch') - container_dict = container.as_dict() - new_container_fields = api_utils.apply_jsonpatch( - container_dict, patch) - except api_utils.JSONPATCH_EXCEPTIONS as e: - raise exception.PatchError(patch=patch, reason=e) - - # Update only the fields that have changed - for field in objects.Container.fields: - try: - patch_val = new_container_fields[field] - except AttributeError: - # Ignore fields that aren't exposed in the API - continue - if getattr(container, field) != patch_val: - setattr(container, field, patch_val) - - container.save(context) + utils.validate_container_state(container, 'update') + if 'memory' in patch: + patch['memory'] = str(patch['memory']) + 'M' + if 'cpu' in patch: + patch['cpu'] = float(patch['cpu']) + compute_api = pecan.request.compute_api + container = compute_api.container_update(context, container, patch) return view.format_container(pecan.request.host_url, container) @pecan.expose('json') diff --git a/zun/api/controllers/v1/schemas/containers.py b/zun/api/controllers/v1/schemas/containers.py index ea87669d1..2fe9cdc0f 100644 --- a/zun/api/controllers/v1/schemas/containers.py +++ b/zun/api/controllers/v1/schemas/containers.py @@ -50,6 +50,17 @@ query_param_create = { 'additionalProperties': False } +_container_update_properties = { + 'cpu': parameter_types.cpu, + 'memory': parameter_types.memory, +} + +container_update = { + 'type': 'object', + 'properties': _container_update_properties, + 'additionalProperties': False +} + query_param_delete = { 'type': 'object', 'properties': { diff --git a/zun/common/utils.py b/zun/common/utils.py index 698297281..622564c0b 100644 --- a/zun/common/utils.py +++ b/zun/common/utils.py @@ -44,6 +44,7 @@ VALID_STATES = { 'unpause': ['Paused'], 'kill': ['Running'], 'execute': ['Running'], + 'update': ['Running', 'Stopped', 'Paused'], } @@ -140,6 +141,7 @@ def translate_exception(function): return function(self, context, *args, **kwargs) except Exception as e: if not isinstance(e, exception.ZunException): + LOG.exception(_LE("Unexpected error: %s"), six.text_type(e)) e = exception.ZunException("Unexpected error: %s" % six.text_type(e)) raise e diff --git a/zun/compute/api.py b/zun/compute/api.py index 23d6bd75f..e624d35a4 100644 --- a/zun/compute/api.py +++ b/zun/compute/api.py @@ -84,6 +84,9 @@ class API(object): def container_kill(self, context, container, *args): return self.rpcapi.container_kill(context, container, *args) + def container_update(self, context, container, *args): + return self.rpcapi.container_update(context, container, *args) + def image_show(self, context, image, *args): return self.rpcapi.image_show(context, image, *args) diff --git a/zun/compute/manager.py b/zun/compute/manager.py index df6946a36..ce0abff5f 100644 --- a/zun/compute/manager.py +++ b/zun/compute/manager.py @@ -327,6 +327,24 @@ class Manager(object): def container_kill(self, context, container, signal): utils.spawn_n(self._do_container_kill, context, container, signal) + @translate_exception + def container_update(self, context, container, patch): + LOG.debug('Updating a container...', container=container) + # Update only the fields that have changed + for field, patch_val in patch.items(): + if getattr(container, field) != patch_val: + setattr(container, field, patch_val) + + try: + self.driver.update(container) + except exception.DockerError as e: + LOG.error(_LE("Error occured while calling docker API: %s"), + six.text_type(e)) + raise + + container.save(context) + return container + def image_pull(self, context, image): utils.spawn_n(self._do_image_pull, context, image) diff --git a/zun/compute/rpcapi.py b/zun/compute/rpcapi.py index 5ec964d8e..62d118c46 100644 --- a/zun/compute/rpcapi.py +++ b/zun/compute/rpcapi.py @@ -78,6 +78,10 @@ class API(rpc_service.API): self._cast(container.host, 'container_kill', container=container, signal=signal) + def container_update(self, context, container, patch): + return self._call(container.host, 'container_update', + container=container, patch=patch) + def image_show(self, context, image): # NOTE(hongbin): Image API doesn't support multiple compute nodes # scenario yet, so we temporarily set host to None and rpc will diff --git a/zun/conf/docker.py b/zun/conf/docker.py index 0c388adf5..6dcc7b1f3 100644 --- a/zun/conf/docker.py +++ b/zun/conf/docker.py @@ -18,7 +18,7 @@ docker_group = cfg.OptGroup(name='docker', docker_opts = [ cfg.StrOpt('docker_remote_api_version', - default='1.20', + default='1.22', help='Docker remote api version. Override it according to ' 'specific docker api version in your environment.'), cfg.IntOpt('default_timeout', diff --git a/zun/container/docker/driver.py b/zun/container/docker/driver.py index 0bdde7fd9..a12decaf6 100644 --- a/zun/container/docker/driver.py +++ b/zun/container/docker/driver.py @@ -230,6 +230,26 @@ class DockerDriver(driver.ContainerDriver): self._populate_container(container, response) return container + @check_container_id + def update(self, container): + patch = container.obj_get_changes() + + args = {} + memory = patch.get('memory') + if memory is not None: + args['mem_limit'] = memory + cpu = patch.get('cpu') + if cpu is not None: + args['cpu_quota'] = int(100000 * cpu) + args['cpu_period'] = 100000 + + with docker_utils.docker_client() as docker: + try: + resp = docker.update_container(container.container_id, **args) + return resp + except errors.APIError: + raise + def _encode_utf8(self, value): if six.PY2 and not isinstance(value, unicode): value = unicode(value) diff --git a/zun/container/driver.py b/zun/container/driver.py index 91f9a2713..65584312e 100644 --- a/zun/container/driver.py +++ b/zun/container/driver.py @@ -139,4 +139,7 @@ class ContainerDriver(object): def get_addresses(self, context, container): """Retrieve IP addresses of the container.""" + + def update(self, container): + """Update a container.""" raise NotImplementedError() diff --git a/zun/tests/tempest/api/clients.py b/zun/tests/tempest/api/clients.py index bcd0e5f55..62aa00240 100644 --- a/zun/tests/tempest/api/clients.py +++ b/zun/tests/tempest/api/clients.py @@ -155,6 +155,11 @@ class ZunClient(rest_client.RestClient): return self.get( self.container_uri(container_id, action='logs'), None, **kwargs) + def update_container(self, container_id, model, **kwargs): + resp, body = self.patch( + self.container_uri(container_id), body=model.to_json(), **kwargs) + return self.deserialize(resp, body, container_model.ContainerEntity) + def list_services(self, **kwargs): resp, body = self.get(self.services_uri(), **kwargs) return self.deserialize(resp, body, diff --git a/zun/tests/tempest/api/common/datagen.py b/zun/tests/tempest/api/common/datagen.py index c831f60cd..8982fc3d9 100644 --- a/zun/tests/tempest/api/common/datagen.py +++ b/zun/tests/tempest/api/common/datagen.py @@ -60,3 +60,15 @@ def container_data(**kwargs): model = container_model.ContainerEntity.from_dict(data) return model + + +def container_patch_data(**kwargs): + data = { + 'cpu': 0.2, + 'memory': '512', + } + + data.update(kwargs) + model = container_model.ContainerPatchEntity.from_dict(data) + + return model diff --git a/zun/tests/tempest/api/models/container_model.py b/zun/tests/tempest/api/models/container_model.py index e938aa963..ec246f46f 100644 --- a/zun/tests/tempest/api/models/container_model.py +++ b/zun/tests/tempest/api/models/container_model.py @@ -28,3 +28,14 @@ class ContainerCollection(base_model.CollectionModel): """Collection Model that represents a list of ContainerData objects""" COLLECTION_NAME = 'containerlists' MODEL_TYPE = ContainerData + + +class ContainerPatchData(base_model.BaseModel): + """Data that encapsulates container update attributes""" + pass + + +class ContainerPatchEntity(base_model.EntityModel): + """Entity Model that represents a single instance of ContainerPatchData""" + ENTITY_NAME = 'containerpatch' + MODEL_TYPE = ContainerPatchData diff --git a/zun/tests/tempest/api/test_containers.py b/zun/tests/tempest/api/test_containers.py index cf280c308..bd0657606 100644 --- a/zun/tests/tempest/api/test_containers.py +++ b/zun/tests/tempest/api/test_containers.py @@ -149,6 +149,33 @@ class TestContainer(base.BaseZunTest): self.assertEqual(200, resp.status) self.assertTrue('hello' in body) + @decorators.idempotent_id('d383f359-3ebd-40ef-9dc5-d36922790230') + def test_update_container(self): + _, model = self._run_container(cpu=0.1, memory=100) + self.assertEqual('100M', model.memory) + self.assertEqual(0.1, model.cpu) + container = self.docker_client.get_container(model.uuid) + self._assert_resource_constraints(container, cpu=0.1, memory=100) + + gen_model = datagen.container_patch_data(cpu=0.2, memory=200) + resp, model = self.container_client.update_container(model.uuid, + gen_model) + self.assertEqual(200, resp.status) + self.assertEqual('200M', model.memory) + self.assertEqual(0.2, model.cpu) + container = self.docker_client.get_container(model.uuid) + self._assert_resource_constraints(container, cpu=0.2, memory=200) + + def _assert_resource_constraints(self, container, cpu=None, memory=None): + if cpu is not None: + cpu_quota = container.get('HostConfig').get('CpuQuota') + self.assertEqual(int(cpu * 100000), cpu_quota) + cpu_period = container.get('HostConfig').get('CpuPeriod') + self.assertEqual(100000, cpu_period) + if memory is not None: + docker_memory = container.get('HostConfig').get('Memory') + self.assertEqual(memory * 1024 * 1024, docker_memory) + def _create_container(self, **kwargs): gen_model = datagen.container_data(**kwargs) resp, model = self.container_client.post_container(gen_model) diff --git a/zun/tests/unit/api/controllers/v1/test_containers.py b/zun/tests/unit/api/controllers/v1/test_containers.py index 7d393f844..6f97a8aeb 100644 --- a/zun/tests/unit/api/controllers/v1/test_containers.py +++ b/zun/tests/unit/api/controllers/v1/test_containers.py @@ -539,43 +539,39 @@ class TestContainerController(api_base.FunctionalTest): self.assertEqual(test_container['uuid'], response.json['uuid']) + @patch('zun.compute.rpcapi.API.container_update') @patch('zun.objects.Container.get_by_uuid') - def test_patch_by_uuid(self, mock_container_get_by_uuid): + def test_patch_by_uuid(self, mock_container_get_by_uuid, mock_update): test_container = utils.get_test_container() test_container_obj = objects.Container(self.context, **test_container) mock_container_get_by_uuid.return_value = test_container_obj + mock_update.return_value = test_container_obj - with patch.object(test_container_obj, 'save') as mock_save: - params = {'patch': [{'path': '/name', - 'value': 'new_name', - 'op': 'replace'}]} - container_uuid = test_container.get('uuid') - response = self.app.patch_json( - '/v1/containers/%s/' % container_uuid, - params=params) + params = {'cpu': 1} + container_uuid = test_container.get('uuid') + response = self.app.patch_json( + '/v1/containers/%s/' % container_uuid, + params=params) - mock_save.assert_called_once() - self.assertEqual(200, response.status_int) - self.assertEqual('new_name', test_container_obj.name) + self.assertEqual(200, response.status_int) + self.assertTrue(mock_update.called) + @patch('zun.compute.rpcapi.API.container_update') @patch('zun.objects.Container.get_by_name') - def test_patch_by_name(self, mock_container_get_by_name): + def test_patch_by_name(self, mock_container_get_by_name, mock_update): test_container = utils.get_test_container() test_container_obj = objects.Container(self.context, **test_container) mock_container_get_by_name.return_value = test_container_obj + mock_update.return_value = test_container_obj - with patch.object(test_container_obj, 'save') as mock_save: - params = {'patch': [{'path': '/name', - 'value': 'new_name', - 'op': 'replace'}]} - container_name = test_container.get('name') - response = self.app.patch_json( - '/v1/containers/%s/' % container_name, - params=params) + params = {'cpu': 1} + container_name = test_container.get('name') + response = self.app.patch_json( + '/v1/containers/%s/' % container_name, + params=params) - mock_save.assert_called_once() - self.assertEqual(200, response.status_int) - self.assertEqual('new_name', test_container_obj.name) + self.assertEqual(200, response.status_int) + self.assertTrue(mock_update.called) def _action_test(self, container, action, ident_field, mock_container_action, status_code, query_param=''): @@ -1131,9 +1127,7 @@ class TestContainerEnforcement(api_base.FunctionalTest): def test_policy_disallow_update(self): container = obj_utils.create_test_container(self.context) - params = {'patch': [{'path': '/name', - 'value': 'new_name', - 'op': 'replace'}]} + params = {'cpu': 1} self._common_policy_check( 'container:update', self.app.patch_json, '/v1/containers/%s/' % container.uuid, params, @@ -1178,8 +1172,7 @@ class TestContainerEnforcement(api_base.FunctionalTest): self._owner_check( "container:update", self.patch_json, '/containers/%s/' % container.uuid, - {'patch': [{ - 'path': '/name', 'value': "new_name", 'op': 'replace'}]}, + {'cpu': 1}, expect_errors=True) def test_policy_only_owner_delete(self): diff --git a/zun/tests/unit/compute/test_compute_manager.py b/zun/tests/unit/compute/test_compute_manager.py index 8a86f0f6b..c53bd8031 100644 --- a/zun/tests/unit/compute/test_compute_manager.py +++ b/zun/tests/unit/compute/test_compute_manager.py @@ -357,3 +357,20 @@ class TestManager(base.TestCase): self.assertRaises(exception.DockerError, self.compute_manager._do_container_kill, self.context, container, None, reraise=True) + + @mock.patch.object(Container, 'save') + @mock.patch.object(fake_driver, 'update') + def test_container_update(self, mock_update, mock_save): + container = Container(self.context, **utils.get_test_container()) + self.compute_manager.container_update(self.context, container, + {'memory': 512}) + mock_save.assert_called_with(self.context) + mock_update.assert_called_once_with(container) + + @mock.patch.object(fake_driver, 'update') + def test_container_update_failed(self, mock_update): + container = Container(self.context, **utils.get_test_container()) + mock_update.side_effect = exception.DockerError + self.assertRaises(exception.DockerError, + self.compute_manager.container_update, + self.context, container, {}) diff --git a/zun/tests/unit/container/fake_driver.py b/zun/tests/unit/container/fake_driver.py index f9f42dd6c..3e9c8aaae 100644 --- a/zun/tests/unit/container/fake_driver.py +++ b/zun/tests/unit/container/fake_driver.py @@ -83,3 +83,7 @@ class FakeDriver(driver.ContainerDriver): def get_addresses(self, context, container): pass + + @check_container_id + def update(self, container): + pass