Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

flux-jobs: filter against all users with --include #6646

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

chu11
Copy link
Member

@chu11 chu11 commented Feb 14, 2025

Problem: The --include option is predominantly used for finding jobs on specific hosts. This implies that the -A option is set (i.e.
filter on all user jobs) but it is currently not. This can be confusing to users wondering why --include isn't finding any jobs running on specific hosts.

Solution: If the user did not specify filtering on a specific user with --user, then assume all user jobs (i.e. -A or --user=all) will be checked when --include is specified.


side note, the 1 tricky part was how to tell if the user specified --user=X on the command line so we didn't assume "-A" when --include was specified. I setup a new filter action that sets a argparse flag "filtereduser". There were probably other ways of doing this, but went with this approach.

Problem: A number of the user filtering options for flux jobs are
not tested because different users submitting jobs was not easy to
test in earlier builds of flux.

Today we can use job-exec's testexec to test different user job submissions.

Solution: Test flux-jobs user filtering in a new test file
t2800-jobs-cmd-multiuser.t.
Problem: The --include option is predominantly used for finding
jobs on specific hosts.  This implies that the -A option is set (i.e.
filter on all user jobs) but it is currently not.  This can be
confusing to users wondering why --include isn't finding any jobs
running on specific hosts.

Solution: If the user did not specify filtering on a specific user
with --user, then assume all user jobs (i.e. -A or --user=all) will
be checked when --include is specified.

Fixes flux-framework#6585
Problem: The --include option in flux-jobs(1) implies that -A (all
user jobs) will be used.  But this is not documented.

Add documentation in flux-jobs(1) that -A is also assumed when
--include is used.
Problem: There are no tests to cover the flux-jobs --include
filtering with multiple user job submissions.

Add some coverage to t2800-jobs-cmd-multiuser.t.
Copy link

codecov bot commented Feb 14, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.86%. Comparing base (9029a38) to head (e8e44e1).
Report is 26 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6646      +/-   ##
==========================================
+ Coverage   83.84%   83.86%   +0.01%     
==========================================
  Files         533      533              
  Lines       88330    88338       +8     
==========================================
+ Hits        74064    74083      +19     
+ Misses      14266    14255      -11     
Files with missing lines Coverage Δ
src/bindings/python/flux/util.py 96.58% <100.00%> (+0.02%) ⬆️
src/cmd/flux-jobs.py 96.58% <100.00%> (+0.04%) ⬆️

... and 7 files with indirect coverage changes

@chu11
Copy link
Member Author

chu11 commented Feb 14, 2025

Had one builder fail with an error i've never seen before

  expecting success: 
  	test_must_fail_or_be_terminated flux start \
  		-Stbon.interface-hint=badiface \
  		${ARGS} -s2 -Stbon.prefertcp=1 true
  
  flux-broker: could not find address associated with badiface
  flux-start: 0 (pid 470505) exited with rc=1
  flux-start: 0: PMI_Abort(): fatal bootstrap error
  test_must_fail_or_be_terminated: died by signal 13: flux start -Stbon.interface-hint=badiface -Sbroker.rc1_path= -Sbroker.rc3_path= -s2 -Stbon.prefertcp=1 true
  not ok 12 - tbon.interface-hint=badiface fails

hmmm signal 13 is SIGPIPE. That's not good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant