]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Switch to using neutron.common.utils:replace_file()
authorBogdan Tabor <bogdan.tabor@ericpol.com>
Wed, 7 Oct 2015 13:49:08 +0000 (15:49 +0200)
committerBogdan Tabor <bogdan.tabor@ericpol.com>
Mon, 16 Nov 2015 07:44:04 +0000 (08:44 +0100)
neutron.agent.linux.utils:replace_file() and
neutron.common.utils:replace_file() have same functionality.

This is the 1st patch in the series of 4 patches.
It modifies neutron.common.utils:replace_file(),
so it can be used by all components as a replacement
for neutron.agent.linux.utils:replace_file().
New keyword parameter 'file_mode=0o644' is added
to neutron.common.utils:replace_file().

Partial-bug: #1504477
Change-Id: Id1a7f1236786e8606c91bb9925cd9ac8e95892b3

neutron/agent/linux/dhcp.py
neutron/agent/linux/dibbler.py
neutron/agent/linux/keepalived.py
neutron/agent/linux/ra.py
neutron/common/utils.py
neutron/tests/functional/common/__init__.py [new file with mode: 0644]
neutron/tests/functional/common/test_utils.py [new file with mode: 0644]
neutron/tests/unit/agent/l3/test_agent.py
neutron/tests/unit/agent/l3/test_dvr_local_router.py
neutron/tests/unit/agent/linux/test_dhcp.py

index 279d2ca7bed33f729075e45246eb363e26c6624d..bcf8ae53736dceb30f35547ea5dcf1961eb833f4 100644 (file)
@@ -26,15 +26,14 @@ from oslo_log import log as logging
 from oslo_utils import uuidutils
 import six
 
-from neutron.agent.common import utils as common_utils
+from neutron.agent.common import utils as agent_common_utils
 from neutron.agent.linux import external_process
 from neutron.agent.linux import ip_lib
 from neutron.agent.linux import iptables_manager
-from neutron.agent.linux import utils
 from neutron.common import constants
 from neutron.common import exceptions
 from neutron.common import ipv6_utils
-from neutron.common import utils as commonutils
+from neutron.common import utils as common_utils
 from neutron.extensions import extra_dhcp_opt as edo_ext
 from neutron.i18n import _LI, _LW, _LE
 
@@ -174,7 +173,7 @@ class DhcpLocalProcess(DhcpBase):
                                                version, plugin)
         self.confs_dir = self.get_confs_dir(conf)
         self.network_conf_dir = os.path.join(self.confs_dir, network.id)
-        commonutils.ensure_dir(self.network_conf_dir)
+        common_utils.ensure_dir(self.network_conf_dir)
 
     @staticmethod
     def get_confs_dir(conf):
@@ -199,7 +198,7 @@ class DhcpLocalProcess(DhcpBase):
         if self.active:
             self.restart()
         elif self._enable_dhcp():
-            commonutils.ensure_dir(self.network_conf_dir)
+            common_utils.ensure_dir(self.network_conf_dir)
             interface_name = self.device_manager.setup(self.network)
             self.interface_name = interface_name
             self.spawn_process()
@@ -260,7 +259,7 @@ class DhcpLocalProcess(DhcpBase):
     @interface_name.setter
     def interface_name(self, value):
         interface_file_path = self.get_conf_file_name('interface')
-        utils.replace_file(interface_file_path, value)
+        common_utils.replace_file(interface_file_path, value)
 
     @property
     def active(self):
@@ -596,7 +595,7 @@ class Dnsmasq(DhcpLocalProcess):
             buf.write('%s %s %s * *\n' %
                       (timestamp, port.mac_address, ip_address))
         contents = buf.getvalue()
-        utils.replace_file(filename, contents)
+        common_utils.replace_file(filename, contents)
         LOG.debug('Done building initial lease file %s with contents:\n%s',
                   filename, contents)
         return filename
@@ -666,7 +665,7 @@ class Dnsmasq(DhcpLocalProcess):
                 buf.write('%s,%s,%s\n' %
                           (port.mac_address, name, ip_address))
 
-        utils.replace_file(filename, buf.getvalue())
+        common_utils.replace_file(filename, buf.getvalue())
         LOG.debug('Done building host file %s with contents:\n%s', filename,
                   buf.getvalue())
         return filename
@@ -737,7 +736,7 @@ class Dnsmasq(DhcpLocalProcess):
             if alloc:
                 buf.write('%s\t%s %s\n' % (alloc.ip_address, fqdn, hostname))
         addn_hosts = self.get_conf_file_name('addn_hosts')
-        utils.replace_file(addn_hosts, buf.getvalue())
+        common_utils.replace_file(addn_hosts, buf.getvalue())
         return addn_hosts
 
     def _output_opts_file(self):
@@ -746,7 +745,7 @@ class Dnsmasq(DhcpLocalProcess):
         options += self._generate_opts_per_port(subnet_index_map)
 
         name = self.get_conf_file_name('opts')
-        utils.replace_file(name, '\n'.join(options))
+        common_utils.replace_file(name, '\n'.join(options))
         return name
 
     def _generate_opts_per_subnet(self):
@@ -979,7 +978,7 @@ class DeviceManager(object):
     def __init__(self, conf, plugin):
         self.conf = conf
         self.plugin = plugin
-        self.driver = common_utils.load_interface_driver(conf)
+        self.driver = agent_common_utils.load_interface_driver(conf)
 
     def get_interface_name(self, network, port):
         """Return interface(device) name for use by the DHCP process."""
@@ -989,7 +988,8 @@ class DeviceManager(object):
         """Return a unique DHCP device ID for this host on the network."""
         # There could be more than one dhcp server per network, so create
         # a device id that combines host and network ids
-        return commonutils.get_dhcp_agent_device_id(network.id, self.conf.host)
+        return common_utils.get_dhcp_agent_device_id(network.id,
+                                                     self.conf.host)
 
     def _set_default_route(self, network, device_name):
         """Sets the default gateway for this dhcp namespace.
index 3a97f620ef1e7a0fcebd5297e9699ad1abba2c53..f68c45c2242bbd53c19a8d18edd97981002b3093 100644 (file)
@@ -24,6 +24,7 @@ from neutron.agent.linux import pd
 from neutron.agent.linux import pd_driver
 from neutron.agent.linux import utils
 from neutron.common import constants
+from neutron.common import utils as common_utils
 from oslo_log import log as logging
 
 
@@ -83,7 +84,7 @@ class PDDibbler(pd_driver.PDDriverBase):
         buf.write('%s' % SCRIPT_TEMPLATE.render(
                              prefix_path=self.prefix_path,
                              l3_agent_pid=os.getpid()))
-        utils.replace_file(script_path, buf.getvalue())
+        common_utils.replace_file(script_path, buf.getvalue())
         os.chmod(script_path, 0o744)
 
         dibbler_conf = utils.get_conf_file_name(dcwa, 'client', 'conf', False)
@@ -95,7 +96,7 @@ class PDDibbler(pd_driver.PDDriverBase):
                              interface_name='"%s"' % ex_gw_ifname,
                              bind_address='%s' % lla))
 
-        utils.replace_file(dibbler_conf, buf.getvalue())
+        common_utils.replace_file(dibbler_conf, buf.getvalue())
         return dcwa
 
     def _spawn_dibbler(self, pmon, router_ns, dibbler_conf):
index 060928e6f4e8db87d68ad63bd813fe63730afc27..09d437e8da31a3f3dd5a9f51c605f10841320c35 100644 (file)
@@ -21,7 +21,6 @@ from oslo_config import cfg
 from oslo_log import log as logging
 
 from neutron.agent.linux import external_process
-from neutron.agent.linux import utils
 from neutron.common import exceptions
 from neutron.common import utils as common_utils
 
@@ -378,7 +377,7 @@ class KeepalivedManager(object):
     def _output_config_file(self):
         config_str = self.config.get_config_str()
         config_path = self.get_full_config_file_path('keepalived.conf')
-        utils.replace_file(config_path, config_str)
+        common_utils.replace_file(config_path, config_str)
 
         return config_path
 
index d9eca8d6475c48bf2312c0a76501b21f720d4a07..a6fffd328252d7fe0abe974f59c8179b1c6d3b99 100644 (file)
@@ -22,6 +22,7 @@ import six
 from neutron.agent.linux import external_process
 from neutron.agent.linux import utils
 from neutron.common import constants
+from neutron.common import utils as common_utils
 
 
 RADVD_SERVICE_NAME = 'radvd'
@@ -94,7 +95,7 @@ class DaemonMonitor(object):
                 prefixes=auto_config_prefixes,
                 constants=constants))
 
-        utils.replace_file(radvd_conf, buf.getvalue())
+        common_utils.replace_file(radvd_conf, buf.getvalue())
         return radvd_conf
 
     def _get_radvd_process_manager(self, callback=None):
index 1fe6ab1ce5ca9fdbaa135ccf627c6acde94ce44b..190d3e6242cd28d861cf61b39538b4325e0bde1d 100644 (file)
@@ -471,7 +471,7 @@ def round_val(val):
                                              rounding=decimal.ROUND_HALF_UP))
 
 
-def replace_file(file_name, data):
+def replace_file(file_name, data, file_mode=0o644):
     """Replaces the contents of file_name with data in a safe manner.
 
     First write to a temp file and then rename. Since POSIX renames are
@@ -485,5 +485,5 @@ def replace_file(file_name, data):
                                      dir=base_dir,
                                      delete=False) as tmp_file:
         tmp_file.write(data)
-    os.chmod(tmp_file.name, 0o644)
+    os.chmod(tmp_file.name, file_mode)
     os.rename(tmp_file.name, file_name)
diff --git a/neutron/tests/functional/common/__init__.py b/neutron/tests/functional/common/__init__.py
new file mode 100644 (file)
index 0000000..e69de29
diff --git a/neutron/tests/functional/common/test_utils.py b/neutron/tests/functional/common/test_utils.py
new file mode 100644 (file)
index 0000000..8515963
--- /dev/null
@@ -0,0 +1,51 @@
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+import os.path
+import stat
+
+from neutron.common import utils
+from neutron.tests import base
+
+
+class TestReplaceFile(base.BaseTestCase):
+    def setUp(self):
+        super(TestReplaceFile, self).setUp()
+        temp_dir = self.get_default_temp_dir().path
+        self.file_name = os.path.join(temp_dir, "new_file")
+        self.data = "data to copy"
+
+    def _verify_result(self, file_mode):
+        self.assertTrue(os.path.exists(self.file_name))
+        with open(self.file_name) as f:
+            content = f.read()
+        self.assertEqual(self.data, content)
+        mode = os.stat(self.file_name).st_mode
+        self.assertEqual(file_mode, stat.S_IMODE(mode))
+
+    def test_replace_file_default_mode(self):
+        file_mode = 0o644
+        utils.replace_file(self.file_name, self.data)
+        self._verify_result(file_mode)
+
+    def test_replace_file_custom_mode(self):
+        file_mode = 0o722
+        utils.replace_file(self.file_name, self.data, file_mode)
+        self._verify_result(file_mode)
+
+    def test_replace_file_custom_mode_twice(self):
+        file_mode = 0o722
+        utils.replace_file(self.file_name, self.data, file_mode)
+        self.data = "new data to copy"
+        file_mode = 0o777
+        utils.replace_file(self.file_name, self.data, file_mode)
+        self._verify_result(file_mode)
index a40a29dcc93cf772781c79d14791e0c004e8ff2e..ae30cdf96345204fd9a9b71ca9281edc7349fdb4 100644 (file)
@@ -93,7 +93,7 @@ class BasicRouterOperationsFramework(base.BaseTestCase):
         self.utils_exec = self.utils_exec_p.start()
 
         self.utils_replace_file_p = mock.patch(
-            'neutron.agent.linux.utils.replace_file')
+            'neutron.common.utils.replace_file')
         self.utils_replace_file = self.utils_replace_file_p.start()
 
         self.external_process_p = mock.patch(
index 048beb305e7dcbc70f0937c85dd3cd7b7fe20bbd..9609ae8d744f4d57b90918a34d93df913cfe8907 100644 (file)
@@ -75,7 +75,7 @@ class TestDvrRouterOperations(base.BaseTestCase):
         self.utils_exec = self.utils_exec_p.start()
 
         self.utils_replace_file_p = mock.patch(
-            'neutron.agent.linux.utils.replace_file')
+            'neutron.common.utils.replace_file')
         self.utils_replace_file = self.utils_replace_file_p.start()
 
         self.external_process_p = mock.patch(
index 1a25247837cc130cf04585eb6f1e999ed41dc39e..1604fe7fb703e357b572045776196a24430e3a5b 100644 (file)
@@ -814,7 +814,7 @@ class TestBase(TestConfBase):
         self.config_parse(self.conf)
         self.conf.set_override('state_path', '')
 
-        self.replace_p = mock.patch('neutron.agent.linux.utils.replace_file')
+        self.replace_p = mock.patch('neutron.common.utils.replace_file')
         self.execute_p = mock.patch('neutron.agent.common.utils.execute')
         self.safe = self.replace_p.start()
         self.execute = self.execute_p.start()
@@ -993,7 +993,7 @@ class TestDhcpLocalProcess(TestBase):
             self.assertEqual(lp.interface_name, 'tap0')
 
     def test_set_interface_name(self):
-        with mock.patch('neutron.agent.linux.utils.replace_file') as replace:
+        with mock.patch('neutron.common.utils.replace_file') as replace:
             lp = LocalChild(self.conf, FakeDualNetwork())
             with mock.patch.object(lp, 'get_conf_file_name') as conf_file:
                 conf_file.return_value = '/interface'
@@ -1965,22 +1965,14 @@ class TestDnsmasq(TestBase):
 
 
 class TestDeviceManager(TestConfBase):
-
-    @mock.patch('neutron.agent.linux.dhcp.ip_lib')
-    @mock.patch('neutron.agent.linux.dhcp.common_utils.load_interface_driver')
-    def test_setup(self, load_interface_driver, ip_lib):
-        """Test new and existing cases of DeviceManager's DHCP port setup
-        logic.
-        """
-        self._test_setup(load_interface_driver, ip_lib, False)
-
-    @mock.patch('neutron.agent.linux.dhcp.ip_lib')
-    @mock.patch('neutron.agent.linux.dhcp.common_utils.load_interface_driver')
-    def test_setup_gateway_ips(self, load_interface_driver, ip_lib):
-        """Test new and existing cases of DeviceManager's DHCP port setup
-        logic.
-        """
-        self._test_setup(load_interface_driver, ip_lib, True)
+    def setUp(self):
+        super(TestDeviceManager, self).setUp()
+        ip_lib_patcher = mock.patch('neutron.agent.linux.dhcp.ip_lib')
+        load_interface_driver_patcher = mock.patch(
+            'neutron.agent.linux.dhcp.agent_common_utils.'
+            'load_interface_driver')
+        self.mock_ip_lib = ip_lib_patcher.start()
+        self.mock_load_interface_driver = load_interface_driver_patcher.start()
 
     def _test_setup(self, load_interface_driver, ip_lib, use_gateway_ips):
         # Create DeviceManager.
@@ -2044,9 +2036,15 @@ class TestDeviceManager(TestConfBase):
                                          'unique-IP-address/64']))
         self.assertFalse(plugin.create_dhcp_port.called)
 
-    @mock.patch('neutron.agent.linux.dhcp.ip_lib')
-    @mock.patch('neutron.agent.linux.dhcp.common_utils.load_interface_driver')
-    def test_setup_reserved(self, load_interface_driver, ip_lib):
+    def test_setup_device_manager_dhcp_port_without_gateway_ips(self):
+        self._test_setup(self.mock_load_interface_driver,
+                         self.mock_ip_lib, use_gateway_ips=False)
+
+    def test_setup_device_manager_dhcp_port_with_gateway_ips(self):
+        self._test_setup(self.mock_load_interface_driver,
+                         self.mock_ip_lib, use_gateway_ips=True)
+
+    def test_setup_reserved(self):
         """Test reserved port case of DeviceManager's DHCP port setup
         logic.
         """
@@ -2056,7 +2054,7 @@ class TestDeviceManager(TestConfBase):
                                            default=False))
         plugin = mock.Mock()
         mgr = dhcp.DeviceManager(self.conf, plugin)
-        load_interface_driver.assert_called_with(self.conf)
+        self.mock_load_interface_driver.assert_called_with(self.conf)
 
         # Setup with a reserved DHCP port.
         network = FakeDualNetworkReserved()
@@ -2072,7 +2070,7 @@ class TestDeviceManager(TestConfBase):
         plugin.update_dhcp_port.side_effect = mock_update
         mgr.driver.get_device_name.return_value = 'ns-XXX'
         mgr.driver.use_gateway_ips = False
-        ip_lib.ensure_device_is_ready.return_value = True
+        self.mock_ip_lib.ensure_device_is_ready.return_value = True
         mgr.setup(network)
         plugin.update_dhcp_port.assert_called_with(reserved_port.id, mock.ANY)
 
@@ -2080,9 +2078,7 @@ class TestDeviceManager(TestConfBase):
                                               ['192.168.0.6/24'],
                                               namespace='qdhcp-ns')
 
-    @mock.patch('neutron.agent.linux.dhcp.ip_lib')
-    @mock.patch('neutron.agent.linux.dhcp.common_utils.load_interface_driver')
-    def test_setup_reserved_2(self, load_interface_driver, ip_lib):
+    def test_setup_reserved_2(self):
         """Test scenario where a network has two reserved ports, and
         update_dhcp_port fails for the first of those.
         """
@@ -2092,7 +2088,7 @@ class TestDeviceManager(TestConfBase):
                                            default=False))
         plugin = mock.Mock()
         mgr = dhcp.DeviceManager(self.conf, plugin)
-        load_interface_driver.assert_called_with(self.conf)
+        self.mock_load_interface_driver.assert_called_with(self.conf)
 
         # Setup with a reserved DHCP port.
         network = FakeDualNetworkReserved2()
@@ -2112,7 +2108,7 @@ class TestDeviceManager(TestConfBase):
         plugin.update_dhcp_port.side_effect = mock_update
         mgr.driver.get_device_name.return_value = 'ns-XXX'
         mgr.driver.use_gateway_ips = False
-        ip_lib.ensure_device_is_ready.return_value = True
+        self.mock_ip_lib.ensure_device_is_ready.return_value = True
         mgr.setup(network)
         plugin.update_dhcp_port.assert_called_with(reserved_port_2.id,
                                                    mock.ANY)