Browse Source

Do not modify passed by reference variables in mechanism_driver

Test test_update_port_with_empty_data [0] from TestOVNMechanismDriverPortsV2
class in neutron is runned agains:
neutron.tests.unit.plugins.ml2.drivers.mechanism_test.TestMechanismDriver
which in fact during update_port_postcommit() does nothing with
the port, only validates it [1].

In networking-ovn this test is runned against real mechanism driver:
networking_ovn.ml2.mech_driver.OVNMechanismDriver. This ends with
little difference - to port dict 'network' information is added
during call of update_port_postcommit(). The port data itself
remains the same.
We shouldn't modify passed by reference variables there. So doing
deepcopy on all provided data.

Here comes also the question if this test inheritance is the
right way.

[0] https://review.opendev.org/#/c/673486
[1] https://github.com/openstack/neutron/blob/master/neutron/tests/unit/plugins/ml2/drivers/mechanism_test.py#L208

Closes-Bug: #1839434
Change-Id: I1ad224960173fe896f6eb8049a188d9e82874068
(cherry picked from commit b2b5e89bca)
(cherry picked from commit 36bd132b40)
changes/03/675603/1
Maciej Józefczyk 1 month ago
parent
commit
25d24f2096

+ 5
- 4
networking_ovn/ml2/mech_driver.py View File

@@ -12,6 +12,7 @@
12 12
 #    under the License.
13 13
 #
14 14
 
15
+import copy
15 16
 import functools
16 17
 import operator
17 18
 import threading
@@ -484,7 +485,7 @@ class OVNMechanismDriver(api.MechanismDriver):
484 485
         drastically affect performance.  Raising an exception will
485 486
         result in the deletion of the resource.
486 487
         """
487
-        port = context.current
488
+        port = copy.deepcopy(context.current)
488 489
         port['network'] = context.network.current
489 490
         self._ovn_client.create_port(port)
490 491
         self._notify_dhcp_updated(port['id'])
@@ -537,9 +538,9 @@ class OVNMechanismDriver(api.MechanismDriver):
537 538
         state. It is up to the mechanism driver to ignore state or
538 539
         state changes that it does not know or care about.
539 540
         """
540
-        port = context.current
541
+        port = copy.deepcopy(context.current)
541 542
         port['network'] = context.network.current
542
-        original_port = context.original
543
+        original_port = copy.deepcopy(context.original)
543 544
         original_port['network'] = context.network.current
544 545
         self._ovn_client.update_port(port, port_object=original_port)
545 546
         self._notify_dhcp_updated(port['id'])
@@ -556,7 +557,7 @@ class OVNMechanismDriver(api.MechanismDriver):
556 557
         expected, and will not prevent the resource from being
557 558
         deleted.
558 559
         """
559
-        port = context.current
560
+        port = copy.deepcopy(context.current)
560 561
         port['network'] = context.network.current
561 562
         self._ovn_client.delete_port(port['id'], port_object=port)
562 563
 

+ 11
- 4
networking_ovn/tests/unit/ml2/test_mech_driver.py View File

@@ -12,6 +12,7 @@
12 12
 #    under the License.
13 13
 #
14 14
 
15
+import copy
15 16
 import datetime
16 17
 import uuid
17 18
 
@@ -1471,10 +1472,10 @@ class TestOVNMechanismDriver(test_plugin.Ml2PluginV2TestCase):
1471 1472
         fake_port = fakes.FakePort.create_one_port(
1472 1473
             attrs={'status': const.PORT_STATUS_DOWN}).info()
1473 1474
         fake_ctx = mock.Mock(current=fake_port)
1474
-
1475 1475
         self.mech_driver.create_port_postcommit(fake_ctx)
1476
-
1477
-        mock_create_port.assert_called_once_with(fake_port)
1476
+        passed_fake_port = copy.deepcopy(fake_port)
1477
+        passed_fake_port['network'] = fake_ctx.network.current
1478
+        mock_create_port.assert_called_once_with(passed_fake_port)
1478 1479
         mock_notify_dhcp.assert_called_once_with(fake_port['id'])
1479 1480
 
1480 1481
     @mock.patch.object(mech_driver.OVNMechanismDriver,
@@ -1487,8 +1488,14 @@ class TestOVNMechanismDriver(test_plugin.Ml2PluginV2TestCase):
1487 1488
             attrs={'status': const.PORT_STATUS_ACTIVE}).info()
1488 1489
         fake_ctx = mock.Mock(current=fake_port, original=fake_port)
1489 1490
         self.mech_driver.update_port_postcommit(fake_ctx)
1491
+
1492
+        passed_fake_port = copy.deepcopy(fake_port)
1493
+        passed_fake_port['network'] = fake_ctx.network.current
1494
+        passed_fake_port_orig = copy.deepcopy(fake_ctx.original)
1495
+        passed_fake_port_orig['network'] = fake_ctx.network.current
1496
+
1490 1497
         mock_update_port.assert_called_once_with(
1491
-            fake_port, port_object=fake_ctx.original)
1498
+            passed_fake_port, port_object=passed_fake_port_orig)
1492 1499
         mock_notify_dhcp.assert_called_once_with(fake_port['id'])
1493 1500
 
1494 1501
     def _add_chassis_agent(self, nb_cfg, agent_type, updated_at=None):

Loading…
Cancel
Save