From 0bfec51913ebd6b3c6ff57464b842da907dc7fa0 Mon Sep 17 00:00:00 2001 From: Kevin Zhao Date: Tue, 28 Nov 2017 10:40:57 +0800 Subject: [PATCH] Add support for create volume when run a container user can use "--mount destination=/data,size=5" to create new volume and attach to the container when run a container Change-Id: I713807b607624c9bfba891a973483b4de7b41226 Signed-off-by: Kevin Zhao --- zun/api/controllers/v1/containers.py | 13 +++-- zun/common/exception.py | 8 ++++ zun/common/validation/parameter_types.py | 3 ++ zun/compute/manager.py | 2 + zun/container/docker/driver.py | 6 +++ zun/container/driver.py | 3 ++ ...b4172_add_auto_remove_to_volume_mapping.py | 27 +++++++++++ zun/db/sqlalchemy/models.py | 1 + zun/objects/volume_mapping.py | 4 +- .../api/controllers/v1/test_containers.py | 48 +++++++++++++++++++ zun/tests/unit/db/utils.py | 1 + zun/tests/unit/objects/test_objects.py | 2 +- zun/tests/unit/volume/test_cinder_api.py | 24 ++++++++++ zun/tests/unit/volume/test_cinder_workflow.py | 15 ++++++ zun/tests/unit/volume/test_driver.py | 11 +++++ zun/volume/cinder_api.py | 17 +++++++ zun/volume/cinder_workflow.py | 8 ++++ zun/volume/driver.py | 8 ++++ 18 files changed, 196 insertions(+), 5 deletions(-) create mode 100644 zun/db/sqlalchemy/alembic/versions/d2affd5b4172_add_auto_remove_to_volume_mapping.py diff --git a/zun/api/controllers/v1/containers.py b/zun/api/controllers/v1/containers.py index 4fc0072f2..744ae7009 100644 --- a/zun/api/controllers/v1/containers.py +++ b/zun/api/controllers/v1/containers.py @@ -428,14 +428,21 @@ class ContainersController(base.Controller): cinder_api = cinder.CinderAPI(context) requested_volumes = [] for mount in mounts: - volume = cinder_api.search_volume(mount['source']) - cinder_api.ensure_volume_usable(volume) + if mount['source'] != '': + volume = cinder_api.search_volume(mount['source']) + cinder_api.ensure_volume_usable(volume) + auto_remove = False + else: + volume = cinder_api.create_volume(mount['size']) + auto_remove = True + volmapp = objects.VolumeMapping( context, volume_id=volume.id, volume_provider='cinder', container_path=mount['destination'], user_id=context.user_id, - project_id=context.project_id) + project_id=context.project_id, + auto_remove=auto_remove) requested_volumes.append(volmapp) return requested_volumes diff --git a/zun/common/exception.py b/zun/common/exception.py index 1d4c751b2..266eb554f 100644 --- a/zun/common/exception.py +++ b/zun/common/exception.py @@ -647,3 +647,11 @@ class PciDeviceInvalidOwner(Invalid): message = _( "PCI device %(compute_node_id)s:%(address)s is owned by %(owner)s " "instead of %(hopeowner)s") + + +class VolumeCreateFailed(Invalid): + message = _("Volume Creation failed: %(creation_failed)s") + + +class VolumeDeleteFailed(Invalid): + message = _("Volume Deletion failed: %(deletion_failed)s") diff --git a/zun/common/validation/parameter_types.py b/zun/common/validation/parameter_types.py index 6b1fcc7f0..eacbf81ad 100644 --- a/zun/common/validation/parameter_types.py +++ b/zun/common/validation/parameter_types.py @@ -123,6 +123,9 @@ mounts = { }, 'destination': { 'type': ['string'], + }, + 'size': { + 'type': ['string'], } }, 'additionalProperties': False, diff --git a/zun/compute/manager.py b/zun/compute/manager.py index 88e80c853..9a88ff26a 100644 --- a/zun/compute/manager.py +++ b/zun/compute/manager.py @@ -286,6 +286,8 @@ class Manager(periodic_task.PeriodicTasks): container.uuid) for volume in volumes: self._detach_volume(context, volume, reraise=reraise) + if volume.auto_remove: + self.driver.delete_volume(context, volume) def _detach_volume(self, context, volume, reraise=True): context = context.elevated() diff --git a/zun/container/docker/driver.py b/zun/container/docker/driver.py index 1fa306baa..02079db01 100644 --- a/zun/container/docker/driver.py +++ b/zun/container/docker/driver.py @@ -750,6 +750,12 @@ class DockerDriver(driver.ContainerDriver): context=context) volume_driver.detach(volume_mapping) + def delete_volume(self, context, volume_mapping): + volume_driver = vol_driver.driver( + provider=volume_mapping.volume_provider, + context=context) + volume_driver.delete(volume_mapping) + def _get_or_create_docker_network(self, context, network_api, neutron_net_id): docker_net_name = self._get_docker_network_name(context, diff --git a/zun/container/driver.py b/zun/container/driver.py index c61fe3774..0f40dc015 100644 --- a/zun/container/driver.py +++ b/zun/container/driver.py @@ -201,6 +201,9 @@ class ContainerDriver(object): def detach_volume(self, context, volume_mapping): raise NotImplementedError() + def delete_volume(self, context, volume_mapping): + raise NotImplementedError() + def add_security_group(self, context, container, security_group, **kwargs): raise NotImplementedError() diff --git a/zun/db/sqlalchemy/alembic/versions/d2affd5b4172_add_auto_remove_to_volume_mapping.py b/zun/db/sqlalchemy/alembic/versions/d2affd5b4172_add_auto_remove_to_volume_mapping.py new file mode 100644 index 000000000..066b04e82 --- /dev/null +++ b/zun/db/sqlalchemy/alembic/versions/d2affd5b4172_add_auto_remove_to_volume_mapping.py @@ -0,0 +1,27 @@ +# Copyright 2017 ARM Limited +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +# revision identifiers, used by Alembic. +revision = 'd2affd5b4172' +down_revision = 'f046346d1d87' +branch_labels = None +depends_on = None + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + op.add_column('volume_mapping', + sa.Column('auto_remove', sa.Boolean, nullable=True)) diff --git a/zun/db/sqlalchemy/models.py b/zun/db/sqlalchemy/models.py index b63212254..d6c5d9173 100644 --- a/zun/db/sqlalchemy/models.py +++ b/zun/db/sqlalchemy/models.py @@ -186,6 +186,7 @@ class VolumeMapping(Base): backref=orm.backref('volume'), foreign_keys=container_uuid, primaryjoin='and_(VolumeMapping.container_uuid==Container.uuid)') + auto_remove = Column(Boolean, default=False) class Image(Base): diff --git a/zun/objects/volume_mapping.py b/zun/objects/volume_mapping.py index ecca5b1fd..685794618 100644 --- a/zun/objects/volume_mapping.py +++ b/zun/objects/volume_mapping.py @@ -34,7 +34,8 @@ def _expected_cols(expected_attrs): @base.ZunObjectRegistry.register class VolumeMapping(base.ZunPersistentObject, base.ZunObject): # Version 1.0: Initial version - VERSION = '1.0' + # Version 1.1: Add field "auto_remove" + VERSION = '1.1' fields = { 'id': fields.IntegerField(), @@ -47,6 +48,7 @@ class VolumeMapping(base.ZunPersistentObject, base.ZunObject): 'container_uuid': fields.UUIDField(nullable=True), 'container': fields.ObjectField('Container', nullable=True), 'connection_info': fields.SensitiveStringField(nullable=True), + 'auto_remove': fields.BooleanField(nullable=True), } @staticmethod diff --git a/zun/tests/unit/api/controllers/v1/test_containers.py b/zun/tests/unit/api/controllers/v1/test_containers.py index 2cc865013..1e8dbd1dc 100644 --- a/zun/tests/unit/api/controllers/v1/test_containers.py +++ b/zun/tests/unit/api/controllers/v1/test_containers.py @@ -754,6 +754,54 @@ class TestContainerController(api_base.FunctionalTest): self.assertEqual(1, len(requested_volumes)) self.assertEqual(fake_volume_id, requested_volumes[0].volume_id) + @patch('zun.network.neutron.NeutronAPI.get_available_network') + @patch('zun.compute.api.API.container_show') + @patch('zun.compute.api.API.container_create') + @patch('zun.common.context.RequestContext.can') + @patch('zun.volume.cinder_api.CinderAPI.create_volume') + @patch('zun.volume.cinder_api.CinderAPI.ensure_volume_usable') + @patch('zun.compute.api.API.image_search') + def test_create_container_with_create_new_volume( + self, mock_search, mock_ensure_volume_usable, mock_create_volume, + mock_authorize, mock_container_create, mock_container_show, + mock_neutron_get_network): + fake_network = {'id': 'foo'} + mock_neutron_get_network.return_value = fake_network + fake_volume_id = 'fakevolid' + fake_volume = mock.Mock(id=fake_volume_id) + mock_create_volume.return_value = fake_volume + # Create a container with a command + params = ('{"name": "MyDocker", "image": "ubuntu",' + '"command": "env", "memory": "512",' + '"mounts": [{"source": "", "destination": "d", ' + '"size": "5"}]}') + response = self.post('/v1/containers/', + params=params, + content_type='application/json') + self.assertEqual(202, response.status_int) + # get all containers + container = objects.Container.list(self.context)[0] + container.status = 'Creating' + mock_container_show.return_value = container + response = self.app.get('/v1/containers/') + self.assertEqual(200, response.status_int) + self.assertEqual(2, len(response.json)) + c = response.json['containers'][0] + self.assertIsNotNone(c.get('uuid')) + self.assertEqual('MyDocker', c.get('name')) + self.assertEqual('env', c.get('command')) + self.assertEqual('Creating', c.get('status')) + self.assertEqual('512M', c.get('memory')) + requested_networks = \ + mock_container_create.call_args[1]['requested_networks'] + self.assertEqual(1, len(requested_networks)) + self.assertEqual(fake_network['id'], requested_networks[0]['network']) + mock_create_volume.assert_called_once() + requested_volumes = \ + mock_container_create.call_args[1]['requested_volumes'] + self.assertEqual(1, len(requested_volumes)) + self.assertEqual(fake_volume_id, requested_volumes[0].volume_id) + @patch('zun.network.neutron.NeutronAPI.get_available_network') @patch('zun.compute.api.API.container_show') @patch('zun.compute.api.API.container_create') diff --git a/zun/tests/unit/db/utils.py b/zun/tests/unit/db/utils.py index 0db925a07..71bae5708 100644 --- a/zun/tests/unit/db/utils.py +++ b/zun/tests/unit/db/utils.py @@ -117,6 +117,7 @@ def get_test_volume_mapping(**kwargs): 'container_uuid': kwargs.get('container_uuid', '1aca1705-20f3-4506-8bc3-59685d86a357'), 'connection_info': kwargs.get('connection_info', 'fake_info'), + 'auto_remove': kwargs.get('auto_remove', False), } diff --git a/zun/tests/unit/objects/test_objects.py b/zun/tests/unit/objects/test_objects.py index 428cb2408..9c106eef3 100644 --- a/zun/tests/unit/objects/test_objects.py +++ b/zun/tests/unit/objects/test_objects.py @@ -345,7 +345,7 @@ class TestObject(test_base.TestCase, _TestObject): # https://docs.openstack.org/zun/latest/ object_data = { 'Container': '1.23-4469205888f8aec51af98375eef6b81a', - 'VolumeMapping': '1.0-187aeb163610315595be729df1c642fc', + 'VolumeMapping': '1.1-50df6202f7846a136a91444c38eba841', 'Image': '1.0-0b976be24f4f6ee0d526e5c981ce0633', 'MyObj': '1.0-34c4b1aadefd177b13f9a2f894cc23cd', 'NUMANode': '1.0-cba878b70b2f8b52f1e031b41ac13b4e', diff --git a/zun/tests/unit/volume/test_cinder_api.py b/zun/tests/unit/volume/test_cinder_api.py index 7d47ccaa0..54ecd4c83 100644 --- a/zun/tests/unit/volume/test_cinder_api.py +++ b/zun/tests/unit/volume/test_cinder_api.py @@ -224,3 +224,27 @@ class CinderApiTestCase(base.TestCase): mock_cinderclient.assert_called_once_with() mock_volumes.terminate_connection.assert_called_once_with('id1', 'connector') + + @mock.patch('zun.common.clients.OpenStackClients.cinder') + def test_create_volume(self, mock_cinderclient): + mock_volumes = mock.MagicMock() + mock_cinderclient.return_value = mock.MagicMock(volumes=mock_volumes) + + volume_size = '5' + self.api = cinder_api.CinderAPI(self.context) + self.api.create_volume(volume_size) + + mock_cinderclient.assert_called_once_with() + mock_volumes.create.assert_called_once_with(volume_size) + + @mock.patch('zun.common.clients.OpenStackClients.cinder') + def test_delete_volume(self, mock_cinderclient): + mock_volumes = mock.MagicMock() + mock_cinderclient.return_value = mock.MagicMock(volumes=mock_volumes) + + volume_id = self.id + self.api = cinder_api.CinderAPI(self.context) + self.api.delete_volume(volume_id) + + mock_cinderclient.assert_called_once_with() + mock_volumes.delete.assert_called_once_with(volume_id) diff --git a/zun/tests/unit/volume/test_cinder_workflow.py b/zun/tests/unit/volume/test_cinder_workflow.py index ec237a061..9dd03c3b9 100644 --- a/zun/tests/unit/volume/test_cinder_workflow.py +++ b/zun/tests/unit/volume/test_cinder_workflow.py @@ -266,3 +266,18 @@ class CinderWorkflowTestCase(base.TestCase): mock_cinder_api.detach.assert_not_called() mock_cinder_api.roll_detaching.assert_called_once_with( self.fake_volume_id) + + @mock.patch('zun.volume.cinder_api.CinderAPI') + def test_delete_volume(self, + mock_cinder_api_cls): + volume = mock.MagicMock() + volume.volume_id = self.fake_volume_id + volume.connection_info = jsonutils.dumps(self.fake_conn_info) + mock_cinder_api = mock.MagicMock() + mock_cinder_api_cls.return_value = mock_cinder_api + + cinder = cinder_workflow.CinderWorkflow(self.context) + cinder.delete_volume(volume) + + mock_cinder_api.delete_volume.assert_called_once_with( + self.fake_volume_id) diff --git a/zun/tests/unit/volume/test_driver.py b/zun/tests/unit/volume/test_driver.py index cb616b59d..87b2b4361 100644 --- a/zun/tests/unit/volume/test_driver.py +++ b/zun/tests/unit/volume/test_driver.py @@ -176,3 +176,14 @@ class VolumeDriverTestCase(base.TestCase): self.assertEqual(self.fake_mountpoint, source) self.assertEqual(self.fake_container_path, destination) mock_get_mountpoint.assert_called_once_with(self.fake_volume_id) + + @mock.patch('zun.volume.cinder_workflow.CinderWorkflow') + def test_delete(self, mock_cinder_workflow_cls): + mock_cinder_workflow = mock.MagicMock() + mock_cinder_workflow_cls.return_value = mock_cinder_workflow + mock_cinder_workflow.delete_volume.return_value = self.fake_volume_id + + volume_driver = driver.Cinder(self.context, 'cinder') + volume_driver.delete(self.volume) + + mock_cinder_workflow.delete_volume.assert_called_once_with(self.volume) diff --git a/zun/volume/cinder_api.py b/zun/volume/cinder_api.py index 417f376f8..be7784eef 100644 --- a/zun/volume/cinder_api.py +++ b/zun/volume/cinder_api.py @@ -134,3 +134,20 @@ class CinderAPI(object): def roll_detaching(self, volume_id): self.cinder.volumes.roll_detaching(volume_id) + + def create_volume(self, size): + try: + volume = self.cinder.volumes.create(size) + except cinder_exception.ClientException as ex: + LOG.error('Volume creation failed: %(ex)s', {'ex': ex}) + raise exception.VolumeCreateFailed(creation_failed=ex) + + return volume + + def delete_volume(self, volume_id): + try: + self.cinder.volumes.delete(volume_id) + except cinder_exception.ClientException as ex: + LOG.error('Volume deletion failed: %(ex)s', + {'ex': ex}) + raise exception.VolumeDeleteFailed(deletion_failed=ex) diff --git a/zun/volume/cinder_workflow.py b/zun/volume/cinder_workflow.py index e92001676..3ac5d7500 100644 --- a/zun/volume/cinder_workflow.py +++ b/zun/volume/cinder_workflow.py @@ -169,3 +169,11 @@ class CinderWorkflow(object): cinder_api.terminate_connection( volume_id, get_volume_connector_properties()) cinder_api.detach(volume_id) + + def delete_volume(self, volume): + volume_id = volume.volume_id + cinder_api = cinder.CinderAPI(self.context) + try: + cinder_api.delete_volume(volume_id) + except cinder_exception as e: + raise exception.Invalid(_("Delete Volume failed: %s") % str(e)) diff --git a/zun/volume/driver.py b/zun/volume/driver.py index 97282f5c9..f0f401668 100644 --- a/zun/volume/driver.py +++ b/zun/volume/driver.py @@ -66,6 +66,9 @@ class VolumeDriver(object): def detach(self, *args, **kwargs): raise NotImplementedError() + def delete(self, *args, **kwargs): + raise NotImplementedError() + def bind_mount(self, *args, **kwargs): raise NotImplementedError() @@ -99,6 +102,11 @@ class Cinder(VolumeDriver): cinder = cinder_workflow.CinderWorkflow(self.context) cinder.detach_volume(volume) + def delete(self, volume): + self._unmount_device(volume) + cinder = cinder_workflow.CinderWorkflow(self.context) + cinder.delete_volume(volume) + def _unmount_device(self, volume): conn_info = jsonutils.loads(volume.connection_info) devpath = conn_info['data']['device_path']