Merge branches 'travis_jh9.0.1', 'run_command_deadlock', 'run_command_debug', 'keepvars_extra', 'spawner_detail', 'prologue_epilogue' and 'exec_prefix_template_separately' into dev

Author: rkdarst

README.md

@@ -24,6 +24,8 @@ This package formerly included WrapSpawner and ProfilesSpawner, which provide me
 
 ## Batch Spawners
 
+For information on the specific spawners, see [SPAWNERS.md](SPAWNERS.md).
+
 ### Overview
 
 This file contains an abstraction layer for batch job queueing systems (`BatchSpawnerBase`), and implements
@@ -82,6 +84,15 @@ to run Jupyter notebooks on an academic supercomputer cluster.
    c.TorqueSpawner.state_exechost_exp = r'int-\1.mesabi.xyz.edu'
    ```
 
+### Security
+
+Unless otherwise stated for a specific spawner, assume that spawners
+*do* evaluate shell environment for users and thus the [security
+requriemnts of JupyterHub security for untrusted
+users](https://jupyterhub.readthedocs.io/en/stable/reference/websecurity.html)
+are not fulfilled.
+
+
 ## Provide different configurations of BatchSpawner
 
 ### Overview

SPAWNERS.md

@@ -0,0 +1,49 @@
+# Notes on specific spawners
+
+## `TorqueSpawner`
+
+Maintainers:
+
+
+## `MoabSpawner`
+
+Subclass of TorqueSpawner
+
+Maintainers:
+
+
+## `SlurmSpawner`
+
+Maintainers: @rkdarst
+
+This spawner enforces the environment if `srun` is used, which is the
+default.  If you *do* want user environment to be used, set
+`req_srun=''`.
+
+
+## `GridengineSpawner`
+
+Maintainers:
+
+
+## `CondorSpawner`
+
+Maintainers:
+
+
+## `LsfSpawner`
+
+Maintainers:
+
+
+# Checklist for making spawners
+
+- Does your spawner read shell environment before starting?
+
+- Does your spawner send SIGTERM to the jupyterhub-singleuser process
+  before SIGKILL?  It should, so that the process can terminate
+  gracefully.  If you don't see the script end (e.g. you can add `echo
+  "terminated gracefully"` to the end of your script and see it), you
+  should check.  PR #75 might help here, ask the poster to finalize
+  it.
+

batchspawner/batchspawner.py

@@ -143,6 +143,12 @@ def _req_homedir_default(self):
     def _req_keepvars_default(self):
         return ','.join(self.get_env().keys())
 
+    req_keepvars_extra = Unicode(
+        help="Extra environment variables which should be configured, "
+              "added to the defaults in keepvars, "
+              "comma separated list.")
+
+
     batch_script = Unicode('', \
         help="Template for job submission script. Traits on this class named like req_xyz "
              "will be substituted in the template for {xyz} using string.Formatter. "
@@ -164,6 +170,8 @@ def get_req_subvars(self):
         subvars = {}
         for t in reqlist:
             subvars[t[4:]] = getattr(self, t)
+        if subvars.get('keepvars_extra'):
+            subvars['keepvars'] += ',' + subvars['keepvars_extra']
         return subvars
 
     batch_submit_cmd = Unicode('', \
@@ -189,17 +197,20 @@ def run_command(self, cmd, input=None, env=None):
                 # Apparently harmless
                 pass
         proc.stdin.close()
-        out = yield proc.stdout.read_until_close()
-        eout = yield proc.stderr.read_until_close()
+        out, eout = yield [proc.stdout.read_until_close(),
+                           proc.stderr.read_until_close()]
         proc.stdout.close()
         proc.stderr.close()
         eout = eout.decode().strip()
         try:
             err = yield proc.wait_for_exit()
         except CalledProcessError:
             self.log.error("Subprocess returned exitcode %s" % proc.returncode)
+            self.log.error('Stdout:')
+            self.log.error(out)
+            self.log.error('Stderr:')
             self.log.error(eout)
-            raise RuntimeError(eout)
+            raise RuntimeError('{} exit status {}: {}'.format(cmd, proc.returncode, eout))
         if err != 0:
             return err # exit error?
         else:
@@ -215,8 +226,8 @@ def _get_batch_script(self, **subvars):
     @gen.coroutine
     def submit_batch_script(self):
         subvars = self.get_req_subvars()
-        cmd = self.exec_prefix + ' ' + self.batch_submit_cmd
-        cmd = format_template(cmd, **subvars)
+        cmd = ' '.join((format_template(self.exec_prefix, **subvars),
+                        format_template(self.batch_submit_cmd, **subvars)))
         subvars['cmd'] = self.cmd_formatted_for_batch()
         if hasattr(self, 'user_options'):
             subvars.update(self.user_options)
@@ -246,8 +257,8 @@ def read_job_state(self):
             return self.job_status
         subvars = self.get_req_subvars()
         subvars['job_id'] = self.job_id
-        cmd = self.exec_prefix + ' ' + self.batch_query_cmd
-        cmd = format_template(cmd, **subvars)
+        cmd = ' '.join((format_template(self.exec_prefix, **subvars),
+                        format_template(self.batch_query_cmd, **subvars)))
         self.log.debug('Spawner querying job: ' + cmd)
         try:
             out = yield self.run_command(cmd)
@@ -266,8 +277,8 @@ def read_job_state(self):
     def cancel_batch_job(self):
         subvars = self.get_req_subvars()
         subvars['job_id'] = self.job_id
-        cmd = self.exec_prefix + ' ' + self.batch_cancel_cmd
-        cmd = format_template(cmd, **subvars)
+        cmd = ' '.join((format_template(self.exec_prefix, **subvars),
+                        format_template(self.batch_cancel_cmd, **subvars)))
         self.log.info('Cancelling job ' + self.job_id + ': ' + cmd)
         yield self.run_command(cmd)
 
@@ -460,7 +471,9 @@ class TorqueSpawner(BatchSpawnerRegexStates):
 #PBS -v {keepvars}
 #PBS {options}
 
+{prologue}
 {cmd}
+{epilogue}
 """).tag(config=True)
 
     # outputs job id string
@@ -582,7 +595,9 @@ class GridengineSpawner(BatchSpawnerBase):
 #$ -v {keepvars}
 #$ {options}
 
+{prologue}
 {cmd}
+{epilogue}
 """).tag(config=True)
 
     # outputs job id string
@@ -668,7 +683,9 @@ class LsfSpawner(BatchSpawnerBase):
 #BSUB -o {homedir}/.jupyterhub.lsf.out
 #BSUB -e {homedir}/.jupyterhub.lsf.err
 
+{prologue}
 {cmd}
+{epilogue}
 ''').tag(config=True)
 
 

batchspawner/tests/test_spawners.py

@@ -279,9 +279,11 @@ def test_torque(db, io_loop):
         'req_nprocs': '5',
         'req_memory': '5678',
         'req_options': 'some_option_asdf',
+        'req_prologue': 'PROLOGUE',
+        'req_epilogue': 'EPILOGUE',
         }
     batch_script_re_list = [
-        re.compile(r'singleuser_command'),
+        re.compile(r'^PROLOGUE.*^singleuser_command.*^EPILOGUE', re.S|re.M),
         re.compile(r'mem=5678'),
         re.compile(r'ppn=5'),
         re.compile(r'^#PBS some_option_asdf', re.M),
@@ -305,9 +307,11 @@ def test_moab(db, io_loop):
         'req_nprocs': '5',
         'req_memory': '5678',
         'req_options': 'some_option_asdf',
+        'req_prologue': 'PROLOGUE',
+        'req_epilogue': 'EPILOGUE',
         }
     batch_script_re_list = [
-        re.compile(r'singleuser_command'),
+        re.compile(r'^PROLOGUE.*^singleuser_command.*^EPILOGUE', re.S|re.M),
         re.compile(r'mem=5678'),
         re.compile(r'ppn=5'),
         re.compile(r'^#PBS some_option_asdf', re.M),
@@ -332,23 +336,41 @@ def test_slurm(db, io_loop):
         'req_nprocs': '5',
         'req_memory': '5678',
         'req_options': 'some_option_asdf',
+        'req_prologue': 'PROLOGUE',
+        'req_epilogue': 'EPILOGUE',
         }
     batch_script_re_list = [
-        re.compile(r'srun .* singleuser_command', re.X|re.M),
+        re.compile(r'PROLOGUE.*srun singleuser_command.*EPILOGUE', re.S),
         re.compile(r'^#SBATCH \s+ --cpus-per-task=5', re.X|re.M),
         re.compile(r'^#SBATCH \s+ --time=3-05:10:10', re.X|re.M),
         re.compile(r'^#SBATCH \s+ some_option_asdf', re.X|re.M),
         ]
-    script = [
+    from .. import SlurmSpawner
+    run_spawner_script(db, io_loop, SlurmSpawner, normal_slurm_script,
+                       batch_script_re_list=batch_script_re_list,
+                       spawner_kwargs=spawner_kwargs)
+# We tend to use slurm as our typical example job.  These allow quick
+# Slurm examples.
+normal_slurm_script = [
         (re.compile(r'sudo.*sbatch'),   str(testjob)),
         (re.compile(r'sudo.*squeue'),   'PENDING '),          # pending
         (re.compile(r'sudo.*squeue'),   'RUNNING '+testhost), # running
         (re.compile(r'sudo.*squeue'),   'RUNNING '+testhost),
         (re.compile(r'sudo.*scancel'),  'STOP'),
         (re.compile(r'sudo.*squeue'),   ''),
         ]
-    from .. import SlurmSpawner
-    run_spawner_script(db, io_loop, SlurmSpawner, script,
+from .. import SlurmSpawner
+def run_typical_slurm_spawner(db, io_loop,
+        spawner=SlurmSpawner,
+        script=normal_slurm_script,
+        batch_script_re_list=None,
+        spawner_kwargs={}):
+    """Run a full slurm job with default (overrideable) parameters.
+
+    This is useful, for example, for changing options and testing effect
+    of batch scripts.
+    """
+    return run_spawner_script(db, io_loop, spawner, script,
                        batch_script_re_list=batch_script_re_list,
                        spawner_kwargs=spawner_kwargs)
 
@@ -407,9 +429,11 @@ def test_lfs(db, io_loop):
         'req_memory': '5678',
         'req_options': 'some_option_asdf',
         'req_queue': 'some_queue',
+        'req_prologue': 'PROLOGUE',
+        'req_epilogue': 'EPILOGUE',
         }
     batch_script_re_list = [
-        re.compile(r'^singleuser_command', re.M),
+        re.compile(r'^PROLOGUE.*^singleuser_command.*^EPILOGUE', re.S|re.M),
         re.compile(r'#BSUB\s+-q\s+some_queue', re.M),
         ]
     script = [
@@ -424,3 +448,28 @@ def test_lfs(db, io_loop):
     run_spawner_script(db, io_loop, LsfSpawner, script,
                        batch_script_re_list=batch_script_re_list,
                        spawner_kwargs=spawner_kwargs)
+
+
+def test_keepvars(db, io_loop):
+    # req_keepvars
+    spawner_kwargs = {
+        'req_keepvars': 'ABCDE',
+        }
+    batch_script_re_list = [
+        re.compile(r'--export=ABCDE', re.X|re.M),
+        ]
+    run_typical_slurm_spawner(db, io_loop,
+                              spawner_kwargs=spawner_kwargs,
+                              batch_script_re_list=batch_script_re_list)
+
+    # req_keepvars AND req_keepvars together
+    spawner_kwargs = {
+        'req_keepvars': 'ABCDE',
+        'req_keepvars_extra': 'XYZ',
+        }
+    batch_script_re_list = [
+        re.compile(r'--export=ABCDE,XYZ', re.X|re.M),
+        ]
+    run_typical_slurm_spawner(db, io_loop,
+                              spawner_kwargs=spawner_kwargs,
+                              batch_script_re_list=batch_script_re_list)