openstack/heat
Code Issues Proposed changes

New resource OS::Neutron::ExtraRouteSet

Browse Source 
I hope I remembered all the discussion points we had about how to design
this plugin, so:

* Instead of changing OS::Neutron::ExtraRoute we introduce
  OS::Neutron::ExtraRouteSet so we can take advantage of Neutron API's
  ability to add/remove multiple extra routes at once.

* Addition and removal of extra routes is supposed to be atomic with
  Neutron extension 'extraroute-atomic'. An update involves a removal
  and an addition, therefore an update is not atomic operation. However
  unless the responsibility for an extra route is moved from one stack
  to another that should not be a problem.

* Sharing the responsibility for an extra route between stacks (that is
  multiple stacks defining the same extra route) is not supported due
  to the Neutron API not allowing this.

Let me know what did I forget.

Example template:

resources:
  extrarouteset0:
    type: OS::Neutron::ExtraRouteSet
    properties:
      router: { get_resource: router0 }
      routes:
        - destination: 10.0.0.0/24
          nexthop: 10.0.0.10
        - destination: 10.0.1.0/24
          nexthop: 10.0.0.11
  ...

Change-Id: Ic1fe593d9821d844fd124b0212d444f6e3a0015e
Depends-On: https://review.opendev.org/675900
Story: #2005522
Task: #36264
This commit is contained in:
Bence Romsics 2019-08-08 10:27:30 +02:00
parent 329570afe7
commit e0a69202d2
5 changed files with 485 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

236
heat/engine/resources/openstack/neutron/extrarouteset.py Normal file
View File

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

237
heat/tests/openstack/neutron/test_neutron_extrarouteset.py Normal file
View File

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

2
lower-constraints.txt
View File

@ -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.3.0
python-openstackclient==3.12.0

10
releasenotes/notes/neutron-extrarouteset-379c5354e1ac7795.yaml Normal file
View File

@ -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.

2
requirements.txt
View File

@ -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.3.0 # Apache-2.0
python-openstackclient>=3.12.0 # Apache-2.0