]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Fix pid file location to avoid I->J changes that break metadata
authorMiguel Angel Ajo <mangelajo@redhat.com>
Wed, 1 Oct 2014 13:16:19 +0000 (15:16 +0200)
committerarmando-migliaccio <armamig@gmail.com>
Wed, 1 Oct 2014 23:31:27 +0000 (16:31 -0700)
Changes in commit 7f8ae630b87392193974dd9cb198c1165cdec93b moved
pid files handled by agent/linux/external_process.py from
$state_path/external/<uuid>.pid  to $state_path/external/<uuid>/pid
that breaks the neutron-ns-metadata-proxy respawn after upgrades
becase the l3 or dhcp agent can't find the old pid file so
they try to start a new neutron-ns-metadata-proxy which won't
succeed, because the old one is holding the port already.

Closes-Bug: #1376128
Change-Id: Id166ec8e508aaab8eea35d89d010a5a0b7fdba1f

neutron/agent/linux/utils.py
neutron/tests/unit/test_linux_external_process.py

index e0a66f843ca1a2b2fe0bab19dbe79508cbac2789..8243468919a84edbebcb60a6dff865b8a517fa03 100644 (file)
@@ -14,9 +14,9 @@
 #    under the License.
 
 import fcntl
+import glob
 import os
 import shlex
-import shutil
 import socket
 import struct
 import tempfile
@@ -134,19 +134,19 @@ def find_child_pids(pid):
     return [x.strip() for x in raw_pids.split('\n') if x.strip()]
 
 
-def _get_conf_dir(cfg_root, uuid, ensure_conf_dir):
-    confs_dir = os.path.abspath(os.path.normpath(cfg_root))
-    conf_dir = os.path.join(confs_dir, uuid)
+def _get_conf_base(cfg_root, uuid, ensure_conf_dir):
+    conf_dir = os.path.abspath(os.path.normpath(cfg_root))
+    conf_base = os.path.join(conf_dir, uuid)
     if ensure_conf_dir:
         if not os.path.isdir(conf_dir):
             os.makedirs(conf_dir, 0o755)
-    return conf_dir
+    return conf_base
 
 
 def get_conf_file_name(cfg_root, uuid, cfg_file, ensure_conf_dir=False):
     """Returns the file name for a given kind of config file."""
-    conf_dir = _get_conf_dir(cfg_root, uuid, ensure_conf_dir)
-    return os.path.join(conf_dir, cfg_file)
+    conf_base = _get_conf_base(cfg_root, uuid, ensure_conf_dir)
+    return "%s.%s" % (conf_base, cfg_file)
 
 
 def get_value_from_conf_file(cfg_root, uuid, cfg_file, converter=None):
@@ -168,15 +168,13 @@ def get_value_from_conf_file(cfg_root, uuid, cfg_file, converter=None):
 
 
 def remove_conf_files(cfg_root, uuid):
-    conf_dir = _get_conf_dir(cfg_root, uuid, False)
-    shutil.rmtree(conf_dir, ignore_errors=True)
+    conf_base = _get_conf_base(cfg_root, uuid, False)
+    for file_path in glob.iglob("%s.*" % conf_base):
+        os.unlink(file_path)
 
 
 def remove_conf_file(cfg_root, uuid, cfg_file):
-    """Remove a config file. Remove the directory if this is the last file."""
+    """Remove a config file."""
     conf_file = get_conf_file_name(cfg_root, uuid, cfg_file)
     if os.path.exists(conf_file):
         os.unlink(conf_file)
-        conf_dir = _get_conf_dir(cfg_root, uuid, False)
-        if not os.listdir(conf_dir):
-            shutil.rmtree(conf_dir, ignore_errors=True)
index 88f4bc075bcd12ef6de34e1765dd5382ea39398b..b16b681e9309cb1435e203c8d4c0ca2e7ed7610a 100644 (file)
@@ -123,7 +123,7 @@ class TestProcessManager(base.BaseTestCase):
             isdir.return_value = True
             manager = ep.ProcessManager(self.conf, 'uuid')
             retval = manager.get_pid_file_name(ensure_pids_dir=True)
-            self.assertEqual(retval, '/var/path/uuid/pid')
+            self.assertEqual(retval, '/var/path/uuid.pid')
 
     def test_get_pid_file_name_not_existing(self):
         with mock.patch.object(ep.utils.os.path, 'isdir') as isdir:
@@ -131,15 +131,15 @@ class TestProcessManager(base.BaseTestCase):
                 isdir.return_value = False
                 manager = ep.ProcessManager(self.conf, 'uuid')
                 retval = manager.get_pid_file_name(ensure_pids_dir=True)
-                self.assertEqual(retval, '/var/path/uuid/pid')
-                makedirs.assert_called_once_with('/var/path/uuid', 0o755)
+                self.assertEqual(retval, '/var/path/uuid.pid')
+                makedirs.assert_called_once_with('/var/path', 0o755)
 
     def test_get_pid_file_name_default(self):
         with mock.patch.object(ep.utils.os.path, 'isdir') as isdir:
             isdir.return_value = True
             manager = ep.ProcessManager(self.conf, 'uuid')
             retval = manager.get_pid_file_name(ensure_pids_dir=False)
-            self.assertEqual(retval, '/var/path/uuid/pid')
+            self.assertEqual(retval, '/var/path/uuid.pid')
             self.assertFalse(isdir.called)
 
     def test_pid(self):