From e2207d7c1268e35244adf0c2241fea2453976b17 Mon Sep 17 00:00:00 2001 From: jorgesece Date: Fri, 28 Oct 2016 14:50:57 +0100 Subject: [PATCH] Check if router exists on network creation List the routers of the tenant and use the first router with external gateway, thus It is linked to the public network. In case no router with those features is found, a new one is created. Change-Id: I696ebe04426fab7e210d86f30084c4fee64cbce8 Closes-bug: #1637497 --- ooi/api/helpers_neutron.py | 41 +++++----- .../middleware/test_network_controller.py | 77 ++++++++++++++++++- .../unit/controllers/test_neutron_helper.py | 73 ++++++++++++++++-- 3 files changed, 161 insertions(+), 30 deletions(-) diff --git a/ooi/api/helpers_neutron.py b/ooi/api/helpers_neutron.py index fb24324..857417e 100644 --- a/ooi/api/helpers_neutron.py +++ b/ooi/api/helpers_neutron.py @@ -349,25 +349,26 @@ class OpenStackNeutron(helpers.BaseHelper): req, 'subnets', subnet_param) # INTERFACE and ROUTER information is agnostic to the user - net_public = self._get_public_network(req) - attributes_router = {"external_gateway_info": { - "network_id": net_public} - } - router = self.create_resource(req, - 'routers', - attributes_router) - try: - # create interface to the network - self._add_router_interface(req, - router['id'], - net['subnet_info']['id'] - ) - except Exception as ex: - self.delete_resource(req, - 'routers', - router['id'] - ) - raise ex + router = None + router_list = self.list_resources(req, "routers") + for r in router_list: + if r["external_gateway_info"]: + router = r + break + if not router: + net_public = self._get_public_network(req) + attributes_router = {"external_gateway_info": { + "network_id": net_public} + } + router = self.create_resource(req, + 'routers', + attributes_router) + + # create interface to the network + self._add_router_interface(req, + router['id'], + net['subnet_info']['id'] + ) except Exception as ex: self.delete_resource(req, 'networks', net['id']) @@ -389,8 +390,6 @@ class OpenStackNeutron(helpers.BaseHelper): port['device_id'], port['id'], ) - self.delete_resource(req, - 'routers', port["device_id"]) else: self.delete_resource(req, 'ports', port["id"]) diff --git a/ooi/tests/functional/middleware/test_network_controller.py b/ooi/tests/functional/middleware/test_network_controller.py index eff941d..33a3690 100644 --- a/ooi/tests/functional/middleware/test_network_controller.py +++ b/ooi/tests/functional/middleware/test_network_controller.py @@ -117,10 +117,14 @@ class TestNetNeutronController(test_middleware.TestMiddleware): 200) mock_subnet = mock.Mock(webob.Request) mock_subnet.get_response.return_value = subnet_out + list_router_out = fakes.create_fake_json_resp( + {"routers": []}, + 200) + mock_list_router = mock.Mock(webob.Request) + mock_list_router.get_response.return_value = list_router_out public_out = fakes.create_fake_json_resp( {"networks": fakes.networks[tenant['id']]}, 200) - mock_public = mock.Mock(webob.Request) mock_public.get_response.return_value = public_out router_out = fakes.create_fake_json_resp( @@ -131,7 +135,76 @@ class TestNetNeutronController(test_middleware.TestMiddleware): mock_iface = mock.Mock(webob.Request) mock_iface.get_response.return_value = fakes.create_fake_json_resp( {"foo": "foo"}, 200) - m.side_effect = [mock_net, mock_subnet, mock_public, + m.side_effect = [mock_net, mock_subnet, mock_list_router, mock_public, + mock_router, mock_iface + ] + name = fakes.networks[tenant["id"]][0]["name"] + net_id = fakes.networks[tenant["id"]][0]["id"] + address = fakes.networks[tenant["id"]][0]["subnet_info"]["cidr"] + headers = { + 'Category': 'network;' + ' scheme=' + '"http://schemas.ogf.org/occi/infrastructure#";' + 'class="kind",' + 'ipnetwork;' + ' scheme=' + '"http://schemas.ogf.org/occi/infrastructure/' + 'network#";' + 'class="mixin",', + 'X-OCCI-Attribute': 'occi.core.title="%s",' + 'occi.network.address="%s"' % + (name, address) + } + req = self._build_req(path="/network", + tenant_id='X', + method="POST", + headers=headers) + + m.return_value = fakes.networks[tenant['id']][0] + resp = req.get_response(self.app) + self.assertEqual(200, resp.status_code) + expected = [("X-OCCI-Location", + utils.join_url(self.application_url + "/", + "network/%s" % net_id))] + self.assertExpectedResult(expected, resp) + + @mock.patch.object(helpers.BaseHelper, "_get_req") + def test_create_router_exists(self, m): + tenant = fakes.tenants["foo"] + net_out = fakes.create_fake_json_resp( + {"network": fakes.networks[tenant['id']][0]}, 200) + mock_net = mock.Mock(webob.Request) + mock_net.get_response.return_value = net_out + subnet_out = fakes.create_fake_json_resp( + {"subnet": fakes.networks[tenant['id']][0]["subnet_info"]}, + 200) + mock_subnet = mock.Mock(webob.Request) + mock_subnet.get_response.return_value = subnet_out + list_router_out = fakes.create_fake_json_resp( + {"routers": [ + {"id": uuid.uuid4().hex, + "external_gateway_info": None}, + {"id": uuid.uuid4().hex, + "external_gateway_info": {"network_id": uuid.uuid4().hex}}, + ] + }, + 200) + mock_list_router = mock.Mock(webob.Request) + mock_list_router.get_response.return_value = list_router_out + public_out = fakes.create_fake_json_resp( + {"networks": fakes.networks[tenant['id']]}, + 200) + mock_public = mock.Mock(webob.Request) + mock_public.get_response.return_value = public_out + router_out = fakes.create_fake_json_resp( + {"router": {"id": uuid.uuid4().hex}}, + 200) + mock_router = mock.Mock(webob.Request) + mock_router.get_response.return_value = router_out + mock_iface = mock.Mock(webob.Request) + mock_iface.get_response.return_value = fakes.create_fake_json_resp( + {"foo": "foo"}, 200) + m.side_effect = [mock_net, mock_subnet, mock_list_router, mock_public, mock_router, mock_iface ] name = fakes.networks[tenant["id"]][0]["name"] diff --git a/ooi/tests/unit/controllers/test_neutron_helper.py b/ooi/tests/unit/controllers/test_neutron_helper.py index 40da61e..85608b1 100644 --- a/ooi/tests/unit/controllers/test_neutron_helper.py +++ b/ooi/tests/unit/controllers/test_neutron_helper.py @@ -379,7 +379,9 @@ class TestNetOpenStackHelper(base.TestCase): }, {"id": router_id}, {"id": 0}] - list_net.return_value = [{'id': public_net}] + router_list = [] + net_list = [{'id': public_net}] + list_net.side_effect = [router_list, net_list] ret = self.helper.create_network(None, name=name, cidr=cidr, @@ -407,6 +409,66 @@ class TestNetOpenStackHelper(base.TestCase): self.assertEqual(cidr, ret['address']) self.assertEqual(gate_way, ret['gateway']) + @mock.patch.object(helpers_neutron.OpenStackNeutron, "create_resource") + @mock.patch.object(helpers_neutron.OpenStackNeutron, "list_resources") + @mock.patch.object(helpers_neutron.OpenStackNeutron, + "_add_router_interface") + def test_create_full_network_router_exists(self, add_if, + list_net, cre_net): + name = "name_net" + net_id = uuid.uuid4().hex + subnet_id = uuid.uuid4().hex + router_id_1 = uuid.uuid4().hex + router_id_2 = uuid.uuid4().hex + state = "ACTIVE" + ip_version = 4 + cidr = "0.0.0.0/24" + gate_way = "0.0.0.1" + parameters = {"occi.core.title": name, + "occi.core.id": net_id, + "occi.network.state": state, + + "org.openstack.network.ip_version": ip_version, + "occi.network.address": cidr, + "occi.network.gateway": gate_way + } + cre_net.side_effect = [{'id': net_id, + "status": 'active', + "name": 'xx'}, + {"id": subnet_id, + "cidr": cidr, + "gateway_ip": gate_way, + } + ] + router_list = [{"id": router_id_1, + "external_gateway_info": None}, + {"id": router_id_2, + "external_gateway_info": + {"network_id": net_id}} + ] + list_net.return_value = router_list + ret = self.helper.create_network(None, + name=name, + cidr=cidr, + gateway=gate_way, + ip_version=ip_version) + self.assertEqual(net_id, ret["id"]) + param = utils.translate_parameters( + self.translation["networks"], parameters) + self.assertEqual((None, 'networks', + param), + cre_net.call_args_list[0][0]) + param_subnet = utils.translate_parameters( + self.translation["subnets"], parameters) + param_subnet['network_id'] = net_id + self.assertEqual((None, 'subnets', + param_subnet), + cre_net.call_args_list[1][0]) + list_net.assert_called_with(None, "routers") + add_if.assert_called_with(None, router_id_2, subnet_id) + self.assertEqual(cidr, ret['address']) + self.assertEqual(gate_way, ret['gateway']) + @mock.patch.object(helpers_neutron.OpenStackNeutron, "delete_resource") @mock.patch.object(helpers_neutron.OpenStackNeutron, "list_resources") @mock.patch.object(helpers_neutron.OpenStackNeutron, @@ -424,19 +486,16 @@ class TestNetOpenStackHelper(base.TestCase): } port2 = {'id': 2, 'device_owner': 'nova'} m_list.return_value = [port1, port2] - m_del.side_effect = [{0}, {0}, []] + m_del.side_effect = [{0}, []] m_if.return_value = [] ret = self.helper.delete_network(None, net_id) self.assertEqual(ret, []) - self.assertEqual((None, 'routers', - port1['device_id']), - m_del.call_args_list[0][0]) self.assertEqual((None, 'ports', port2['id']), - m_del.call_args_list[1][0]) + m_del.call_args_list[0][0]) self.assertEqual((None, 'networks', net_id), - m_del.call_args_list[2][0]) + m_del.call_args_list[1][0]) @mock.patch.object(helpers_neutron.OpenStackNeutron, "_make_delete_request")