diff --git a/rs/cli/src/commands/proposals/analyze.rs b/rs/cli/src/commands/proposals/analyze.rs index 3f92a3719..9fcd584fe 100644 --- a/rs/cli/src/commands/proposals/analyze.rs +++ b/rs/cli/src/commands/proposals/analyze.rs @@ -37,7 +37,12 @@ impl ExecutableCommand for Analyze { let runner = ctx.runner().await?; match filter_map_nns_function_proposals::(&[proposal]).first() { - Some((_, change_membership)) => runner.decentralization_change(change_membership, None, proposal_summary).await, + Some((_, change_membership)) => { + let change = runner.decentralization_change(change_membership, None, proposal_summary).await?; + + println!("{}", change); + Ok(()) + } _ => Err(anyhow::anyhow!( "Proposal {} must have {} type", self.proposal_id, diff --git a/rs/cli/src/commands/subnet/force_replace.rs b/rs/cli/src/commands/subnet/force_replace.rs new file mode 100644 index 000000000..6e9ac6682 --- /dev/null +++ b/rs/cli/src/commands/subnet/force_replace.rs @@ -0,0 +1,164 @@ +use std::collections::BTreeSet; + +use clap::Args; +use ic_management_types::Node; +use ic_types::PrincipalId; +use indexmap::IndexMap; +use itertools::Itertools; +use registry_canister::mutations::do_change_subnet_membership::ChangeSubnetMembershipPayload; + +use crate::{ + exe::ExecutableCommand, + forum::ForumPostKind, + submitter::{SubmissionParameters, Submitter}, +}; + +#[derive(Args, Debug)] +pub struct ForceReplace { + /// Subnet id to perform force replacement from + #[clap(long)] + subnet_id: PrincipalId, + + /// Nodes to remove from the given subnet + #[clap(long, num_args = 1..)] + from: Vec, + + /// Nodes to include into a given subnet + #[clap(long, num_args = 1..)] + to: Vec, + + /// Additional motivation + #[clap(long)] + motivation: Option, + + #[clap(flatten)] + pub submission_parameters: SubmissionParameters, +} + +impl ExecutableCommand for ForceReplace { + fn require_auth(&self) -> crate::auth::AuthRequirement { + crate::auth::AuthRequirement::Neuron + } + + fn validate(&self, _args: &crate::exe::args::GlobalArgs, cmd: &mut clap::Command) { + let from: BTreeSet = self.from.iter().cloned().collect(); + let to: BTreeSet = self.to.iter().cloned().collect(); + + if from.len() != to.len() { + cmd.error( + clap::error::ErrorKind::InvalidValue, + "`from` and `to` have to contain the same number of elements".to_string(), + ) + .exit(); + } + + let duplicates = from.intersection(&to).collect_vec(); + + if duplicates.is_empty() { + return; + } + + let duplicates = duplicates.iter().map(|p| p.to_string().split_once("-").unwrap().0.to_string()).join(", "); + + cmd.error( + clap::error::ErrorKind::ValueValidation, + format!("`from` and `to` contain the following duplicates: [{duplicates}]"), + ) + .exit() + } + + async fn execute(&self, ctx: crate::ctx::DreContext) -> anyhow::Result<()> { + let registry = ctx.registry().await; + + let subnets = registry.subnets().await?; + let subnet = subnets + .get(&self.subnet_id) + .ok_or_else(|| anyhow::anyhow!("Subnet {} is not present in the registry.", self.subnet_id))?; + + // Ensure that the `from` nodes are in the subnet + let wrong_from_nodes: Vec<&PrincipalId> = self + .from + .iter() + .filter(|p| !subnet.nodes.iter().any(|node| node.principal == **p)) + .collect(); + + if !wrong_from_nodes.is_empty() { + return Err(anyhow::anyhow!( + "The following nodes are not members of subnet {}: [{}]", + self.subnet_id, + wrong_from_nodes.iter().map(|p| p.to_string()).join(", ") + )); + } + + // Ensure that the `to` nodes are not in any subnet + let nodes = registry.nodes().await?; + let unassigned_nodes: IndexMap = nodes + .iter() + .filter(|(_, n)| n.subnet_id.is_none()) + .map(|(k, v)| (*k, v.clone())) + .collect(); + + let wrong_to_nodes: Vec<&PrincipalId> = self.to.iter().filter(|p| !unassigned_nodes.contains_key(*p)).collect(); + + if !wrong_to_nodes.is_empty() { + return Err(anyhow::anyhow!( + "The following nodes are not found in unassigned nodes: [{}]", + wrong_to_nodes.iter().map(|p| p.to_string()).join(", ") + )); + } + + // Check that no included nodes have open proposals. + let nodes_with_proposals: Vec = self + .from + .iter() + .chain(self.to.iter()) + .map(|node| nodes.get(node).unwrap()) + .filter(|node| node.proposal.is_some()) + .cloned() + .collect(); + + if !nodes_with_proposals.is_empty() { + return Err(anyhow::anyhow!( + "The following nodes have open proposals:\n{}", + nodes_with_proposals + .iter() + .map(|node| format!(" - {} - {}", node.principal, node.proposal.clone().unwrap().id)) + .join("\n") + )); + } + + // Create a request + let runner = ctx.runner().await?; + + let change_membership = ChangeSubnetMembershipPayload { + subnet_id: self.subnet_id, + node_ids_add: self.to.iter().map(|id| (*id).into()).collect(), + node_ids_remove: self.from.iter().map(|id| (*id).into()).collect(), + }; + + let subnet_change_response = runner.decentralization_change(&change_membership, None, self.motivation.clone()).await?; + + let runner_proposal = match ctx.runner().await?.propose_subnet_change(&subnet_change_response).await? { + Some(runner_proposal) => runner_proposal, + None => return Ok(()), + }; + + Submitter::from(&self.submission_parameters) + .propose_and_print( + ctx.ic_admin_executor().await?.execution(runner_proposal.clone()), + match subnet_change_response.subnet_id { + Some(id) => ForumPostKind::ReplaceNodes { + subnet_id: id, + body: match (&runner_proposal.options.motivation, &runner_proposal.options.summary) { + (Some(motivation), None) => motivation.to_string(), + (Some(motivation), Some(summary)) => format!("{}\nMotivation:\n{}", summary, motivation), + (None, Some(summary)) => summary.to_string(), + (None, None) => anyhow::bail!("Expected to have `motivation` or `summary` for this proposal"), + }, + }, + None => ForumPostKind::Generic, + }, + ) + .await + } +} diff --git a/rs/cli/src/commands/subnet/mod.rs b/rs/cli/src/commands/subnet/mod.rs index f338cabb0..242ea4310 100644 --- a/rs/cli/src/commands/subnet/mod.rs +++ b/rs/cli/src/commands/subnet/mod.rs @@ -1,6 +1,7 @@ use clap::Parser; use create::Create; use deploy::Deploy; +use force_replace::ForceReplace; use replace::Replace; use rescue::Rescue; use resize::Resize; @@ -11,6 +12,7 @@ use crate::exe::impl_executable_command_for_enums; mod create; mod deploy; +mod force_replace; mod replace; mod rescue; mod resize; @@ -23,4 +25,4 @@ pub struct Subnet { pub subcommands: Subcommands, } -impl_executable_command_for_enums! { Subnet, WhatifDecentralization, Deploy, Replace, Resize, Create, Rescue, SetAuthorization } +impl_executable_command_for_enums! { Subnet, WhatifDecentralization, Deploy, Replace, Resize, Create, Rescue, SetAuthorization, ForceReplace } diff --git a/rs/cli/src/commands/subnet/whatif.rs b/rs/cli/src/commands/subnet/whatif.rs index 16cf451cd..392821980 100644 --- a/rs/cli/src/commands/subnet/whatif.rs +++ b/rs/cli/src/commands/subnet/whatif.rs @@ -38,9 +38,12 @@ impl ExecutableCommand for WhatifDecentralization { node_ids_remove: self.remove_nodes.iter().map(|id| (*id).into()).collect(), }; - runner + let change = runner .decentralization_change(&change_membership, self.subnet_nodes_initial.clone(), None) - .await + .await?; + + println!("{}", change); + Ok(()) } fn validate(&self, _args: &GlobalArgs, _cmd: &mut clap::Command) {} diff --git a/rs/cli/src/ctx/mod.rs b/rs/cli/src/ctx/mod.rs index d51036379..636c758c7 100644 --- a/rs/cli/src/ctx/mod.rs +++ b/rs/cli/src/ctx/mod.rs @@ -259,6 +259,10 @@ impl DreContext { pub fn health_client(&self) -> Arc { self.health_client.clone() } + + pub fn cordoned_features_fetcher(&self) -> Arc { + self.cordoned_features_fetcher.clone() + } } #[cfg(test)] diff --git a/rs/cli/src/runner.rs b/rs/cli/src/runner.rs index e5f89c303..54641123a 100644 --- a/rs/cli/src/runner.rs +++ b/rs/cli/src/runner.rs @@ -872,7 +872,7 @@ impl Runner { change: &ChangeSubnetMembershipPayload, override_subnet_nodes: Option>, summary: Option, - ) -> anyhow::Result<()> { + ) -> anyhow::Result { let subnet_before = match override_subnet_nodes { Some(nodes) => { let nodes = self.registry.get_nodes_from_ids(&nodes).await?; @@ -904,8 +904,7 @@ impl Runner { removed_nodes: removed_nodes.clone(), ..Default::default() }; - println!("{}", SubnetChangeResponse::new(&subnet_change, &health_of_nodes, summary)); - Ok(()) + Ok(SubnetChangeResponse::new(&subnet_change, &health_of_nodes, summary)) } pub async fn subnet_rescue(&self, subnet: &PrincipalId, keep_nodes: Option>) -> anyhow::Result> { diff --git a/rs/decentralization/src/nakamoto/mod.rs b/rs/decentralization/src/nakamoto/mod.rs index ba1a3539d..a7711034a 100644 --- a/rs/decentralization/src/nakamoto/mod.rs +++ b/rs/decentralization/src/nakamoto/mod.rs @@ -533,7 +533,6 @@ mod tests { (NodeFeature::NodeOperator, 1.), (NodeFeature::DataCenter, 1.), (NodeFeature::DataCenterOwner, 1.), - (NodeFeature::Area, 1.), (NodeFeature::Country, 1.), ]), value_counts: IndexMap::new(), @@ -542,7 +541,6 @@ mod tests { (NodeFeature::NodeOperator, 1), (NodeFeature::DataCenter, 1), (NodeFeature::DataCenterOwner, 1), - (NodeFeature::Area, 1), (NodeFeature::Country, 1), ]), avg_linear: 1., diff --git a/rs/ic-management-types/src/lib.rs b/rs/ic-management-types/src/lib.rs index 4982ee5e5..b3cf7672a 100644 --- a/rs/ic-management-types/src/lib.rs +++ b/rs/ic-management-types/src/lib.rs @@ -346,15 +346,8 @@ impl Node { .as_ref() .map(|d| d.country.clone()) .unwrap_or_else(|| "unknown".to_string()); - let area = self - .operator - .datacenter - .as_ref() - .map(|d| d.area.clone()) - .unwrap_or_else(|| "unknown".to_string()); NodeFeatures::from_iter([ - (NodeFeature::Area, area), (NodeFeature::Country, country), ( NodeFeature::Continent, @@ -475,7 +468,6 @@ pub enum NodeFeature { NodeProvider, DataCenter, DataCenterOwner, - Area, // Represents smaller geographic entities like cities and states Country, // Covers larger contexts, like countries or broader regions under shared legal jurisdiction Continent, }