Correct environment= docs, add test for actual behavior

It was observed in Supervisor#550 that when environment= is set in the
[supervisord] section of the config file, that the environment
of the supervisord process itself was never actually changed.
This was contrary to the documentation, which said it would be.
It has been this way for a very long time (always?).

Now the documentation has been corrected to indicate that
environment= in the [supervisord] section will be inherited by
all subprocesses, but will not actually change the environment
of supervisord itself.  This may actually be safer, as changing
some environment variables such as PYTHONPATH could unexpectedly
affect the execution of supervisord (unsure).

Closes Supervisor#550

Author: mnaberez

docs/configuration.rst

@@ -445,8 +445,8 @@ follows.
 ``environment``
 
   A list of key/value pairs in the form ``KEY="val",KEY2="val2"`` that
-  will be placed in the :program:`supervisord` process' environment
-  (and as a result in all of its child process' environments).  This
+  will be placed in the environment of all child processes.  This does
+  not change the environment of :program:`supervisord` itself.  This
   option can include the value ``%(here)s``, which expands to the
   directory in which the supervisord configuration file was found.
   Values containing non-alphanumeric characters should be quoted

supervisor/options.py

@@ -406,7 +406,6 @@ class ServerOptions(Options):
     passwdfile = None
     nodaemon = None
     silent = None
-    environment = None
     httpservers = ()
     unlink_pidfile = False
     unlink_socketfiles = False
@@ -1584,9 +1583,6 @@ def readfd(self, fd):
             data = b''
         return data
 
-    def process_environment(self):
-        os.environ.update(self.environment or {})
-
     def chdir(self, dir):
         os.chdir(dir)
 

supervisor/supervisord.py

@@ -84,7 +84,6 @@ def run(self):
         try:
             for config in self.options.process_group_configs:
                 self.add_process_group(config)
-            self.options.process_environment()
             self.options.openhttpservers(self)
             self.options.setsignals()
             if (not self.options.nodaemon) and self.options.first:

supervisor/tests/base.py

@@ -77,7 +77,6 @@ def __init__(self):
         self.setuid_msg = None
         self.privsdropped = None
         self.logs_reopened = False
-        self.environment_processed = False
         self.write_accept = None
         self.tempfile_name = '/foo/bar'
         self.removed = []
@@ -232,9 +231,6 @@ def readfd(self, fd):
     def reopenlogs(self):
         self.logs_reopened = True
 
-    def process_environment(self):
-        self.environment_processed = True
-
     def mktempfile(self, prefix, suffix, dir):
         return self.tempfile_name
 

supervisor/tests/fixtures/issue-550.conf

@@ -0,0 +1,21 @@
+[supervisord]
+loglevel=info                ; log level; default info; others: debug,warn,trace
+logfile=/tmp/issue-550.log   ; main log file; default $CWD/supervisord.log
+pidfile=/tmp/issue-550.pid   ; supervisord pidfile; default supervisord.pid
+nodaemon=true                ; start in foreground if true; default false
+environment=THIS_SHOULD=BE_IN_CHILD_ENV
+
+[rpcinterface:supervisor]
+supervisor.rpcinterface_factory = supervisor.rpcinterface:make_main_rpcinterface
+
+[unix_http_server]
+file=/tmp/issue-550.sock     ; the path to the socket file
+
+[supervisorctl]
+serverurl=unix:///tmp/issue-550.sock ; use a unix:// URL  for a unix socket
+
+[program:print_env]
+command=python %(here)s/print_env.py
+startsecs=0
+startretries=0
+autorestart=false

supervisor/tests/fixtures/print_env.py

@@ -0,0 +1,5 @@
+#!<<PYTHON>>
+import os
+
+for k, v in os.environ.items():
+    print("%s=%s" % (k,v))

supervisor/tests/test_end_to_end.py

@@ -20,6 +20,19 @@
 
 class EndToEndTests(BaseTestCase):
 
+    def test_issue_550(self):
+        filename = pkg_resources.resource_filename(__name__, 'fixtures/issue-550.conf')
+        args = ['-m', 'supervisor.supervisord', '-c', filename]
+        supervisord = pexpect.spawn(sys.executable, args, encoding='utf-8')
+        self.addCleanup(supervisord.kill, signal.SIGINT)
+        supervisord.expect_exact('success: print_env entered RUNNING state')
+
+        args = ['-m', 'supervisor.supervisorctl', '-c', filename, 'tail -100000', 'print_env']
+        supervisorctl = pexpect.spawn(sys.executable, args, encoding='utf-8')
+        self.addCleanup(supervisorctl.kill, signal.SIGINT)
+        supervisorctl.expect_exact("THIS_SHOULD=BE_IN_CHILD_ENV")
+        supervisorctl.expect(pexpect.EOF)
+
     def test_issue_565(self):
         filename = pkg_resources.resource_filename(__name__, 'fixtures/issue-565.conf')
         args = ['-m', 'supervisor.supervisord', '-c', filename]

supervisor/tests/test_supervisord.py

@@ -132,7 +132,6 @@ def test_main_first(self):
         options.first = True
         supervisord = self._makeOne(options)
         supervisord.main()
-        self.assertEqual(options.environment_processed, True)
         self.assertEqual(options.fds_cleaned_up, False)
         self.assertEqual(options.rlimits_set, True)
         self.assertEqual(options.parse_criticals, ['setuid_called'])
@@ -142,7 +141,6 @@ def test_main_first(self):
         self.assertEqual(len(supervisord.process_groups), 1)
         self.assertEqual(supervisord.process_groups['foo'].config.options,
                          options)
-        self.assertEqual(options.environment_processed, True)
         self.assertEqual(options.httpservers_opened, True)
         self.assertEqual(options.signals_set, True)
         self.assertEqual(options.daemonized, True)
@@ -158,7 +156,6 @@ def test_main_notfirst(self):
         options.first = False
         supervisord = self._makeOne(options)
         supervisord.main()
-        self.assertEqual(options.environment_processed, True)
         self.assertEqual(options.fds_cleaned_up, True)
         self.assertFalse(hasattr(options, 'rlimits_set'))
         self.assertEqual(options.parse_criticals, ['setuid_called'])
@@ -168,7 +165,6 @@ def test_main_notfirst(self):
         self.assertEqual(len(supervisord.process_groups), 1)
         self.assertEqual(supervisord.process_groups['foo'].config.options,
                          options)
-        self.assertEqual(options.environment_processed, True)
         self.assertEqual(options.httpservers_opened, True)
         self.assertEqual(options.signals_set, True)
         self.assertEqual(options.daemonized, False)