diff --git a/neutron/agent/dhcp/agent.py b/neutron/agent/dhcp/agent.py index 0d6624d0237..613d7fcbc76 100644 --- a/neutron/agent/dhcp/agent.py +++ b/neutron/agent/dhcp/agent.py @@ -25,6 +25,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 86b7169f328..cf693e39694 100644 --- a/neutron/agent/l3/ha.py +++ b/neutron/agent/l3/ha.py @@ -19,6 +19,7 @@ from oslo_config import cfg from oslo_log import log as logging 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 @@ -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 1b0e0e64dd4..53752a8a319 100644 --- 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 8164af0b094..04fba57b61d 100644 --- 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 22ed9f3fe2f..44ea9b062e2 100644 --- a/neutron/agent/metadata/agent.py +++ b/neutron/agent/metadata/agent.py @@ -28,6 +28,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 @@ -312,7 +313,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 79166c558dd..4f374f357b3 100644 --- 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 026f4e649e6..8a9415a234e 100644 --- a/neutron/tests/unit/test_l3_agent.py +++ b/neutron/tests/unit/test_l3_agent.py @@ -197,8 +197,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() @@ -306,6 +307,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 62de4ef9269..e4f943b9b6f 100644 --- a/neutron/tests/unit/test_linux_dhcp.py +++ b/neutron/tests/unit/test_linux_dhcp.py @@ -24,6 +24,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.tests import base @@ -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 4cf5bf3aa56..d6211d53be8 100644 --- 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 b85ce2d6054..27a450a751e 100644 --- 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 + @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() - 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()] - ) + 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: