Hi Chris/Mike,
I found and fixed one other issue and updated the patch. Here's the
summary:
- Added support for setting the ownership and permissions for an FCGI socket
- Fixed bug where FCGI socket reference count was not getting decremented on
spawn error
I updated the unit tests to verify the reference count fix.
Thanks,
Roger
On Fri, Oct 23, 2009 at 11:55 AM, Roger Hoover <[email protected]>wrote:
> Thanks, everyone. Attached is the patch with full unit tests.
>
> Chris, I need to update the manual as well for the new fcgi-program
> options, socket_owner and socket_mode. I forget where that is. In another
> svn repo?
>
> Thanks,
>
> Roger
>
>
> On Fri, Oct 23, 2009 at 9:57 AM, Chris McDonough <[email protected]> wrote:
>
>> Roger Hoover wrote:
>>
>>> Hmmm...the more I think about it., socket_owner and socket_mode are
>>> indeed better names. Chris/Mike, do you want to weigh in on consistency vs.
>>> clarity?
>>>
>>
>> I'd be fine with either.
>>
>> - C
>>
>>
>>
>>> On Fri, Oct 23, 2009 at 9:08 AM, Roger Hoover
>>> <[email protected]<mailto:
>>> [email protected]>> wrote:
>>>
>>>
>>>
>>> On Fri, Oct 23, 2009 at 8:57 AM, Grzegorz Nosek <[email protected]
>>> <mailto:[email protected]>> wrote:
>>>
>>> On Fri, Oct 23, 2009 at 08:33:28AM -0700, Roger Hoover wrote:
>>> > I'm most of the way there on #2. The issue is that as far as
>>> I can tell
>>> > there's no way to find out the umask for a user so I don't
>>> automatically
>>> > know what permissions to chmod the FCGI socket with. Now,
>>> the choices are
>>> >
>>> > a) Don't chmod the FCGI socket, just chown it to the uid/gid
>>> of the user the
>>> > process will run as
>>> >
>>> > b) Add socket_chown, socket_chmod args that only apply to
>>> unix domain
>>> > sockets. This allows the most control for the user but the
>>> fact that the
>>> > params don't always make sense is a bit awkward.
>>> >
>>> > [fcgi-program:test]
>>> > command=/foo/bar.fcgi
>>> > socket=unix:///tmp/test.socket
>>> > socket_chown=rhoover:wheel ; this option would only apply to
>>> unix domain
>>> > sockets
>>> > socket_chmod=0777 ; this option would only apply to unix
>>> domain sockets
>>> > user=nobody
>>> > process_name=foo_%(process_num)s
>>> > numprocs=2
>>> >
>>> > Anyone have an opinion here?
>>>
>>> I'm for explicit owner and mode options. Apache-style FastCGI
>>> wrappers
>>> are a pain.
>>>
>>>
>>> Thanks. I was leaning this direction.
>>>
>>>
>>> Also, my vote would go to naming these options "socket_owner" and
>>> "socket_mode" (or "socket_perm(s)"?) as I've heard enough of
>>> "setting
>>> chmods" in my day ;)
>>>
>>>
>>> That makes sense but I'm going for consistency with the existing
>>> unix_http_server section of the config.
>>> http://supervisord.org/manual/current/configuration.html#unix_http_server
>>>
>>>
>>> Best regards,
>>> Grzegorz Nosek
>>>
>>>
>>>
>>>
>>
>
Index: src/supervisor/options.py
===================================================================
--- src/supervisor/options.py (revision 898)
+++ src/supervisor/options.py (working copy)
@@ -645,6 +645,25 @@
continue
program_name = section.split(':', 1)[1]
priority = integer(get(section, 'priority', 999))
+
+ proc_uid = name_to_uid(get(section, 'user', None))
+
+ socket_owner = get(section, 'socket_owner', None)
+ if socket_owner is not None:
+ try:
+ socket_owner = colon_separated_user_group(socket_owner)
+ except ValueError:
+ raise ValueError('Invalid socket_owner value %s'
+ %
socket_owner)
+
+ socket_mode = get(section, 'socket_mode', None)
+ if socket_mode is not None:
+ try:
+ socket_mode = octal_type(socket_mode)
+ except (TypeError, ValueError):
+ raise ValueError('Invalid socket_mode value %s'
+ % socket_mode)
+
socket = get(section, 'socket', None)
if not socket:
raise ValueError('[%s] section requires a "socket" line' %
@@ -654,7 +673,8 @@
'program_name':program_name}
socket = expand(socket, expansions, 'socket')
try:
- socket_config = self.parse_fcgi_socket(socket)
+ socket_config = self.parse_fcgi_socket(socket, proc_uid,
+ socket_owner, socket_mode)
except ValueError, e:
raise ValueError('%s in [%s] socket' % (str(e), section))
@@ -669,7 +689,7 @@
groups.sort()
return groups
- def parse_fcgi_socket(self, sock):
+ def parse_fcgi_socket(self, sock, proc_uid, socket_owner, socket_mode):
if sock.startswith('unix://'):
path = sock[7:]
#Check it's an absolute path
@@ -677,8 +697,22 @@
raise ValueError("Unix socket path %s is not an absolute path",
path)
path = normalize_path(path)
- return UnixStreamSocketConfig(path)
+
+ if socket_owner is None:
+ uid = os.getuid()
+ if proc_uid is not None and proc_uid != uid:
+ socket_owner = (proc_uid, self.get_gid_for_uid(proc_uid))
+
+ if socket_mode is None:
+ socket_mode = 0700
+
+ return UnixStreamSocketConfig(path, owner=socket_owner,
+ mode=socket_mode)
+ if socket_owner is not None or socket_mode is not None:
+ raise ValueError("socket_owner and socket_mode params should"
+ + " only be used with a Unix domain socket")
+
m = re.match(r'tcp://([^\s:]+):(\d+)$', sock)
if m:
host = m.group(1)
@@ -686,6 +720,11 @@
return InetStreamSocketConfig(host, port)
raise ValueError("Bad socket format %s", sock)
+
+ def get_gid_for_uid(self, uid):
+ import pwd
+ pwrec = pwd.getpwuid(uid)
+ return pwrec[3]
def processes_from_section(self, parser, section, group_name,
klass=None):
Index: src/supervisor/datatypes.py
===================================================================
--- src/supervisor/datatypes.py (revision 898)
+++ src/supervisor/datatypes.py (working copy)
@@ -202,7 +202,7 @@
def addr(self): # pragma: no cover
raise NotImplementedError
- def create(self): # pragma: no cover
+ def create_and_bind(self): # pragma: no cover
raise NotImplementedError
class InetStreamSocketConfig(SocketConfig):
@@ -219,28 +219,65 @@
def addr(self):
return (self.host, self.port)
- def create(self):
+ def create_and_bind(self):
sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
+ sock.bind(self.addr())
return sock
class UnixStreamSocketConfig(SocketConfig):
""" Unix domain socket config helper """
path = None # Unix domain socket path
+ mode = None # Unix permission mode bits for socket
+ owner = None # Tuple (uid, gid) for Unix ownership of socket
+ sock = None # socket object
- def __init__(self, path):
+ def __init__(self, path, **kwargs):
self.path = path
self.url = 'unix://%s' % (path)
+ self.mode = kwargs.get('mode', None)
+ self.owner = kwargs.get('owner', None)
def addr(self):
return self.path
- def create(self):
+ def create_and_bind(self):
if os.path.exists(self.path):
os.unlink(self.path)
sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
+ sock.bind(self.addr())
+ try:
+ self._chown()
+ self._chmod()
+ except:
+ sock.close()
+ os.unlink(self.path)
+ raise
return sock
+
+ def get_mode(self):
+ return self.mode
+
+ def get_owner(self):
+ return self.owner
+
+ def _chmod(self):
+ if self.mode is not None:
+ try:
+ os.chmod(self.path, self.mode)
+ except Exception, e:
+ raise ValueError("Could not change permissions of socket "
+ + "file: %s" % (e))
+
+ def _chown(self):
+ if self.owner is not None:
+ try:
+ os.chown(self.path, self.owner[0], self.owner[1])
+ except Exception, e:
+ raise ValueError("Could not change ownership of socket file: "
+ + "%s" % (e))
+
def colon_separated_user_group(arg):
try:
Index: src/supervisor/tests/test_options.py
===================================================================
--- src/supervisor/tests/test_options.py (revision 898)
+++ src/supervisor/tests/test_options.py (working copy)
@@ -8,6 +8,7 @@
import signal
import shutil
import errno
+from mock import Mock, patch, sentinel
from supervisor.tests.base import DummySupervisor
from supervisor.tests.base import DummyLogger
@@ -784,49 +785,93 @@
config.read_string(text)
instance = self._makeOne()
self.assertRaises(ValueError,instance.process_groups_from_parser,config)
-
+
def test_fcgi_programs_from_parser(self):
from supervisor.options import FastCGIGroupConfig
from supervisor.options import FastCGIProcessConfig
text = lstrip("""\
[fcgi-program:foo]
- socket=unix:///tmp/%(program_name)s.sock
+ socket = unix:///tmp/%(program_name)s.sock
+ socket_owner = testuser:testgroup
+ socket_mode = 0666
process_name = %(program_name)s_%(process_num)s
command = /bin/foo
numprocs = 2
priority = 1
[fcgi-program:bar]
- socket=tcp://localhost:6000
+ socket = unix:///tmp/%(program_name)s.sock
process_name = %(program_name)s_%(process_num)s
command = /bin/bar
+ user = testuser
numprocs = 3
+
+ [fcgi-program:flub]
+ socket = unix:///tmp/%(program_name)s.sock
+ command = /bin/flub
+
+ [fcgi-program:cub]
+ socket = tcp://localhost:6000
+ command = /bin/cub
""")
from supervisor.options import UnhosedConfigParser
config = UnhosedConfigParser()
config.read_string(text)
instance = self._makeOne()
- gconfigs = instance.process_groups_from_parser(config)
- self.assertEqual(len(gconfigs), 2)
+
+ #Patch pwd and grp module functions to give us sentinel
+ #uid/gid values so that the test does not depend on
+ #any specific system users
+ pwd_mock = Mock()
+ pwd_mock.return_value = (None, None, sentinel.uid, sentinel.gid)
+ grp_mock = Mock()
+ grp_mock.return_value = (None, None, sentinel.gid)
+ @patch('pwd.getpwuid', pwd_mock)
+ @patch('pwd.getpwnam', pwd_mock)
+ @patch('grp.getgrnam', grp_mock)
+ def get_process_groups(instance, config):
+ return instance.process_groups_from_parser(config)
- gconfig0 = gconfigs[0]
- self.assertEqual(gconfig0.__class__, FastCGIGroupConfig)
- self.assertEqual(gconfig0.name, 'foo')
- self.assertEqual(gconfig0.priority, 1)
- self.assertEqual(gconfig0.socket_config.url,
- 'unix:///tmp/foo.sock')
- self.assertEqual(len(gconfig0.process_configs), 2)
- self.assertEqual(gconfig0.process_configs[0].__class__,
- FastCGIProcessConfig)
- self.assertEqual(gconfig0.process_configs[1].__class__,
- FastCGIProcessConfig)
+ gconfigs = get_process_groups(instance, config)
+ exp_owner = (sentinel.uid, sentinel.gid)
+
+ self.assertEqual(len(gconfigs), 4)
+
+ gconf_foo = gconfigs[0]
+ self.assertEqual(gconf_foo.__class__, FastCGIGroupConfig)
+ self.assertEqual(gconf_foo.name, 'foo')
+ self.assertEqual(gconf_foo.priority, 1)
+ self.assertEqual(gconf_foo.socket_config.url,
+ 'unix:///tmp/foo.sock')
+ self.assertEqual(exp_owner, gconf_foo.socket_config.get_owner())
+ self.assertEqual(0666, gconf_foo.socket_config.get_mode())
+ self.assertEqual(len(gconf_foo.process_configs), 2)
+ pconfig_foo = gconf_foo.process_configs[0]
+ self.assertEqual(pconfig_foo.__class__, FastCGIProcessConfig)
- gconfig1 = gconfigs[1]
- self.assertEqual(gconfig1.name, 'bar')
- self.assertEqual(gconfig1.priority, 999)
- self.assertEqual(gconfig1.socket_config.url,
+ gconf_bar = gconfigs[2]
+ self.assertEqual(gconf_bar.name, 'bar')
+ self.assertEqual(gconf_bar.priority, 999)
+ self.assertEqual(gconf_bar.socket_config.url,
+ 'unix:///tmp/bar.sock')
+ self.assertEqual(exp_owner, gconf_bar.socket_config.get_owner())
+ self.assertEqual(0700, gconf_bar.socket_config.get_mode())
+ self.assertEqual(len(gconf_bar.process_configs), 3)
+
+ gconf_flub = gconfigs[1]
+ self.assertEqual(gconf_flub.name, 'flub')
+ self.assertEqual(gconf_flub.socket_config.url,
+ 'unix:///tmp/flub.sock')
+ self.assertEqual(None, gconf_flub.socket_config.get_owner())
+ self.assertEqual(0700, gconf_flub.socket_config.get_mode())
+ self.assertEqual(len(gconf_flub.process_configs), 1)
+
+ gconf_cub = gconfigs[3]
+ self.assertEqual(gconf_cub.name, 'cub')
+ self.assertEqual(gconf_cub.socket_config.url,
'tcp://localhost:6000')
- self.assertEqual(len(gconfig1.process_configs), 3)
+ self.assertEqual(len(gconf_cub.process_configs), 1)
+
def test_fcgi_program_no_socket(self):
text = lstrip("""\
@@ -901,6 +946,58 @@
config.read_string(text)
instance = self._makeOne()
self.assertRaises(ValueError,instance.process_groups_from_parser,config)
+
+ def test_fcgi_program_socket_owner_set_for_tcp(self):
+ text = lstrip("""\
+ [fcgi-program:foo]
+ socket=tcp://localhost:8000
+ socket_owner=nobody:nobody
+ command = /bin/foo
+ """)
+ from supervisor.options import UnhosedConfigParser
+ config = UnhosedConfigParser()
+ config.read_string(text)
+ instance = self._makeOne()
+
self.assertRaises(ValueError,instance.process_groups_from_parser,config)
+
+ def test_fcgi_program_socket_mode_set_for_tcp(self):
+ text = lstrip("""\
+ [fcgi-program:foo]
+ socket = tcp://localhost:8000
+ socket_mode = 0777
+ command = /bin/foo
+ """)
+ from supervisor.options import UnhosedConfigParser
+ config = UnhosedConfigParser()
+ config.read_string(text)
+ instance = self._makeOne()
+
self.assertRaises(ValueError,instance.process_groups_from_parser,config)
+
+ def test_fcgi_program_bad_socket_owner(self):
+ text = lstrip("""\
+ [fcgi-program:foo]
+ socket = unix:///tmp/foo.sock
+ socket_owner = sometotaljunkuserthatshouldnobethere
+ command = /bin/foo
+ """)
+ from supervisor.options import UnhosedConfigParser
+ config = UnhosedConfigParser()
+ config.read_string(text)
+ instance = self._makeOne()
+
self.assertRaises(ValueError,instance.process_groups_from_parser,config)
+
+ def test_fcgi_program_bad_socket_mode(self):
+ text = lstrip("""\
+ [fcgi-program:foo]
+ socket = unix:///tmp/foo.sock
+ socket_mode = junk
+ command = /bin/foo
+ """)
+ from supervisor.options import UnhosedConfigParser
+ config = UnhosedConfigParser()
+ config.read_string(text)
+ instance = self._makeOne()
+
self.assertRaises(ValueError,instance.process_groups_from_parser,config)
def test_heterogeneous_process_groups_from_parser(self):
text = lstrip("""\
Index: src/supervisor/tests/test_datatypes.py
===================================================================
--- src/supervisor/tests/test_datatypes.py (revision 898)
+++ src/supervisor/tests/test_datatypes.py (working copy)
@@ -5,6 +5,7 @@
import unittest
import socket
import tempfile
+from mock import Mock, patch, sentinel
from supervisor import datatypes
class DatatypesTest(unittest.TestCase):
@@ -176,7 +177,7 @@
def test_url_rejects_urlparse_unrecognized_scheme_with_path(self):
bad_url = "bad://path"
self.assertRaises(ValueError, datatypes.url, bad_url)
-
+
class InetStreamSocketConfigTests(unittest.TestCase):
def _getTargetClass(self):
return datatypes.InetStreamSocketConfig
@@ -187,6 +188,10 @@
def test_url(self):
conf = self._makeOne('127.0.0.1', 8675)
self.assertEqual(conf.url, 'tcp://127.0.0.1:8675')
+
+ def test___str__(self):
+ cfg = self._makeOne('localhost', 65531)
+ self.assertEqual(str(cfg), 'tcp://localhost:65531')
def test_repr(self):
conf = self._makeOne('127.0.0.1', 8675)
@@ -205,32 +210,32 @@
addr = conf.addr()
self.assertEqual(addr, ('localhost', 5001))
- def test_create(self):
+ def test_create_and_bind(self):
conf = self._makeOne('127.0.0.1', 8675)
- sock = conf.create()
+ sock = conf.create_and_bind()
reuse = sock.getsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR)
self.assertTrue(reuse)
- sock.close
+ self.assertEquals(conf.addr(), sock.getsockname()) #verifies that bind
was called
+ sock.close()
def test_same_urls_are_equal(self):
- conf1 = self._makeOne('localhost', '5001')
- conf2 = self._makeOne('localhost', '5001')
+ conf1 = self._makeOne('localhost', 5001)
+ conf2 = self._makeOne('localhost', 5001)
self.assertTrue(conf1 == conf2)
self.assertFalse(conf1 != conf2)
def test_diff_urls_are_not_equal(self):
- conf1 = self._makeOne('localhost', '5001')
- conf2 = self._makeOne('localhost', '5002')
+ conf1 = self._makeOne('localhost', 5001)
+ conf2 = self._makeOne('localhost', 5002)
self.assertTrue(conf1 != conf2)
self.assertFalse(conf1 == conf2)
def test_diff_objs_are_not_equal(self):
- conf1 = self._makeOne('localhost', '5001')
+ conf1 = self._makeOne('localhost', 5001)
conf2 = 'blah'
self.assertTrue(conf1 != conf2)
- self.assertFalse(conf1 == conf2)
-
-
+ self.assertFalse(conf1 == conf2)
+
class UnixStreamSocketConfigTests(unittest.TestCase):
def _getTargetClass(self):
return datatypes.UnixStreamSocketConfig
@@ -241,6 +246,10 @@
def test_url(self):
conf = self._makeOne('/tmp/foo.sock')
self.assertEqual(conf.url, 'unix:///tmp/foo.sock')
+
+ def test___str__(self):
+ cfg = self._makeOne('foo/bar')
+ self.assertEqual(str(cfg), 'unix://foo/bar')
def test_repr(self):
conf = self._makeOne('/tmp/foo.sock')
@@ -254,14 +263,43 @@
addr = conf.addr()
self.assertEqual(addr, '/tmp/foo.sock')
- def test_create(self):
+ def test_create_and_bind(self):
(tf_fd, tf_name) = tempfile.mkstemp()
- conf = self._makeOne(tf_name)
- os.close(tf_fd)
- sock = conf.create()
- self.assertFalse(os.path.exists(tf_name))
- sock.close
+ owner = (sentinel.uid, sentinel.gid)
+ mode = sentinel.mode
+ conf = self._makeOne(tf_name, owner=owner, mode=mode)
+ #Patch os.chmod and os.chown functions with mocks
+ #objects so that the test does not depend on
+ #any specific system users or permissions
+ chown_mock = Mock()
+ chmod_mock = Mock()
+ @patch('os.chown', chown_mock)
+ @patch('os.chmod', chmod_mock)
+ def call_create_and_bind(conf):
+ return conf.create_and_bind()
+
+ sock = call_create_and_bind(conf)
+ self.assertTrue(os.path.exists(tf_name))
+ self.assertEquals(conf.addr(), sock.getsockname()) #verifies that bind
was called
+ sock.close()
+ self.assertTrue(os.path.exists(tf_name))
+ os.unlink(tf_name)
+ #Verify that os.chown was called with correct args
+ self.assertEquals(1, chown_mock.call_count)
+ path_arg = chown_mock.call_args[0][0]
+ uid_arg = chown_mock.call_args[0][1]
+ gid_arg = chown_mock.call_args[0][2]
+ self.assertEquals(tf_name, path_arg)
+ self.assertEquals(owner[0], uid_arg)
+ self.assertEquals(owner[1], gid_arg)
+ #Verify that os.chmod was called with correct args
+ self.assertEquals(1, chmod_mock.call_count)
+ path_arg = chmod_mock.call_args[0][0]
+ mode_arg = chmod_mock.call_args[0][1]
+ self.assertEquals(tf_name, path_arg)
+ self.assertEquals(mode, mode_arg)
+
def test_same_paths_are_equal(self):
conf1 = self._makeOne('/tmp/foo.sock')
conf2 = self._makeOne('/tmp/foo.sock')
@@ -278,8 +316,8 @@
conf1 = self._makeOne('/tmp/foo.sock')
conf2 = 'blah'
self.assertTrue(conf1 != conf2)
- self.assertFalse(conf1 == conf2)
-
+ self.assertFalse(conf1 == conf2)
+
class RangeCheckedConversionTests(unittest.TestCase):
def _getTargetClass(self):
from supervisor.datatypes import RangeCheckedConversion
@@ -341,72 +379,6 @@
self.assertEqual(addr.family, socket.AF_INET)
self.assertEqual(addr.address, ('localhost', 8080))
-class TestInetStreamSocketConfig(unittest.TestCase):
- def _getTargetClass(self):
- from supervisor.datatypes import InetStreamSocketConfig
- return InetStreamSocketConfig
-
- def _makeOne(self, host, port):
- return self._getTargetClass()(host, port)
-
- def test_addr(self):
- cfg = self._makeOne('localhost', 8080)
- self.assertEqual(cfg.addr(), ('localhost', 8080))
-
- def test_create(self):
- cfg = self._makeOne('localhost', 65531)
- sock = cfg.create()
- sock.close()
-
- def test___str__(self):
- cfg = self._makeOne('localhost', 65531)
- self.assertEqual(str(cfg), 'tcp://localhost:65531')
-
- def test__eq__(self):
- cfg = self._makeOne('localhost', 65531)
- cfg2 = self._makeOne('localhost', 65531)
- self.failUnless(cfg == cfg2)
-
- def test__ne__(self):
- cfg = self._makeOne('localhost', 65531)
- cfg2 = self._makeOne('localhost', 65532)
- self.failUnless(cfg != cfg2)
-
-class TestUnixStreamSocketConfig(unittest.TestCase):
- def _getTargetClass(self):
- from supervisor.datatypes import UnixStreamSocketConfig
- return UnixStreamSocketConfig
-
- def _makeOne(self, path):
- return self._getTargetClass()(path)
-
- def test_addr(self):
- cfg = self._makeOne('/foo/bar')
- self.assertEqual(cfg.addr(), '/foo/bar')
-
- def test_create(self):
- import tempfile
- fn = tempfile.mktemp()
- cfg = self._makeOne(fn)
- sock = cfg.create()
- sock.close()
- if os.path.exists(fn):
- os.unlink(fn)
-
- def test___str__(self):
- cfg = self._makeOne('foo/bar')
- self.assertEqual(str(cfg), 'unix://foo/bar')
-
- def test__eq__(self):
- cfg = self._makeOne('/foo/bar')
- cfg2 = self._makeOne('/foo/bar')
- self.failUnless(cfg == cfg2)
-
- def test__ne__(self):
- cfg = self._makeOne('/foo/bar')
- cfg2 = self._makeOne('/foo/bar2')
- self.failUnless(cfg != cfg2)
-
class TestColonSeparatedUserGroup(unittest.TestCase):
def _callFUT(self, arg):
from supervisor.datatypes import colon_separated_user_group
Index: src/supervisor/tests/base.py
===================================================================
--- src/supervisor/tests/base.py (revision 898)
+++ src/supervisor/tests/base.py (working copy)
@@ -333,7 +333,7 @@
def __ne__(self, other):
return not self.__eq__(other)
- def create(self):
+ def create_and_bind(self):
return DummySocket(self.fd)
class DummySocketManager:
Index: src/supervisor/tests/test_process.py
===================================================================
--- src/supervisor/tests/test_process.py (revision 898)
+++ src/supervisor/tests/test_process.py (working copy)
@@ -4,6 +4,7 @@
import unittest
import sys
import errno
+from mock import Mock, patch_object, sentinel
from supervisor.tests.base import DummyOptions
from supervisor.tests.base import DummyPConfig
@@ -17,6 +18,8 @@
from supervisor.tests.base import DummyFCGIProcessGroup
from supervisor.tests.base import DummySocketManager
+from supervisor.process import Subprocess
+
class SubprocessTests(unittest.TestCase):
def _getTargetClass(self):
from supervisor.process import Subprocess
@@ -1234,7 +1237,61 @@
instance.fcgi_sock = 'hello'
instance.after_finish()
self.assertTrue(instance.fcgi_sock is None)
+
+ #Patch Subprocess.finish() method for this test to verify override
+ @patch_object(Subprocess, 'finish',
Mock(return_value=sentinel.finish_result))
+ def test_finish_override(self):
+ options = DummyOptions()
+ config = DummyPConfig(options, 'good', '/good/filename', uid=1)
+ instance = self._makeOne(config)
+ instance.after_finish = Mock()
+ result = instance.finish(sentinel.pid, sentinel.sts)
+ self.assertEqual(sentinel.finish_result, result,
+ 'FastCGISubprocess.finish() did not pass thru result')
+ self.assertEqual(1, instance.after_finish.call_count,
+ 'FastCGISubprocess.after_finish() not called once')
+ finish_mock = Subprocess.finish
+ self.assertEqual(1, finish_mock.call_count,
+ 'Subprocess.finish() not called once')
+ pid_arg = finish_mock.call_args[0][1]
+ sts_arg = finish_mock.call_args[0][2]
+ self.assertEqual(sentinel.pid, pid_arg,
+ 'Subprocess.finish() pid arg was not passed')
+ self.assertEqual(sentinel.sts, sts_arg,
+ 'Subprocess.finish() sts arg was not passed')
+
+ #Patch Subprocess.spawn() method for this test to verify override
+ @patch_object(Subprocess, 'spawn', Mock(return_value=sentinel.ppid))
+ def test_spawn_override_success(self):
+ options = DummyOptions()
+ config = DummyPConfig(options, 'good', '/good/filename', uid=1)
+ instance = self._makeOne(config)
+ instance.before_spawn = Mock()
+ result = instance.spawn()
+ self.assertEqual(sentinel.ppid, result,
+ 'FastCGISubprocess.spawn() did not pass thru result')
+ self.assertEqual(1, instance.before_spawn.call_count,
+ 'FastCGISubprocess.before_spawn() not called once')
+ spawn_mock = Subprocess.spawn
+ self.assertEqual(1, spawn_mock.call_count,
+ 'Subprocess.spawn() not called once')
+ #Patch Subprocess.spawn() method for this test to verify error handling
+ @patch_object(Subprocess, 'spawn', Mock(return_value=None))
+ def test_spawn_error(self):
+ options = DummyOptions()
+ config = DummyPConfig(options, 'good', '/good/filename', uid=1)
+ instance = self._makeOne(config)
+ instance.before_spawn = Mock()
+ instance.fcgi_sock = 'nuke me on error'
+ result = instance.spawn()
+ self.assertEqual(None, result,
+ 'FastCGISubprocess.spawn() did return None on error')
+ self.assertEqual(1, instance.before_spawn.call_count,
+ 'FastCGISubprocess.before_spawn() not called once')
+ self.assertEqual(None, instance.fcgi_sock,
+ 'FastCGISubprocess.spawn() did not remove sock ref on error')
+
class ProcessGroupBaseTests(unittest.TestCase):
def _getTargetClass(self):
from supervisor.process import ProcessGroupBase
Index: src/supervisor/tests/test_socket_manager.py
===================================================================
--- src/supervisor/tests/test_socket_manager.py (revision 898)
+++ src/supervisor/tests/test_socket_manager.py (working copy)
@@ -174,8 +174,7 @@
sock_manager = self._makeOne(conf)
sock = sock_manager.get_socket()
self.assertTrue(sock_manager.is_prepared())
- self.assertTrue(sock.bind_called)
- self.assertEqual(sock.bind_addr, 'dummy addr')
+ self.assertFalse(sock.bind_called)
self.assertTrue(sock.listen_called)
self.assertEqual(sock.listen_backlog, socket.SOMAXCONN)
self.assertFalse(sock.close_called)
Index: src/supervisor/process.py
===================================================================
--- src/supervisor/process.py (revision 898)
+++ src/supervisor/process.py (working copy)
@@ -579,7 +579,11 @@
Overrides Subprocess.spawn() so we can hook in before it happens
"""
self.before_spawn()
- Subprocess.spawn(self)
+ pid = Subprocess.spawn(self)
+ if pid is None:
+ #Remove object reference to decrement the reference count on error
+ self.fcgi_sock = None
+ return pid
def after_finish(self):
"""
@@ -592,8 +596,9 @@
"""
Overrides Subprocess.finish() so we can hook in after it happens
"""
- Subprocess.finish(self, pid, sts)
+ retval = Subprocess.finish(self, pid, sts)
self.after_finish()
+ return retval
def _prepare_child_fds(self):
"""
Index: src/supervisor/socket_manager.py
===================================================================
--- src/supervisor/socket_manager.py (revision 898)
+++ src/supervisor/socket_manager.py (working copy)
@@ -107,8 +107,7 @@
if not self.prepared:
if self.logger:
self.logger.info('Creating socket %s' % self.socket_config)
- self.socket = self.socket_config.create()
- self.socket.bind(self.socket_config.addr())
+ self.socket = self.socket_config.create_and_bind()
self.socket.listen(socket.SOMAXCONN)
self.prepared = True
Index: setup.py
===================================================================
--- setup.py (revision 898)
+++ setup.py (working copy)
@@ -87,7 +87,7 @@
)],
install_requires = requires,
extras_require = {'iterparse':['cElementTree >= 1.0.2']},
- tests_require = requires,
+ tests_require = requires + ['mock >= 0.5.0'],
include_package_data = True,
zip_safe = False,
namespace_packages = ['supervisor'],
_______________________________________________
Supervisor-users mailing list
[email protected]
http://lists.supervisord.org/mailman/listinfo/supervisor-users
