broker: emit error when running interactive shell without tty #4253
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Problem: Some tests need to run under a tty but still want stdin closed after starting, e.g. if running an interactive shell. If --input=none in runpty.py, then immediately send eof (^D or 0x04) to the child, which effectively closes stdin. This is as a convenience over doing the equivalent using an asciicast file as input with embedded eof. E.g.: runpty.py --input=none cat will exit instead of hanging.
Problem: Python has issues with utf-8 sequences when LC_ALL=C is set in the environment (which it is for the testsuite). Even when we're very careful about encoding data with utf-8 and errors=replace, when the data is written to stdout/err, Python will senselessly raise an exception and abort. Re-open stdout and stderr with encoding="utf-8" and errors="surrogateescape" to work around this problem, even when the current LANG is set to C.
Problem: The test 'default initial program in $SHELL' in t0014-runlevel.t executes the interactive shell without a tty, which may break in future flux-core versions that issue an error in this situation. Run the test under runpty.py to avoid the potential failure. Use the runpty --input=none option instead of redirecting from /dev/null. In case of a hang due to problems with sending EOF over the pty, also execute the test under run_timeout(), adding SHELL=/bin/sh to the --env option of this program. Since the runtpy.py script must be able to load the flux Python module, execute this script under flux(1) so that paths are set up correctly, otherwise the Python modules may not be found.
Problem: A test in t2212-job-manager-plugins.t runs an interactive shell. This works even without a tty since the test expects a failure, but soon the broker may exit with a different error in this situation, breaking the test. Add /bin/true to the arguments in the test so that the shell is no longer interactive. This doesn't change the semantics of the test, but will prevent future failures if the broker checks for a tty.
Problem: It is a common folly to run `flux start` via flux-mini run or foreign RM launcher and forget to allocate a pty to the job. This can waste time as a user waits for a shell prompt that will never come. Since it is almost guaranteed that running an interactive shell without a pty is a mistake, catch this condition in the broker and emit a useful error message and exit. Now, `flux mini run flux start` without `-o pty` or `-o pty.interactive` will result in the error: flux-broker: stdin is not a tty - can't run interactive shell flux-job: task(s) exited with exit code 1 Fixes flux-framework#2032
Problem: No tests check that launching an interactive shell without a tty generates an error. Add a test to t0014-runlevel.t that ensures launching an interactive session without a tty results in an error.
garlick
approved these changes
Mar 31, 2022
|
Thanks, and no problem! |
|
Can I run it without interactive? E.g., I'm trying to run flux in a container base on github actions and I'm seeing this error - I can't figure out a way to disable or, or to run flux (and test with snakemake) with needing to start flux. |
|
@vsoch: Yes, as discussed offline, this check and error for an interactive tty only occurs when there are no arguments provided to You can run without a tty by providing any argument to |
|
For future us, the workflow that worked wound up being: - name: Start Flux and Test Workflow
run: |
su fluxuser
cd examples/flux
which snakemake
flux start snakemake --flux --jobs=1 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While going through some old issues I found one that seemed easy and valuable to fix
This PR has the broker check for a tty on stdin when invoking an interactive shell. If there is no tty the broker exits with an error:
This prevents wasting time when you think an interactive job is taking a long time to start, but really you've forgotten
-o pty/-o pty.interactiveor the--ptyoption withsrun.There was a little test fallout from this one, and most of my time was actually spent trying to fix Python encoding errors in the
runptytest script (as well as working around timing issues getting EOF to the shell in the one case that is required for testing)