mutest: new output arg for out-of-order cmd execution

[liambrady]

Adds a new argument, output: bool, for the mutest method step() that allows for a cmd to be executed within a node without waiting for any output. This introduces the ability to effectively run a series of (possibly lengthy) commands in the background while the mutest is allowed to continue executing further steps (some of which may require a previous command to still be in the middle of executing).

This commit also modifies _cmd_status() in base.py to support executing a command without returning output in the first place.

[codecov]

Codecov Report

Attention: Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 59.33%. Comparing base (f0447ca) to head (67b3716).
Report is 60 commits behind head on main.

Files with missing lines	Patch %	Lines
munet/base.py	60.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #39      +/-   ##
==========================================
+ Coverage   58.94%   59.33%   +0.38%     
==========================================
  Files          18       19       +1     
  Lines        5286     5545     +259     
==========================================
+ Hits         3116     3290     +174     
- Misses       2170     2255      +85     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[choppsv1]

This call is expecting to run after the process has completed (one side effect of the p.communicate call above is that it waits until this happens). If we look at _cmd_status_finish you'll see it checks p.returncode as a boolean (expecting 0 to mean success); however, in the not-completed-running case p.returncode will be None so this also looks like success.

[liambrady]

I figured that in the case that an immediate error occurred within the process, then maybe catching it isn't a bad thing. In retrospect though, leaving that up to chance is probably a bad idea and consistency would be preferred so I will probably modify this to skip the _cmd_status_finish entirely.

[choppsv1]

I understand you're looking for a "fire-and-forget" command, I just wonder if this is the right way to do that.

I think that perhaps instead of output the new param should be no_wait=False.

I'm also wondering if we should explicitly set stdout=subprocess.DEVNULL and stderr=subprocess.DEVNULL.

[choppsv1]

In general creating these fire-and-forget processes is messy b/c there's no guarantee they will complete before the test exists (in which case they will probably be killed by the kernel, or have PIPE closed signals or something). For non-mutest uses one uses popen to start processes that you want to run in the background and you get back a p process object that you can kill or wait on later. I don't know if this is the right pattern to use for mutest though -- so maybe your way here is ok.

[choppsv1]

I wonder about the actual use case. Might a new API that runs multiple commands simultaneously and waits for them all to complete would be a cleaner solution (step_multi or step_parallel)? It depends on the problem we're trying to solve I guess.

[liambrady]

The use case I have encountered twice now is the desire to create some sort of traffic generator on a node (or two nodes in the case of setting up a client/server pair situation) and then run a series of tests based on the results (perhaps on separate nodes). While it would probably be feasible to do this with some sort of new waitstep_multi or waitstep_parallel, such would require that API call to be responsible for multiple parallel steps plus multiple waiting steps which seems excessive. I figured that accepting the risk and running a few fire-and-forget cmd through a step (then using match/wait calls as usual but written by the tester to be robust to the state of the background process) was cleanest.

I do agree that letting the kernel kill the processes is a naive solution though, so perhaps the best solution would be to keep track of all fire-and-forget processes in a list and clean it up later when a node is being deleted?