Merge pull request #44 from stackhpc/upstream/yoga-2023-05-08

Synchronise yoga with upstream

Author: mnasiadka

doc/source/admin/config-dhcp-ha.rst

@@ -442,6 +442,38 @@ To test the HA of DHCP agent:
 
 #. Start DHCP agent on HostB. The VM gets the wanted IP again.
 
+No HA for metadata service on isolated networks
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+All Neutron backends using the DHCP agent can also provide `metadata service
+<https://docs.openstack.org/nova/latest/user/metadata.html>`_ in isolated
+networks (i.e. networks without a router). In this case the DHCP agent manages
+the metadata service (see config option `enable_isolated_metadata
+<https://docs.openstack.org/neutron/latest/configuration/dhcp-agent.html#DEFAULT.enable_isolated_metadata>`_).
+
+Note however that the metadata service is only redundant for IPv4, and not
+IPv6, even when the DHCP service is configured to be highly available
+(config option `dhcp_agents_per_network
+<https://docs.openstack.org/neutron/latest/configuration/neutron.html#DEFAULT.dhcp_agents_per_network>`_
+> 1). This is because the DHCP agent will insert a route to the well known
+metadata IPv4 address (`169.254.169.254`) via its own IP address, so it will
+be reachable as long as the DHCP service is available at that IP address.
+This also means that recovery after a failure is tied to the renewal of the
+DHCP lease, since that route will only change if the DHCP server for a VM
+changes.
+
+With IPv6, the well known metadata IPv6 address (`fe80::a9fe:a9fe`) is used,
+but directly configured in the DHCP agent network namespace.
+Due to the enforcement of duplicate address detection (DAD), this address
+can only be configured in at most one DHCP network namespaces at any time.
+See `RFC 4862 <https://www.rfc-editor.org/rfc/rfc4862#section-5.4>`_ for
+details on the DAD process.
+
+For this reason, even when you have multiple DHCP agents, an arbitrary one
+(where the metadata IPv6 address is not in `dadfailed` state) will serve all
+metadata requests over IPv6. When that metadata service instance becomes
+unreachable there is no failover and the service will become unreachable.
+
 Disabling and removing an agent
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 

neutron/agent/linux/dhcp.py

@@ -40,6 +40,7 @@
 from neutron.agent.linux import ip_lib
 from neutron.agent.linux import iptables_manager
 from neutron.cmd import runtime_checks as checks
+from neutron.common import _constants as common_constants
 from neutron.common import utils as common_utils
 from neutron.ipam import utils as ipam_utils
 from neutron.privileged.agent.linux import dhcp as priv_dhcp
@@ -1773,7 +1774,7 @@ def setup(self, network):
         if self.conf.force_metadata or self.conf.enable_isolated_metadata:
             ip_cidrs.append(constants.METADATA_CIDR)
             if netutils.is_ipv6_enabled():
-                ip_cidrs.append(constants.METADATA_V6_CIDR)
+                ip_cidrs.append(common_constants.METADATA_V6_CIDR)
 
         self.driver.init_l3(interface_name, ip_cidrs,
                             namespace=network.namespace)

neutron/agent/linux/ip_lib.py

@@ -103,6 +103,10 @@ class AddressNotReady(exceptions.NeutronException):
                 "become ready: %(reason)s")
 
 
+class DADFailed(AddressNotReady):
+    pass
+
+
 InvalidArgument = privileged.InvalidArgument
 
 
@@ -581,7 +585,7 @@ def wait_until_address_ready(self, address, wait_time=30):
         """Wait until an address is no longer marked 'tentative' or 'dadfailed'
 
         raises AddressNotReady if times out, address not present on interface
-               or DAD fails
+        raises DADFailed if Duplicate Address Detection fails
         """
         def is_address_ready():
             try:
@@ -593,7 +597,7 @@ def is_address_ready():
             # Since both 'dadfailed' and 'tentative' will be set if DAD fails,
             # check 'dadfailed' first just to be explicit
             if addr_info['dadfailed']:
-                raise AddressNotReady(
+                raise DADFailed(
                     address=address, reason=_('Duplicate address detected'))
             if addr_info['tentative']:
                 return False

neutron/agent/metadata/driver.py

@@ -33,6 +33,7 @@
 from neutron.agent.linux import external_process
 from neutron.agent.linux import ip_lib
 from neutron.agent.linux import utils as linux_utils
+from neutron.common import _constants as common_constants
 from neutron.common import coordination
 from neutron.common import utils as common_utils
 
@@ -266,9 +267,30 @@ def spawn_monitored_metadata_proxy(cls, monitor, ns_name, port, conf,
             # HAProxy cannot bind() until IPv6 Duplicate Address Detection
             # completes. We must wait until the address leaves its 'tentative'
             # state.
-            ip_lib.IpAddrCommand(
-                parent=ip_lib.IPDevice(name=bind_interface, namespace=ns_name)
-            ).wait_until_address_ready(address=bind_address_v6)
+            try:
+                ip_lib.IpAddrCommand(
+                    parent=ip_lib.IPDevice(name=bind_interface,
+                                           namespace=ns_name)
+                ).wait_until_address_ready(address=bind_address_v6)
+            except ip_lib.DADFailed as exc:
+                # This failure means that another DHCP agent has already
+                # configured this metadata address, so all requests will
+                # be via that single agent.
+                LOG.info('DAD failed for address %(address)s on interface '
+                         '%(interface)s in namespace %(namespace)s on network '
+                         '%(network)s, deleting it. Exception: %(exception)s',
+                         {'address': bind_address_v6,
+                          'interface': bind_interface,
+                          'namespace': ns_name,
+                          'network': network_id,
+                          'exception': str(exc)})
+                try:
+                    ip_lib.delete_ip_address(bind_address_v6, bind_interface,
+                                             namespace=ns_name)
+                except Exception as exc:
+                    # do not re-raise a delete failure, just log
+                    LOG.info('Address deletion failure: %s', str(exc))
+                return
         pm.enable()
         monitor.register(uuid, METADATA_SERVICE_NAME, pm)
         cls.monitors[router_id] = pm
@@ -363,6 +385,6 @@ def apply_metadata_nat_rules(router, proxy):
     if netutils.is_ipv6_enabled():
         for c, r in proxy.metadata_nat_rules(
                 proxy.metadata_port,
-                metadata_address=(constants.METADATA_V6_IP + '/128')):
+                metadata_address=(common_constants.METADATA_V6_CIDR)):
             router.iptables_manager.ipv6['nat'].add_rule(c, r)
     router.iptables_manager.apply()

neutron/common/_constants.py

@@ -81,3 +81,6 @@
 
 # The lowest binding index for L3 agents and DHCP agents.
 LOWEST_AGENT_BINDING_INDEX = 1
+
+# Neutron-lib defines this with a /64 but it should be /128
+METADATA_V6_CIDR = constants.METADATA_V6_IP + '/128'

neutron/conf/agent/database/agentschedulers_db.py

@@ -32,7 +32,9 @@
                       'network. If this number is greater than 1, the '
                       'scheduler automatically assigns multiple DHCP agents '
                       'for a given tenant network, providing high '
-                      'availability for DHCP service.')),
+                      'availability for the DHCP service. However this does '
+                      'not provide high availability for the IPv6 metadata '
+                      'service in isolated networks.')),
     cfg.BoolOpt('enable_services_on_agents_with_admin_state_down',
                 default=False,
                 help=_('Enable services on an agent with admin_state_up '

neutron/db/securitygroups_db.py

@@ -253,6 +253,12 @@ def delete_security_group(self, context, id):
             if sg['name'] == 'default' and not context.is_admin:
                 raise ext_sg.SecurityGroupCannotRemoveDefault()
 
+            # Check if there are rules with remote_group_id ponting to
+            # the security_group to be deleted
+            rules_ids_as_remote = self._get_security_group_rules_by_remote(
+                context=context, remote_id=id,
+            )
+
         self._registry_publish(resources.SECURITY_GROUP,
                                events.BEFORE_DELETE,
                                exc_cls=ext_sg.SecurityGroupInUse, id=id,
@@ -285,6 +291,20 @@ def delete_security_group(self, context, id):
                              context, resource_id=id, states=(sec_group,),
                              metadata={'security_group_rule_ids': sgr_ids,
                                        'name': sg['name']}))
+        for rule in rules_ids_as_remote:
+            registry.publish(
+                resources.SECURITY_GROUP_RULE,
+                events.AFTER_DELETE,
+                self,
+                payload=events.DBEventPayload(
+                    context,
+                    resource_id=rule['id'],
+                    metadata={'security_group_id': rule['security_group_id'],
+                              'remote_group_id': rule['remote_group_id'],
+                              'rule': rule
+                              }
+                )
+            )
 
     @db_api.retry_if_session_inactive()
     def update_security_group(self, context, id, security_group):
@@ -371,6 +391,23 @@ def _get_port_security_group_bindings(self, context,
             self._make_security_group_binding_dict,
             filters=filters, fields=fields)
 
+    def _get_security_group_rules_by_remote(self, context, remote_id):
+        return model_query.get_collection(
+            context, sg_models.SecurityGroupRule,
+            self._make_security_group_rule_dict,
+            filters={'remote_group_id': [remote_id]},
+            fields=['id',
+                    'remote_group_id',
+                    'security_group_id',
+                    'direction',
+                    'ethertype',
+                    'protocol',
+                    'port_range_min',
+                    'port_range_max',
+                    'normalized_cidr'
+                    ]
+        )
+
     @db_api.retry_if_session_inactive()
     def _delete_port_security_group_bindings(self, context, port_id):
         with db_api.CONTEXT_WRITER.using(context):

neutron/objects/db/api.py

@@ -13,6 +13,7 @@
 # TODO(ihrachys): cover the module with functional tests targeting supported
 # backends
 
+from neutron_lib.db import api as db_api
 from neutron_lib.db import model_query
 from neutron_lib import exceptions as n_exc
 from neutron_lib.objects import utils as obj_utils
@@ -31,16 +32,18 @@ def _get_filter_query(obj_cls, context, query_field=None, query_limit=None,
 
 
 def get_object(obj_cls, context, **kwargs):
-    return _get_filter_query(obj_cls, context, **kwargs).first()
+    with db_api.CONTEXT_READER.using(context):
+        return _get_filter_query(obj_cls, context, **kwargs).first()
 
 
 def count(obj_cls, context, query_field=None, query_limit=None, **kwargs):
-    if not query_field and obj_cls.primary_keys:
-        query_field = obj_cls.primary_keys[0]
-        if query_field in obj_cls.fields_need_translation:
-            query_field = obj_cls.fields_need_translation[query_field]
-    return _get_filter_query(obj_cls, context, query_field=query_field,
-                             query_limit=query_limit, **kwargs).count()
+    with db_api.CONTEXT_READER.using(context):
+        if not query_field and obj_cls.primary_keys:
+            query_field = obj_cls.primary_keys[0]
+            if query_field in obj_cls.fields_need_translation:
+                query_field = obj_cls.fields_need_translation[query_field]
+        return _get_filter_query(obj_cls, context, query_field=query_field,
+                                 query_limit=query_limit, **kwargs).count()
 
 
 def _kwargs_to_filters(**kwargs):

neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py

@@ -253,9 +253,6 @@ def subscribe(self):
             registry.subscribe(self._create_security_group,
                                resources.SECURITY_GROUP,
                                events.AFTER_CREATE)
-            registry.subscribe(self._delete_security_group_precommit,
-                               resources.SECURITY_GROUP,
-                               events.PRECOMMIT_DELETE)
             registry.subscribe(self._delete_security_group,
                                resources.SECURITY_GROUP,
                                events.AFTER_DELETE)
@@ -268,6 +265,9 @@ def subscribe(self):
             registry.subscribe(self._process_sg_rule_notification,
                                resources.SECURITY_GROUP_RULE,
                                events.BEFORE_DELETE)
+            registry.subscribe(self._process_sg_rule_after_del_notification,
+                               resources.SECURITY_GROUP_RULE,
+                               events.AFTER_DELETE)
 
     def _clean_hash_ring(self, *args, **kwargs):
         admin_context = n_context.get_admin_context()
@@ -384,14 +384,6 @@ def _create_security_group(self, resource, event, trigger, payload):
         self._ovn_client.create_security_group(context,
                                                security_group)
 
-    def _delete_security_group_precommit(self, resource, event, trigger,
-                                         payload):
-        context = n_context.get_admin_context()
-        security_group_id = payload.resource_id
-        for sg_rule in self._plugin.get_security_group_rules(
-                context, filters={'remote_group_id': [security_group_id]}):
-            self._ovn_client.delete_security_group_rule(context, sg_rule)
-
     def _delete_security_group(self, resource, event, trigger, payload):
         context = payload.context
         security_group_id = payload.resource_id
@@ -449,6 +441,12 @@ def _process_sg_rule_notification(
                 context,
                 sg_rule)
 
+    def _process_sg_rule_after_del_notification(
+            self, resource, event, trigger, payload):
+        context = payload.context
+        sg_rule = payload.metadata['rule']
+        self._ovn_client.delete_security_group_rule(context, sg_rule)
+
     def _sg_has_rules_with_same_normalized_cidr(self, sg_rule):
         compare_keys = [
             'ethertype', 'direction', 'protocol',

neutron/services/qos/qos_plugin.py

@@ -366,7 +366,7 @@ def _extend_port_resource_request_bulk(ports_res, noop):
     def _get_ports_with_policy(self, context, policy):
         networks_ids = policy.get_bound_networks()
         ports_with_net_policy = ports_object.Port.get_objects(
-            context, network_id=networks_ids)
+            context, network_id=networks_ids) if networks_ids else []
 
         # Filter only this ports which don't have overwritten policy
         ports_with_net_policy = [
@@ -376,7 +376,7 @@ def _get_ports_with_policy(self, context, policy):
 
         ports_ids = policy.get_bound_ports()
         ports_with_policy = ports_object.Port.get_objects(
-            context, id=ports_ids)
+            context, id=ports_ids) if ports_ids else []
         return list(set(ports_with_policy + ports_with_net_policy))
 
     def _validate_create_port_callback(self, resource, event, trigger,