From 648a9cdaaa9ae0461e2f4072d68f6bad3a232d43 Mon Sep 17 00:00:00 2001 From: Hongbin Lu Date: Sat, 8 Jul 2017 22:25:20 +0000 Subject: [PATCH] Implement create with existing neutron port Support creating container with existing neutron port. Users can pass the uuid/name of an existing neutron port via the 'nets' parameter. If port is specified, Zun will use the existing port instead of creating a new port. If a container is deleted, its neutron port will be deleted regardless of existing or not. A future work is to determine if the port is pre-existing and avoid deleting existing port. Closes-Bug: #1702581 Change-Id: I204b4bbe3de9e275690c4a64acdfe557f134b7ee --- api-ref/source/containers.inc | 1 + api-ref/source/parameters.yaml | 24 ++++ .../source/samples/container-create-req.json | 10 +- requirements.txt | 1 + zun/api/controllers/v1/containers.py | 13 +- zun/common/exception.py | 17 +++ zun/container/docker/driver.py | 3 +- zun/network/kuryr_network.py | 39 ++++-- zun/network/neutron.py | 46 +++++- .../api/controllers/v1/test_containers.py | 132 +++++++++++++++--- 10 files changed, 250 insertions(+), 36 deletions(-) diff --git a/api-ref/source/containers.inc b/api-ref/source/containers.inc index a62ac02a8..d21f86d33 100644 --- a/api-ref/source/containers.inc +++ b/api-ref/source/containers.inc @@ -46,6 +46,7 @@ Request - interactive: interactive - image_driver: image_driver - security_groups: security_groups + - nets: nets Request Example ---------------- diff --git a/api-ref/source/parameters.yaml b/api-ref/source/parameters.yaml index b7dd87aff..bc3e00a36 100644 --- a/api-ref/source/parameters.yaml +++ b/api-ref/source/parameters.yaml @@ -162,6 +162,30 @@ name: in: body required: true type: string +nets: + description: | + A list of networks for the container. When you do not specify the nets + parameter, the container attaches to the only network created for the + current tenant. To provision the container with a NIC for a network, + specify the UUID or name of the network in the ``network`` attribute. + To provision the container with a NIC for an already existing port, + specify the port id or name in the ``port`` attribute. + + If multiple networks are defined, the order in which they appear in the + container will not necessarily reflect the order in which they are given + in the request. Users should therefore not depend on device order + to deduce any information about their network devices. + + The special values *auto* and can be specified for networks. *auto* tells + the Containers service to use a network that is available to the project, + if one exists. If one does not exist, the Containers service will attempt + to automatically allocate a network for the project (if possible). *none* + tells the Containers service to not allocate a network for the instance. + The *auto* and *none* values cannot be used with any other network values, + including other network uuids, ports, fixed IPs or device tags. These are + requested as strings for the networks value, not in a list. + in: body + type: object new_name: description: | The new name for the container. diff --git a/api-ref/source/samples/container-create-req.json b/api-ref/source/samples/container-create-req.json index 18ef77dae..2e56da31d 100644 --- a/api-ref/source/samples/container-create-req.json +++ b/api-ref/source/samples/container-create-req.json @@ -18,5 +18,13 @@ }, "interactive": "False", "image_driver": "docker", - "security_groups": null + "security_groups": null, + "nets": [ + { + "v4-fixed-ip": "", + "network": "", + "v6-fixed-ip": "", + "port": "890699a9-4690-4bd6-8b70-3a9c1be77ecb" + } + ] } diff --git a/requirements.txt b/requirements.txt index d8c9cc88c..104e84d69 100644 --- a/requirements.txt +++ b/requirements.txt @@ -32,4 +32,5 @@ SQLAlchemy!=1.1.5,!=1.1.6,!=1.1.7,!=1.1.8,>=1.0.10 # MIT stevedore>=1.20.0 # Apache-2.0 docker>=2.0.0 # Apache-2.0 netaddr!=0.7.16,>=0.7.13 # BSD +neutron-lib>=1.9.0 # Apache-2.0 websockify>=0.8.0 # LGPLv3 diff --git a/zun/api/controllers/v1/containers.py b/zun/api/controllers/v1/containers.py index 455afb96b..40d2d81f0 100644 --- a/zun/api/controllers/v1/containers.py +++ b/zun/api/controllers/v1/containers.py @@ -299,11 +299,20 @@ class ContainersController(base.Controller): return view.format_container(pecan.request.host_url, new_container) def _build_requested_networks(self, context, nets): + neutron_api = neutron.NeutronAPI(context) requested_networks = [] - if not nets: + for net in nets: + if 'port' in net: + port = neutron_api.get_neutron_port(net['port']) + neutron_api.ensure_neutron_port_usable(port) + requested_networks.append({'network': port['network_id'], + 'port': port['id'], + 'v4-fixed-ip': '', + 'v6-fixed-ip': ''}) + + if not requested_networks: # Find an available neutron net and create docker network by # wrapping the neutron net. - neutron_api = neutron.NeutronAPI(context) neutron_net = neutron_api.get_available_network() requested_networks.append({'network': neutron_net['id'], 'port': '', diff --git a/zun/common/exception.py b/zun/common/exception.py index 67ca7d4f3..67080637c 100644 --- a/zun/common/exception.py +++ b/zun/common/exception.py @@ -352,6 +352,10 @@ class ComputeNodeNotFound(HTTPNotFound): message = _("Compute node %(compute_node)s could not be found.") +class PortNotFound(HTTPNotFound): + message = _("Neutron port %(port)s could not be found.") + + class ImageNotFound(HTTPNotFound): message = _("Image %(image)s could not be found.") @@ -400,6 +404,19 @@ class ResourceClassAlreadyExists(ResourceExists): message = _("A resource class with %(field)s %(value)s already exists.") +class PortNotUsable(Invalid): + message = _("Port %(port)s not usable for the container.") + + +class PortInUse(Invalid): + message = _("Port %(port)s is still in use.") + + +class PortBindingFailed(Invalid): + message = _("Binding failed for port %(port)s, please check neutron " + "logs for more information.") + + class UniqueConstraintViolated(ResourceExists): message = _("A resource with %(fields)s violates unique constraint.") diff --git a/zun/container/docker/driver.py b/zun/container/docker/driver.py index 0e36a4a5d..d6f30e951 100644 --- a/zun/container/docker/driver.py +++ b/zun/container/docker/driver.py @@ -182,7 +182,8 @@ class DockerDriver(driver.ContainerDriver): docker_net_name = self._get_docker_network_name( context, network['network']) addrs = network_api.connect_container_to_network( - container, docker_net_name, security_groups=security_group_ids) + container, docker_net_name, network, + security_groups=security_group_ids) addresses[docker_net_name] = addrs return addresses diff --git a/zun/network/kuryr_network.py b/zun/network/kuryr_network.py index d05a7aacc..53f54a267 100644 --- a/zun/network/kuryr_network.py +++ b/zun/network/kuryr_network.py @@ -22,6 +22,7 @@ from zun.common import exception from zun.common.i18n import _ import zun.conf from zun.network import network +from zun.network import neutron CONF = zun.conf.CONF @@ -32,6 +33,8 @@ LOG = logging.getLogger(__name__) class KuryrNetwork(network.Network): def init(self, context, docker_api): self.docker = docker_api + self.neutron_api = neutron.NeutronAPI(context) + # TODO(hongbin): Move all neutron calls to NeutronAPI self.neutron = clients.OpenStackClients(context).neutron() self.context = context @@ -116,7 +119,7 @@ class KuryrNetwork(network.Network): return self.docker.networks(**kwargs) def connect_container_to_network(self, container, network_name, - security_groups=None): + requested_network, security_groups=None): """Connect container to the network This method will create a neutron port, retrieve the ip address(es) @@ -126,20 +129,30 @@ class KuryrNetwork(network.Network): if not container_id: container_id = container.container_id - network = self.inspect_network(network_name) - neutron_net_id = network['Options']['neutron.net.uuid'] - port_dict = { - 'network_id': neutron_net_id, - 'tenant_id': self.context.project_id - } - if security_groups is not None: - port_dict['security_groups'] = security_groups - neutron_port = self.neutron.create_port({'port': port_dict}) + if requested_network.get('port'): + neutron_port_id = requested_network.get('port') + neutron_port = self.neutron_api.get_neutron_port(neutron_port_id) + # NOTE(hongbin): If existing port is specified, security_group_ids + # is ignored because existing port already has security groups. + # We might revisit this behaviour later. Alternatively, we could + # either throw an exception or overwrite the port's security + # groups. + else: + network = self.inspect_network(network_name) + neutron_net_id = network['Options']['neutron.net.uuid'] + port_dict = { + 'network_id': neutron_net_id, + 'tenant_id': self.context.project_id + } + if security_groups is not None: + port_dict['security_groups'] = security_groups + neutron_port = self.neutron.create_port({'port': port_dict}) + neutron_port = neutron_port['port'] ipv4_address = None ipv6_address = None addresses = [] - for fixed_ip in neutron_port['port']['fixed_ips']: + for fixed_ip in neutron_port['fixed_ips']: ip_address = fixed_ip['ip_address'] ip = ipaddress.ip_address(six.text_type(ip_address)) if ip.version == 4: @@ -147,14 +160,14 @@ class KuryrNetwork(network.Network): addresses.append({ 'addr': ip_address, 'version': 4, - 'port': neutron_port['port']['id'] + 'port': neutron_port['id'] }) else: ipv6_address = ip_address addresses.append({ 'addr': ip_address, 'version': 6, - 'port': neutron_port['port']['id'] + 'port': neutron_port['id'] }) kwargs = {} diff --git a/zun/network/neutron.py b/zun/network/neutron.py index 3fd5e4987..c92efcbbc 100644 --- a/zun/network/neutron.py +++ b/zun/network/neutron.py @@ -10,6 +10,9 @@ # License for the specific language governing permissions and limitations # under the License. +from neutron_lib import constants as n_const +from oslo_utils import uuidutils + from zun.common import clients from zun.common import exception from zun.common.i18n import _ @@ -19,13 +22,52 @@ class NeutronAPI(object): def __init__(self, context): self.context = context + self.neutron = clients.OpenStackClients(self.context).neutron() def get_available_network(self): - neutron = clients.OpenStackClients(self.context).neutron() search_opts = {'tenant_id': self.context.project_id, 'shared': False} - nets = neutron.list_networks(**search_opts).get('networks', []) + nets = self.neutron.list_networks(**search_opts).get('networks', []) if not nets: raise exception.Conflict(_( "There is no neutron network available")) nets.sort(key=lambda x: x['created_at']) return nets[0] + + def get_neutron_port(self, port): + if uuidutils.is_uuid_like(port): + ports = self.neutron.list_ports(id=port)['ports'] + else: + ports = self.neutron.list_ports(name=port)['ports'] + + if len(ports) == 0: + raise exception.PortNotFound(port=port) + elif len(ports) > 1: + raise exception.Conflict(_( + 'Multiple neutron ports exist with same name. ' + 'Please use the uuid instead.')) + + port = ports[0] + return port + + def ensure_neutron_port_usable(self, port, project_id=None): + if project_id is None: + project_id = self.context.project_id + + # Make sure the container has access to the port. + if port['tenant_id'] != project_id: + raise exception.PortNotUsable(port=port['id']) + + # Make sure the port isn't already attached to another + # container or Nova instance. + if port.get('device_id'): + raise exception.PortInUse(port=port['id']) + + if port.get('status') == (n_const.PORT_STATUS_ACTIVE, + n_const.PORT_STATUS_BUILD, + n_const.PORT_STATUS_ERROR): + raise exception.PortNotUsable(port=port['id']) + + # Make sure the port is usable + binding_vif_type = port.get('binding:vif_type') + if binding_vif_type == 'binding_failed': + raise exception.PortBindingFailed(port=port['id']) diff --git a/zun/tests/unit/api/controllers/v1/test_containers.py b/zun/tests/unit/api/controllers/v1/test_containers.py index ef1e87418..1ded60b2b 100644 --- a/zun/tests/unit/api/controllers/v1/test_containers.py +++ b/zun/tests/unit/api/controllers/v1/test_containers.py @@ -56,8 +56,8 @@ class TestContainerController(api_base.FunctionalTest): content_type='application/json') @patch('zun.network.neutron.NeutronAPI.get_available_network') - @patch('zun.compute.rpcapi.API.container_run') - @patch('zun.compute.rpcapi.API.image_search') + @patch('zun.compute.api.API.container_run') + @patch('zun.compute.api.API.image_search') def test_run_container_with_false(self, mock_search, mock_container_run, mock_neutron_get_network): @@ -73,8 +73,8 @@ class TestContainerController(api_base.FunctionalTest): self.assertFalse(mock_container_run.called) mock_neutron_get_network.assert_called_once() - @patch('zun.compute.rpcapi.API.container_run') - @patch('zun.compute.rpcapi.API.image_search') + @patch('zun.compute.api.API.container_run') + @patch('zun.compute.api.API.image_search') def test_run_container_with_wrong(self, mock_search, mock_container_run): mock_container_run.side_effect = exception.InvalidValue @@ -181,6 +181,8 @@ class TestContainerController(api_base.FunctionalTest): mock_container_show, mock_neutron_get_network): mock_container_create.side_effect = lambda x, y, z, v: y + fake_network = {'id': 'foo'} + mock_neutron_get_network.return_value = fake_network # Create a container with a command params = ('{"name": "MyDocker", "image": "ubuntu",' '"command": "env", "memory": "512",' @@ -204,6 +206,9 @@ class TestContainerController(api_base.FunctionalTest): self.assertEqual('512M', c.get('memory')) self.assertEqual({"key1": "val1", "key2": "val2"}, c.get('environment')) + requested_networks = mock_container_create.call_args[0][3] + self.assertEqual(1, len(requested_networks)) + self.assertEqual(fake_network['id'], requested_networks[0]['network']) def side_effect(*args, **kwargs): (ctx, cnt, force) = args @@ -231,6 +236,8 @@ class TestContainerController(api_base.FunctionalTest): mock_container_show, mock_neutron_get_network): mock_container_create.side_effect = lambda x, y, z, v: y + fake_network = {'id': 'foo'} + mock_neutron_get_network.return_value = fake_network # Create a container with a command params = ('{"name": "MyDocker", "image": "ubuntu",' '"command": "env",' @@ -255,6 +262,9 @@ class TestContainerController(api_base.FunctionalTest): self.assertEqual({"key1": "val1", "key2": "val2"}, c.get('environment')) mock_neutron_get_network.assert_called_once() + requested_networks = mock_container_create.call_args[0][3] + self.assertEqual(1, len(requested_networks)) + self.assertEqual(fake_network['id'], requested_networks[0]['network']) @patch('zun.network.neutron.NeutronAPI.get_available_network') @patch('zun.compute.api.API.container_show') @@ -265,6 +275,8 @@ class TestContainerController(api_base.FunctionalTest): mock_container_show, mock_neutron_get_network): mock_container_create.side_effect = lambda x, y, z, v: y + fake_network = {'id': 'foo'} + mock_neutron_get_network.return_value = fake_network # Create a container with a command params = ('{"name": "MyDocker", "image": "ubuntu",' '"command": "env", "memory": "512"}') @@ -287,6 +299,9 @@ class TestContainerController(api_base.FunctionalTest): self.assertEqual('512M', c.get('memory')) self.assertEqual({}, c.get('environment')) mock_neutron_get_network.assert_called_once() + requested_networks = mock_container_create.call_args[0][3] + self.assertEqual(1, len(requested_networks)) + self.assertEqual(fake_network['id'], requested_networks[0]['network']) @patch('zun.network.neutron.NeutronAPI.get_available_network') @patch('zun.compute.api.API.container_show') @@ -298,6 +313,8 @@ class TestContainerController(api_base.FunctionalTest): mock_neutron_get_network): # No name param mock_container_create.side_effect = lambda x, y, z, v: y + fake_network = {'id': 'foo'} + mock_neutron_get_network.return_value = fake_network params = ('{"image": "ubuntu", "command": "env", "memory": "512",' '"environment": {"key1": "val1", "key2": "val2"}}') response = self.app.post('/v1/containers/', @@ -320,10 +337,13 @@ class TestContainerController(api_base.FunctionalTest): self.assertEqual({"key1": "val1", "key2": "val2"}, c.get('environment')) mock_neutron_get_network.assert_called_once() + requested_networks = mock_container_create.call_args[0][3] + self.assertEqual(1, len(requested_networks)) + self.assertEqual(fake_network['id'], requested_networks[0]['network']) @patch('zun.network.neutron.NeutronAPI.get_available_network') - @patch('zun.compute.rpcapi.API.container_show') - @patch('zun.compute.rpcapi.API.container_create') + @patch('zun.compute.api.API.container_show') + @patch('zun.compute.api.API.container_create') @patch('zun.compute.api.API.image_search') def test_create_container_with_restart_policy_no_retry_0( self, @@ -332,6 +352,8 @@ class TestContainerController(api_base.FunctionalTest): mock_container_show, mock_neutron_get_network): mock_container_create.side_effect = lambda x, y, z, v: y + fake_network = {'id': 'foo'} + mock_neutron_get_network.return_value = fake_network # Create a container with a command params = ('{"name": "MyDocker", "image": "ubuntu",' '"command": "env", "memory": "512",' @@ -357,10 +379,13 @@ class TestContainerController(api_base.FunctionalTest): self.assertEqual({"Name": "no", "MaximumRetryCount": "0"}, c.get('restart_policy')) mock_neutron_get_network.assert_called_once() + requested_networks = mock_container_create.call_args[0][3] + self.assertEqual(1, len(requested_networks)) + self.assertEqual(fake_network['id'], requested_networks[0]['network']) @patch('zun.network.neutron.NeutronAPI.get_available_network') - @patch('zun.compute.rpcapi.API.container_show') - @patch('zun.compute.rpcapi.API.container_create') + @patch('zun.compute.api.API.container_show') + @patch('zun.compute.api.API.container_create') @patch('zun.compute.api.API.image_search') def test_create_container_with_restart_policy_no_retry_6( self, @@ -369,6 +394,8 @@ class TestContainerController(api_base.FunctionalTest): mock_container_show, mock_neutron_get_network): mock_container_create.side_effect = lambda x, y, z, v: y + fake_network = {'id': 'foo'} + mock_neutron_get_network.return_value = fake_network # Create a container with a command params = ('{"name": "MyDocker", "image": "ubuntu",' '"command": "env", "memory": "512",' @@ -394,10 +421,13 @@ class TestContainerController(api_base.FunctionalTest): self.assertEqual({"Name": "no", "MaximumRetryCount": "0"}, c.get('restart_policy')) mock_neutron_get_network.assert_called_once() + requested_networks = mock_container_create.call_args[0][3] + self.assertEqual(1, len(requested_networks)) + self.assertEqual(fake_network['id'], requested_networks[0]['network']) @patch('zun.network.neutron.NeutronAPI.get_available_network') - @patch('zun.compute.rpcapi.API.container_show') - @patch('zun.compute.rpcapi.API.container_create') + @patch('zun.compute.api.API.container_show') + @patch('zun.compute.api.API.container_create') @patch('zun.compute.api.API.image_search') def test_create_container_with_restart_policy_miss_retry( self, @@ -406,6 +436,8 @@ class TestContainerController(api_base.FunctionalTest): mock_container_show, mock_neutron_get_network): mock_container_create.side_effect = lambda x, y, z, v: y + fake_network = {'id': 'foo'} + mock_neutron_get_network.return_value = fake_network # Create a container with a command params = ('{"name": "MyDocker", "image": "ubuntu",' '"command": "env", "memory": "512",' @@ -430,10 +462,13 @@ class TestContainerController(api_base.FunctionalTest): self.assertEqual({"Name": "no", "MaximumRetryCount": "0"}, c.get('restart_policy')) mock_neutron_get_network.assert_called_once() + requested_networks = mock_container_create.call_args[0][3] + self.assertEqual(1, len(requested_networks)) + self.assertEqual(fake_network['id'], requested_networks[0]['network']) @patch('zun.network.neutron.NeutronAPI.get_available_network') - @patch('zun.compute.rpcapi.API.container_show') - @patch('zun.compute.rpcapi.API.container_create') + @patch('zun.compute.api.API.container_show') + @patch('zun.compute.api.API.container_create') @patch('zun.compute.api.API.image_search') def test_create_container_with_restart_policy_unless_stopped( self, @@ -442,6 +477,8 @@ class TestContainerController(api_base.FunctionalTest): mock_container_show, mock_neutron_get_network): mock_container_create.side_effect = lambda x, y, z, v: y + fake_network = {'id': 'foo'} + mock_neutron_get_network.return_value = fake_network # Create a container with a command params = ('{"name": "MyDocker", "image": "ubuntu",' '"command": "env", "memory": "512",' @@ -467,10 +504,71 @@ class TestContainerController(api_base.FunctionalTest): self.assertEqual({"Name": "unless-stopped", "MaximumRetryCount": "0"}, c.get('restart_policy')) mock_neutron_get_network.assert_called_once() + requested_networks = mock_container_create.call_args[0][3] + self.assertEqual(1, len(requested_networks)) + self.assertEqual(fake_network['id'], requested_networks[0]['network']) + + @patch('zun.network.neutron.NeutronAPI.get_neutron_port') + @patch('zun.network.neutron.NeutronAPI.ensure_neutron_port_usable') + @patch('zun.compute.api.API.container_show') + @patch('zun.compute.api.API.container_create') + @patch('zun.compute.api.API.container_delete') + @patch('zun.compute.api.API.image_search') + def test_create_container_with_requested_neutron_port( + self, mock_search, mock_container_delete, mock_container_create, + mock_container_show, mock_ensure_port_usable, mock_get_port): + mock_container_create.side_effect = lambda x, y, z, v: y + fake_port = {'network_id': 'foo', 'id': 'bar'} + mock_get_port.return_value = fake_port + # Create a container with a command + params = ('{"name": "MyDocker", "image": "ubuntu",' + '"command": "env", "memory": "512",' + '"environment": {"key1": "val1", "key2": "val2"},' + '"nets": [{"port": "testport"}]}') + response = self.app.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 = 'Stopped' + 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('Stopped', c.get('status')) + self.assertEqual('512M', c.get('memory')) + self.assertEqual({"key1": "val1", "key2": "val2"}, + c.get('environment')) + requested_networks = mock_container_create.call_args[0][3] + self.assertEqual(1, len(requested_networks)) + self.assertEqual(fake_port['network_id'], + requested_networks[0]['network']) + self.assertEqual(fake_port['id'], requested_networks[0]['port']) + + def side_effect(*args, **kwargs): + (ctx, cnt, force) = args + cnt.destroy(ctx) + + # Delete the container we created + mock_container_delete.side_effect = side_effect + response = self.app.delete( + '/v1/containers/%s?force=True' % c.get('uuid')) + self.assertEqual(204, response.status_int) + + response = self.app.get('/v1/containers/') + self.assertEqual(200, response.status_int) + c = response.json['containers'] + self.assertEqual(0, len(c)) + self.assertTrue(mock_container_create.called) @patch('zun.network.neutron.NeutronAPI.get_available_network') - @patch('zun.compute.rpcapi.API.container_show') - @patch('zun.compute.rpcapi.API.container_create') + @patch('zun.compute.api.API.container_show') + @patch('zun.compute.api.API.container_create') @patch('zun.compute.api.API.image_search') def test_create_container_with_restart_policy_always_and_retrycount( self, @@ -492,8 +590,8 @@ class TestContainerController(api_base.FunctionalTest): self.assertTrue(mock_container_create.not_called) mock_neutron_get_network.assert_called_once() - @patch('zun.compute.rpcapi.API.container_create') - @patch('zun.compute.rpcapi.API.image_search') + @patch('zun.compute.api.API.container_create') + @patch('zun.compute.api.API.image_search') def test_create_container_invalid_long_name(self, mock_search, mock_container_create): # Long name @@ -652,7 +750,7 @@ class TestContainerController(api_base.FunctionalTest): self.assertEqual(test_container['uuid'], response.json['uuid']) - @patch('zun.compute.rpcapi.API.container_update') + @patch('zun.compute.api.API.container_update') @patch('zun.objects.Container.get_by_uuid') def test_patch_by_uuid(self, mock_container_get_by_uuid, mock_update): test_container = utils.get_test_container()