Use common agent.linux.utils.ensure_dir method

We repeated os.makedirs(dir, 0o755) in several places. We should use
common neutron.agent.linux.utils.ensure_dir. Unit tests are also added.

Change-Id: Iaeae5ff7dc6676420c000d6501f69a5997ad4b6c
Closes-bug: 1419042

diff --git a/neutron/agent/dhcp/agent.py b/neutron/agent/dhcp/agent.py
index 8f0647c8992f817001d24d03aaaad849cb0331e6..13a3a944c85ce1cf354b5e0d8bb04b56ff6e0fce 100644 (file)
--- a/neutron/agent/dhcp/agent.py
+++ b/neutron/agent/dhcp/agent.py
@@ -24,6 +24,7 @@ from oslo_utils import importutils
 
 from neutron.agent.linux import dhcp
 from neutron.agent.linux import external_process
+from neutron.agent.linux import utils as linux_utils
 from neutron.agent.metadata import driver as metadata_driver
 from neutron.agent import rpc as agent_rpc
 from neutron.common import constants
@@ -62,8 +63,7 @@ class DhcpAgent(manager.Manager):
                                         ctx, self.conf.use_namespaces)
         # create dhcp dir to store dhcp info
         dhcp_dir = os.path.dirname("/%s/dhcp/" % self.conf.state_path)
-        if not os.path.isdir(dhcp_dir):
-            os.makedirs(dhcp_dir, 0o755)
+        linux_utils.ensure_dir(dhcp_dir)
         self.dhcp_version = self.dhcp_driver_cls.check_version()
         self._populate_networks_cache()
         self._process_monitor = external_process.ProcessMonitor(

diff --git a/neutron/agent/l3/ha.py b/neutron/agent/l3/ha.py
index 5758abba09a1065750d34ee44efaae5cc9446a84..99be4a101b78d9e35a1631749dd4f8439999e15f 100644 (file)
--- a/neutron/agent/l3/ha.py
+++ b/neutron/agent/l3/ha.py
@@ -18,6 +18,7 @@ import os
 from oslo_config import cfg
 
 from neutron.agent.linux import keepalived
+from neutron.agent.linux import utils
 from neutron.common import constants as l3_constants
 from neutron.i18n import _LE
 from neutron.openstack.common import log as logging
@@ -51,8 +52,7 @@ class AgentMixin(object):
 
     def _init_ha_conf_path(self):
         ha_full_path = os.path.dirname("/%s/" % self.conf.ha_confs_path)
-        if not os.path.isdir(ha_full_path):
-            os.makedirs(ha_full_path, 0o755)
+        utils.ensure_dir(ha_full_path)
 
     def process_ha_router_added(self, ri):
         ha_port = ri.router.get(l3_constants.HA_INTERFACE_KEY)

diff --git a/neutron/agent/linux/dhcp.py b/neutron/agent/linux/dhcp.py
index 08e3c84b46d4344bd5af8b28e9d6b7e0338d39bd..2378841f837979caf5943a87e49ddc43616f5e23 100644 (file)
--- a/neutron/agent/linux/dhcp.py
+++ b/neutron/agent/linux/dhcp.py
@@ -170,7 +170,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)
-        self._ensure_network_conf_dir()
+        utils.ensure_dir(self.network_conf_dir)
 
     @staticmethod
     def get_confs_dir(conf):
@@ -180,11 +180,6 @@ class DhcpLocalProcess(DhcpBase):
         """Returns the file name for a given kind of config file."""
         return os.path.join(self.network_conf_dir, kind)
 
-    def _ensure_network_conf_dir(self):
-        """Ensures the directory for configuration files is created."""
-        if not os.path.isdir(self.network_conf_dir):
-            os.makedirs(self.network_conf_dir, 0o755)
-
     def _remove_config_files(self):
         shutil.rmtree(self.network_conf_dir, ignore_errors=True)
 
@@ -200,7 +195,7 @@ class DhcpLocalProcess(DhcpBase):
         if self.active:
             self.restart()
         elif self._enable_dhcp():
-            self._ensure_network_conf_dir()
+            utils.ensure_dir(self.network_conf_dir)
             interface_name = self.device_manager.setup(self.network)
             self.interface_name = interface_name
             self.spawn_process()

diff --git a/neutron/agent/linux/keepalived.py b/neutron/agent/linux/keepalived.py
index c448f41ae3599cba88e98e05ba3bacd3e2595d8e..6c18cb0e354c37eed1e0f0d2fd8289371ce7b9eb 100644 (file)
--- a/neutron/agent/linux/keepalived.py
+++ b/neutron/agent/linux/keepalived.py
@@ -351,8 +351,8 @@ class KeepalivedNotifierMixin(object):
 
     def _get_full_config_file_path(self, filename, ensure_conf_dir=True):
         conf_dir = self.get_conf_dir()
-        if ensure_conf_dir and not os.path.isdir(conf_dir):
-            os.makedirs(conf_dir, 0o755)
+        if ensure_conf_dir:
+            utils.ensure_dir(conf_dir)
         return os.path.join(conf_dir, filename)
 
 

diff --git a/neutron/agent/metadata/agent.py b/neutron/agent/metadata/agent.py
index 21ecea3e842d10b80ad860e8f8b3b75c585587cd..53c9f6df51ed8b0441b49e825cf85b461ac1d776 100644 (file)
--- a/neutron/agent/metadata/agent.py
+++ b/neutron/agent/metadata/agent.py
@@ -26,6 +26,7 @@ from oslo_utils import excutils
 import six.moves.urllib.parse as urlparse
 import webob
 
+from neutron.agent.linux import utils as linux_utils
 from neutron.agent import rpc as agent_rpc
 from neutron.common import constants as n_const
 from neutron.common import rpc as n_rpc
@@ -311,7 +312,7 @@ class UnixDomainMetadataProxy(object):
                     if not os.path.exists(cfg.CONF.metadata_proxy_socket):
                         ctxt.reraise = False
         else:
-            os.makedirs(dirname, 0o755)
+            linux_utils.ensure_dir(dirname)
 
         self._init_state_reporting()
 

diff --git a/neutron/tests/unit/agent/linux/test_utils.py b/neutron/tests/unit/agent/linux/test_utils.py
index 79166c558dd2a8e93ef6a3bfb955b3c85439c76a..4f374f357b3c4202990db1d0d13e49748f2234c3 100644 (file)
--- a/neutron/tests/unit/agent/linux/test_utils.py
+++ b/neutron/tests/unit/agent/linux/test_utils.py
@@ -11,6 +11,8 @@
 #    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
+
 import mock
 import testtools
 
@@ -203,3 +205,19 @@ class TestPathUtilities(base.BaseTestCase):
         self.assertFalse(utils.cmdlines_are_equal(
             ['ping', '8.8.8.8'],
             ['/usr/bin/ping6', '8.8.8.8']))
+
+
+class TestBaseOSUtils(base.BaseTestCase):
+    @mock.patch.object(os.path, 'isdir', return_value=False)
+    @mock.patch.object(os, 'makedirs')
+    def test_ensure_dir_not_exist(self, makedirs, isdir):
+        utils.ensure_dir('/the')
+        isdir.assert_called_once_with('/the')
+        makedirs.assert_called_once_with('/the', 0o755)
+
+    @mock.patch.object(os.path, 'isdir', return_value=True)
+    @mock.patch.object(os, 'makedirs')
+    def test_ensure_dir_exist(self, makedirs, isdir):
+        utils.ensure_dir('/the')
+        isdir.assert_called_once_with('/the')
+        self.assertFalse(makedirs.called)

diff --git a/neutron/tests/unit/test_l3_agent.py b/neutron/tests/unit/test_l3_agent.py
index bb7272d83e2d1dd9f7dd8c71d578b2fd35ad02bd..dd34b469b40154d984b16c16ee62d7c435bd4b5c 100644 (file)
--- a/neutron/tests/unit/test_l3_agent.py
+++ b/neutron/tests/unit/test_l3_agent.py
@@ -198,8 +198,9 @@ class BasicRouterOperationsFramework(base.BaseTestCase):
             'neutron.agent.linux.ip_lib.device_exists')
         self.device_exists = self.device_exists_p.start()
 
-        mock.patch('neutron.agent.l3.ha.AgentMixin'
-                   '._init_ha_conf_path').start()
+        self.ensure_dir = mock.patch('neutron.agent.linux.utils'
+                                     '.ensure_dir').start()
+
         mock.patch('neutron.agent.linux.keepalived.KeepalivedNotifierMixin'
                    '._get_full_config_file_path').start()
 
@@ -307,6 +308,11 @@ class BasicRouterOperationsFramework(base.BaseTestCase):
 
 
 class TestBasicRouterOperations(BasicRouterOperationsFramework):
+    def test_init_ha_conf(self):
+        with mock.patch('os.path.dirname', return_value='/etc/ha/'):
+            l3_agent.L3NATAgent(HOSTNAME, self.conf)
+            self.ensure_dir.assert_called_once_with('/etc/ha/')
+
     def test_periodic_sync_routers_task_raise_exception(self):
         agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
         self.plugin_api.get_routers.side_effect = ValueError

diff --git a/neutron/tests/unit/test_linux_dhcp.py b/neutron/tests/unit/test_linux_dhcp.py
index 779bd7108e7b14bd33f2322026d4840ef5555802..b32f11bbd2d54a2cd0a60aa64024a92c4269cc4d 100644 (file)
--- a/neutron/tests/unit/test_linux_dhcp.py
+++ b/neutron/tests/unit/test_linux_dhcp.py
@@ -23,6 +23,7 @@ from neutron.agent.common import config
 from neutron.agent.dhcp import config as dhcp_config
 from neutron.agent.linux import dhcp
 from neutron.agent.linux import external_process
+from neutron.agent.linux import utils
 from neutron.common import config as base_config
 from neutron.common import constants
 from neutron.openstack.common import log as logging
@@ -674,15 +675,11 @@ class TestDhcpLocalProcess(TestBase):
         lp = LocalChild(self.conf, FakeV4Network())
         self.assertEqual(lp.get_conf_file_name('dev'), tpl)
 
-    def test_ensure_network_conf_dir(self):
+    @mock.patch.object(utils, 'ensure_dir')
+    def test_ensure_dir_called(self, ensure_dir):
         LocalChild(self.conf, FakeV4Network())
-        self.makedirs.assert_called_once_with(
-            '/dhcp/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa', mock.ANY)
-
-    def test_ensure_network_conf_existing_dir(self):
-        self.isdir.return_value = True
-        LocalChild(self.conf, FakeV4Network())
-        self.assertFalse(self.makedirs.called)
+        ensure_dir.assert_called_once_with(
+            '/dhcp/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa')
 
     def test_enable_already_active(self):
         with mock.patch.object(LocalChild, 'active') as patched:
@@ -693,16 +690,15 @@ class TestDhcpLocalProcess(TestBase):
             self.assertEqual(lp.called, ['restart'])
             self.assertFalse(self.mock_mgr.return_value.setup.called)
 
-    def test_enable(self):
+    @mock.patch.object(utils, 'ensure_dir')
+    def test_enable(self, ensure_dir):
         attrs_to_mock = dict(
             [(a, mock.DEFAULT) for a in
-                ['active', 'get_conf_file_name', 'interface_name',
-                 '_ensure_network_conf_dir']]
+                ['active', 'interface_name']]
         )
 
         with mock.patch.multiple(LocalChild, **attrs_to_mock) as mocks:
             mocks['active'].__get__ = mock.Mock(return_value=False)
-            mocks['get_conf_file_name'].return_value = '/dir'
             mocks['interface_name'].__set__ = mock.Mock()
             lp = LocalChild(self.conf,
                             FakeDualNetwork())
@@ -713,7 +709,8 @@ class TestDhcpLocalProcess(TestBase):
                  mock.call().setup(mock.ANY)])
             self.assertEqual(lp.called, ['spawn'])
             self.assertTrue(mocks['interface_name'].__set__.called)
-            self.assertTrue(mocks['_ensure_network_conf_dir'].called)
+            ensure_dir.assert_called_with(
+                '/dhcp/cccccccc-cccc-cccc-cccc-cccccccccccc')
 
     def _assert_disabled(self, lp):
         self.assertTrue(lp.process_monitor.unregister.called)

diff --git a/neutron/tests/unit/test_linux_external_process.py b/neutron/tests/unit/test_linux_external_process.py
index 4cf5bf3aa56f2551473de022643807faae3de80b..d6211d53be82fea7927d79233ecf6043146d7a9c 100644 (file)
--- a/neutron/tests/unit/test_linux_external_process.py
+++ b/neutron/tests/unit/test_linux_external_process.py
@@ -16,6 +16,7 @@ import mock
 import os.path
 
 from neutron.agent.linux import external_process as ep
+from neutron.agent.linux import utils
 from neutron.tests import base
 
 
@@ -26,7 +27,8 @@ class TestProcessManager(base.BaseTestCase):
         self.execute = self.execute_p.start()
         self.delete_if_exists = mock.patch(
             'neutron.openstack.common.fileutils.delete_if_exists').start()
-        self.makedirs = mock.patch('os.makedirs').start()
+        self.ensure_dir = mock.patch.object(
+            utils, 'ensure_dir').start()
 
         self.conf = mock.Mock()
         self.conf.external_pids = '/var/path'
@@ -34,7 +36,7 @@ class TestProcessManager(base.BaseTestCase):
     def test_processmanager_ensures_pid_dir(self):
         pid_file = os.path.join(self.conf.external_pids, 'pid')
         ep.ProcessManager(self.conf, 'uuid', pid_file=pid_file)
-        self.makedirs.assert_called_once_with(self.conf.external_pids, 0o755)
+        self.ensure_dir.assert_called_once_with(self.conf.external_pids)
 
     def test_enable_no_namespace(self):
         callback = mock.Mock()

diff --git a/neutron/tests/unit/test_metadata_agent.py b/neutron/tests/unit/test_metadata_agent.py
index b85ce2d6054718282f189fce6f02cd8f3d14c010..27a450a751e65ffd35c8f3d4c282e015c3c1eba5 100644 (file)
--- a/neutron/tests/unit/test_metadata_agent.py
+++ b/neutron/tests/unit/test_metadata_agent.py
@@ -19,6 +19,7 @@ import mock
 import testtools
 import webob
 
+from neutron.agent.linux import utils as linux_utils
 from neutron.agent.metadata import agent
 from neutron.agent import metadata_agent
 from neutron.common import constants
@@ -566,12 +567,10 @@ class TestUnixDomainMetadataProxy(base.BaseTestCase):
         self.cfg.CONF.metadata_workers = 0
         self.cfg.CONF.metadata_backlog = 128
 
-    def test_init_doesnot_exists(self):
-        with mock.patch('os.path.isdir') as isdir:
-            with mock.patch('os.makedirs') as makedirs:
-                isdir.return_value = False
-                agent.UnixDomainMetadataProxy(mock.Mock())
-                makedirs.assert_called_once_with('/the', 0o755)
+    @mock.patch.object(linux_utils, 'ensure_dir')
+    def test_init_doesnot_exists(self, ensure_dir):
+        agent.UnixDomainMetadataProxy(mock.Mock())
+        ensure_dir.assert_called_once_with('/the')
 
     def test_init_exists(self):
         with mock.patch('os.path.isdir') as isdir:
@@ -603,24 +602,21 @@ class TestUnixDomainMetadataProxy(base.BaseTestCase):
                         agent.UnixDomainMetadataProxy(mock.Mock())
                     unlink.assert_called_once_with('/the/path')
 
-    def test_run(self):
-        with mock.patch.object(agent, 'MetadataProxyHandler') as handler:
-            with mock.patch.object(agent, 'UnixDomainWSGIServer') as server:
-                with mock.patch('os.path.isdir') as isdir:
-                    with mock.patch('os.makedirs') as makedirs:
-                        isdir.return_value = False
-
-                        p = agent.UnixDomainMetadataProxy(self.cfg.CONF)
-                        p.run()
-
-                        makedirs.assert_called_once_with('/the', 0o755)
-                        server.assert_has_calls([
-                            mock.call('neutron-metadata-agent'),
-                            mock.call().start(handler.return_value,
-                                              '/the/path', workers=0,
-                                              backlog=128),
-                            mock.call().wait()]
-                        )
+    @mock.patch.object(agent, 'MetadataProxyHandler')
+    @mock.patch.object(agent, 'UnixDomainWSGIServer')
+    @mock.patch.object(linux_utils, 'ensure_dir')
+    def test_run(self, ensure_dir, server, handler):
+        p = agent.UnixDomainMetadataProxy(self.cfg.CONF)
+        p.run()
+
+        ensure_dir.assert_called_once_with('/the')
+        server.assert_has_calls([
+            mock.call('neutron-metadata-agent'),
+            mock.call().start(handler.return_value,
+                              '/the/path', workers=0,
+                              backlog=128),
+            mock.call().wait()]
+        )
 
     def test_main(self):
         with mock.patch.object(agent, 'UnixDomainMetadataProxy') as proxy: