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

Terminate process tree on ctrl+c #213

Merged
merged 9 commits into from
Oct 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
"./osinfo/Cargo.toml",
"./pal/Cargo.toml",
"./registry/Cargo.toml",
"./test_group_resource/Cargo.toml",
"./tools/test_group_resource/Cargo.toml",
"./tools/dsctest/Cargo.toml",
"./y2j/Cargo.toml"
],
"rust-analyzer.showUnlinkedFileNotification": true,
Expand Down
29 changes: 18 additions & 11 deletions build.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,9 @@ if (Test-Path $target) {
}
New-Item -ItemType Directory $target > $null

# make sure dependencies are built first so clippy runs correctly
$windows_projects = @("pal", "ntreg", "ntstatuserror", "ntuserinfo", "registry")
$projects = @("dsc_lib", "dsc", "osinfo", "process", "test_group_resource", "y2j", "powershellgroup")
$projects = @("dsc_lib", "dsc", "osinfo", "process", "tools/test_group_resource", "y2j", "powershellgroup", "tools/dsctest")
$pedantic_unclean_projects = @("ntreg")

if ($IsWindows) {
Expand Down Expand Up @@ -126,11 +127,13 @@ foreach ($project in $projects) {
$failed = $true
}

$binary = Split-Path $project -Leaf

if ($IsWindows) {
Copy-Item "$path/$project.exe" $target -ErrorAction Ignore
Copy-Item "$path/$binary.exe" $target -ErrorAction Ignore
}
else {
Copy-Item "$path/$project" $target -ErrorAction Ignore
Copy-Item "$path/$binary" $target -ErrorAction Ignore
}

Copy-Item "*.dsc.resource.json" $target -Force -ErrorAction Ignore
Expand All @@ -152,6 +155,16 @@ Copy-Item $PSScriptRoot/tools/add-path.ps1 $target -Force -ErrorAction Ignore
$relative = Resolve-Path $target -Relative
Write-Host -ForegroundColor Green "`nEXE's are copied to $target ($relative)"

# remove the other target in case switching between them
$dirSeparator = [System.IO.Path]::DirectorySeparatorChar
if ($Release) {
$oldTarget = $target.Replace($dirSeparator + 'release', $dirSeparator + 'debug')
}
else {
$oldTarget = $target.Replace($dirSeparator + 'debug', $dirSeparator + 'release')
}
$env:PATH = $env:PATH.Replace($oldTarget, '')

$paths = $env:PATH.Split([System.IO.Path]::PathSeparator)
$found = $false
foreach ($path in $paths) {
Expand All @@ -161,14 +174,8 @@ foreach ($path in $paths) {
}
}

# remove the other target in case switching between them
if ($Release) {
$oldTarget = $target.Replace('\release', '\debug')
}
else {
$oldTarget = $target.Replace('\debug', '\release')
}
$env:PATH = $env:PATH.Replace(';' + $oldTarget, '')
# remove empty entries from path
$env:PATH = [string]::Join([System.IO.Path]::PathSeparator, $env:PATH.Split([System.IO.Path]::PathSeparator, [StringSplitOptions]::RemoveEmptyEntries))

if (!$found) {
Write-Host -ForegroundCOlor Yellow "Adding $target to `$env:PATH"
Expand Down
3 changes: 2 additions & 1 deletion dsc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ lto = true
atty = { version = "0.2" }
clap = { version = "4.1", features = ["derive"] }
clap_complete = { version = "4.4" }
crossterm = { version = "0.26.1" }
crossterm = { version = "0.27" }
ctrlc = { version = "3.4.0" }
dsc_lib = { path = "../dsc_lib" }
jsonschema = "0.17"
Expand All @@ -23,6 +23,7 @@ serde = { version = "1.0", features = ["derive"] }
serde_json = { version = "1.0", features = ["preserve_order"] }
serde_yaml = { version = "0.9" }
syntect = { version = "5.0", features = ["default-fancy"], default-features = false }
sysinfo = { version = "0.29.10" }
thiserror = "1.0"
tracing = "0.1.37"
tracing-subscriber = "0.3.17"
36 changes: 33 additions & 3 deletions dsc/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ use clap::{CommandFactory, Parser};
use clap_complete::generate;
use std::io::{self, Read};
use std::process::exit;
use tracing::{error, info};
use sysinfo::{Process, ProcessExt, RefreshKind, System, SystemExt, get_current_pid, ProcessRefreshKind};
use tracing::{error, info, warn};

#[cfg(debug_assertions)]
use crossterm::event;
Expand All @@ -26,7 +27,9 @@ fn main() {

// create subscriber that writes all events to stderr
let subscriber = tracing_subscriber::fmt().pretty().with_writer(std::io::stderr).finish();
let _ = tracing::subscriber::set_global_default(subscriber).map_err(|_err| eprintln!("Unable to set global default subscriber"));
if tracing::subscriber::set_global_default(subscriber).is_err() {
eprintln!("Unable to set global default subscriber");
}

if ctrlc::set_handler(ctrlc_handler).is_err() {
error!("Error: Failed to set Ctrl-C handler");
Expand Down Expand Up @@ -78,10 +81,37 @@ fn main() {
}

fn ctrlc_handler() {
error!("Ctrl-C received");
warn!("Ctrl-C received");

// get process tree for current process and terminate all processes
let sys = System::new_with_specifics(RefreshKind::new().with_processes(ProcessRefreshKind::new()));
info!("Found {} processes", sys.processes().len());
let Ok(current_pid) = get_current_pid() else {
error!("Could not get current process id");
exit(util::EXIT_CTRL_C);
};
info!("Current process id: {}", current_pid);
let Some(current_process) = sys.process(current_pid) else {
error!("Could not get current process");
exit(util::EXIT_CTRL_C);
};

terminate_subprocesses(&sys, current_process);
exit(util::EXIT_CTRL_C);
}

fn terminate_subprocesses(sys: &System, process: &Process) {
info!("Terminating subprocesses of process {} {}", process.name(), process.pid());
for subprocess in sys.processes().values().filter(|p| p.parent().map_or(false, |parent| parent == process.pid())) {
terminate_subprocesses(sys, subprocess);
}

info!("Terminating process {} {}", process.name(), process.pid());
if !process.kill() {
error!("Failed to terminate process {} {}", process.name(), process.pid());
}
}

#[cfg(debug_assertions)]
fn check_debug() {
if env::var("DEBUG_DSC").is_ok() {
Expand Down
88 changes: 68 additions & 20 deletions dsc_lib/src/dscresources/command_resource.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use serde_json::Value;
use std::{collections::HashMap, process::Command, io::{Write, Read}, process::Stdio};
use crate::dscerror::DscError;
use super::{dscresource::get_diff,resource_manifest::{ResourceManifest, InputKind, ReturnKind, SchemaKind}, invoke_result::{GetResult, SetResult, TestResult, ValidateResult, ExportResult}};
use tracing::debug;
use tracing::{debug, info};

pub const EXIT_PROCESS_TERMINATED: i32 = 0x102;

Expand All @@ -30,18 +30,25 @@ pub fn invoke_get(resource: &ResourceManifest, cwd: &str, filter: &str) -> Resul

let mut env: Option<HashMap<String, String>> = None;
let mut input_filter: Option<&str> = None;
let mut get_args = resource.get.args.clone();
if !filter.is_empty() {
verify_json(resource, cwd, filter)?;

if input_kind == InputKind::Env {
env = Some(json_to_hashmap(filter)?);
}
else {
input_filter = Some(filter);
match input_kind {
InputKind::Env => {
env = Some(json_to_hashmap(filter)?);
},
InputKind::Stdin => {
input_filter = Some(filter);
},
InputKind::Arg(arg_name) => {
replace_token(&mut get_args, &arg_name, filter)?;
},
}
}

let (exit_code, stdout, stderr) = invoke_command(&resource.get.executable, resource.get.args.clone(), input_filter, Some(cwd), env)?;
info!("Invoking get {} using {}", &resource.resource_type, &resource.get.executable);
let (exit_code, stdout, stderr) = invoke_command(&resource.get.executable, get_args, input_filter, Some(cwd), env)?;
if exit_code != 0 {
return Err(DscError::Command(resource.resource_type.clone(), exit_code, stderr));
}
Expand Down Expand Up @@ -69,6 +76,7 @@ pub fn invoke_get(resource: &ResourceManifest, cwd: &str, filter: &str) -> Resul
/// # Errors
///
/// Error returned if the resource does not successfully set the desired state
#[allow(clippy::too_many_lines)]
pub fn invoke_set(resource: &ResourceManifest, cwd: &str, desired: &str, skip_test: bool) -> Result<SetResult, DscError> {
let Some(set) = resource.set.as_ref() else {
return Err(DscError::NotImplemented("set".to_string()));
Expand All @@ -77,15 +85,22 @@ pub fn invoke_set(resource: &ResourceManifest, cwd: &str, desired: &str, skip_te

let mut env: Option<HashMap<String, String>> = None;
let mut input_desired: Option<&str> = None;
if set.input == InputKind::Env {
env = Some(json_to_hashmap(desired)?);
}
else {
input_desired = Some(desired);
let mut args = set.args.clone();
match &set.input {
InputKind::Env => {
env = Some(json_to_hashmap(desired)?);
},
InputKind::Stdin => {
input_desired = Some(desired);
},
InputKind::Arg(arg_token) => {
replace_token(&mut args, arg_token, desired)?;
},
}

// if resource doesn't implement a pre-test, we execute test first to see if a set is needed
if !skip_test && !set.pre_test.unwrap_or_default() {
info!("No pretest, invoking test {}", &resource.resource_type);
let test_result = invoke_test(resource, cwd, desired)?;
if test_result.in_desired_state {
return Ok(SetResult {
Expand All @@ -98,19 +113,24 @@ pub fn invoke_set(resource: &ResourceManifest, cwd: &str, desired: &str, skip_te

let mut get_env: Option<HashMap<String, String>> = None;
let mut get_input: Option<&str> = None;
let mut get_args = resource.get.args.clone();
match &resource.get.input {
Some(InputKind::Env) => {
get_env = Some(json_to_hashmap(desired)?);
},
Some(InputKind::Stdin) => {
get_input = Some(desired);
},
Some(InputKind::Arg(arg_token)) => {
replace_token(&mut get_args, arg_token, desired)?;
},
None => {
// leave input as none
},
}

let (exit_code, stdout, stderr) = invoke_command(&resource.get.executable, resource.get.args.clone(), get_input, Some(cwd), get_env)?;
info!("Getting current state for set by invoking get {} using {}", &resource.resource_type, &resource.get.executable);
let (exit_code, stdout, stderr) = invoke_command(&resource.get.executable, get_args, get_input, Some(cwd), get_env)?;
if exit_code != 0 {
return Err(DscError::Command(resource.resource_type.clone(), exit_code, stderr));
}
Expand All @@ -122,6 +142,7 @@ pub fn invoke_set(resource: &ResourceManifest, cwd: &str, desired: &str, skip_te
return Err(DscError::Command(resource.resource_type.clone(), exit_code, stderr));
};

info!("Invoking set {} using {}", &resource.resource_type, &set.executable);
let (exit_code, stdout, stderr) = invoke_command(&set.executable, set.args.clone(), input_desired, Some(cwd), env)?;
if exit_code != 0 {
return Err(DscError::Command(resource.resource_type.clone(), exit_code, stderr));
Expand Down Expand Up @@ -195,14 +216,21 @@ pub fn invoke_test(resource: &ResourceManifest, cwd: &str, expected: &str) -> Re

let mut env: Option<HashMap<String, String>> = None;
let mut input_expected: Option<&str> = None;
if test.input == InputKind::Env {
env = Some(json_to_hashmap(expected)?);
}
else {
input_expected = Some(expected);
let mut args = test.args.clone();
match &test.input {
InputKind::Env => {
env = Some(json_to_hashmap(expected)?);
},
InputKind::Stdin => {
input_expected = Some(expected);
},
InputKind::Arg(arg_token) => {
replace_token(&mut args, arg_token, expected)?;
},
}

let (exit_code, stdout, stderr) = invoke_command(&test.executable, test.args.clone(), input_expected, Some(cwd), env)?;
info!("Invoking test {} using {}", &resource.resource_type, &test.executable);
let (exit_code, stdout, stderr) = invoke_command(&test.executable, args, input_expected, Some(cwd), env)?;
if exit_code != 0 {
return Err(DscError::Command(resource.resource_type.clone(), exit_code, stderr));
}
Expand Down Expand Up @@ -381,6 +409,7 @@ pub fn invoke_export(resource: &ResourceManifest, cwd: &str) -> Result<ExportRes
/// Error is returned if the command fails to execute or stdin/stdout/stderr cannot be opened.
#[allow(clippy::implicit_hasher)]
pub fn invoke_command(executable: &str, args: Option<Vec<String>>, input: Option<&str>, cwd: Option<&str>, env: Option<HashMap<String, String>>) -> Result<(i32, String, String), DscError> {
debug!("Invoking command {} with args {:?}", executable, args);
anmenaga marked this conversation as resolved.
Show resolved Hide resolved
let mut command = Command::new(executable);
if input.is_some() {
command.stdin(Stdio::piped());
Expand Down Expand Up @@ -421,13 +450,32 @@ pub fn invoke_command(executable: &str, args: Option<Vec<String>>, input: Option
child_stderr.read_to_end(&mut stderr_buf)?;

let exit_status = child.wait()?;

let exit_code = exit_status.code().unwrap_or(EXIT_PROCESS_TERMINATED);
let stdout = String::from_utf8_lossy(&stdout_buf).to_string();
let stderr = String::from_utf8_lossy(&stderr_buf).to_string();
Ok((exit_code, stdout, stderr))
}

fn replace_token(args: &mut Option<Vec<String>>, token: &str, value: &str) -> Result<(), DscError> {
let Some(arg_values) = args else {
return Err(DscError::Operation("No args to replace".to_string()));
};

let mut found = false;
for arg in arg_values {
if arg == token {
found = true;
*arg = value.to_string();
}
}

if !found {
return Err(DscError::Operation(format!("Token {token} not found in args")));
}

Ok(())
}

fn verify_json(resource: &ResourceManifest, cwd: &str, json: &str) -> Result<(), DscError> {

debug!("resource_type - {}", resource.resource_type);
Expand Down
3 changes: 3 additions & 0 deletions dsc_lib/src/dscresources/resource_manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ pub struct ResourceManifest {

#[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema)]
pub enum InputKind {
/// The input replaces arguments with this token in the command.
#[serde(rename = "arg")]
Arg(String),
Comment on lines +53 to +55
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure about this change/implementation from the user-facing perspective and the schema - it moves the schema from a set of enum values (in JSON Schema syntax) to a oneOf that is more complicated and harder for users to adjust/grok.

I would propose an enum value like ReplaceArg (replaceArg in the JSON) that always replaces a static string argument in the list, instead of allowing an arbitrary string.

We could have that argument be something like {{instance_json}} to be very unlikely to conflict with any actual arguments.

Then the manifest would look like:

"set": {
  "executable": "foo",
  "args": ["config", "set", "--input", "{{instance_json}}"],
  "input": "replaceArg"
}

From a schema perspective, we can then validate that when input is replaceArg that args includes the static string and not when input is any other value.

Are there cases where users will need to specify a custom value for the replacement string?

allOf:
  - if: { properties: { input: { const: replaceArg } } }
    then:
      properties:
        args:
          contains:  { const: '{{instance_json}}' }
          minContains: 1
          maxContains: 1
    else:
      properties:
        args:
          not: { contains:  { const: '{{instance_json}}' } }

Copy link
Member Author

Choose a reason for hiding this comment

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

@michaeltlombardi please open a new issue to discuss

/// The input is accepted as environmental variables.
#[serde(rename = "env")]
Env,
Expand Down
12 changes: 12 additions & 0 deletions tools/dsctest/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
[package]
name = "dsctest"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
clap = { version = "4.1", features = ["derive"] }
schemars = { version = "0.8" }
serde = { version = "1.0", features = ["derive"] }
serde_json = { version = "1.0", features = ["preserve_order"] }
14 changes: 14 additions & 0 deletions tools/dsctest/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# DSCTest Resource

## Sleep

Example config:

```yaml
$schema: https://raw.githubusercontent.com/PowerShell/DSC/main/schemas/2023/08/config/document.json
resources:
- name: Sleep1
type: Test/Sleep
properties:
seconds: 30
```
Loading