From 0674236eedd76ae06f168db0da2b249d3ba756b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Piliszek?= Date: Tue, 19 May 2020 19:49:14 +0200 Subject: [PATCH] Fix kolla_address in IPv6 fully-routed topo case This includes some lightweight refactoring to avoid code duplication. This patch is made to be backportable to Train. We now include Ansible in testing since Ussuri so the comments about the bool filter are wrong. Change-Id: Ia2e0f7f24988763bacfeafefb7977021f5949f4e Closes-bug: #1848941 --- kolla_ansible/filters.py | 11 +- kolla_ansible/helpers.py | 21 ++++ kolla_ansible/kolla_address.py | 30 ++++- kolla_ansible/tests/unit/helpers.py | 23 ++++ .../tests/unit/test_address_filters.py | 111 ++++++++++++++++++ kolla_ansible/tests/unit/test_filters.py | 12 +- .../notes/bug-1848941-7e192be1885af513.yaml | 6 + 7 files changed, 187 insertions(+), 27 deletions(-) create mode 100644 kolla_ansible/helpers.py create mode 100644 kolla_ansible/tests/unit/helpers.py create mode 100644 releasenotes/notes/bug-1848941-7e192be1885af513.yaml diff --git a/kolla_ansible/filters.py b/kolla_ansible/filters.py index 10bed9e46d..94c4ae58a5 100644 --- a/kolla_ansible/filters.py +++ b/kolla_ansible/filters.py @@ -15,16 +15,7 @@ import jinja2 from kolla_ansible import exception - - -def _call_bool_filter(context, value): - """Pass a value through the 'bool' filter. - - :param context: Jinja2 Context object. - :param value: Value to pass through bool filter. - :returns: A boolean. - """ - return context.environment.call_filter("bool", value, context=context) +from kolla_ansible.helpers import _call_bool_filter @jinja2.contextfilter diff --git a/kolla_ansible/helpers.py b/kolla_ansible/helpers.py new file mode 100644 index 0000000000..d6b3d3b7a2 --- /dev/null +++ b/kolla_ansible/helpers.py @@ -0,0 +1,21 @@ +# 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. + + +def _call_bool_filter(context, value): + """Pass a value through the 'bool' filter. + + :param context: Jinja2 Context object. + :param value: Value to pass through bool filter. + :returns: A boolean. + """ + return context.environment.call_filter("bool", value, context=context) diff --git a/kolla_ansible/kolla_address.py b/kolla_ansible/kolla_address.py index c1082973e3..888ee734c2 100644 --- a/kolla_ansible/kolla_address.py +++ b/kolla_ansible/kolla_address.py @@ -18,6 +18,7 @@ from jinja2.filters import contextfilter from jinja2.runtime import Undefined from kolla_ansible.exception import FilterError +from kolla_ansible.helpers import _call_bool_filter @contextfilter @@ -50,14 +51,15 @@ def kolla_address(context, network_name, hostname=None): if isinstance(hostvars, Undefined): raise FilterError("'hostvars' variable is unavailable") - del context # remove for sanity - host = hostvars.get(hostname) if isinstance(host, Undefined): raise FilterError("'{hostname}' not in 'hostvars'" .format(hostname=hostname)) - del hostvars # remove for sanity (no 'Undefined' beyond this point) + del hostvars # remove for clarity (no need for other hosts) + + # NOTE(yoctozepto): variable "host" will *not* return Undefined + # same applies to all its children (act like plain dictionary) interface_name = host.get(network_name + '_interface') if interface_name is None: @@ -101,11 +103,27 @@ def kolla_address(context, network_name, hostname=None): address = af_interface.get('address') elif address_family == 'ipv6': # ipv6 has no concept of a secondary address - # prefix 128 is the default from keepalived - # it needs to be excluded here + # explicitly exclude the vip addresses + # to avoid excluding all /128 + + haproxy_enabled = host.get('enable_haproxy') + if haproxy_enabled is None: + raise FilterError("'enable_haproxy' variable is unavailable") + haproxy_enabled = _call_bool_filter(context, haproxy_enabled) + + if haproxy_enabled: + vip_addresses = [ + host.get('kolla_internal_vip_address'), + host.get('kolla_external_vip_address'), + ] + else: + # no addresses are virtual (kolla-wise) + vip_addresses = [] + global_ipv6_addresses = [x for x in af_interface if x['scope'] == 'global' and - x['prefix'] != '128'] + x['address'] not in vip_addresses] + if global_ipv6_addresses: address = global_ipv6_addresses[0]['address'] else: diff --git a/kolla_ansible/tests/unit/helpers.py b/kolla_ansible/tests/unit/helpers.py new file mode 100644 index 0000000000..5854fb976d --- /dev/null +++ b/kolla_ansible/tests/unit/helpers.py @@ -0,0 +1,23 @@ +# 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. + + +def _to_bool(value): + """Simplified version of the bool filter. + + Avoids having a dependency on Ansible in unit tests. + """ + if value == 'yes': + return True + if value == 'no': + return False + return bool(value) diff --git a/kolla_ansible/tests/unit/test_address_filters.py b/kolla_ansible/tests/unit/test_address_filters.py index de2aadfe65..43bfc568a8 100644 --- a/kolla_ansible/tests/unit/test_address_filters.py +++ b/kolla_ansible/tests/unit/test_address_filters.py @@ -22,6 +22,8 @@ from kolla_ansible.exception import FilterError from kolla_ansible.kolla_address import kolla_address from kolla_ansible.put_address_in_context import put_address_in_context +from kolla_ansible.tests.unit.helpers import _to_bool + class TestAddressContextFilter(unittest.TestCase): @@ -58,6 +60,7 @@ class TestKollaAddressFilter(unittest.TestCase): def setUp(self): # Bandit complains about Jinja2 autoescaping without nosec. self.env = jinja2.Environment() # nosec + self.env.filters['bool'] = _to_bool def _make_context(self, parent): return self.env.context_class( @@ -97,6 +100,7 @@ class TestKollaAddressFilter(unittest.TestCase): 'inventory_hostname': 'primary', 'hostvars': { 'primary': { + 'enable_haproxy': 'yes', 'api_address_family': 'ipv6', 'api_interface': 'fake-interface', 'ansible_fake_interface': { @@ -133,6 +137,7 @@ class TestKollaAddressFilter(unittest.TestCase): 'inventory_hostname': 'primary', 'hostvars': { 'primary': { + 'enable_haproxy': 'yes', 'api_address_family': 'ipv6', 'api_interface': 'fake-interface', 'ansible_fake_interface': { @@ -185,3 +190,109 @@ class TestKollaAddressFilter(unittest.TestCase): }, }) self.assertRaises(FilterError, kolla_address, context, 'api') + + def test_valid_ipv6_config_prefix_128(self): + addr = 'fd::' + context = self._make_context({ + 'inventory_hostname': 'primary', + 'hostvars': { + 'primary': { + 'enable_haproxy': 'yes', + 'api_address_family': 'ipv6', + 'api_interface': 'fake-interface', + 'ansible_fake_interface': { + 'ipv6': [ + { + 'address': addr, + 'scope': 'global', + 'prefix': 128, + }, + ], + }, + }, + }, + }) + self.assertEqual(addr, kolla_address(context, 'api')) + + def test_valid_ipv6_config_ignore_internal_vip_address(self): + addr = 'fd::' + context = self._make_context({ + 'inventory_hostname': 'primary', + 'hostvars': { + 'primary': { + 'enable_haproxy': 'yes', + 'kolla_internal_vip_address': addr + '1', + 'api_address_family': 'ipv6', + 'api_interface': 'fake-interface', + 'ansible_fake_interface': { + 'ipv6': [ + { + 'address': addr + '1', + 'scope': 'global', + 'prefix': 128, + }, + { + 'address': addr, + 'scope': 'global', + 'prefix': 128, + }, + ], + }, + }, + }, + }) + self.assertEqual(addr, kolla_address(context, 'api')) + + def test_valid_ipv6_config_ignore_external_vip_address(self): + addr = 'fd::' + context = self._make_context({ + 'inventory_hostname': 'primary', + 'hostvars': { + 'primary': { + 'enable_haproxy': 'yes', + 'kolla_external_vip_address': addr + '1', + 'api_address_family': 'ipv6', + 'api_interface': 'fake-interface', + 'ansible_fake_interface': { + 'ipv6': [ + { + 'address': addr + '1', + 'scope': 'global', + 'prefix': 128, + }, + { + 'address': addr, + 'scope': 'global', + 'prefix': 128, + }, + ], + }, + }, + }, + }) + self.assertEqual(addr, kolla_address(context, 'api')) + + def test_valid_ipv6_config_do_not_ignore_any_vip_address(self): + addr = 'fd::' + context = self._make_context({ + 'inventory_hostname': 'primary', + 'hostvars': { + 'primary': { + 'enable_haproxy': 'no', + 'kolla_external_vip_address': addr, + 'kolla_internal_vip_address': addr, + 'api_address_family': 'ipv6', + 'api_interface': 'fake-interface', + 'ansible_fake_interface': { + 'ipv6': [ + { + 'address': addr, + 'scope': 'global', + 'prefix': 128, + }, + ], + }, + }, + }, + }) + self.assertEqual(addr, kolla_address(context, 'api')) diff --git a/kolla_ansible/tests/unit/test_filters.py b/kolla_ansible/tests/unit/test_filters.py index e35f503d97..859cfd7aae 100644 --- a/kolla_ansible/tests/unit/test_filters.py +++ b/kolla_ansible/tests/unit/test_filters.py @@ -20,17 +20,7 @@ import jinja2 from kolla_ansible import exception from kolla_ansible import filters - -def _to_bool(value): - """Simplified version of the bool filter. - - Avoids having a dependency on Ansible in unit tests. - """ - if value == 'yes': - return True - if value == 'no': - return False - return bool(value) +from kolla_ansible.tests.unit.helpers import _to_bool class TestFilters(unittest.TestCase): diff --git a/releasenotes/notes/bug-1848941-7e192be1885af513.yaml b/releasenotes/notes/bug-1848941-7e192be1885af513.yaml new file mode 100644 index 0000000000..4b33368034 --- /dev/null +++ b/releasenotes/notes/bug-1848941-7e192be1885af513.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + IPv6 fully-routed topology (/128 addressing) is now allowed + (where applicable). + `LP#1848941 `__