Skip to content

Commit

Permalink
Since OVN 20.06, config is stored in "Chassis.other_config"
Browse files Browse the repository at this point in the history
Since OVN 20.06 [1], the OVN configuration is stored in
"Chassis.other_config".

Since OVN 22.09, the "Chassis" configuration stored in
"Chassis.other_config" will not be replicated to
"Chassis.external_ids".

The ML2/OVN plugin tries to retrieve the "Chassis"
configuration from the "other_config" field first; if this
field does not exist (in OVN versions before 20.06), the plugin
will use "external_ids" field instead. Neutron will be compatible
with the different OVN versions (with and without "other_config"
field).

[1]ovn-org/ovn@74d90c2
[2]ovn-org/ovn@5130942

NOTE: this patch is similar to [1], but in this case neutron keeps
compatibility with the different OVN versions (with and without
"other_config" field). Since [2], the Neutron CI has a new job that
uses the OVN/OVS packages distributed by the operating system
installed by the CI (in this case, Ubuntu 20.04 and OVN 20.03).

[1]https://review.opendev.org/c/openstack/neutron/+/859642
[2]https://review.opendev.org/c/openstack/neutron/+/860636

Closes-Bug: #1990229
Change-Id: I54c8fd4d065ae537f396408df16832b158ee8998
  • Loading branch information
ralonsoh committed Oct 4, 2022
1 parent 98d76ec commit 536498a
Show file tree
Hide file tree
Showing 19 changed files with 156 additions and 103 deletions.
12 changes: 10 additions & 2 deletions neutron/common/ovn/utils.py
Expand Up @@ -641,8 +641,8 @@ def compute_address_pairs_diff(ovn_port, neutron_port):

def get_ovn_cms_options(chassis):
"""Return the list of CMS options in a Chassis."""
return [opt.strip() for opt in chassis.external_ids.get(
constants.OVN_CMS_OPTIONS, '').split(',')]
return [opt.strip() for opt in get_ovn_chassis_other_config(chassis).get(
constants.OVN_CMS_OPTIONS, '').split(',')]


def is_gateway_chassis(chassis):
Expand Down Expand Up @@ -831,3 +831,11 @@ def create_neutron_pg_drop():
}]

OvsdbClientTransactCommand.run(command)


def get_ovn_chassis_other_config(chassis):
# NOTE(ralonsoh): LP#1990229 to be removed when min OVN version is 22.09
try:
return chassis.other_config
except AttributeError:
return chassis.external_ids
19 changes: 11 additions & 8 deletions neutron/plugins/ml2/drivers/ovn/agent/neutron_agent.py
Expand Up @@ -89,7 +89,8 @@ def as_dict(self):
'configurations': {
'chassis_name': self.chassis.name,
'bridge-mappings':
self.chassis.external_ids.get('ovn-bridge-mappings', '')},
ovn_utils.get_ovn_chassis_other_config(self.chassis).get(
'ovn-bridge-mappings', '')},
'start_flag': True,
'agent_type': self.agent_type,
'id': self.agent_id,
Expand Down Expand Up @@ -141,9 +142,9 @@ class ControllerAgent(NeutronAgent):

@staticmethod # it is by default, but this makes pep8 happy
def __new__(cls, chassis_private, driver):
external_ids = cls.chassis_from_private(chassis_private).external_ids
if ('enable-chassis-as-gw' in
external_ids.get('ovn-cms-options', [])):
_chassis = cls.chassis_from_private(chassis_private)
other_config = ovn_utils.get_ovn_chassis_other_config(_chassis)
if 'enable-chassis-as-gw' in other_config.get('ovn-cms-options', []):
cls = ControllerGatewayAgent
return super().__new__(cls)

Expand All @@ -166,8 +167,9 @@ def description(self):

def update(self, chassis_private, clear_down=False):
super().update(chassis_private, clear_down)
external_ids = self.chassis_from_private(chassis_private).external_ids
if 'enable-chassis-as-gw' in external_ids.get('ovn-cms-options', []):
_chassis = self.chassis_from_private(chassis_private)
other_config = ovn_utils.get_ovn_chassis_other_config(_chassis)
if 'enable-chassis-as-gw' in other_config.get('ovn-cms-options', []):
self.__class__ = ControllerGatewayAgent


Expand All @@ -176,9 +178,10 @@ class ControllerGatewayAgent(ControllerAgent):

def update(self, chassis_private, clear_down=False):
super().update(chassis_private, clear_down)
external_ids = self.chassis_from_private(chassis_private).external_ids
_chassis = self.chassis_from_private(chassis_private)
other_config = ovn_utils.get_ovn_chassis_other_config(_chassis)
if ('enable-chassis-as-gw' not in
external_ids.get('ovn-cms-options', [])):
other_config.get('ovn-cms-options', [])):
self.__class__ = ControllerAgent


Expand Down
8 changes: 5 additions & 3 deletions neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py
Expand Up @@ -191,7 +191,8 @@ def sb_ovn(self, val):
def get_supported_vif_types(self):
vif_types = set()
for ch in self.sb_ovn.chassis_list().execute(check_error=True):
dp_type = ch.external_ids.get('datapath-type', '')
other_config = ovn_utils.get_ovn_chassis_other_config(ch)
dp_type = other_config.get('datapath-type', '')
if dp_type == ovn_const.CHASSIS_DATAPATH_NETDEV:
vif_types.add(portbindings.VIF_TYPE_VHOST_USER)
else:
Expand Down Expand Up @@ -962,8 +963,9 @@ def bind_port(self, context):
'agent': agent})
return
chassis = agent.chassis
datapath_type = chassis.external_ids.get('datapath-type', '')
iface_types = chassis.external_ids.get('iface-types', '')
other_config = ovn_utils.get_ovn_chassis_other_config(chassis)
datapath_type = other_config.get('datapath-type', '')
iface_types = other_config.get('iface-types', '')
iface_types = iface_types.split(',') if iface_types else []
chassis_physnets = self.sb_ovn._get_chassis_physnets(chassis)
for segment_to_bind in context.segments_to_bind:
Expand Down
Expand Up @@ -42,7 +42,8 @@ def _parse_ovn_cms_options(chassis):


def _parse_bridge_mappings(chassis):
bridge_mappings = chassis.external_ids.get('ovn-bridge-mappings', '')
other_config = ovn_utils.get_ovn_chassis_other_config(chassis)
bridge_mappings = other_config.get('ovn-bridge-mappings', '')
bridge_mappings = helpers.parse_mappings(bridge_mappings.split(','),
unique_values=False)
return {k: [v] for k, v in bridge_mappings.items()}
Expand Down
Expand Up @@ -847,7 +847,8 @@ def from_worker(cls, worker_class, driver=None):
return cls(conn)

def _get_chassis_physnets(self, chassis):
bridge_mappings = chassis.external_ids.get('ovn-bridge-mappings', '')
other_config = utils.get_ovn_chassis_other_config(chassis)
bridge_mappings = other_config.get('ovn-bridge-mappings', '')
mapping_dict = helpers.parse_mappings(bridge_mappings.split(','))
return list(mapping_dict.keys())

Expand All @@ -865,7 +866,8 @@ def get_gateway_chassis_from_cms_options(self, name_only=True):
return [ch.name if name_only else ch
for ch in self.chassis_list().execute(check_error=True)
if ovn_const.CMS_OPT_CHASSIS_AS_GW in
ch.external_ids.get(ovn_const.OVN_CMS_OPTIONS, '').split(',')]
utils.get_ovn_chassis_other_config(ch).get(
ovn_const.OVN_CMS_OPTIONS, '').split(',')]

def get_chassis_and_physnets(self):
chassis_info_dict = {}
Expand All @@ -890,7 +892,7 @@ def get_chassis_by_card_serial_from_cms_options(self,
if ('{}={}'
.format(ovn_const.CMS_OPT_CARD_SERIAL_NUMBER,
card_serial_number)
in ch.external_ids.get(
in utils.get_ovn_chassis_other_config(ch).get(
ovn_const.OVN_CMS_OPTIONS, '').split(',')):
return ch
msg = _('Chassis with %s %s %s does not exist'
Expand Down
41 changes: 29 additions & 12 deletions neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py
Expand Up @@ -172,12 +172,21 @@ def handle_ha_chassis_group_changes(self, event, row, old):
def match_fn(self, event, row, old):
if event != self.ROW_UPDATE:
return True
# NOTE(lucasgomes): If the external_ids column wasn't updated
# (meaning, Chassis "gateway" status didn't change) just returns
if not hasattr(old, 'external_ids') and event == self.ROW_UPDATE:

# NOTE(ralonsoh): LP#1990229 to be removed when min OVN version is
# 22.09
other_config = ('other_config' if hasattr(row, 'other_config') else
'external_ids')
# NOTE(lucasgomes): If the other_config/external_ids column wasn't
# updated (meaning, Chassis "gateway" status didn't change) just
# returns
if not hasattr(old, other_config) and event == self.ROW_UPDATE:
return False
if (old.external_ids.get('ovn-bridge-mappings') !=
row.external_ids.get('ovn-bridge-mappings')):
old_br_mappings = utils.get_ovn_chassis_other_config(old).get(
'ovn-bridge-mappings')
new_br_mappings = utils.get_ovn_chassis_other_config(row).get(
'ovn-bridge-mappings')
if old_br_mappings != new_br_mappings:
return True
# Check if either the Gateway status or Availability Zones has
# changed in the Chassis
Expand All @@ -192,8 +201,9 @@ def match_fn(self, event, row, old):
def run(self, event, row, old):
host = row.hostname
phy_nets = []
new_other_config = utils.get_ovn_chassis_other_config(row)
if event != self.ROW_DELETE:
bridge_mappings = row.external_ids.get('ovn-bridge-mappings', '')
bridge_mappings = new_other_config.get('ovn-bridge-mappings', '')
mapping_dict = helpers.parse_mappings(bridge_mappings.split(','))
phy_nets = list(mapping_dict)

Expand All @@ -208,9 +218,10 @@ def run(self, event, row, old):
if event == self.ROW_DELETE:
kwargs['event_from_chassis'] = row.name
elif event == self.ROW_UPDATE:
old_mappings = old.external_ids.get('ovn-bridge-mappings',
old_other_config = utils.get_ovn_chassis_other_config(old)
old_mappings = old_other_config.get('ovn-bridge-mappings',
set()) or set()
new_mappings = row.external_ids.get('ovn-bridge-mappings',
new_mappings = new_other_config.get('ovn-bridge-mappings',
set()) or set()
if old_mappings:
old_mappings = set(old_mappings.split(','))
Expand Down Expand Up @@ -339,11 +350,17 @@ class ChassisAgentTypeChangeEvent(ChassisEvent):
events = (BaseEvent.ROW_UPDATE,)

def match_fn(self, event, row, old=None):
if not getattr(old, 'external_ids', False):
# NOTE(ralonsoh): LP#1990229 to be removed when min OVN version is
# 22.09
other_config = ('other_config' if hasattr(row, 'other_config') else
'external_ids')
if not getattr(old, other_config, False):
return False
agent_type_change = n_agent.NeutronAgent.chassis_from_private(
row).external_ids.get('ovn-cms-options', []) != (
old.external_ids.get('ovn-cms-options', []))
chassis = n_agent.NeutronAgent.chassis_from_private(row)
new_other_config = utils.get_ovn_chassis_other_config(chassis)
old_other_config = utils.get_ovn_chassis_other_config(old)
agent_type_change = new_other_config.get('ovn-cms-options', []) != (
old_other_config.get('ovn-cms-options', []))
return agent_type_change

def run(self, event, row, old):
Expand Down
15 changes: 9 additions & 6 deletions neutron/tests/functional/base.py
Expand Up @@ -423,7 +423,8 @@ def restart(self):
self._start_ovn_northd()

def add_fake_chassis(self, host, physical_nets=None, external_ids=None,
name=None, azs=None, enable_chassis_as_gw=False):
name=None, azs=None, enable_chassis_as_gw=False,
other_config=None):
def append_cms_options(ext_ids, value):
if 'ovn-cms-options' not in ext_ids:
ext_ids['ovn-cms-options'] = value
Expand All @@ -432,19 +433,20 @@ def append_cms_options(ext_ids, value):

physical_nets = physical_nets or []
external_ids = external_ids or {}
other_config = other_config or {}
if azs is None:
azs = ['ovn']
if azs:
append_cms_options(external_ids, 'availability-zones=')
external_ids['ovn-cms-options'] += ':'.join(azs)
append_cms_options(other_config, 'availability-zones=')
other_config['ovn-cms-options'] += ':'.join(azs)
if enable_chassis_as_gw:
append_cms_options(external_ids, 'enable-chassis-as-gw')
append_cms_options(other_config, 'enable-chassis-as-gw')

bridge_mapping = ",".join(["%s:br-provider%s" % (phys_net, i)
for i, phys_net in enumerate(physical_nets)])
if name is None:
name = uuidutils.generate_uuid()
external_ids['ovn-bridge-mappings'] = bridge_mapping
other_config['ovn-bridge-mappings'] = bridge_mapping
# We'll be using different IP addresses every time for the Encap of
# the fake chassis as the SB schema doesn't allow to have two entries
# with same (ip,type) pairs as of OVS 2.11. This shouldn't have any
Expand All @@ -455,7 +457,8 @@ def append_cms_options(ext_ids, value):
self._counter += 1
chassis = self.sb_api.chassis_add(
name, ['geneve'], '172.24.4.%d' % self._counter,
external_ids=external_ids, hostname=host).execute(check_error=True)
external_ids=external_ids, hostname=host,
other_config=other_config).execute(check_error=True)
if self.sb_api.is_table_present('Chassis_Private'):
nb_cfg_timestamp = timeutils.utcnow_ts() * 1000
self.sb_api.db_create(
Expand Down
Expand Up @@ -72,7 +72,7 @@ def setUp(self, maintenance_worker=False, service_plugins=None):
self.mock_send_batch = mock.patch.object(
placement_extension, '_send_deferred_batch').start()

def _build_external_ids(self, bandwidths, inventory_defaults, hypervisors):
def _build_other_config(self, bandwidths, inventory_defaults, hypervisors):
options = []
if bandwidths:
options.append(n_const.RP_BANDWIDTHS + '=' + bandwidths)
Expand All @@ -85,17 +85,17 @@ def _build_external_ids(self, bandwidths, inventory_defaults, hypervisors):

def _create_chassis(self, host, name, physical_nets=None, bandwidths=None,
inventory_defaults=None, hypervisors=None):
external_ids = self._build_external_ids(bandwidths, inventory_defaults,
other_config = self._build_other_config(bandwidths, inventory_defaults,
hypervisors)
self.add_fake_chassis(host, physical_nets=physical_nets,
external_ids=external_ids, name=name)
other_config=other_config, name=name)

def _update_chassis(self, name, bandwidths=None, inventory_defaults=None,
hypervisors=None):
external_ids = self._build_external_ids(bandwidths, inventory_defaults,
other_config = self._build_other_config(bandwidths, inventory_defaults,
hypervisors)
self.sb_api.db_set(
'Chassis', name, ('external_ids', external_ids)
'Chassis', name, ('other_config', other_config)
).execute(check_error=True)

def _check_placement_config(self, expected_chassis):
Expand Down
Expand Up @@ -52,11 +52,11 @@ def setUp(self):
super(TestSbApi, self).setUp()
self.data = {
'chassis': [
{'external_ids': {'ovn-bridge-mappings':
{'other_config': {'ovn-bridge-mappings':
'public:br-ex,private:br-0'}},
{'external_ids': {'ovn-bridge-mappings':
{'other_config': {'ovn-bridge-mappings':
'public:br-ex,public2:br-ex2'}},
{'external_ids': {'ovn-bridge-mappings':
{'other_config': {'ovn-bridge-mappings':
'public:br-ex'}},
]
}
Expand All @@ -70,7 +70,7 @@ def load_test_data(self):
txn.add(self.api.chassis_add(
chassis['name'], ['geneve'], chassis['hostname'],
hostname=chassis['hostname'],
external_ids=chassis['external_ids']))
other_config=chassis['other_config']))

def test_get_chassis_hostname_and_physnets(self):
mapping = self.api.get_chassis_hostname_and_physnets()
Expand All @@ -97,7 +97,7 @@ def test_get_chassis_and_physnets(self):
def test_multiple_physnets_in_one_bridge(self):
self.data = {
'chassis': [
{'external_ids': {'ovn-bridge-mappings': 'p1:br-ex,p2:br-ex'}}
{'other_config': {'ovn-bridge-mappings': 'p1:br-ex,p2:br-ex'}}
]
}
self.load_test_data()
Expand Down
Expand Up @@ -441,21 +441,20 @@ def setUp(self):
chassis_name, self.mech_driver.agent_chassis_table)
self.mech_driver.sb_ovn.idl.notify_handler.watch_event(row_event)
self.chassis_name = self.add_fake_chassis(
self.FAKE_CHASSIS_HOST,
external_ids={'ovn-cms-options': 'enable-chassis-as-gw'},
name=chassis_name)
self.FAKE_CHASSIS_HOST, name=chassis_name,
enable_chassis_as_gw=True)
self.assertTrue(row_event.wait())
n_utils.wait_until_true(
lambda: len(list(neutron_agent.AgentCache())) == 1)

def test_agent_change_controller(self):
self.assertEqual(neutron_agent.ControllerGatewayAgent,
type(neutron_agent.AgentCache()[self.chassis_name]))
self.sb_api.db_set('Chassis', self.chassis_name, ('external_ids',
self.sb_api.db_set('Chassis', self.chassis_name, ('other_config',
{'ovn-cms-options': ''})).execute(check_error=True)
n_utils.wait_until_true(lambda:
neutron_agent.AgentCache()[self.chassis_name].
chassis.external_ids['ovn-cms-options'] == '')
chassis.other_config['ovn-cms-options'] == '')
self.assertEqual(neutron_agent.ControllerAgent,
type(neutron_agent.AgentCache()[self.chassis_name]))

Expand Down
Expand Up @@ -66,16 +66,16 @@ def setUp(self):
self.add_fake_chassis(self.ovs_host)
self.add_fake_chassis(
self.dpdk_host,
external_ids={'datapath-type': 'netdev',
other_config={'datapath-type': 'netdev',
'iface-types': 'dummy,dummy-internal,dpdkvhostuser'})

self.add_fake_chassis(
self.invalid_dpdk_host,
external_ids={'datapath-type': 'netdev',
other_config={'datapath-type': 'netdev',
'iface-types': 'dummy,dummy-internal,geneve,vxlan'})
self.add_fake_chassis(
self.smartnic_dpu_host,
external_ids={ovn_const.OVN_CMS_OPTIONS: '{}={}'.format(
other_config={ovn_const.OVN_CMS_OPTIONS: '{}={}'.format(
ovn_const.CMS_OPT_CARD_SERIAL_NUMBER,
self.smartnic_dpu_serial)})
self.n1 = self._make_network(self.fmt, 'n1', True)
Expand Down

0 comments on commit 536498a

Please sign in to comment.