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

add check for binaries in native #245

Merged
merged 1 commit into from
Aug 15, 2024
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
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
Loading