feat(dre): Update the DRE cli tool to work with the new IC-OS proposa…

…ls (#411) - revise-elected-guestos-versions - revise-elected-hostos-versions - deploy-guestos-to-all-subnet-nodes - deploy-guestos-to-all-unassigned-nodes - deploy-hostos-to-some-nodes
dfinity · May 23, 2024 · 125b147 · 125b147
1 parent e651338
commit 125b147
Show file tree

Hide file tree

Showing 8 changed files with 114 additions and 78 deletions.
diff --git a/rs/cli/src/cli.rs b/rs/cli/src/cli.rs
@@ -73,12 +73,9 @@ pub enum Commands {
 
     /// Place a proposal for updating unassigned nodes config
     UpdateUnassignedNodes {
-        /// NNS subnet id. By default 'tdb26-jop6k-aogll-7ltgs-eruif-6kk7m-qpktf-gdiqx-mxtrf-vb5e6-eqe'
-        #[clap(
-            long,
-            default_value = "tdb26-jop6k-aogll-7ltgs-eruif-6kk7m-qpktf-gdiqx-mxtrf-vb5e6-eqe"
-        )]
-        nns_subnet_id: String,
+        /// NNS subnet id
+        #[clap(long)]
+        nns_subnet_id: Option<String>,
     },
 
     /// Manage replica/host-os versions blessing
@@ -295,9 +292,9 @@ pub mod version {
 
     #[derive(Subcommand, Clone)]
     pub enum UpdateCommands {
-        /// Update the elected/blessed replica versions in the registry
+        /// Update the elected/blessed GuestOS versions in the registry
         /// by adding a new version and potentially removing obsolete versions
-        Replica {
+        GuestOS {
             /// Specify the commit hash of the version that is being elected.
             version: String,
 
@@ -325,7 +322,7 @@ pub mod version {
     impl From<UpdateCommands> for Artifact {
         fn from(value: UpdateCommands) -> Self {
             match value {
-                UpdateCommands::Replica { .. } => Artifact::Replica,
+                UpdateCommands::GuestOS { .. } => Artifact::GuestOs,
                 UpdateCommands::HostOS { .. } => Artifact::HostOs,
             }
         }

diff --git a/rs/cli/src/ic_admin.rs b/rs/cli/src/ic_admin.rs
@@ -235,6 +235,7 @@ impl IcAdminWrapper {
             .concat()
             .as_slice(),
             !as_simulation,
+            false,
         )
         .await
     }
@@ -268,7 +269,12 @@ impl IcAdminWrapper {
         }
     }
 
-    async fn _run_ic_admin_with_args(&self, ic_admin_args: &[String], with_auth: bool) -> anyhow::Result<String> {
+    async fn _run_ic_admin_with_args(
+        &self,
+        ic_admin_args: &[String],
+        with_auth: bool,
+        silent: bool,
+    ) -> anyhow::Result<String> {
         let ic_admin_path = self.ic_admin_bin_path.clone().unwrap_or_else(|| "ic-admin".to_string());
         let mut cmd = Command::new(ic_admin_path);
         let auth_options = if with_auth {
@@ -283,7 +289,11 @@ impl IcAdminWrapper {
         .concat();
         let cmd = cmd.args([&root_options, ic_admin_args].concat());
 
-        self.print_ic_admin_command_line(cmd).await;
+        if silent {
+            cmd.stderr(Stdio::piped());
+        } else {
+            self.print_ic_admin_command_line(cmd).await;
+        }
         cmd.stdout(Stdio::piped());
 
         match cmd.spawn() {
@@ -295,15 +305,28 @@ impl IcAdminWrapper {
                             output
                                 .read_to_end(&mut readbuf)
                                 .map_err(|e| anyhow::anyhow!("Error reading output: {:?}", e))?;
-                            let converted = String::from_utf8_lossy(&readbuf).to_string();
-                            println!("{}", converted);
+                            let converted = String::from_utf8_lossy(&readbuf).trim().to_string();
+                            if !silent {
+                                println!("{}", converted);
+                            }
                             return Ok(converted);
                         }
                         Ok("".to_string())
                     } else {
+                        let readbuf = match child.stderr {
+                            Some(mut stderr) => {
+                                let mut readbuf = String::new();
+                                stderr
+                                    .read_to_string(&mut readbuf)
+                                    .map_err(|e| anyhow::anyhow!("Error reading output: {:?}", e))?;
+                                readbuf
+                            }
+                            None => "".to_string(),
+                        };
                         Err(anyhow::anyhow!(
-                            "ic-admin failed with non-zero exit code {}",
-                            s.code().map(|c| c.to_string()).unwrap_or_else(|| "<none>".to_string())
+                            "ic-admin failed with non-zero exit code {} stderr ==>\n{}",
+                            s.code().map(|c| c.to_string()).unwrap_or_else(|| "<none>".to_string()),
+                            readbuf
                         ))
                     }
                 }
@@ -313,9 +336,9 @@ impl IcAdminWrapper {
         }
     }
 
-    pub async fn run(&self, command: &str, args: &[String], with_auth: bool) -> anyhow::Result<String> {
+    pub async fn run(&self, command: &str, args: &[String], with_auth: bool, silent: bool) -> anyhow::Result<String> {
         let ic_admin_args = [&[command.to_string()], args].concat();
-        self._run_ic_admin_with_args(&ic_admin_args, with_auth).await
+        self._run_ic_admin_with_args(&ic_admin_args, with_auth, silent).await
     }
 
     /// Run ic-admin and parse sub-commands that it lists with "--help",
@@ -348,7 +371,7 @@ impl IcAdminWrapper {
     }
 
     /// Run an `ic-admin get-*` command directly, and without an HSM
-    pub async fn run_passthrough_get(&self, args: &[String]) -> anyhow::Result<()> {
+    pub async fn run_passthrough_get(&self, args: &[String], silent: bool) -> anyhow::Result<String> {
         if args.is_empty() {
             println!("List of available ic-admin 'get' sub-commands:\n");
             for subcmd in self.grep_subcommands(r"\s+get-(.+?)\s") {
@@ -375,9 +398,15 @@ impl IcAdminWrapper {
             args_with_get_prefix
         };
 
-        self.run(&args[0], &args.iter().skip(1).cloned().collect::<Vec<_>>(), false)
+        let stdout = self
+            .run(
+                &args[0],
+                &args.iter().skip(1).cloned().collect::<Vec<_>>(),
+                false,
+                silent,
+            )
             .await?;
-        Ok(())
+        Ok(stdout)
     }
 
     /// Run an `ic-admin propose-to-*` command directly
@@ -711,7 +740,7 @@ must be identical, and must match the SHA256 from the payload of the NNS proposa
             nns.replica_version_id, unassigned_version
         );
 
-        let command = ProposeCommand::UpdateUnassignedNodes {
+        let command = ProposeCommand::DeployGuestosToAllUnassignedNodes {
             replica_version: nns.replica_version_id.clone(),
         };
         let options = ProposeOptions {
@@ -924,32 +953,32 @@ pub enum ProposeCommand {
         node_ids_add: Vec<PrincipalId>,
         node_ids_remove: Vec<PrincipalId>,
     },
-    UpdateSubnetReplicaVersion {
+    DeployGuestosToAllSubnetNodes {
         subnet: PrincipalId,
         version: String,
     },
-    Raw {
-        command: String,
-        args: Vec<String>,
+    DeployGuestosToAllUnassignedNodes {
+        replica_version: String,
     },
-    UpdateNodesHostosVersion {
+    DeployHostosToSomeNodes {
         nodes: Vec<PrincipalId>,
         version: String,
     },
+    Raw {
+        command: String,
+        args: Vec<String>,
+    },
     RemoveNodes {
         nodes: Vec<PrincipalId>,
     },
-    UpdateElectedVersions {
+    ReviseElectedVersions {
         release_artifact: Artifact,
         args: Vec<String>,
     },
     CreateSubnet {
         node_ids: Vec<PrincipalId>,
         replica_version: String,
     },
-    UpdateUnassignedNodes {
-        replica_version: String,
-    },
 }
 
 impl ProposeCommand {
@@ -959,11 +988,12 @@ impl ProposeCommand {
             "{PROPOSE_CMD_PREFIX}{}",
             match self {
                 Self::Raw { command, args: _ } => command.trim_start_matches(PROPOSE_CMD_PREFIX).to_string(),
-                Self::UpdateElectedVersions {
+                Self::ReviseElectedVersions {
                     release_artifact,
                     args: _,
-                } => format!("update-elected-{}-versions", release_artifact),
-                Self::UpdateUnassignedNodes { replica_version: _ } => "update-unassigned-nodes-config".to_string(),
+                } => format!("revise-elected-{}-versions", release_artifact),
+                Self::DeployGuestosToAllUnassignedNodes { replica_version: _ } =>
+                    "deploy-guestos-to-all-unassigned-nodes".to_string(),
                 _ => self.to_string(),
             }
         )
@@ -999,17 +1029,17 @@ impl ProposeCommand {
                 },
             ]
             .concat(),
-            Self::UpdateSubnetReplicaVersion { subnet, version } => {
+            Self::DeployGuestosToAllSubnetNodes { subnet, version } => {
                 vec![subnet.to_string(), version.clone()]
             }
             Self::Raw { command: _, args } => args.clone(),
-            Self::UpdateNodesHostosVersion { nodes, version } => [
+            Self::DeployHostosToSomeNodes { nodes, version } => [
                 nodes.iter().map(|n| n.to_string()).collect::<Vec<_>>(),
                 vec!["--hostos-version-id".to_string(), version.to_string()],
             ]
             .concat(),
             Self::RemoveNodes { nodes } => nodes.iter().map(|n| n.to_string()).collect(),
-            Self::UpdateElectedVersions {
+            Self::ReviseElectedVersions {
                 release_artifact: _,
                 args,
             } => args.clone(),
@@ -1027,7 +1057,7 @@ impl ProposeCommand {
                 }
                 args
             }
-            Self::UpdateUnassignedNodes { replica_version } => {
+            Self::DeployGuestosToAllUnassignedNodes { replica_version } => {
                 vec!["--replica-version-id".to_string(), replica_version.clone()]
             }
         }
@@ -1114,7 +1144,7 @@ oSMDIQBa2NLmSmaqjDXej4rrJEuEhKIz7/pGXpxztViWhB+X9Q==
                 node_ids_add: vec![Default::default()],
                 node_ids_remove: vec![Default::default()],
             },
-            ProposeCommand::UpdateSubnetReplicaVersion {
+            ProposeCommand::DeployGuestosToAllSubnetNodes {
                 subnet: Default::default(),
                 version: "0000000000000000000000000000000000000000".to_string(),
             },
@@ -1178,7 +1208,7 @@ oSMDIQBa2NLmSmaqjDXej4rrJEuEhKIz7/pGXpxztViWhB+X9Q==
             .concat()
             .to_vec();
             let out = with_ic_admin(Default::default(), async {
-                cli.run(&cmd.get_command_name(), &vector, true)
+                cli.run(&cmd.get_command_name(), &vector, true, false)
                     .await
                     .map_err(|e| anyhow::anyhow!(e))
             })

diff --git a/rs/cli/src/main.rs b/rs/cli/src/main.rs
@@ -222,15 +222,24 @@ async fn async_main() -> Result<(), anyhow::Error> {
             }
 
             cli::Commands::Get { args } => {
-                runner_instance.ic_admin.run_passthrough_get(args).await
+                runner_instance.ic_admin.run_passthrough_get(args, false).await?;
+                Ok(())
             },
 
             cli::Commands::Propose { args } => {
                 runner_instance.ic_admin.run_passthrough_propose(args, simulate).await
             },
 
             cli::Commands::UpdateUnassignedNodes { nns_subnet_id } => {
-                runner_instance.ic_admin.update_unassigned_nodes( nns_subnet_id, &target_network, simulate).await
+                let nns_subnet_id = match nns_subnet_id {
+                    Some(subnet_id) => subnet_id.to_owned(),
+                    None => {
+                        let res = runner_instance.ic_admin.run_passthrough_get(&["get-subnet-list".to_string()], true).await?;
+                        let subnet_list: Vec<String> = serde_json::from_str(&res)?;
+                        subnet_list.first().ok_or_else(|| anyhow::anyhow!("No subnet found"))?.clone()
+                    }
+                };
+                runner_instance.ic_admin.update_unassigned_nodes(&nns_subnet_id, &target_network, simulate).await
             },
 
             cli::Commands::Version(version_command) => {
@@ -239,7 +248,7 @@ async fn async_main() -> Result<(), anyhow::Error> {
                         let release_artifact: &Artifact = &update_command.subcommand.clone().into();
 
                         let update_version = match &update_command.subcommand {
-                            cli::version::UpdateCommands::Replica { version, release_tag, force} | cli::version::UpdateCommands::HostOS { version, release_tag, force} => {
+                            cli::version::UpdateCommands::GuestOS { version, release_tag, force} | cli::version::UpdateCommands::HostOS { version, release_tag, force} => {
                                 ic_admin::IcAdminWrapper::prepare_to_propose_to_update_elected_versions(
                                     release_artifact,
                                     version,
@@ -250,7 +259,7 @@ async fn async_main() -> Result<(), anyhow::Error> {
                             }
                         }.await?;
 
-                        runner_instance.ic_admin.propose_run(ic_admin::ProposeCommand::UpdateElectedVersions {
+                        runner_instance.ic_admin.propose_run(ic_admin::ProposeCommand::ReviseElectedVersions {
                                                  release_artifact: update_version.release_artifact.clone(),
                                                  args: dre::parsed_cli::ParsedCli::get_update_cmd_args(&update_version)
                                              },

diff --git a/rs/cli/src/runner.rs b/rs/cli/src/runner.rs
@@ -26,13 +26,13 @@ impl Runner {
     pub async fn deploy(&self, subnet: &PrincipalId, version: &str, simulate: bool) -> anyhow::Result<()> {
         self.ic_admin
             .propose_run(
-                ic_admin::ProposeCommand::UpdateSubnetReplicaVersion {
+                ic_admin::ProposeCommand::DeployGuestosToAllSubnetNodes {
                     subnet: *subnet,
                     version: version.to_string(),
                 },
                 ic_admin::ProposeOptions {
-                    title: format!("Update subnet {subnet} to replica version {version}").into(),
-                    summary: format!("Update subnet {subnet} to replica version {version}").into(),
+                    title: format!("Update subnet {subnet} to GuestOS version {version}").into(),
+                    summary: format!("Update subnet {subnet} to GuestOS version {version}").into(),
                     motivation: None,
                 },
                 simulate,
@@ -108,7 +108,7 @@ impl Runner {
             self.dashboard_backend_client
                 .get_nns_replica_version()
                 .await
-                .expect("Should get a replica version"),
+                .expect("Failed to get a GuestOS version of the NNS subnet"),
         );
 
         self.ic_admin
@@ -252,13 +252,13 @@ impl Runner {
             .collect::<Vec<_>>();
 
             if versions.is_empty() {
-                warn!("Empty list of replica versions to unelect");
+                warn!("Empty list of GuestOS versions to unelect");
             }
             versions
         };
 
         let mut template =
-            "Removing the obsolete IC replica versions from the registry, to prevent unintended version downgrades in the future"
+            "Removing the obsolete GuestOS versions from the registry, to prevent unintended version downgrades in the future"
                 .to_string();
         if edit_summary {
             info!("Edit summary");
@@ -413,7 +413,7 @@ impl Runner {
         let title = format!("Set HostOS version: {version} on {} nodes", nodes.clone().len());
         self.ic_admin
             .propose_run(
-                ic_admin::ProposeCommand::UpdateNodesHostosVersion {
+                ic_admin::ProposeCommand::DeployHostosToSomeNodes {
                     nodes: nodes.clone(),
                     version: version.to_string(),
                 },