diff --git a/rs/cli/src/cli.rs b/rs/cli/src/cli.rs index fe1d952b..41053248 100644 --- 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, }, /// 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 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 index 6ee221de..f37f78c6 100644 --- 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 { + async fn _run_ic_admin_with_args( + &self, + ic_admin_args: &[String], + with_auth: bool, + silent: bool, + ) -> anyhow::Result { 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(|| "".to_string()) + "ic-admin failed with non-zero exit code {} stderr ==>\n{}", + s.code().map(|c| c.to_string()).unwrap_or_else(|| "".to_string()), + readbuf )) } } @@ -313,9 +336,9 @@ impl IcAdminWrapper { } } - pub async fn run(&self, command: &str, args: &[String], with_auth: bool) -> anyhow::Result { + pub async fn run(&self, command: &str, args: &[String], with_auth: bool, silent: bool) -> anyhow::Result { 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 { 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::>(), false) + let stdout = self + .run( + &args[0], + &args.iter().skip(1).cloned().collect::>(), + 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,22 +953,25 @@ pub enum ProposeCommand { node_ids_add: Vec, node_ids_remove: Vec, }, - UpdateSubnetReplicaVersion { + DeployGuestosToAllSubnetNodes { subnet: PrincipalId, version: String, }, - Raw { - command: String, - args: Vec, + DeployGuestosToAllUnassignedNodes { + replica_version: String, }, - UpdateNodesHostosVersion { + DeployHostosToSomeNodes { nodes: Vec, version: String, }, + Raw { + command: String, + args: Vec, + }, RemoveNodes { nodes: Vec, }, - UpdateElectedVersions { + ReviseElectedVersions { release_artifact: Artifact, args: Vec, }, @@ -947,9 +979,6 @@ pub enum ProposeCommand { node_ids: Vec, 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!["--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 index ce118ebe..b1826a62 100644 --- a/rs/cli/src/main.rs +++ b/rs/cli/src/main.rs @@ -222,7 +222,8 @@ 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 } => { @@ -230,7 +231,15 @@ async fn async_main() -> Result<(), anyhow::Error> { }, 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 = 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 index 08f8fa73..f908f085 100644 --- 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::>(); 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(), }, diff --git a/rs/ic-management-backend/src/registry.rs b/rs/ic-management-backend/src/registry.rs index 35bb9841..50355343 100644 --- a/rs/ic-management-backend/src/registry.rs +++ b/rs/ic-management-backend/src/registry.rs @@ -78,7 +78,7 @@ pub struct RegistryState { node_labels_guests: Vec, known_subnets: BTreeMap, - replica_releases: ArtifactReleases, + guestos_releases: ArtifactReleases, hostos_releases: ArtifactReleases, ic_repo: Option, } @@ -245,7 +245,7 @@ impl RegistryState { nodes: BTreeMap::new(), operators: BTreeMap::new(), node_labels_guests: Vec::new(), - replica_releases: ArtifactReleases::new(Artifact::Replica), + guestos_releases: ArtifactReleases::new(Artifact::GuestOs), hostos_releases: ArtifactReleases::new(Artifact::HostOs), ic_repo: Some(IcRepo::new().expect("failed to init ic repo")), known_subnets: [ @@ -309,7 +309,7 @@ impl RegistryState { Ok(()) } - pub async fn get_blessed_replica_versions(&self) -> Result, anyhow::Error> { + pub async fn get_elected_guestos_versions(&self) -> Result, anyhow::Error> { match self.local_registry.get_value( &make_blessed_replica_versions_key(), self.local_registry.get_latest_version(), @@ -320,7 +320,7 @@ impl RegistryState { Ok(cfg.blessed_version_ids) } - _ => Err(anyhow::anyhow!("No blessed replica version found".to_string(),)), + _ => Err(anyhow::anyhow!("No elected GuestOS versions found".to_string(),)), } } pub async fn get_elected_hostos_versions(&self) -> Result, anyhow::Error> { @@ -359,7 +359,7 @@ impl RegistryState { *DATETIME_NAME_GROUP, )).unwrap(); } - let blessed_replica_versions = self.get_blessed_replica_versions().await?; + let blessed_replica_versions = self.get_elected_guestos_versions().await?; let elected_hostos_versions = self.get_elected_hostos_versions().await?; let blessed_versions: HashSet<&String> = blessed_replica_versions @@ -429,7 +429,7 @@ impl RegistryState { }); for (blessed_versions, ArtifactReleases { artifact, releases }) in [ - (blessed_replica_versions, &mut self.replica_releases), + (blessed_replica_versions, &mut self.guestos_releases), (elected_hostos_versions, &mut self.hostos_releases), ] { releases.clear(); @@ -640,7 +640,7 @@ impl RegistryState { }, replica_version: sr.replica_version_id.clone(), replica_release: self - .replica_releases + .guestos_releases .releases .iter() .find(|r| r.commit_hash == sr.replica_version_id) @@ -727,14 +727,14 @@ impl RegistryState { pub async fn retireable_versions(&self, artifact: &Artifact) -> Result> { match artifact { Artifact::HostOs => self.retireable_hostos_versions().await, - Artifact::Replica => self.retireable_replica_versions().await, + Artifact::GuestOs => self.retireable_guestos_versions().await, } } pub async fn blessed_versions(&self, artifact: &Artifact) -> Result> { match artifact { Artifact::HostOs => self.get_elected_hostos_versions().await, - Artifact::Replica => self.get_blessed_replica_versions().await, + Artifact::GuestOs => self.get_elected_guestos_versions().await, } } @@ -777,8 +777,8 @@ impl RegistryState { .collect()) } - async fn retireable_replica_versions(&self) -> Result> { - let active_releases = self.replica_releases.get_active_branches(); + async fn retireable_guestos_versions(&self) -> Result> { + let active_releases = self.guestos_releases.get_active_branches(); let subnet_versions: BTreeSet = self.subnets.values().map(|s| s.replica_version.clone()).collect(); let version_on_unassigned_nodes = self.get_unassigned_nodes_replica_version().await?; let versions_in_proposals: BTreeSet = self @@ -790,16 +790,16 @@ impl RegistryState { .collect(); info!("Active releases: {}", active_releases.iter().join(", ")); info!( - "Replica versions in use on subnets: {}", + "GuestOS versions in use on subnets: {}", subnet_versions.iter().join(", ") ); - info!("Replica version on unassigned nodes: {}", version_on_unassigned_nodes); + info!("GuestOS version on unassigned nodes: {}", version_on_unassigned_nodes); info!( - "Replica versions in open proposals: {}", + "GuestOS versions in open proposals: {}", versions_in_proposals.iter().join(", ") ); Ok(self - .replica_releases + .guestos_releases .releases .clone() .into_iter() @@ -846,7 +846,7 @@ impl RegistryState { } pub fn replica_releases(&self) -> Vec { - self.replica_releases.releases.clone() + self.guestos_releases.releases.clone() } pub fn get_nns_urls(&self) -> &Vec { @@ -867,7 +867,7 @@ impl RegistryState { Ok(cfg.replica_version) } _ => Err(anyhow::anyhow!( - "No replica version for unassigned nodes found".to_string(), + "No GuestOS version for unassigned nodes found".to_string(), )), } } diff --git a/rs/ic-management-types/src/lib.rs b/rs/ic-management-types/src/lib.rs index c90420e4..fea8cf8b 100644 --- a/rs/ic-management-types/src/lib.rs +++ b/rs/ic-management-types/src/lib.rs @@ -567,20 +567,20 @@ impl ArtifactReleases { #[strum(serialize_all = "lowercase")] #[serde(rename_all = "lowercase")] pub enum Artifact { - Replica, + GuestOs, HostOs, } impl Artifact { pub fn s3_folder(&self) -> String { match self { - Artifact::Replica => String::from("guest-os"), + Artifact::GuestOs => String::from("guest-os"), Artifact::HostOs => String::from("host-os"), } } pub fn capitalized(&self) -> String { match self { - Artifact::Replica => String::from("Replica"), + Artifact::GuestOs => String::from("Guestos"), Artifact::HostOs => String::from("Hostos"), } } diff --git a/rs/rollout-controller/src/actions/mod.rs b/rs/rollout-controller/src/actions/mod.rs index be518b7c..a4c40401 100644 --- a/rs/rollout-controller/src/actions/mod.rs +++ b/rs/rollout-controller/src/actions/mod.rs @@ -84,15 +84,15 @@ impl<'a> SubnetAction { } = self { if !blessed_replica_versions.contains(version) { - return Err(anyhow::anyhow!("Replica version '{}' is not blessed.", version)); + return Err(anyhow::anyhow!("GuestOS version '{}' is not elected.", version)); } let principal_string = subnet_principal.to_string(); let proposal = match is_unassigned { - true => ProposeCommand::UpdateUnassignedNodes { + true => ProposeCommand::DeployGuestosToAllUnassignedNodes { replica_version: version.to_string(), }, - false => ProposeCommand::UpdateSubnetReplicaVersion { + false => ProposeCommand::DeployGuestosToAllSubnetNodes { subnet: *subnet_principal, version: version.to_string(), }, @@ -100,12 +100,12 @@ impl<'a> SubnetAction { let opts = ProposeOptions { title: Some(format!( - "Update subnet {} to replica version {}", + "Update subnet {} to GuestOS version {}", principal_string.split_once('-').expect("Should contain '-'").0, version.split_at(8).0 )), summary: Some(format!( - "Update subnet {} to replica version {}", + "Update subnet {} to GuestOS version {}", principal_string, version )), ..Default::default() diff --git a/rs/rollout-controller/src/main.rs b/rs/rollout-controller/src/main.rs index 0cbd0968..3156ecb9 100644 --- a/rs/rollout-controller/src/main.rs +++ b/rs/rollout-controller/src/main.rs @@ -92,8 +92,8 @@ async fn main() -> anyhow::Result<()> { } }; - // Get blessed replica versions for later - let blessed_versions = match registry_state.get_blessed_replica_versions().await { + // Get elected GuestOS versions for later + let elected_guestos_versions = match registry_state.get_elected_guestos_versions().await { Ok(versions) => versions, Err(e) => { warn!(logger, "{:?}", e); @@ -119,7 +119,7 @@ async fn main() -> anyhow::Result<()> { break; } info!(logger, "Calculated actions: {:#?}", actions); - match executor.execute(&actions, &blessed_versions).await { + match executor.execute(&actions, &elected_guestos_versions).await { Ok(()) => info!(logger, "Actions taken successfully"), Err(e) => warn!(logger, "{:?}", e), };