Skip to content

Add create/kill background process#8

Open
mattia-lecci wants to merge 7 commits into
opentap:mainfrom
mattia-lecci:background-process
Open

Add create/kill background process#8
mattia-lecci wants to merge 7 commits into
opentap:mainfrom
mattia-lecci:background-process

Conversation

@mattia-lecci

Copy link
Copy Markdown

The current implementation of the SSH plugin only allows to run comands in place, blocking the flow execution.
While SSH commands can be started in a parallel branch, stopping them is not trivial.

Possible use cases are:

  • monitoring scripts
  • processes with no a-priori end

This pull request addresses this by running processes in nohup mode. In this way, processes can continue running on the remote machine even after the completion of the test step.
The step yields as output the PID of the background process.

Later, whenever the user wants (e.g., after a delay or whenever a set of steps terminate), the background process can be killed.
The target PID is used as input.

Example

  • Background SSH Command with command, e.g., sleep 30
  • Delay 2s
  • Check Background SSH Command (should succeed)
  • Kill Background SSH Command (should succeed)
  • Check Background SSH Command (should fail)

Possible issues

  • The user might want to log outputs from the background command
  • Not sure if the kill logic is robust

@mattia-lecci

Copy link
Copy Markdown
Author

Somewhat related to #1 , although with no logging

@mattia-lecci

Copy link
Copy Markdown
Author

One improvement i would like to add is the following:
In both Kill and Check bg SSH command, i would like InputPid to not just be a generic Input<str>, which in principle could come from any string Output, but rather only allow the selection for Background SSH Command steps. This would greatly improve the user experience when this is used within a very large TapPlan, only allowig the user to select among relevant Outputs.
Is that possible? If so, how?

@mattia-lecci

Copy link
Copy Markdown
Author

Improvements added:

  • created a Pid class to making things cleaner. now input/outputs are using this class instead of a simple string
  • all operations to launch/check/kill background processes are now implemented in SshResource. Test steps simply call its public methods
  • SshResource keeps track of all background processes' PIDs
  • when the resource is closed, all bg processes are killed
    • note: the resource is closed whenever the test plan stops running, not when the ks8400 is closed. this means that starting a background process alone (e.g., F10 on the desired test step) is useless, because the resource is immediately closed and thus the process immediately killed
  • added a "linux only" information, but no runtime checks over the ssh connection are done to check whether the SshResource is running over linux or not

Comment on lines +209 to +214
if (command.ExitStatus == 0)
{
Log.Debug($"Gracefully killed PID={pid}");
BackgroundProcesses.Remove(pid);
return true;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This exit status doesn't check if the background process exited. It checks if the kill command successfully sent the SIGTERM signal to the process. If the process ignores the signal, or doesn't handle it (because it's hanging for example) then kill will still return 0 without exiting the process

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.

2 participants