From 7012041845f7e56a18a8be37a41aa73fba3df70a Mon Sep 17 00:00:00 2001 From: Michael Still Date: Tue, 27 Nov 2018 09:59:11 +1100 Subject: [PATCH] Move bridge creation to privsep. Change-Id: I4b34e8ff2f43f2d94762202c20501209862bc1d5 --- nova/network/linux_net.py | 3 +-- nova/privsep/linux_net.py | 9 +++++++++ .../api_sample_tests/api_sample_base.py | 5 +++++ nova/tests/unit/network/test_linux_net.py | 11 ++++------- nova/tests/unit/network/test_manager.py | 16 +++++++++++----- nova/tests/unit/virt/xenapi/test_xenapi.py | 3 ++- 6 files changed, 32 insertions(+), 15 deletions(-) diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py index 66645239cbaa..bbda8d9b06d4 100644 --- a/nova/network/linux_net.py +++ b/nova/network/linux_net.py @@ -1422,8 +1422,7 @@ class LinuxBridgeInterfaceDriver(LinuxNetInterfaceDriver): """ if not linux_net_utils.device_exists(bridge): LOG.debug('Starting Bridge %s', bridge) - out, err = _execute('brctl', 'addbr', bridge, - check_exit_code=False, run_as_root=True) + out, err = nova.privsep.linux_net.add_bridge(bridge) if (err and err != "device %s already exists; can't create " "bridge with the same name\n" % (bridge)): msg = _('Failed to add bridge: %s') % err diff --git a/nova/privsep/linux_net.py b/nova/privsep/linux_net.py index 5d3cb23e8c55..5bcf5f61712b 100644 --- a/nova/privsep/linux_net.py +++ b/nova/privsep/linux_net.py @@ -22,6 +22,15 @@ from oslo_concurrency import processutils import nova.privsep +@nova.privsep.sys_admin_pctxt.entrypoint +def add_bridge(interface): + """Add a bridge. + + :param interface: the name of the bridge + """ + processutils.execute('brctl', 'addbr', interface) + + @nova.privsep.sys_admin_pctxt.entrypoint def delete_bridge(interface): """Delete a bridge. diff --git a/nova/tests/functional/api_sample_tests/api_sample_base.py b/nova/tests/functional/api_sample_tests/api_sample_base.py index 05df35d1d820..b3d8128acee7 100644 --- a/nova/tests/functional/api_sample_tests/api_sample_base.py +++ b/nova/tests/functional/api_sample_tests/api_sample_base.py @@ -120,5 +120,10 @@ class ApiSampleTestBaseV21(testscenarios.WithScenarios, # this is used to generate sample docs self.generate_samples = os.getenv('GENERATE_SAMPLES') is not None + # NOTE(mikal): this is used to stub away privsep helpers + def fake_noop(*args, **kwargs): + return '', '' + self.stub_out('nova.privsep.linux_net.add_bridge', fake_noop) + def _setup_services(self): pass diff --git a/nova/tests/unit/network/test_linux_net.py b/nova/tests/unit/network/test_linux_net.py index bba9f77271ef..d5af089d22f2 100644 --- a/nova/tests/unit/network/test_linux_net.py +++ b/nova/tests/unit/network/test_linux_net.py @@ -590,21 +590,18 @@ class LinuxNetworkTestCase(test.NoDBTestCase): self.assertEqual(expected, actual) @mock.patch.object(linux_net.iptables_manager.ipv4['filter'], 'add_rule') - @mock.patch.object(utils, 'execute') - def test_linux_bridge_driver_plug(self, mock_execute, mock_add_rule): + @mock.patch('nova.privsep.linux_net.add_bridge', + return_value=('', '')) + def test_linux_bridge_driver_plug(self, mock_add_bridge, mock_add_rule): """Makes sure plug doesn't drop FORWARD by default. Ensures bug 890195 doesn't reappear. """ - def fake_execute(*args, **kwargs): - return "", "" - def verify_add_rule(chain, rule): self.assertEqual('FORWARD', chain) self.assertIn('ACCEPT', rule) - mock_execute.side_effect = fake_execute mock_add_rule.side_effect = verify_add_rule driver = linux_net.LinuxBridgeInterfaceDriver() @@ -1183,7 +1180,7 @@ class LinuxNetworkTestCase(test.NoDBTestCase): with test.nested( mock.patch('nova.network.linux_utils.device_exists', return_value=False), - mock.patch.object(linux_net, '_execute', fake_execute) + mock.patch('nova.privsep.linux_net.add_bridge', fake_execute) ) as (device_exists, _): driver = linux_net.LinuxBridgeInterfaceDriver() driver.ensure_bridge('brq1234567-89', '') diff --git a/nova/tests/unit/network/test_manager.py b/nova/tests/unit/network/test_manager.py index 0cfe44288709..4e75bc0b6951 100644 --- a/nova/tests/unit/network/test_manager.py +++ b/nova/tests/unit/network/test_manager.py @@ -935,7 +935,8 @@ class VlanNetworkTestCase(test.TestCase): self.assertEqual(objects.QuotasNoOp, self.network.quotas_cls) - def test_vpn_allocate_fixed_ip(self): + @mock.patch('nova.privsep.linux_net.add_bridge', return_value=('', '')) + def test_vpn_allocate_fixed_ip(self, mock_add_bridge): self.mox.StubOutWithMock(db, 'fixed_ip_associate') self.mox.StubOutWithMock(db, 'fixed_ip_update') self.mox.StubOutWithMock(db, @@ -968,7 +969,8 @@ class VlanNetworkTestCase(test.TestCase): self.network.allocate_fixed_ip(self.context, FAKEUUID, network, vpn=True) - def test_allocate_fixed_ip(self): + @mock.patch('nova.privsep.linux_net.add_bridge', return_value=('', '')) + def test_allocate_fixed_ip(self, mock_add_bridge): self.stubs.Set(self.network, '_do_trigger_security_group_members_refresh_for_instance', lambda *a, **kw: None) @@ -1685,7 +1687,9 @@ class VlanNetworkTestCase(test.TestCase): ctxt, mox.IgnoreArg()) - def test_add_fixed_ip_instance_without_vpn_requested_networks(self): + @mock.patch('nova.privsep.linux_net.add_bridge', return_value=('', '')) + def test_add_fixed_ip_instance_without_vpn_requested_networks( + self, mock_add_bridge): self.stubs.Set(self.network, '_do_trigger_security_group_members_refresh_for_instance', lambda *a, **kw: None) @@ -2829,7 +2833,8 @@ class AllocateTestCase(test.TestCase): self.user_context = context.RequestContext('testuser', fakes.FAKE_PROJECT_ID) - def test_allocate_for_instance(self): + @mock.patch('nova.privsep.linux_net.add_bridge', return_value=('', '')) + def test_allocate_for_instance(self, mock_add_bridge): address = "10.10.10.10" self.flags(auto_assign_floating_ip=True) @@ -2893,7 +2898,8 @@ class AllocateTestCase(test.TestCase): project_id=self.context.project_id, macs=None, requested_networks=requested_networks) - def test_allocate_for_instance_with_mac(self): + @mock.patch('nova.privsep.linux_net.add_bridge', return_value=('', '')) + def test_allocate_for_instance_with_mac(self, mock_add_bridge): available_macs = set(['ca:fe:de:ad:be:ef']) inst = db.instance_create(self.context, {'host': HOST, 'display_name': HOST, diff --git a/nova/tests/unit/virt/xenapi/test_xenapi.py b/nova/tests/unit/virt/xenapi/test_xenapi.py index f9ce7bf7c00f..4f706f32912f 100644 --- a/nova/tests/unit/virt/xenapi/test_xenapi.py +++ b/nova/tests/unit/virt/xenapi/test_xenapi.py @@ -1135,7 +1135,8 @@ class XenAPIVMTestCase(stubs.XenAPITestBase, mock.ANY) @mock.patch.object(vmops.VMOps, '_create_vifs') - def test_spawn_vlanmanager(self, mock_create_vifs): + @mock.patch('nova.privsep.linux_net.add_bridge', return_value=('', '')) + def test_spawn_vlanmanager(self, mock_add_bridge, mock_create_vifs): self.flags(network_manager='nova.network.manager.VlanManager', vlan_interface='fake0') # Reset network table