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

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

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')

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)

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):