Merge "New resource OS::Neutron::ExtraRouteSet"

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,
+    }

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'},
+                ]}})

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

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.

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