From 901df0e1f17bd552f388898787958cbc15fc9e1e Mon Sep 17 00:00:00 2001 From: Frode Nordahl Date: Tue, 1 Oct 2019 07:54:37 +0200 Subject: [PATCH] Use hostname from ovs relation when available When creating a Neutron port the value of ``binding:host_id`` needs to match the host identity of the Neutron OpenvSwitch subordinate. Depends-On: I3b11eec3f1c4b8a673ccb6f9e6320d79dbde8f7a Change-Id: I6bec4c5a5dade1295414ff8eefc070e2e8127f37 Closes-Bug: #1845303 --- src/lib/charm/openstack/api_crud.py | 91 ++++++++++++------- src/reactive/octavia_handlers.py | 4 +- .../test_lib_charm_openstack_api_crud.py | 22 ++++- unit_tests/test_octavia_handlers.py | 6 +- 4 files changed, 86 insertions(+), 37 deletions(-) diff --git a/src/lib/charm/openstack/api_crud.py b/src/lib/charm/openstack/api_crud.py index 1daabe12..9e1ad0d7 100644 --- a/src/lib/charm/openstack/api_crud.py +++ b/src/lib/charm/openstack/api_crud.py @@ -138,7 +138,8 @@ def get_nova_flavor(identity_service): raise APIUnavailable('nova', 'flavors', e) -def get_hm_port(identity_service, local_unit_name, local_unit_address): +def get_hm_port(identity_service, local_unit_name, local_unit_address, + ovs_hostname=None): """Get or create a per unit Neutron port for Octavia Health Manager. A side effect of calling this function is that a port is created if one @@ -151,6 +152,8 @@ def get_hm_port(identity_service, local_unit_name, local_unit_address): :param local_unit_address: DNS resolvable IP address of unit, used to build Neutron port ``binding:host_id`` :type local_unit_address: str + :param ovs_hostname: Hostname used by neutron-openvswitch + :type ovs_hostname: Option[None,str] :returns: Port details extracted from result of call to neutron_client.list_ports or neutron_client.create_port :rtype: dict @@ -200,44 +203,63 @@ def get_hm_port(identity_service, local_unit_name, local_unit_address): except NEUTRON_TEMP_EXCS as e: raise APIUnavailable('neutron', 'ports', e) + port_template = { + 'port': { + # avoid race with OVS agent attempting to bind port + # before it is created in the local units OVSDB + 'admin_state_up': False, + 'binding:host_id': (ovs_hostname or + socket.gethostname()), + # NOTE(fnordahl): device_owner has special meaning + # for Neutron [0], and things may break if set to + # an arbritary value. Using a value known by Neutron + # is_dvr_serviced() function [1] gets us the correct + # rules appiled to the port to allow IPv6 Router + # Advertisement packets through LP: #1813931 + # 0: https://github.com/openstack/neutron/blob/ + # 916347b996684c82b29570cd2962df3ea57d4b16/ + # neutron/plugins/ml2/drivers/openvswitch/ + # agent/ovs_dvr_neutron_agent.py#L592 + # 1: https://github.com/openstack/neutron/blob/ + # 50308c03c960bd6e566f328a790b8e05f5e92ead/ + # neutron/common/utils.py#L200 + 'device_owner': ( + neutron_lib.constants.DEVICE_OWNER_LOADBALANCERV2), + 'security_groups': [ + health_secgrp['id'], + ], + 'name': 'octavia-health-manager-{}-listen-port' + .format(local_unit_name), + 'network_id': network['id'], + }, + } n_resp = len(resp.get('ports', [])) if n_resp == 1: hm_port = resp['ports'][0] + # Ensure binding:host_id is up to date on a existing port + # + # In the event of a need to update it, we bring the port down to make + # sure Neutron rebuilds the port correctly. + # + # Our caller, ``setup_hm_port``, will toggle the port admin status. + if hm_port and hm_port.get( + 'binding:host_id') != port_template['port']['binding:host_id']: + try: + nc.update_port(hm_port['id'], { + 'port': { + 'admin_state_up': False, + 'binding:host_id': port_template['port'][ + 'binding:host_id'], + } + }) + except NEUTRON_TEMP_EXCS as e: + raise APIUnavailable('neutron', 'ports', e) elif n_resp > 1: raise DuplicateResource('neutron', 'ports', data=resp) else: # create new port try: - resp = nc.create_port( - { - 'port': { - # avoid race with OVS agent attempting to bind port - # before it is created in the local units OVSDB - 'admin_state_up': False, - 'binding:host_id': socket.gethostname(), - # NOTE(fnordahl): device_owner has special meaning - # for Neutron [0], and things may break if set to - # an arbritary value. Using a value known by Neutron - # is_dvr_serviced() function [1] gets us the correct - # rules appiled to the port to allow IPv6 Router - # Advertisement packets through LP: #1813931 - # 0: https://github.com/openstack/neutron/blob/ - # 916347b996684c82b29570cd2962df3ea57d4b16/ - # neutron/plugins/ml2/drivers/openvswitch/ - # agent/ovs_dvr_neutron_agent.py#L592 - # 1: https://github.com/openstack/neutron/blob/ - # 50308c03c960bd6e566f328a790b8e05f5e92ead/ - # neutron/common/utils.py#L200 - 'device_owner': ( - neutron_lib.constants.DEVICE_OWNER_LOADBALANCERV2), - 'security_groups': [ - health_secgrp['id'], - ], - 'name': 'octavia-health-manager-{}-listen-port' - .format(local_unit_name), - 'network_id': network['id'], - }, - }) + resp = nc.create_port(port_template) hm_port = resp['port'] ch_core.hookenv.log('Created port {}'.format(hm_port['id']), ch_core.hookenv.INFO) @@ -275,7 +297,7 @@ def toggle_hm_port(identity_service, local_unit_name, enabled=True): nc.update_port(port['id'], {'port': {'admin_state_up': enabled}}) -def setup_hm_port(identity_service, octavia_charm): +def setup_hm_port(identity_service, octavia_charm, ovs_hostname=None): """Create a per unit Neutron and OVS port for Octavia Health Manager. This is used to plug the unit into the overlay network for direct @@ -286,6 +308,8 @@ def setup_hm_port(identity_service, octavia_charm): :type identity_service: RelationBase class :param ocataiva_charm: charm instance :type octavia_charm: OctaviaCharm class instance + :param ovs_hostname: Hostname used by neutron-openvswitch + :type ovs_hostname: Option[None,str] :retruns: True on change to local unit, False otherwise :rtype: bool :raises: api_crud.APIUnavailable, api_crud.DuplicateResource @@ -294,7 +318,8 @@ def setup_hm_port(identity_service, octavia_charm): hm_port = get_hm_port( identity_service, octavia_charm.local_unit_name, - octavia_charm.local_address) + octavia_charm.local_address, + ovs_hostname=ovs_hostname) if not hm_port: ch_core.hookenv.log('No network tagged with `charm-octavia` ' 'exists, deferring port setup awaiting ' diff --git a/src/reactive/octavia_handlers.py b/src/reactive/octavia_handlers.py index dfef8b57..6712a089 100644 --- a/src/reactive/octavia_handlers.py +++ b/src/reactive/octavia_handlers.py @@ -100,13 +100,15 @@ def setup_hm_port(): communication with the octavia managed load balancer instances running within the deployed cloud. """ + ovs = reactive.endpoint_from_flag('neutron-openvswitch.connected') with charm.provide_charm_instance() as octavia_charm: identity_service = reactive.endpoint_from_flag( 'identity-service.available') try: if api_crud.setup_hm_port( identity_service, - octavia_charm): + octavia_charm, + ovs_hostname=ovs.host()): # trigger config render to make systemd-networkd bring up # automatic IP configuration of the new port right now. reactive.set_flag('config.changed') diff --git a/unit_tests/test_lib_charm_openstack_api_crud.py b/unit_tests/test_lib_charm_openstack_api_crud.py index 44da21b1..42e88882 100644 --- a/unit_tests/test_lib_charm_openstack_api_crud.py +++ b/unit_tests/test_lib_charm_openstack_api_crud.py @@ -169,6 +169,25 @@ class TestAPICrud(test_utils.PatchHelper): nc.add_tag.assert_called_with('ports', port_uuid, 'charm-octavia') self.assertEqual(result, {'id': 'fake-port-uuid', 'mac_address': 'fake-mac-address'}) + nc.create_port.reset_mock() + result = api_crud.get_hm_port(identity_service, + 'fake-unit-name', + '192.0.2.42', + ovs_hostname='fake-unit-name.fqdn') + nc.create_port.assert_called_once_with( + { + 'port': { + 'admin_state_up': False, + 'binding:host_id': 'fake-unit-name.fqdn', + 'device_owner': 'fakeowner', + 'security_groups': ['fake-secgrp-uuid'], + 'name': 'octavia-health-manager-' + 'fake-unit-name-listen-port', + 'network_id': 'fake-network-uuid', + }, + }) + self.assertEqual(result, {'id': 'fake-port-uuid', + 'mac_address': 'fake-mac-address'}) def test_toggle_hm_port(self): self.patch_object(api_crud, 'neutron_client') @@ -204,7 +223,8 @@ class TestAPICrud(test_utils.PatchHelper): self.get_hm_port.assert_called_with( identity_service, octavia_charm.local_unit_name, - octavia_charm.local_address) + octavia_charm.local_address, + ovs_hostname=None) self.check_output.assert_called_with( ['ip', 'link', 'show', api_crud.octavia.OCTAVIA_MGMT_INTF], stderr=-2, universal_newlines=True) diff --git a/unit_tests/test_octavia_handlers.py b/unit_tests/test_octavia_handlers.py index 38ed06d8..327ea14e 100644 --- a/unit_tests/test_octavia_handlers.py +++ b/unit_tests/test_octavia_handlers.py @@ -122,8 +122,10 @@ class TestOctaviaHandlers(test_utils.PatchHelper): self.patch('charms.reactive.set_flag', 'set_flag') self.patch_object(handlers.api_crud, 'setup_hm_port') handlers.setup_hm_port() - self.setup_hm_port.assert_called_with(self.endpoint_from_flag(), - self.octavia_charm) + self.setup_hm_port.assert_called_with( + self.endpoint_from_flag(), + self.octavia_charm, + ovs_hostname=self.endpoint_from_flag().host()) self.set_flag.assert_called_once_with('config.changed') def test_update_controller_ip_port_list(self):