diff --git a/heat/engine/resources/openstack/neutron/extrarouteset.py b/heat/engine/resources/openstack/neutron/extrarouteset.py new file mode 100644 index 0000000000..af953474ea --- /dev/null +++ b/heat/engine/resources/openstack/neutron/extrarouteset.py @@ -0,0 +1,236 @@ +# Copyright 2019 Ericsson Software Technology +# +# 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. + +from operator import itemgetter +import six + +from oslo_log import log as logging + +from heat.common import exception +from heat.common.i18n import _ +from heat.engine import constraints +from heat.engine import properties +from heat.engine.resources.openstack.neutron import neutron +from heat.engine.resources.openstack.neutron import router +from heat.engine import support + + +LOG = logging.getLogger(__name__) + + +class ExtraRouteSet(neutron.NeutronResource): + """Resource for specifying extra routes for a Neutron router. + + Requires Neutron ``extraroute-atomic`` extension to be enabled:: + + $ openstack extension show extraroute-atomic + + An extra route is a static routing table entry that is added beyond + the routes managed implicitly by router interfaces and router gateways. + + The ``destination`` of an extra route is any IP network in /CIDR notation. + The ``nexthop`` of an extra route is an IP in a subnet that is directly + connected to the router. + + In a single OS::Neutron::ExtraRouteSet resource you can specify a + set of extra routes (represented as a list) on the same virtual + router. As an improvement over the (never formally supported) + OS::Neutron::ExtraRoute resource this resource plugin uses a Neutron + API extension (``extraroute-atomic``) that is not prone to race + conditions when used to manage multiple extra routes of the same + router. It is safe to manage multiple extra routes of the same router + from multiple stacks. + + On the other hand use of the same route on the same router is not safe + from multiple stacks (or between Heat and non-Heat managed Neutron extra + routes). + """ + + support_status = support.SupportStatus(version='14.0.0') + + required_service_extension = 'extraroute-atomic' + + PROPERTIES = ( + ROUTER, ROUTES, + ) = ( + 'router', 'routes', + ) + + _ROUTE_KEYS = ( + DESTINATION, NEXTHOP, + ) = ( + 'destination', 'nexthop', + ) + + properties_schema = { + ROUTER: properties.Schema( + properties.Schema.STRING, + description=_('The router id.'), + required=True, + constraints=[ + constraints.CustomConstraint('neutron.router') + ], + ), + ROUTES: properties.Schema( + properties.Schema.LIST, + _('A set of route dictionaries for the router.'), + schema=properties.Schema( + properties.Schema.MAP, + schema={ + DESTINATION: properties.Schema( + properties.Schema.STRING, + _('The destination network in CIDR notation.'), + required=True, + constraints=[ + constraints.CustomConstraint('net_cidr') + ] + ), + NEXTHOP: properties.Schema( + properties.Schema.STRING, + _('The next hop for the destination.'), + required=True, + constraints=[ + constraints.CustomConstraint('ip_addr') + ] + ), + }, + ), + default=[], + update_allowed=True, + ), + } + + def add_dependencies(self, deps): + super(ExtraRouteSet, self).add_dependencies(deps) + for resource in six.itervalues(self.stack): + # depend on any RouterInterface in this template with the same + # router as this router + if resource.has_interface('OS::Neutron::RouterInterface'): + try: + router_id = self.properties[self.ROUTER] + dep_router_id = resource.properties.get( + router.RouterInterface.ROUTER) + except (ValueError, TypeError): + # Properties errors will be caught later in validation, + # where we can report them in their proper context. + continue + if dep_router_id == router_id: + deps += (self, resource) + + def handle_create(self): + router = self.properties[self.ROUTER] + routes = self.properties[self.ROUTES] + + _raise_if_duplicate(self.client().show_router(router), routes) + + self.client().add_extra_routes_to_router( + router, {'router': {'routes': routes}}) + + # A set of extra routes does not have a physical ID, so all + # we can do is to set the resource ID to something at least + # informative, that is the router's ID. + self.resource_id_set(router) + + def handle_delete(self): + if not self.resource_id: + return + with self.client_plugin().ignore_not_found: + self.client().remove_extra_routes_from_router( + self.properties[self.ROUTER], + {'router': {'routes': self.properties[self.ROUTES]}}) + + def handle_update(self, json_snippet, tmpl_diff, prop_diff): + """Handle updates correctly. + + Implementing handle_update() here is not just an optimization but a + must, because the default create/delete behavior would delete the + unchanged part of the extra route set. + """ + + # Ignore the shallow diff done in prop_diff. + if self.ROUTES in prop_diff: + del prop_diff[self.ROUTES] + + # Do a deep diff instead. + old = self.properties[self.ROUTES] or [] + new = json_snippet.properties( + self.properties_schema)[self.ROUTES] or [] + + add = _set_to_routes(_routes_to_set(new) - _routes_to_set(old)) + remove = _set_to_routes(_routes_to_set(old) - _routes_to_set(new)) + + router = self.properties[self.ROUTER] + + _raise_if_duplicate(self.client().show_router(router), add) + + # Neither the remove-add nor the add-remove order is perfect. + # Likely both will produce transient packet loss. + # The remove-add order seems to be conceptually simpler, + # never producing unexpected routing tables. + self.client().remove_extra_routes_from_router( + router, {'router': {'routes': remove}}) + self.client().add_extra_routes_to_router( + router, {'router': {'routes': add}}) + + +def _routes_to_set(route_list): + """Convert routes to a set that can be diffed. + + Convert the in-API/in-template routes format to another data type that + has the same information content but that is hashable, so we can put + routes in a set and perform set operations on them. + """ + return set(frozenset(r.items()) for r in route_list) + + +def _set_to_routes(route_set): + """The reverse of _routes_to_set. + + _set_to_routes(_routes_to_set(routes)) == routes + """ + return [dict(r) for r in route_set] + + +def _generate_name(router, routes): + return ','.join( + ['%s' % router] + + ['%(destination)s=%(nexthop)s' % r for r in sorted( + # sort by destination as primary key and + # by nexthop as secondary key + routes, key=itemgetter('destination', 'nexthop'))]) + + +def _raise_if_duplicate(router_existing, routes_to_add): + """Detect trying to add duplicate routes in create/update + + Take the response of show_router() for an existing router and a list of + routes to add and raise PhysicalResourceExists if we try to add a route + already existing on the router. Otherwise do not raise and return None. + + You cannot use this to detect duplicate routes atomically while adding + a route so when you use this you'll inevitably create race conditions. + """ + routes_existing = _routes_to_set( + router_existing['router']['routes']) + for route in _routes_to_set(routes_to_add): + if route in routes_existing: + original = _set_to_routes(set([route])) + name = _generate_name(router, original) + raise exception.PhysicalResourceExists(name=name) + + +def resource_mapping(): + return { + 'OS::Neutron::ExtraRouteSet': ExtraRouteSet, + } diff --git a/heat/tests/openstack/neutron/test_neutron_extrarouteset.py b/heat/tests/openstack/neutron/test_neutron_extrarouteset.py new file mode 100644 index 0000000000..fb9294e7b8 --- /dev/null +++ b/heat/tests/openstack/neutron/test_neutron_extrarouteset.py @@ -0,0 +1,237 @@ +# Copyright 2019 Ericsson Software Technology +# +# 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. + +import copy + +from oslo_log import log as logging + +from heat.common import exception +from heat.common import template_format +from heat.engine.clients.os import neutron +from heat.engine.resources.openstack.neutron import extrarouteset +from heat.engine import scheduler +from heat.tests import common +from heat.tests import utils +from neutronclient.common import exceptions as ncex +from neutronclient.neutron import v2_0 as neutronV20 +from neutronclient.v2_0 import client as neutronclient + + +LOG = logging.getLogger(__name__) + +template = ''' +heat_template_version: rocky +description: Test create OS::Neutron::ExtraRouteSet +resources: + extrarouteset0: + type: OS::Neutron::ExtraRouteSet + properties: + router: 88ce38c4-be8e-11e9-a0a5-5f64570eeec8 + routes: + - destination: 10.0.1.0/24 + nexthop: 10.0.0.11 + - destination: 10.0.2.0/24 + nexthop: 10.0.0.12 +''' + + +class NeutronExtraRouteSetTest(common.HeatTestCase): + + def setUp(self): + super(NeutronExtraRouteSetTest, self).setUp() + + self.patchobject( + neutron.NeutronClientPlugin, 'has_extension', return_value=True) + + self.add_extra_routes_mock = self.patchobject( + neutronclient.Client, 'add_extra_routes_to_router') + self.add_extra_routes_mock.return_value = { + 'router': { + 'id': '85b91046-be84-11e9-b518-2714ef1d76c3', + 'routes': [ + {'destination': '10.0.1.0/24', 'nexthop': '10.0.0.11'}, + {'destination': '10.0.2.0/24', 'nexthop': '10.0.0.12'}, + ], + } + } + + self.remove_extra_routes_mock = self.patchobject( + neutronclient.Client, 'remove_extra_routes_from_router') + self.remove_extra_routes_mock.return_value = { + 'router': { + 'id': '85b91046-be84-11e9-b518-2714ef1d76c3', + 'routes': [ + {'destination': '10.0.1.0/24', 'nexthop': '10.0.0.11'}, + {'destination': '10.0.2.0/24', 'nexthop': '10.0.0.12'}, + ], + } + } + + self.show_router_mock = self.patchobject( + neutronclient.Client, 'show_router') + self.show_router_mock.return_value = { + 'router': { + 'id': '85b91046-be84-11e9-b518-2714ef1d76c3', + 'routes': [], + } + } + + def find_resourceid_by_name_or_id( + _client, _resource, name_or_id, **_kwargs): + return name_or_id + + self.find_resource_mock = self.patchobject( + neutronV20, 'find_resourceid_by_name_or_id') + self.find_resource_mock.side_effect = find_resourceid_by_name_or_id + + def test_routes_to_set_to_routes(self): + routes = [{'destination': '10.0.1.0/24', 'nexthop': '10.0.0.11'}] + self.assertEqual( + routes, + extrarouteset._set_to_routes(extrarouteset._routes_to_set(routes)) + ) + + def test_diff_routes(self): + old = [ + {'destination': '10.0.1.0/24', 'nexthop': '10.0.0.11'}, + {'destination': '10.0.2.0/24', 'nexthop': '10.0.0.12'}, + ] + new = [ + {'destination': '10.0.1.0/24', 'nexthop': '10.0.0.11'}, + {'destination': '10.0.3.0/24', 'nexthop': '10.0.0.13'}, + ] + + add = extrarouteset._set_to_routes( + extrarouteset._routes_to_set(new) - + extrarouteset._routes_to_set(old)) + remove = extrarouteset._set_to_routes( + extrarouteset._routes_to_set(old) - + extrarouteset._routes_to_set(new)) + + self.assertEqual( + [{'destination': '10.0.3.0/24', 'nexthop': '10.0.0.13'}], add) + self.assertEqual( + [{'destination': '10.0.2.0/24', 'nexthop': '10.0.0.12'}], remove) + + def test__raise_if_duplicate_positive(self): + self.assertRaises( + exception.PhysicalResourceExists, + extrarouteset._raise_if_duplicate, + {'router': {'routes': [ + {'destination': 'dst1', 'nexthop': 'hop1'}, + ]}}, + [{'destination': 'dst1', 'nexthop': 'hop1'}], + ) + + def test__raise_if_duplicate_negative(self): + try: + extrarouteset._raise_if_duplicate( + {'router': {'routes': [ + {'destination': 'dst1', 'nexthop': 'hop1'}, + ]}}, + [{'destination': 'dst2', 'nexthop': 'hop2'}], + ) + except exception.PhysicalResourceExists: + self.fail('Unexpected exception in detecting duplicate routes') + + def test_create(self): + t = template_format.parse(template) + stack = utils.parse_stack(t) + + extra_routes = stack['extrarouteset0'] + scheduler.TaskRunner(extra_routes.create)() + + self.assertEqual( + (extra_routes.CREATE, extra_routes.COMPLETE), extra_routes.state) + self.add_extra_routes_mock.assert_called_once_with( + '88ce38c4-be8e-11e9-a0a5-5f64570eeec8', + {'router': { + 'routes': [ + {'destination': '10.0.1.0/24', 'nexthop': '10.0.0.11'}, + {'destination': '10.0.2.0/24', 'nexthop': '10.0.0.12'}, + ]}}) + + def test_delete_proper(self): + t = template_format.parse(template) + stack = utils.parse_stack(t) + + extra_routes = stack['extrarouteset0'] + scheduler.TaskRunner(extra_routes.create)() + scheduler.TaskRunner(extra_routes.delete)() + + self.assertEqual( + (extra_routes.DELETE, extra_routes.COMPLETE), extra_routes.state) + self.remove_extra_routes_mock.assert_called_once_with( + '88ce38c4-be8e-11e9-a0a5-5f64570eeec8', + {'router': { + 'routes': [ + {'destination': '10.0.1.0/24', 'nexthop': '10.0.0.11'}, + {'destination': '10.0.2.0/24', 'nexthop': '10.0.0.12'}, + ]}}) + + def test_delete_router_already_gone(self): + t = template_format.parse(template) + stack = utils.parse_stack(t) + + self.remove_extra_routes_mock.side_effect = ( + ncex.NeutronClientException(status_code=404)) + + extra_routes = stack['extrarouteset0'] + scheduler.TaskRunner(extra_routes.create)() + scheduler.TaskRunner(extra_routes.delete)() + + self.assertEqual( + (extra_routes.DELETE, extra_routes.COMPLETE), extra_routes.state) + self.remove_extra_routes_mock.assert_called_once_with( + '88ce38c4-be8e-11e9-a0a5-5f64570eeec8', + {'router': { + 'routes': [ + {'destination': '10.0.1.0/24', 'nexthop': '10.0.0.11'}, + {'destination': '10.0.2.0/24', 'nexthop': '10.0.0.12'}, + ]}}) + + def test_update(self): + t = template_format.parse(template) + stack = utils.parse_stack(t) + + extra_routes = stack['extrarouteset0'] + scheduler.TaskRunner(extra_routes.create)() + self.assertEqual( + (extra_routes.CREATE, extra_routes.COMPLETE), extra_routes.state) + + self.add_extra_routes_mock.reset_mock() + + rsrc_defn = stack.defn.resource_definition('extrarouteset0') + + props = copy.deepcopy(t['resources']['extrarouteset0']['properties']) + props['routes'][1] = { + 'destination': '10.0.3.0/24', 'nexthop': '10.0.0.13'} + rsrc_defn = rsrc_defn.freeze(properties=props) + + scheduler.TaskRunner(extra_routes.update, rsrc_defn)() + self.assertEqual( + (extra_routes.UPDATE, extra_routes.COMPLETE), extra_routes.state) + + self.remove_extra_routes_mock.assert_called_once_with( + '88ce38c4-be8e-11e9-a0a5-5f64570eeec8', + {'router': { + 'routes': [ + {'destination': '10.0.2.0/24', 'nexthop': '10.0.0.12'}, + ]}}) + self.add_extra_routes_mock.assert_called_once_with( + '88ce38c4-be8e-11e9-a0a5-5f64570eeec8', + {'router': { + 'routes': [ + {'destination': '10.0.3.0/24', 'nexthop': '10.0.0.13'}, + ]}}) diff --git a/lower-constraints.txt b/lower-constraints.txt index b29c57e726..a73946db9e 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -116,7 +116,7 @@ python-manilaclient==1.16.0 python-mimeparse==1.6.0 python-mistralclient==3.1.0 python-monascaclient==1.12.0 -python-neutronclient==6.7.0 +python-neutronclient==6.14.0 python-novaclient==9.1.0 python-octaviaclient==1.8.0 python-openstackclient==3.12.0 diff --git a/releasenotes/notes/neutron-extrarouteset-379c5354e1ac7795.yaml b/releasenotes/notes/neutron-extrarouteset-379c5354e1ac7795.yaml new file mode 100644 index 0000000000..cb2c8f54c7 --- /dev/null +++ b/releasenotes/notes/neutron-extrarouteset-379c5354e1ac7795.yaml @@ -0,0 +1,10 @@ +--- +features: + - | + New resource ``OS::Neutron::ExtraRouteSet`` is added to manage extra + routes of a Neutron router. +deprecations: + - | + Unsupported contrib resource ``OS::Neutron::ExtraRoute`` is deprecated + in favor of ``OS::Neutron::ExtraRouteSet`` on all OpenStack clouds where + Neutron extension ``extraroute-atomic`` is available. diff --git a/requirements.txt b/requirements.txt index a62119c609..05d431891a 100644 --- a/requirements.txt +++ b/requirements.txt @@ -43,7 +43,7 @@ python-magnumclient>=2.3.0 # Apache-2.0 python-manilaclient>=1.16.0 # Apache-2.0 python-mistralclient!=3.2.0,>=3.1.0 # Apache-2.0 python-monascaclient>=1.12.0 # Apache-2.0 -python-neutronclient>=6.7.0 # Apache-2.0 +python-neutronclient>=6.14.0 # Apache-2.0 python-novaclient>=9.1.0 # Apache-2.0 python-octaviaclient>=1.8.0 # Apache-2.0 python-openstackclient>=3.12.0 # Apache-2.0