From d02fb560aa35c4931d40000e8ca4c0e4105299b7 Mon Sep 17 00:00:00 2001 From: Lucas Alvares Gomes Date: Thu, 28 Oct 2021 14:17:20 +0100 Subject: [PATCH] [OVN] Update check_for_mcast_flood_reports() to check for mcast_flood The patch [0] changed the value of the "mcast_flood" option for the localnet ports in OVN to "false" to avoid multicast packet duplication but, the maintenance task check_for_mcast_flood_reports() was never updated to account for that change. So, localnet ports created prior to [0] would continue to have the "mcast_flood" value as "true" and new ports would have it as "false". This patch fixes the maintenance task. [0] https://review.opendev.org/c/openstack/neutron/+/797418 Change-Id: Ib90704a85c9ed659d71ac8ed42b1b542bd2496a2 Closes-Bug: #1949081 Signed-off-by: Lucas Alvares Gomes --- .../ovn/mech_driver/ovsdb/maintenance.py | 13 ++++++---- .../ovn/mech_driver/ovsdb/test_maintenance.py | 25 ++++++++++++++----- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py index 343dcde57a3..f36b176954a 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py @@ -677,7 +677,7 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase): txn.add(cmd) raise periodics.NeverAgain() - # TODO(lucasagomes): Remove this in the Y cycle + # TODO(lucasagomes): Remove this in the Z cycle # A static spacing value is used here, but this method will only run # once per lock due to the use of periodics.NeverAgain(). @periodics.periodic(spacing=600, run_immediately=True) @@ -692,13 +692,16 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase): continue options = port.options - if ovn_const.LSP_OPTIONS_MCAST_FLOOD_REPORTS in options: + if port_type == ovn_const.LSP_TYPE_LOCALNET: + mcast_flood_value = options.get( + ovn_const.LSP_OPTIONS_MCAST_FLOOD_REPORTS) + if mcast_flood_value == 'false': + continue + options.update({ovn_const.LSP_OPTIONS_MCAST_FLOOD: 'false'}) + elif ovn_const.LSP_OPTIONS_MCAST_FLOOD_REPORTS in options: continue options.update({ovn_const.LSP_OPTIONS_MCAST_FLOOD_REPORTS: 'true'}) - if port_type == ovn_const.LSP_TYPE_LOCALNET: - options.update({ovn_const.LSP_OPTIONS_MCAST_FLOOD: 'false'}) - cmds.append(self._nb_idl.lsp_set_options(port.name, **options)) if cmds: diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py index 21828277a37..9e5055cace6 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py @@ -455,7 +455,8 @@ class TestDBInconsistenciesPeriodics(testlib_api.SqlTestCaseLight, nb_idl = self.fake_ovn_client._nb_idl lsp0 = fakes.FakeOvsdbRow.create_one_ovsdb_row( attrs={'name': 'lsp0', - 'options': {'mcast_flood_reports': 'true'}, + 'options': { + constants.LSP_OPTIONS_MCAST_FLOOD_REPORTS: 'true'}, 'type': ""}) lsp1 = fakes.FakeOvsdbRow.create_one_ovsdb_row( attrs={'name': 'lsp1', 'options': {}, 'type': ""}) @@ -470,21 +471,33 @@ class TestDBInconsistenciesPeriodics(testlib_api.SqlTestCaseLight, 'type': "router"}) lsp5 = fakes.FakeOvsdbRow.create_one_ovsdb_row( attrs={'name': 'lsp5', 'options': {}, 'type': 'localnet'}) + lsp6 = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={'name': 'lsp6', + 'options': { + constants.LSP_OPTIONS_MCAST_FLOOD_REPORTS: 'true', + constants.LSP_OPTIONS_MCAST_FLOOD: 'true'}, + 'type': 'localnet'}) + lsp7 = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={'name': 'lsp7', + 'options': { + constants.LSP_OPTIONS_MCAST_FLOOD_REPORTS: 'true', + constants.LSP_OPTIONS_MCAST_FLOOD: 'false'}, + 'type': 'localnet'}) nb_idl.lsp_list.return_value.execute.return_value = [ - lsp0, lsp1, lsp2, lsp3, lsp4, lsp5] + lsp0, lsp1, lsp2, lsp3, lsp4, lsp5, lsp6, lsp7] # Invoke the periodic method, it meant to run only once at startup # so NeverAgain will be raised at the end self.assertRaises(periodics.NeverAgain, self.periodic.check_for_mcast_flood_reports) - # Assert only lsp1 and lsp5 were called because they are the only - # ones meeting the criteria ("mcast_flood_reports" not yet set, - # and type "" or localnet) + # Assert only lsp1, lsp5 and lsp6 were called because they are the + # only ones meeting the criteria expected_calls = [ mock.call('lsp1', mcast_flood_reports='true'), - mock.call('lsp5', mcast_flood_reports='true', mcast_flood='false')] + mock.call('lsp5', mcast_flood_reports='true', mcast_flood='false'), + mock.call('lsp6', mcast_flood_reports='true', mcast_flood='false')] nb_idl.lsp_set_options.assert_has_calls(expected_calls)