Skip to content

Commit

Permalink
add check for binaries in native
Browse files Browse the repository at this point in the history
  • Loading branch information
pepoviola committed Aug 15, 2024
1 parent 9afc1d2 commit a70825d
Show file tree
Hide file tree
Showing 8 changed files with 151 additions and 45 deletions.
1 change: 0 additions & 1 deletion crates/orchestrator/src/generators/chain_spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,6 @@ impl ChainSpec {
let sanitized_cmd = if replacement_value.is_empty() {
// we need to remove the `--chain` flag
self.command.as_ref().unwrap().cmd().replace("--chain", "")
//.as_ref().unwrap().replace("--chain", "")
} else {
self.command.as_ref().unwrap().cmd().to_owned()
};
Expand Down
151 changes: 124 additions & 27 deletions crates/orchestrator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ pub mod shared;
mod spawner;

use std::{
collections::HashSet,
net::IpAddr,
path::{Path, PathBuf},
time::Duration,
Expand Down Expand Up @@ -411,36 +412,96 @@ fn validate_spec_with_provider_capabilities(
network_spec: &NetworkSpec,
capabilities: &ProviderCapabilities,
) -> Result<(), anyhow::Error> {
if !capabilities.requires_image {
return Ok(());
}

// Relaychain
if network_spec.relaychain.default_image.is_none() {
// we should check if each node have an image
let nodes = &network_spec.relaychain.nodes;
if nodes.iter().any(|node| node.image.is_none()) {
return Err(anyhow::anyhow!(
"missing image for node, and not default is set at relaychain"
));
}
};
let mut errs: Vec<String> = vec![];

// Paras
for para in &network_spec.parachains {
if para.default_image.is_none() {
let nodes = &para.collators;
if capabilities.requires_image {
// Relaychain
if network_spec.relaychain.default_image.is_none() {
// we should check if each node have an image
let nodes = &network_spec.relaychain.nodes;
if nodes.iter().any(|node| node.image.is_none()) {
return Err(anyhow::anyhow!(
"missing image for node, and not default is set at parachain {}",
para.id
errs.push(String::from(
"Missing image for node, and not default is set at relaychain",
));
}
};

// Paras
for para in &network_spec.parachains {
if para.default_image.is_none() {
let nodes = &para.collators;
if nodes.iter().any(|node| node.image.is_none()) {
errs.push(format!(
"Missing image for node, and not default is set at parachain {}",
para.id
));
}
}
}
} else {
// native
// We need to get all the `cmds` and verify if are part of the path
let mut cmds: HashSet<&str> = Default::default();
if let Some(cmd) = network_spec.relaychain.default_command.as_ref() {
cmds.insert(cmd.as_str());
}
for node in network_spec.relaychain().nodes.iter() {
cmds.insert(node.command());
}

// Paras
for para in &network_spec.parachains {
if let Some(cmd) = para.default_command.as_ref() {
cmds.insert(cmd.as_str());
}

for node in para.collators.iter() {
cmds.insert(node.command());
}
}

// now check the binaries
let path = std::env::var("PATH").unwrap_or_default(); // path should always be set
println!("PATH es: {path}");
let parts: Vec<_> = path.split(":").collect();
for cmd in cmds {
let missing = if cmd.contains('/') {
std::fs::metadata(cmd).is_err()
} else {
// should be in the PATH
!parts
.iter()
.any(|part| std::fs::metadata(format!("{}/{}", part, cmd)).is_ok())
};

if missing {
errs.push(help_msg(cmd));
}
}
}

if !errs.is_empty() {
let msg = errs.join("\n");
return Err(anyhow::anyhow!(format!("Invalid configuration: \n {msg}")));
}

Ok(())
}

fn help_msg(cmd: &str) -> String {
match cmd {
"parachain-template-node" | "solochain-template-node" | "minimal-template-node" => {
format!("Missing binary {cmd}, compile by running: \n\tcargo build --package {cmd} --release")
},
"polkadot" => {
format!("Missing binary {cmd}, compile by running: \n\t cargo build --locked --release --features fast-runtime --bin {cmd} --bin polkadot-prepare-worker --bin polkadot-execute-worker")
},
_ => {
format!("Missing binary {cmd}, please compile it.")
},
}
}

// TODO: get the fs from `DynNamespace` will make this not needed
// but the FileSystem trait isn't object-safe so we can't pass around
// as `dyn FileSystem`. We can refactor or using some `erase` techniques
Expand Down Expand Up @@ -543,12 +604,15 @@ mod tests {

use super::*;

fn generate(with_image: bool) -> Result<NetworkConfig, Vec<anyhow::Error>> {
fn generate(
with_image: bool,
with_cmd: Option<&'static str>,
) -> Result<NetworkConfig, Vec<anyhow::Error>> {
NetworkConfigBuilder::new()
.with_relaychain(|r| {
let mut relay = r
.with_chain("rococo-local")
.with_default_command("polkadot");
.with_default_command(with_cmd.unwrap_or("polkadot"));
if with_image {
relay = relay.with_default_image("docker.io/parity/polkadot")
}
Expand All @@ -559,7 +623,9 @@ mod tests {
})
.with_parachain(|p| {
p.with_id(2000).cumulus_based(true).with_collator(|n| {
let node = n.with_name("collator").with_command("polkadot-parachain");
let node = n
.with_name("collator")
.with_command(with_cmd.unwrap_or("polkadot-parachain"));
if with_image {
node.with_image("docker.io/paritypr/test-parachain")
} else {
Expand All @@ -572,7 +638,7 @@ mod tests {

#[tokio::test]
async fn valid_config_with_image() {
let network_config = generate(true).unwrap();
let network_config = generate(true, None).unwrap();
let spec = NetworkSpec::from_config(&network_config).await.unwrap();
let caps = ProviderCapabilities {
requires_image: true,
Expand All @@ -586,8 +652,8 @@ mod tests {
}

#[tokio::test]
async fn invalid_config() {
let network_config = generate(false).unwrap();
async fn invalid_config_without_image() {
let network_config = generate(false, None).unwrap();
let spec = NetworkSpec::from_config(&network_config).await.unwrap();
let caps = ProviderCapabilities {
requires_image: true,
Expand All @@ -599,4 +665,35 @@ mod tests {
let valid = validate_spec_with_provider_capabilities(&spec, &caps);
assert!(valid.is_err())
}

#[tokio::test]
async fn invalid_config_missing_cmd() {
let network_config = generate(false, Some("other")).unwrap();
let spec = NetworkSpec::from_config(&network_config).await.unwrap();
let caps = ProviderCapabilities {
requires_image: false,
has_resources: false,
prefix_with_full_path: false,
use_default_ports_in_cmd: false,
};

let valid = validate_spec_with_provider_capabilities(&spec, &caps);
assert!(valid.is_err())
}

#[tokio::test]
async fn valid_config_present_cmd() {
let network_config = generate(false, Some("cargo")).unwrap();
let spec = NetworkSpec::from_config(&network_config).await.unwrap();
let caps = ProviderCapabilities {
requires_image: false,
has_resources: false,
prefix_with_full_path: false,
use_default_ports_in_cmd: false,
};

let valid = validate_spec_with_provider_capabilities(&spec, &caps);
println!("{:?}", valid);
assert!(valid.is_ok())
}
}
27 changes: 18 additions & 9 deletions crates/orchestrator/src/network_spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,16 +49,25 @@ impl NetworkSpec {
}
}

Ok(NetworkSpec {
relaychain,
parachains,
hrmp_channels: network_config
.hrmp_channels()
if errs.is_empty() {
Ok(NetworkSpec {
relaychain,
parachains,
hrmp_channels: network_config
.hrmp_channels()
.into_iter()
.cloned()
.collect(),
global_settings: network_config.global_settings().clone(),
})
} else {
let errs_str = errs
.into_iter()
.cloned()
.collect(),
global_settings: network_config.global_settings().clone(),
})
.map(|e| e.to_string())
.collect::<Vec<String>>()
.join("\n");
Err(OrchestratorError::InvalidConfig(errs_str))
}
}

pub async fn populate_nodes_available_args(
Expand Down
9 changes: 5 additions & 4 deletions crates/orchestrator/src/network_spec/parachain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,14 +77,15 @@ impl ParachainSpec {
cmd
} else if let Some(first_node) = config.collators().first() {
let Some(cmd) = first_node.command() else {
return Err(OrchestratorError::InvalidConfig("Parachain, either default_command or command in the first node needs to be set.".to_string()));
return Err(OrchestratorError::InvalidConfig(format!("Parachain {}, either default_command or command in the first node needs to be set.", config.id())));
};

cmd
} else {
return Err(OrchestratorError::InvalidConfig(
"Parachain without nodes and default_command isn't set.".to_string(),
));
return Err(OrchestratorError::InvalidConfig(format!(
"Parachain {}, without nodes and default_command isn't set.",
config.id()
)));
};

// TODO: internally we use image as String
Expand Down
2 changes: 1 addition & 1 deletion crates/provider/src/docker/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ where
.map_err(|err| {
ProviderError::RunCommandError(
format!("sh -c {}", &command.join(" ")),
self.name.to_string(),
format!("in pod {}", self.name),
err.into(),
)
})
Expand Down
2 changes: 1 addition & 1 deletion crates/provider/src/kubernetes/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,7 @@ where
.map_err(|err| {
ProviderError::RunCommandError(
format!("sh -c {}", &command.join(" ")),
self.name.to_string(),
format!("in pod {}", self.name),
err.into(),
)
})
Expand Down
2 changes: 1 addition & 1 deletion crates/provider/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ pub enum ProviderError {
#[error("Failed to spawn node '{0}': {1}")]
NodeSpawningFailed(String, anyhow::Error),

#[error("Error running command '{0}' in pod {1}: {2}")]
#[error("Error running command '{0}' {1}: {2}")]
RunCommandError(String, String, anyhow::Error),

#[error("Error running script'{0}': {1}")]
Expand Down
2 changes: 1 addition & 1 deletion crates/provider/src/native/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ where
.map_err(|err| {
ProviderError::RunCommandError(
format!("{} {}", &options.program, &options.args.join(" ")),
options.program,
"locally".to_string(),
err.into(),
)
})?;
Expand Down

0 comments on commit a70825d

Please sign in to comment.