From c7ce50c0e54c7efd505d18c0d77fe169f7df0465 Mon Sep 17 00:00:00 2001
From: Javier Viola <363911+pepoviola@users.noreply.github.com>
Date: Wed, 8 May 2024 19:06:59 +0200
Subject: [PATCH] fix build chain-spec locally with custom command (#207)

Couple of fixes for docker provider and to use an external tool to build
the chain-spec.
---
 crates/configuration/src/parachain.rs         |  46 ++++++-
 crates/configuration/src/relaychain.rs        |  46 ++++++-
 crates/configuration/src/utils.rs             |   4 +
 .../orchestrator/src/generators/chain_spec.rs | 130 ++++++++++++++----
 crates/orchestrator/src/lib.rs                |   6 +-
 .../src/network_spec/parachain.rs             |   4 +-
 .../src/network_spec/relaychain.rs            |   4 +-
 crates/orchestrator/src/spawner.rs            |  17 +--
 crates/provider/src/docker/client.rs          |   8 ++
 crates/provider/src/docker/node.rs            |   8 ++
 crates/provider/src/kubernetes/node.rs        |   4 +
 crates/provider/src/lib.rs                    |   2 +
 crates/provider/src/native/node.rs            |   5 +
 13 files changed, 237 insertions(+), 47 deletions(-)

diff --git a/crates/configuration/src/parachain.rs b/crates/configuration/src/parachain.rs
index e2533f0..6e5574b 100644
--- a/crates/configuration/src/parachain.rs
+++ b/crates/configuration/src/parachain.rs
@@ -18,7 +18,7 @@ use crate::{
             Arg, AssetLocation, Chain, ChainDefaultContext, Command, Image, ValidationContext, U128,
         },
     },
-    utils::{default_as_true, default_initial_balance},
+    utils::{default_as_true, default_initial_balance, is_false},
 };
 
 /// The registration strategy that will be used for the parachain.
@@ -131,6 +131,9 @@ pub struct ParachainConfig {
     // and executed for generate the chain-spec.
     // available tokens {{chainName}} / {{disableBootnodes}}
     chain_spec_command: Option<String>,
+    // Does the chain_spec_command needs to be run locally
+    #[serde(skip_serializing_if = "is_false", default)]
+    chain_spec_command_is_local: bool,
     #[serde(rename = "cumulus_based", default = "default_as_true")]
     is_cumulus_based: bool,
     #[serde(skip_serializing_if = "std::vec::Vec::is_empty", default)]
@@ -226,6 +229,11 @@ impl ParachainConfig {
         self.chain_spec_command.as_deref()
     }
 
+    /// Does the chain_spec_command needs to be run locally
+    pub fn chain_spec_command_is_local(&self) -> bool {
+        self.chain_spec_command_is_local
+    }
+
     /// Whether the parachain is based on cumulus.
     pub fn is_cumulus_based(&self) -> bool {
         self.is_cumulus_based
@@ -292,6 +300,7 @@ impl<C: Context> Default for ParachainConfigBuilder<Initial, C> {
                 genesis_overrides: None,
                 chain_spec_path: None,
                 chain_spec_command: None,
+                chain_spec_command_is_local: false, // remote by default
                 is_cumulus_based: true,
                 bootnodes_addresses: vec![],
                 collators: vec![],
@@ -674,6 +683,18 @@ impl<C: Context> ParachainConfigBuilder<WithId, C> {
         )
     }
 
+    /// Set if the chain-spec command needs to be run locally or not (false by default)
+    pub fn chain_spec_command_is_local(self, choice: bool) -> Self {
+        Self::transition(
+            ParachainConfig {
+                chain_spec_command_is_local: choice,
+                ..self.config
+            },
+            self.validation_context,
+            self.errors,
+        )
+    }
+
     /// Set whether the parachain is based on cumulus (true in a majority of case, except adder or undying collators).
     pub fn cumulus_based(self, choice: bool) -> Self {
         Self::transition(
@@ -1244,16 +1265,35 @@ mod tests {
     #[test]
     fn parachain_config_builder_should_works_with_chain_spec_command() {
         const CMD_TPL: &str = "./bin/chain-spec-generator {% raw %} {{chainName}} {% endraw %}";
-        let relaychain_config = ParachainConfigBuilder::new(Default::default())
+        let config = ParachainConfigBuilder::new(Default::default())
+            .with_id(2000)
+            .with_chain("some-chain")
+            .with_default_image("myrepo:myimage")
+            .with_default_command("default_command")
+            .with_chain_spec_command(CMD_TPL)
+            .with_collator(|collator| collator.with_name("collator"))
+            .build()
+            .unwrap();
+
+        assert_eq!(config.chain_spec_command(), Some(CMD_TPL));
+        assert!(!config.chain_spec_command_is_local());
+    }
+
+    #[test]
+    fn parachain_config_builder_should_works_with_chain_spec_command_and_local() {
+        const CMD_TPL: &str = "./bin/chain-spec-generator {% raw %} {{chainName}} {% endraw %}";
+        let config = ParachainConfigBuilder::new(Default::default())
             .with_id(2000)
             .with_chain("some-chain")
             .with_default_image("myrepo:myimage")
             .with_default_command("default_command")
             .with_chain_spec_command(CMD_TPL)
+            .chain_spec_command_is_local(true)
             .with_collator(|collator| collator.with_name("collator"))
             .build()
             .unwrap();
 
-        assert_eq!(relaychain_config.chain_spec_command(), Some(CMD_TPL));
+        assert_eq!(config.chain_spec_command(), Some(CMD_TPL));
+        assert!(config.chain_spec_command_is_local());
     }
 }
diff --git a/crates/configuration/src/relaychain.rs b/crates/configuration/src/relaychain.rs
index e605103..5d0f5f3 100644
--- a/crates/configuration/src/relaychain.rs
+++ b/crates/configuration/src/relaychain.rs
@@ -14,7 +14,7 @@ use crate::{
             Arg, AssetLocation, Chain, ChainDefaultContext, Command, Image, ValidationContext,
         },
     },
-    utils::default_command_polkadot,
+    utils::{default_command_polkadot, is_false},
 };
 
 /// A relay chain configuration, composed of nodes and fine-grained configuration options.
@@ -29,10 +29,12 @@ pub struct RelaychainConfig {
     #[serde(skip_serializing_if = "std::vec::Vec::is_empty", default)]
     default_args: Vec<Arg>,
     chain_spec_path: Option<AssetLocation>,
-    // Full _template_ command, will be rendered using [tera]
+    // Full _template_ command, will be rendered (using custom token replacements)
     // and executed for generate the chain-spec.
     // available tokens {{chainName}} / {{disableBootnodes}}
     chain_spec_command: Option<String>,
+    #[serde(skip_serializing_if = "is_false", default)]
+    chain_spec_command_is_local: bool,
     random_nominators_count: Option<u32>,
     max_nominations: Option<u8>,
     #[serde(skip_serializing_if = "std::vec::Vec::is_empty", default)]
@@ -82,6 +84,11 @@ impl RelaychainConfig {
         self.chain_spec_command.as_deref()
     }
 
+    /// Does the chain_spec_command needs to be run locally
+    pub fn chain_spec_command_is_local(&self) -> bool {
+        self.chain_spec_command_is_local
+    }
+
     /// The non-default command used for nodes.
     pub fn command(&self) -> Option<&Command> {
         self.command.as_ref()
@@ -140,6 +147,7 @@ impl Default for RelaychainConfigBuilder<Initial> {
                 default_args: vec![],
                 chain_spec_path: None,
                 chain_spec_command: None,
+                chain_spec_command_is_local: false, // remote cmd by default
                 command: None,
                 random_nominators_count: None,
                 max_nominations: None,
@@ -335,6 +343,18 @@ impl RelaychainConfigBuilder<WithChain> {
         )
     }
 
+    /// Set if the chain-spec command needs to be run locally or not (false by default)
+    pub fn chain_spec_command_is_local(self, choice: bool) -> Self {
+        Self::transition(
+            RelaychainConfig {
+                chain_spec_command_is_local: choice,
+                ..self.config
+            },
+            self.validation_context,
+            self.errors,
+        )
+    }
+
     /// Set the number of `random nominators` to create for chains using staking, this is used in tandem with `max_nominations` to simulate the amount of nominators and nominations.
     pub fn with_random_nominators_count(self, random_nominators_count: u32) -> Self {
         Self::transition(
@@ -683,15 +703,33 @@ mod tests {
     #[test]
     fn relaychain_config_builder_should_works_with_chain_spec_command() {
         const CMD_TPL: &str = "./bin/chain-spec-generator {% raw %} {{chainName}} {% endraw %}";
-        let relaychain_config = RelaychainConfigBuilder::new(Default::default())
+        let config = RelaychainConfigBuilder::new(Default::default())
+            .with_chain("polkadot")
+            .with_default_image("myrepo:myimage")
+            .with_default_command("default_command")
+            .with_chain_spec_command(CMD_TPL)
+            .with_node(|node| node.with_name("node1").bootnode(true))
+            .build()
+            .unwrap();
+
+        assert_eq!(config.chain_spec_command(), Some(CMD_TPL));
+        assert!(!config.chain_spec_command_is_local());
+    }
+
+    #[test]
+    fn relaychain_config_builder_should_works_with_chain_spec_command_locally() {
+        const CMD_TPL: &str = "./bin/chain-spec-generator {% raw %} {{chainName}} {% endraw %}";
+        let config = RelaychainConfigBuilder::new(Default::default())
             .with_chain("polkadot")
             .with_default_image("myrepo:myimage")
             .with_default_command("default_command")
             .with_chain_spec_command(CMD_TPL)
+            .chain_spec_command_is_local(true)
             .with_node(|node| node.with_name("node1").bootnode(true))
             .build()
             .unwrap();
 
-        assert_eq!(relaychain_config.chain_spec_command(), Some(CMD_TPL));
+        assert_eq!(config.chain_spec_command(), Some(CMD_TPL));
+        assert!(config.chain_spec_command_is_local());
     }
 }
diff --git a/crates/configuration/src/utils.rs b/crates/configuration/src/utils.rs
index 5f68d88..a26d7e8 100644
--- a/crates/configuration/src/utils.rs
+++ b/crates/configuration/src/utils.rs
@@ -4,6 +4,10 @@ pub(crate) fn is_true(value: &bool) -> bool {
     *value
 }
 
+pub(crate) fn is_false(value: &bool) -> bool {
+    !(*value)
+}
+
 pub(crate) fn default_as_true() -> bool {
     true
 }
diff --git a/crates/orchestrator/src/generators/chain_spec.rs b/crates/orchestrator/src/generators/chain_spec.rs
index 777bd65..6632dee 100644
--- a/crates/orchestrator/src/generators/chain_spec.rs
+++ b/crates/orchestrator/src/generators/chain_spec.rs
@@ -12,6 +12,7 @@ use provider::{
 };
 use serde_json::json;
 use support::{constants::THIS_IS_A_BUG, fs::FileSystem, replacer::apply_replacements};
+use tokio::process::Command;
 use tracing::{debug, trace, warn};
 
 use super::errors::GeneratorError;
@@ -37,6 +38,21 @@ enum KeyType {
     Aura,
     Grandpa,
 }
+
+#[derive(Debug, Clone)]
+pub enum CommandInContext {
+    Local(String),
+    Remote(String),
+}
+
+impl CommandInContext {
+    fn cmd(&self) -> &str {
+        match self {
+            CommandInContext::Local(cmd) | CommandInContext::Remote(cmd) => cmd.as_ref(),
+        }
+    }
+}
+
 #[derive(Debug)]
 pub struct ParaGenesisConfig<T: AsRef<Path>> {
     pub(crate) state_path: T,
@@ -54,11 +70,9 @@ pub struct ChainSpec {
     chain_name: Option<String>,
     raw_path: Option<PathBuf>,
     // The binary to build the chain-spec
-    command: Option<String>,
+    command: Option<CommandInContext>,
     // Imgae to use for build the chain-spec
     image: Option<String>,
-    // full command to build the spec, we will use as provided
-    build_command: Option<String>,
     // Contex of the network (e.g relay or para)
     context: Context,
 }
@@ -67,7 +81,6 @@ impl ChainSpec {
     pub(crate) fn new(chain_spec_name: impl Into<String>, context: Context) -> Self {
         Self {
             chain_spec_name: chain_spec_name.into(),
-            build_command: None,
             chain_name: None,
             maybe_plain_path: None,
             asset_location: None,
@@ -96,8 +109,13 @@ impl ChainSpec {
         self
     }
 
-    pub(crate) fn command(mut self, command: impl Into<String>) -> Self {
-        self.command = Some(command.into());
+    pub(crate) fn command(mut self, command: impl Into<String>, is_local: bool) -> Self {
+        let cmd = if is_local {
+            CommandInContext::Local(command.into())
+        } else {
+            CommandInContext::Remote(command.into())
+        };
+        self.command = Some(cmd);
         self
     }
 
@@ -155,9 +173,10 @@ impl ChainSpec {
             // default as empty
             let sanitized_cmd = if replacement_value.is_empty() {
                 // we need to remove the `--chain` flag
-                self.command.as_ref().unwrap().replace("--chain", "")
+                self.command.as_ref().unwrap().cmd().replace("--chain", "")
+                //.as_ref().unwrap().replace("--chain", "")
             } else {
-                self.command.as_ref().unwrap().clone()
+                self.command.as_ref().unwrap().cmd().to_owned()
             };
 
             let full_cmd = apply_replacements(
@@ -176,8 +195,14 @@ impl ChainSpec {
 
             let generate_command =
                 GenerateFileCommand::new(cmd, maybe_plain_spec_path.clone()).args(args);
-            let options = GenerateFilesOptions::new(vec![generate_command], self.image.clone());
-            ns.generate_files(options).await?;
+            if let Some(CommandInContext::Local(_)) = self.command {
+                // local
+                build_locally(generate_command, scoped_fs).await?;
+            } else {
+                // remote
+                let options = GenerateFilesOptions::new(vec![generate_command], self.image.clone());
+                ns.generate_files(options).await?;
+            }
         }
 
         if is_raw(maybe_plain_spec_path.clone(), scoped_fs).await? {
@@ -200,7 +225,14 @@ impl ChainSpec {
         Ok(())
     }
 
-    pub async fn build_raw(&mut self, ns: &DynNamespace) -> Result<(), GeneratorError> {
+    pub async fn build_raw<'a, T>(
+        &mut self,
+        ns: &DynNamespace,
+        scoped_fs: &ScopedFilesystem<'a, T>,
+    ) -> Result<(), GeneratorError>
+    where
+        T: FileSystem,
+    {
         let None = self.raw_path else {
             return Ok(());
         };
@@ -233,7 +265,9 @@ impl ChainSpec {
         // Remote path to be injected
         let chain_spec_path_in_pod = format!("{}/{}", NODE_CONFIG_DIR, maybe_plain_path.display());
         // Path in the context of the node, this can be different in the context of the providers (e.g native)
-        let chain_spec_path_in_args = if ns.capabilities().prefix_with_full_path {
+        let chain_spec_path_in_args = if matches!(self.command, Some(CommandInContext::Local(_))) {
+            chain_spec_path_local.clone()
+        } else if ns.capabilities().prefix_with_full_path {
             // In native
             format!(
                 "{}/{}{}",
@@ -246,7 +280,7 @@ impl ChainSpec {
         };
 
         let mut full_cmd = apply_replacements(
-            cmd,
+            cmd.cmd(),
             &HashMap::from([("chainName", chain_spec_path_in_args.as_str())]),
         );
 
@@ -264,17 +298,24 @@ impl ChainSpec {
         trace!("cmd: {:?} - args: {:?}", cmd, args);
 
         let generate_command = GenerateFileCommand::new(cmd, raw_spec_path.clone()).args(args);
-        let options = GenerateFilesOptions::with_files(
-            vec![generate_command],
-            self.image.clone(),
-            &[TransferedFile::new(
-                chain_spec_path_local,
-                chain_spec_path_in_pod,
-            )],
-        )
-        .temp_name(temp_name);
-        trace!("calling generate_files with options: {:#?}", options);
-        ns.generate_files(options).await?;
+
+        if let Some(CommandInContext::Local(_)) = self.command {
+            // local
+            build_locally(generate_command, scoped_fs).await?;
+        } else {
+            // remote
+            let options = GenerateFilesOptions::with_files(
+                vec![generate_command],
+                self.image.clone(),
+                &[TransferedFile::new(
+                    chain_spec_path_local,
+                    chain_spec_path_in_pod,
+                )],
+            )
+            .temp_name(temp_name);
+            trace!("calling generate_files with options: {:#?}", options);
+            ns.generate_files(options).await?;
+        }
 
         self.raw_path = Some(raw_spec_path);
 
@@ -604,6 +645,47 @@ impl ChainSpec {
 
 type GenesisNodeKey = (String, String, HashMap<String, String>);
 
+async fn build_locally<'a, T>(
+    generate_command: GenerateFileCommand,
+    scoped_fs: &ScopedFilesystem<'a, T>,
+) -> Result<(), GeneratorError>
+where
+    T: FileSystem,
+{
+    // generate_command.
+
+    let result = Command::new(generate_command.program.clone())
+        .args(generate_command.args.clone())
+        .current_dir(scoped_fs.base_dir)
+        .output()
+        .await
+        .map_err(|err| {
+            GeneratorError::ChainSpecGeneration(format!(
+                "Error running cmd: {} args: {}, err: {}",
+                &generate_command.program,
+                &generate_command.args.join(" "),
+                err
+            ))
+        })?;
+
+    if result.status.success() {
+        scoped_fs
+            .write(
+                generate_command.local_output_path,
+                String::from_utf8_lossy(&result.stdout).to_string(),
+            )
+            .await?;
+        Ok(())
+    } else {
+        Err(GeneratorError::ChainSpecGeneration(format!(
+            "Error running cmd: {} args: {}, err: {}",
+            &generate_command.program,
+            &generate_command.args.join(" "),
+            String::from_utf8_lossy(&result.stderr)
+        )))
+    }
+}
+
 async fn is_raw<'a, T>(
     file: PathBuf,
     scoped_fs: &ScopedFilesystem<'a, T>,
diff --git a/crates/orchestrator/src/lib.rs b/crates/orchestrator/src/lib.rs
index deebc4d..03e63d1 100644
--- a/crates/orchestrator/src/lib.rs
+++ b/crates/orchestrator/src/lib.rs
@@ -182,7 +182,11 @@ where
             .await?;
 
         // Build raw version
-        network_spec.relaychain.chain_spec.build_raw(&ns).await?;
+        network_spec
+            .relaychain
+            .chain_spec
+            .build_raw(&ns, &scoped_fs)
+            .await?;
 
         // get the bootnodes to spawn first and calculate the bootnode string for use later
         let mut bootnodes = vec![];
diff --git a/crates/orchestrator/src/network_spec/parachain.rs b/crates/orchestrator/src/network_spec/parachain.rs
index da9f9b7..7c6ec53 100644
--- a/crates/orchestrator/src/network_spec/parachain.rs
+++ b/crates/orchestrator/src/network_spec/parachain.rs
@@ -126,7 +126,7 @@ impl ParachainSpec {
 
                 Some(
                     chain_spec_builder
-                        .command(tmpl.as_str())
+                        .command(tmpl.as_str(), config.chain_spec_command_is_local())
                         .image(main_image.clone()),
                 )
             }
@@ -236,7 +236,7 @@ impl ParachainSpec {
                 .customize_para(&cloned, relay_chain_id, scoped_fs)
                 .await?;
             debug!("parachain chain-spec customized!");
-            chain_spec.build_raw(ns).await?;
+            chain_spec.build_raw(ns, scoped_fs).await?;
             debug!("parachain chain-spec raw built!");
             let chain_spec_raw_path =
                 chain_spec
diff --git a/crates/orchestrator/src/network_spec/relaychain.rs b/crates/orchestrator/src/network_spec/relaychain.rs
index 3e2f1e7..78d883b 100644
--- a/crates/orchestrator/src/network_spec/relaychain.rs
+++ b/crates/orchestrator/src/network_spec/relaychain.rs
@@ -88,7 +88,9 @@ impl RelaychainSpec {
                 apply_replacements(DEFAULT_CHAIN_SPEC_TPL_COMMAND, &replacements)
             };
 
-            chain_spec.command(tmpl.as_str()).image(main_image.clone())
+            chain_spec
+                .command(tmpl.as_str(), config.chain_spec_command_is_local())
+                .image(main_image.clone())
         };
 
         // build the `node_specs`
diff --git a/crates/orchestrator/src/spawner.rs b/crates/orchestrator/src/spawner.rs
index 0bf332d..f81c895 100644
--- a/crates/orchestrator/src/spawner.rs
+++ b/crates/orchestrator/src/spawner.rs
@@ -220,20 +220,13 @@ where
     let prometheus_uri = format!("http://{}:{}/metrics", ip_to_use, prometheus_port_external);
     info!("🚀 {}, should be running now", node.name);
     info!(
-        "🚀 {}: direct link https://polkadot.js.org/apps/?rpc={ws_uri}#/explorer",
+        "💻 {}: direct link https://polkadot.js.org/apps/?rpc={ws_uri}#/explorer",
         node.name
     );
-    info!("🚀 {}: metrics link {prometheus_uri}", node.name);
-    // TODO: the cmd for the logs should live on the node or ns.
-    if ctx.ns.capabilities().requires_image {
-        info!(
-            "📓 logs cmd: kubectl -n {} logs {}",
-            ctx.ns.name(),
-            node.name
-        );
-    } else {
-        info!("📓 logs cmd: tail -f {}/{}.log", base_dir, node.name);
-    }
+    info!("📊 {}: metrics link {prometheus_uri}", node.name);
+
+    info!("📓 logs cmd: {}", running_node.log_cmd());
+
     Ok(NetworkNode::new(
         node.name.clone(),
         ws_uri,
diff --git a/crates/provider/src/docker/client.rs b/crates/provider/src/docker/client.rs
index 5fdc271..2f8dca5 100644
--- a/crates/provider/src/docker/client.rs
+++ b/crates/provider/src/docker/client.rs
@@ -177,6 +177,14 @@ impl DockerClient {
         Ok(DockerClient { using_podman })
     }
 
+    pub fn client_binary(&self) -> String {
+        String::from(if self.using_podman {
+            "podman"
+        } else {
+            "docker"
+        })
+    }
+
     async fn is_using_podman() -> Result<bool> {
         let result = tokio::process::Command::new("docker")
             .arg("--version")
diff --git a/crates/provider/src/docker/node.rs b/crates/provider/src/docker/node.rs
index e78e6a9..42af95f 100644
--- a/crates/provider/src/docker/node.rs
+++ b/crates/provider/src/docker/node.rs
@@ -343,6 +343,14 @@ where
         &self.log_path
     }
 
+    fn log_cmd(&self) -> String {
+        format!(
+            "{} logs -f {}",
+            self.docker_client.client_binary(),
+            self.container_name
+        )
+    }
+
     fn path_in_node(&self, file: &Path) -> PathBuf {
         // here is just a noop op since we will receive the path
         // for the file inside the pod
diff --git a/crates/provider/src/kubernetes/node.rs b/crates/provider/src/kubernetes/node.rs
index 402c317..0fa419c 100644
--- a/crates/provider/src/kubernetes/node.rs
+++ b/crates/provider/src/kubernetes/node.rs
@@ -433,6 +433,10 @@ where
         &self.log_path
     }
 
+    fn log_cmd(&self) -> String {
+        format!("kubectl -n {} logs {}", self.namespace_name(), self.name)
+    }
+
     fn path_in_node(&self, file: &Path) -> PathBuf {
         // here is just a noop op since we will receive the path
         // for the file inside the pod
diff --git a/crates/provider/src/lib.rs b/crates/provider/src/lib.rs
index c76df22..0e0abec 100644
--- a/crates/provider/src/lib.rs
+++ b/crates/provider/src/lib.rs
@@ -175,6 +175,8 @@ pub trait ProviderNode {
 
     fn log_path(&self) -> &PathBuf;
 
+    fn log_cmd(&self) -> String;
+
     // Return the absolute path to the file in the `node` perspective
     // TODO: purpose?
     fn path_in_node(&self, file: &Path) -> PathBuf;
diff --git a/crates/provider/src/native/node.rs b/crates/provider/src/native/node.rs
index ea2cb94..08c66a6 100644
--- a/crates/provider/src/native/node.rs
+++ b/crates/provider/src/native/node.rs
@@ -396,6 +396,11 @@ where
         &self.log_path
     }
 
+    fn log_cmd(&self) -> String {
+        let base_dir = format!("{}/{}", self.base_dir().to_string_lossy(), self.name());
+        format!("tail -f {}/{}.log", base_dir, self.name())
+    }
+
     fn path_in_node(&self, file: &Path) -> PathBuf {
         let full_path = format!(
             "{}/{}",
-- 
GitLab