diff --git a/rs/cli/src/commands/hostos/rollout_from_node_group.rs b/rs/cli/src/commands/hostos/rollout_from_node_group.rs index 30cbc2627..c680c08cd 100644 --- a/rs/cli/src/commands/hostos/rollout_from_node_group.rs +++ b/rs/cli/src/commands/hostos/rollout_from_node_group.rs @@ -1,5 +1,3 @@ -use std::str::FromStr; - use clap::{Args, ValueEnum}; use crate::{ @@ -74,9 +72,10 @@ pub struct RolloutFromNodeGroup { #[clap( long, help = r#"How many nodes in the group to update with the version specified -supported values are absolute numbers (10) or percentage (10%)"# +supported values are absolute numbers (10) or percentage (10%)"#, + default_value = "100%" )] - pub nodes_in_group: String, + pub nodes_in_group: NumberOfNodes, #[clap(flatten)] pub submission_parameters: SubmissionParameters, @@ -88,7 +87,7 @@ impl ExecutableCommand for RolloutFromNodeGroup { } async fn execute(&self, ctx: crate::ctx::DreContext) -> anyhow::Result<()> { - let update_group = NodeGroupUpdate::new(self.assignment, self.owner, NumberOfNodes::from_str(&self.nodes_in_group)?); + let update_group = NodeGroupUpdate::new(self.assignment, self.owner, self.nodes_in_group); let runner = ctx.runner().await?; let (nodes_to_update, summary) = match runner diff --git a/rs/cli/src/commands/propose.rs b/rs/cli/src/commands/propose.rs index c5fed2386..bb48c90c1 100644 --- a/rs/cli/src/commands/propose.rs +++ b/rs/cli/src/commands/propose.rs @@ -1,6 +1,7 @@ use clap::{error::ErrorKind, Args}; use crate::auth::AuthRequirement; +use crate::confirm::DryRunType; use crate::exe::args::GlobalArgs; use crate::exe::ExecutableCommand; use crate::{ @@ -49,7 +50,7 @@ impl ExecutableCommand for Propose { // This automatically bypasses interactive prompts, as they are unnecessary and unexpected when displaying help. let mut submission_params = self.submission_parameters.clone(); if args.contains(&String::from("--help")) { - submission_params.confirmation_mode.dry_run = true; + submission_params.confirmation_mode.dry_run = DryRunType::HumanReadable; } Submitter::from(&submission_params) diff --git a/rs/cli/src/commands/update_default_subnets.rs b/rs/cli/src/commands/update_default_subnets.rs index 202787bf1..6d8b632c0 100644 --- a/rs/cli/src/commands/update_default_subnets.rs +++ b/rs/cli/src/commands/update_default_subnets.rs @@ -137,7 +137,7 @@ impl ExecutableCommand for UpdateDefaultSubnets { .collect(); if new_authorized == default_subnets { - println!("There are no diffs. Skipping proposal creation."); + info!("There are no diffs. Skipping proposal creation."); return Ok(()); } diff --git a/rs/cli/src/commands/vote.rs b/rs/cli/src/commands/vote.rs index ddba7d3a6..0302accd9 100644 --- a/rs/cli/src/commands/vote.rs +++ b/rs/cli/src/commands/vote.rs @@ -9,6 +9,7 @@ use log::info; use spinners::{Spinner, Spinners}; use crate::auth::AuthRequirement; +use crate::confirm::DryRunFormat; use crate::exe::{args::GlobalArgs, ExecutableCommand}; use crate::{ confirm::{ConfirmationModeOptions, HowToProceed}, @@ -119,7 +120,10 @@ impl ExecutableCommand for Vote { } } } - HowToProceed::DryRun => info!("Would have voted for proposal {}", prop_id), + HowToProceed::DryRun(format) => match format { + DryRunFormat::HumanReadable => info!("Would have voted for proposal {}", prop_id), + DryRunFormat::Json => println!("{}", prop_id), + }, } voted_proposals.insert(prop_id); } diff --git a/rs/cli/src/confirm.rs b/rs/cli/src/confirm.rs index 25ac11273..d9bed9582 100644 --- a/rs/cli/src/confirm.rs +++ b/rs/cli/src/confirm.rs @@ -1,10 +1,18 @@ -use clap::Args as ClapArgs; +use std::str::FromStr; + +use clap::{Args as ClapArgs, Parser}; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum DryRunFormat { + HumanReadable, + Json, +} #[derive(Debug, Clone, PartialEq, Eq)] pub enum HowToProceed { Confirm, Unconditional, - DryRun, + DryRun(DryRunFormat), #[allow(dead_code)] UnitTests, // Necessary for unit tests, otherwise confirmation is requested. // Generally this is hit when DreContext (created by get_mocked_ctx) has @@ -12,9 +20,31 @@ pub enum HowToProceed { // The net effect is that both the dry run and the final command are run. } -#[derive(ClapArgs, Debug, Clone)] +#[derive(Debug, Clone, Parser)] +pub(crate) enum DryRunType { + NoDryRun, + HumanReadable, + Json, +} + +impl FromStr for DryRunType { + type Err = clap::Error; + + fn from_str(s: &str) -> Result { + match s { + "no" => Ok(DryRunType::NoDryRun), + "yes" => Ok(DryRunType::HumanReadable), + "json" => Ok(DryRunType::Json), + _ => { + let mut cmd = clap::Command::new("dre"); + Err(cmd.error(clap::error::ErrorKind::InvalidValue, format!("invalid value {} for --dry-run", s))) + } + } + } +} /// Options for commands that may require confirmation. +#[derive(ClapArgs, Debug, Clone)] pub struct ConfirmationModeOptions { /// To skip the confirmation prompt #[clap( @@ -28,25 +58,29 @@ pub struct ConfirmationModeOptions { )] yes: bool, - #[clap(long, aliases = [ "dry-run", "dryrun", "simulate", "no"], env = "DRY_RUN", global = true, conflicts_with = "yes", help = r#"Dry-run, or simulate operation. If specified will not make any changes; instead, it will show what would be done or submitted."#,help_heading = "Options on how to proceed")] - pub(crate) dry_run: bool, + #[clap(long, aliases = [ "dry-run", "dryrun", "simulate", "no"], env = "DRY_RUN", global = true, conflicts_with = "yes", help = r#"Dry-run, or simulate operation. If specified will not make any changes; instead, it will show what would be done or submitted. If specified as --dry-run=json, it will print machine-readable JSON to standard output."#, help_heading = "Options on how to proceed", num_args = 0..=1, default_value="no", default_missing_value="yes")] + pub(crate) dry_run: DryRunType, } #[cfg(test)] impl ConfirmationModeOptions { /// Return an option set for unit tests, not instantiable via command line due to conflict. pub fn for_unit_tests() -> Self { - ConfirmationModeOptions { yes: true, dry_run: true } + ConfirmationModeOptions { + yes: true, + dry_run: DryRunType::HumanReadable, + } } } impl From<&ConfirmationModeOptions> for HowToProceed { fn from(o: &ConfirmationModeOptions) -> Self { - match (o.dry_run, o.yes) { - (false, true) => Self::Unconditional, - (true, false) => Self::DryRun, - (false, false) => Self::Confirm, - (true, true) => Self::UnitTests, // This variant cannot be instantiated via the command line. + match (&o.dry_run, o.yes) { + (DryRunType::NoDryRun, true) => Self::Unconditional, + (DryRunType::NoDryRun, false) => Self::Confirm, + (DryRunType::HumanReadable, false) => Self::DryRun(DryRunFormat::HumanReadable), + (DryRunType::Json, false) => Self::DryRun(DryRunFormat::Json), + (_, true) => Self::UnitTests, // These variants cannot be instantiated via the command line. } } } diff --git a/rs/cli/src/forum/impls.rs b/rs/cli/src/forum/impls.rs index 5e2ec764f..5cce448e9 100644 --- a/rs/cli/src/forum/impls.rs +++ b/rs/cli/src/forum/impls.rs @@ -178,7 +178,7 @@ impl Discourse { warn!("While creating a new post in topic {}, Discourse returned an error: {:?}", topic_url, e); } warn!("Please create a post in topic {} with the following content", topic_url); - println!("{}", body); + eprintln!("{}", body); let forum_post_link = dialoguer::Input::::new() .with_prompt("Forum post link") .allow_empty(false) diff --git a/rs/cli/src/governance.rs b/rs/cli/src/governance.rs index b6a45aeda..09cbe1686 100644 --- a/rs/cli/src/governance.rs +++ b/rs/cli/src/governance.rs @@ -2,6 +2,7 @@ use futures::future::BoxFuture; use ic_canisters::governance::GovernanceCanisterWrapper; use ic_nns_common::pb::v1::NeuronId; use ic_nns_governance_api::MakeProposalRequest; +use serde_json::json; use url::Url; use crate::proposal_executors::{ProposableViaGovernanceCanister, ProposalExecution, ProposalResponseWithId}; @@ -32,14 +33,22 @@ impl GovernanceCanisterProposalExecutor { pub fn simulate<'c, 'd, W: ProposableViaGovernanceCanister + 'c>( &'d self, cmd: &'c W, + machine_readable: bool, forum_post_link_description: Option, ) -> BoxFuture<'c, anyhow::Result<()>> where 'd: 'c, { Box::pin(async move { - println!("Proposal that would be submitted:\n{:#?}", cmd); - println!("Forum post link: {}", forum_post_link_description.unwrap_or("None".to_string())); + match machine_readable { + false => { + println!("Proposal that would be submitted:\n{:#?}", cmd); + println!("Forum post link: {}", forum_post_link_description.unwrap_or("None".to_string())); + } + true => { + println!("{}", json!(cmd)); + } + } Ok(()) }) } @@ -85,8 +94,12 @@ impl ProposalExecution for ProposalExecutionViaGovernanceCanister where T: ProposableViaGovernanceCanister, { - fn simulate(&self, forum_post_link_description: Option) -> BoxFuture<'_, anyhow::Result<()>> { - Box::pin(async { self.executor.simulate(&self.proposal, forum_post_link_description).await }) + fn simulate(&self, machine_readable: bool, forum_post_link_description: Option) -> BoxFuture<'_, anyhow::Result<()>> { + Box::pin(async move { + self.executor + .simulate(&self.proposal, machine_readable, forum_post_link_description) + .await + }) } fn submit<'a, 'b>(&'a self, forum_post_link: Option) -> BoxFuture<'b, anyhow::Result> diff --git a/rs/cli/src/ic_admin.rs b/rs/cli/src/ic_admin.rs index 340edb04a..f27db2e92 100644 --- a/rs/cli/src/ic_admin.rs +++ b/rs/cli/src/ic_admin.rs @@ -32,7 +32,12 @@ pub trait IcAdmin: Send + Sync + Debug { fn ic_admin_path(&self) -> Option; /// Runs the proposal in simulation mode (--dry-run). Prints out the result. - fn simulate_proposal(&self, cmd: Vec, forum_post_link_description: Option) -> BoxFuture<'_, anyhow::Result<()>>; + fn simulate_proposal( + &self, + cmd: Vec, + machine_readable: bool, + forum_post_link_description: Option, + ) -> BoxFuture<'_, anyhow::Result<()>>; /// Runs the proposal in forrealz mode. Result is returned and logged at debug level. fn submit_proposal<'a, 'b>(&'a self, cmd: Vec, forum_post_link: Option) -> BoxFuture<'b, anyhow::Result> @@ -148,7 +153,12 @@ impl IcAdmin for IcAdminImpl { }) } - fn simulate_proposal(&self, cmd: Vec, forum_post_link_description: Option) -> BoxFuture<'_, anyhow::Result<()>> { + fn simulate_proposal( + &self, + cmd: Vec, + machine_readable: bool, + forum_post_link_description: Option, + ) -> BoxFuture<'_, anyhow::Result<()>> { Box::pin(async move { debug!("Simulating proposal {:?}.", cmd); @@ -168,11 +178,15 @@ impl IcAdmin for IcAdminImpl { if !args.contains(&String::from("--dry-run")) { args.push("--dry-run".into()) }; + // When we want to simulate as JSON. + if machine_readable { + args.push("--json".into()) + } self.run(args.as_slice(), true).await.map(|r| r.trim().to_string())?; // Add the forum post link description as a string after the ic-admin command. - if forum_post_link_description.is_some() && !is_forum_post_link_description_url { + if forum_post_link_description.is_some() && !is_forum_post_link_description_url && !machine_readable { println!("Forum post link: {}", forum_post_link_description.unwrap()); } @@ -546,6 +560,7 @@ impl IcAdminProposalExecutor { pub fn simulate<'c, 'd, T: RunnableViaIcAdmin + 'c>( &'d self, cmd: &'c T, + machine_readable: bool, forum_post_link_description: Option, ) -> BoxFuture<'c, anyhow::Result<()>> where @@ -553,7 +568,9 @@ impl IcAdminProposalExecutor { { Box::pin(async move { let propose_command = cmd.to_ic_admin_arguments()?; - self.ic_admin.simulate_proposal(propose_command, forum_post_link_description).await?; + self.ic_admin + .simulate_proposal(propose_command, machine_readable, forum_post_link_description) + .await?; Ok(()) }) } @@ -587,8 +604,12 @@ where T: RunnableViaIcAdmin, T: ProducesProposalResult, { - fn simulate(&self, forum_post_link_description: Option) -> BoxFuture<'_, anyhow::Result<()>> { - Box::pin(async { self.executor.simulate(&self.proposal, forum_post_link_description).await }) + fn simulate(&self, machine_readable: bool, forum_post_link_description: Option) -> BoxFuture<'_, anyhow::Result<()>> { + Box::pin(async move { + self.executor + .simulate(&self.proposal, machine_readable, forum_post_link_description) + .await + }) } fn submit<'a, 'b>(&'a self, forum_post_link: Option) -> BoxFuture<'b, anyhow::Result> diff --git a/rs/cli/src/operations/hostos_rollout.rs b/rs/cli/src/operations/hostos_rollout.rs index d77eb7c48..4c8981bb8 100644 --- a/rs/cli/src/operations/hostos_rollout.rs +++ b/rs/cli/src/operations/hostos_rollout.rs @@ -58,21 +58,38 @@ impl std::fmt::Display for NodeGroup { } #[derive(Copy, Clone, Debug, Ord, Eq, PartialEq, PartialOrd)] pub enum NumberOfNodes { - Percentage(i32), - Absolute(i32), + Percentage(u32), + Absolute(u32), } impl FromStr for NumberOfNodes { - type Err = anyhow::Error; + type Err = clap::Error; fn from_str(input: &str) -> Result { if input.ends_with('%') { - let percentage = i32::from_str(input.trim_end_matches('%'))?; + let percentage = match u32::from_str(input.trim_end_matches('%')) { + Ok(p) => p, + Err(e) => { + return Err( + clap::Command::new("dre").error(clap::error::ErrorKind::InvalidValue, format!("while parsing --nodes-in-group: {}", e)) + ) + } + }; if (0..=100).contains(&percentage) { Ok(NumberOfNodes::Percentage(percentage)) } else { - Err(anyhow!("Percentage must be between 0 and 100")) + Err(clap::Command::new("dre").error( + clap::error::ErrorKind::InvalidValue, + "while parsing --nodes-in-group: percentage must be between 0 and 100", + )) } } else { - Ok(NumberOfNodes::Absolute(i32::from_str(input)?)) + Ok(NumberOfNodes::Absolute(match u32::from_str(input) { + Ok(v) => v, + Err(e) => { + return Err( + clap::Command::new("dre").error(clap::error::ErrorKind::InvalidValue, format!("while parsing --nodes-in-group: {}", e)) + ) + } + })) } } } @@ -95,13 +112,13 @@ impl std::fmt::Display for NumberOfNodes { #[derive(Copy, Clone, Debug, Ord, Eq, PartialEq, PartialOrd)] pub struct NodeGroupUpdate { pub node_group: NodeGroup, - pub maybe_number_nodes: Option, + pub number_nodes: NumberOfNodes, } impl NodeGroupUpdate { pub fn new(assignment: Option, owner: Option, nodes_per_subnet: NumberOfNodes) -> Self { NodeGroupUpdate { node_group: NodeGroup::new(assignment.unwrap_or_default(), owner.unwrap_or_default()), - maybe_number_nodes: Some(nodes_per_subnet), + number_nodes: nodes_per_subnet, } } @@ -111,11 +128,11 @@ impl NodeGroupUpdate { assignment, owner: self.node_group.owner, }, - maybe_number_nodes: self.maybe_number_nodes, + number_nodes: self.number_nodes, } } pub fn nodes_to_take(&self, group_size: usize) -> usize { - match self.maybe_number_nodes.unwrap_or_default() { + match self.number_nodes { NumberOfNodes::Percentage(percent_to_update) => (group_size as f32 * percent_to_update as f32 / 100.0).floor() as usize, NumberOfNodes::Absolute(number_nodes) => number_nodes as usize, } @@ -378,7 +395,7 @@ impl HostosRollout { } }) .count(), - update_group.maybe_number_nodes.unwrap_or_default(), + update_group.number_nodes, update_group.node_group.owner, update_group.node_group.assignment ); @@ -393,8 +410,9 @@ impl HostosRollout { { CandidatesSelection::Ok(candidates_unassigned) => { let nodes_to_take = update_group.nodes_to_take(unassigned_nodes.len()); - let nodes_to_update = candidates_unassigned.into_iter().take(nodes_to_take).collect::>(); + let nodes_to_update = candidates_unassigned.clone().into_iter().take(nodes_to_take).collect::>(); info!("{} candidate nodes selected for: {}", nodes_to_update.len(), update_group.node_group); + Ok(HostosRolloutResponse::Ok(nodes_to_update, None)) } CandidatesSelection::None(reason) => { @@ -532,6 +550,7 @@ pub mod test { use ic_management_backend::health::MockHealthStatusQuerier; use ic_management_backend::proposal::ProposalAgentImpl; use ic_management_types::{Network, Node, Operator, Provider, Subnet}; + use ic_protobuf::registry::node::v1::NodeRewardType; use std::collections::BTreeMap; use std::sync::OnceLock; @@ -596,7 +615,7 @@ pub mod test { open_proposals.clone(), NodeGroupUpdate { node_group: NodeGroup::new(Assigned, Others), - maybe_number_nodes: None, + number_nodes: NumberOfNodes::Percentage(100), }, ) .await; @@ -635,7 +654,7 @@ pub mod test { open_proposals.clone(), NodeGroupUpdate { node_group: NodeGroup::new(Assigned, Others), - maybe_number_nodes: None, + number_nodes: NumberOfNodes::Percentage(100), }, ) .await @@ -688,6 +707,7 @@ pub mod test { for i in start_at_number..start_at_number + num_nodes { let node = Node { principal: PrincipalId::new_node_test_id(i), + node_reward_type: Some(NodeRewardType::Type3), ip_addr: None, operator: Operator { principal: PrincipalId::new_node_test_id(i), @@ -700,6 +720,7 @@ pub mod test { datacenter: None, rewardable_nodes: BTreeMap::new(), ipv6: "".to_string(), + max_rewardable_nodes: BTreeMap::new(), }, cached_features: OnceLock::new(), hostname: None, diff --git a/rs/cli/src/proposal_executors.rs b/rs/cli/src/proposal_executors.rs index ee306f25f..54c3ae291 100644 --- a/rs/cli/src/proposal_executors.rs +++ b/rs/cli/src/proposal_executors.rs @@ -98,7 +98,7 @@ pub trait ProducesProposalResult { /// Represents anything that can be turned into a proposal request /// suitable to be made through the GovernanceCanisterWrapper, and /// also has the ability to deserialize to a ProposalResponseWithId. -pub trait ProposableViaGovernanceCanister: Debug + Send + Sync + Clone + Into { +pub trait ProposableViaGovernanceCanister: Debug + Send + Sync + Clone + Into + serde::Serialize { type ProposalResult: TryFrom + TryInto; } @@ -112,7 +112,7 @@ impl ProposableViaGovernanceCanister for MakeProposalRequest { /// of a proposal (any object that implements either RunnableViaIcAdmin or /// ProposableViaGovernanceCanister, and produces a ProposalResponseWithId). pub trait ProposalExecution: Send + Sync { - fn simulate(&self, forum_post_link_description: Option) -> BoxFuture<'_, anyhow::Result<()>>; + fn simulate(&self, machine_readable: bool, forum_post_link_description: Option) -> BoxFuture<'_, anyhow::Result<()>>; /// Runs the proposal in forrealz mode. Result is returned and logged at debug level. fn submit<'a, 'b>(&'a self, forum_post_link: Option) -> BoxFuture<'b, anyhow::Result> diff --git a/rs/cli/src/runner.rs b/rs/cli/src/runner.rs index e5f89c303..b4665ec39 100644 --- a/rs/cli/src/runner.rs +++ b/rs/cli/src/runner.rs @@ -33,6 +33,7 @@ use ic_management_types::TopologyChangePayload; use ic_types::PrincipalId; use indexmap::{IndexMap, IndexSet}; use itertools::Itertools; +use log::error; use log::info; use log::warn; @@ -157,10 +158,10 @@ impl Runner { if self.verbose { if let Some(run_log) = &subnet_creation_data.run_log { - println!("{}\n", run_log.join("\n")); + info!("{}\n", run_log.join("\n")); } } - println!("{}", subnet_creation_data); + info!("{}", subnet_creation_data); let replica_version = replica_version.unwrap_or( self.registry .nns_replica_version() @@ -192,7 +193,7 @@ impl Runner { pub async fn propose_subnet_change(&self, change: &SubnetChangeResponse) -> anyhow::Result> { if self.verbose { if let Some(run_log) = &change.run_log { - println!("{}\n", run_log.join("\n")); + info!("{}\n", run_log.join("\n")); } } @@ -411,7 +412,7 @@ impl Runner { HostosRolloutResponse::None(reason) => { reason .iter() - .for_each(|(group, reason)| println!("No nodes to update in group: {} because: {}", group, reason)); + .for_each(|(group, reason)| error!("No nodes to update in group: {} because: {}", group, reason)); Ok(None) } } @@ -424,8 +425,8 @@ impl Runner { .iter() .map(|p| p.to_string().split('-').next().unwrap().to_string()) .collect::>(); - println!("Will submit proposal to update the following nodes: {:?}", nodes_short); - println!("You will be able to follow the upgrade progress at https://grafana.mainnet.dfinity.network/explore?orgId=1&left=%7B%22datasource%22:%22PE62C54679EC3C073%22,%22queries%22:%5B%7B%22refId%22:%22A%22,%22datasource%22:%7B%22type%22:%22prometheus%22,%22uid%22:%22PE62C54679EC3C073%22%7D,%22editorMode%22:%22code%22,%22expr%22:%22hostos_version%7Bic_node%3D~%5C%22{}%5C%22%7D%5Cn%22,%22legendFormat%22:%22__auto%22,%22range%22:true,%22instant%22:true%7D%5D,%22range%22:%7B%22from%22:%22now-1h%22,%22to%22:%22now%22%7D%7D", nodes_short.iter().map(|n| n.to_string() + ".%2B").join("%7C")); + info!("Will submit proposal to update the following nodes: {:?}", nodes_short); + info!("You will be able to follow the upgrade progress at https://grafana.mainnet.dfinity.network/explore?orgId=1&left=%7B%22datasource%22:%22PE62C54679EC3C073%22,%22queries%22:%5B%7B%22refId%22:%22A%22,%22datasource%22:%7B%22type%22:%22prometheus%22,%22uid%22:%22PE62C54679EC3C073%22%7D,%22editorMode%22:%22code%22,%22expr%22:%22hostos_version%7Bic_node%3D~%5C%22{}%5C%22%7D%5Cn%22,%22legendFormat%22:%22__auto%22,%22range%22:true,%22instant%22:true%7D%5D,%22range%22:%7B%22from%22:%22now-1h%22,%22to%22:%22now%22%7D%7D", nodes_short.iter().map(|n| n.to_string() + ".%2B").join("%7C")); Ok(IcAdminProposal::new( IcAdminProposalCommand::DeployHostosToSomeNodes { @@ -473,7 +474,7 @@ impl Runner { row.add_cell(nr.reason.message()); table.add_row(row); } - println!("{}", table); + info!("{}", table); Ok(IcAdminProposal::new( ic_admin::IcAdminProposalCommand::RemoveNodes { @@ -541,7 +542,7 @@ impl Runner { .run_membership_change(change, replace_proposal_options(change).await?) .await .map_err(|e| { - println!("{}", e); + error!("{}", e); errors.push(e); }); changes.push(current) @@ -904,7 +905,7 @@ impl Runner { removed_nodes: removed_nodes.clone(), ..Default::default() }; - println!("{}", SubnetChangeResponse::new(&subnet_change, &health_of_nodes, summary)); + info!("{}", SubnetChangeResponse::new(&subnet_change, &health_of_nodes, summary)); Ok(()) } diff --git a/rs/cli/src/submitter.rs b/rs/cli/src/submitter.rs index 39033d792..cd9468dfd 100644 --- a/rs/cli/src/submitter.rs +++ b/rs/cli/src/submitter.rs @@ -2,7 +2,7 @@ use clap::Args as ClapArgs; use log::warn; use crate::{ - confirm::{ConfirmationModeOptions, HowToProceed}, + confirm::{ConfirmationModeOptions, DryRunFormat, HowToProceed}, forum::{ForumContext, ForumParameters, ForumPostKind}, proposal_executors::{ProposalExecution, ProposalResponseWithId}, util::yesno, @@ -43,7 +43,10 @@ impl Submitter { pub async fn propose(&self, execution: Box, kind: ForumPostKind) -> anyhow::Result> { if let HowToProceed::Unconditional = self.mode { } else { - execution.simulate(self.forum_parameters.forum_post_link_for_simulation()).await?; + let machine_readable = matches!(self.mode, HowToProceed::DryRun(DryRunFormat::Json)); + execution + .simulate(machine_readable, self.forum_parameters.forum_post_link_for_simulation()) + .await?; }; if let HowToProceed::Confirm = self.mode { @@ -53,7 +56,7 @@ impl Submitter { } } - if let HowToProceed::DryRun = self.mode { + if let HowToProceed::DryRun(_) = self.mode { Ok(None) } else { let forum_post = ForumContext::from(&self.forum_parameters).client()?.forum_post(kind).await?; diff --git a/rs/cli/src/unit_tests/cordoned_feature_fetcher.rs b/rs/cli/src/unit_tests/cordoned_feature_fetcher.rs index 2d8a5a28e..511c88f74 100644 --- a/rs/cli/src/unit_tests/cordoned_feature_fetcher.rs +++ b/rs/cli/src/unit_tests/cordoned_feature_fetcher.rs @@ -101,7 +101,7 @@ fn cordoned_feature_fetcher_tests() { let mut failed_scenarios = vec![]; for scenario in &scenarios { - println!("### Starting scenario `{}`", scenario.name); + eprintln!("### Starting scenario `{}`", scenario.name); let store = Store::new(scenario.offline).unwrap(); match &scenario.cache_contents { @@ -117,7 +117,7 @@ fn cordoned_feature_fetcher_tests() { if let Ok(features) = maybe_cordoned_features { failed_scenarios.push((features, scenario)); } - println!("### Ending scenario `{}`", scenario.name); + eprintln!("### Ending scenario `{}`", scenario.name); continue; } @@ -128,7 +128,7 @@ fn cordoned_feature_fetcher_tests() { if !cordoned_features.eq(&cordoned_features_from_cache) { failed_scenarios.push((cordoned_features, scenario)); } - println!("### Ending scenario `{}`", scenario.name); + eprintln!("### Ending scenario `{}`", scenario.name); } assert!( diff --git a/rs/cli/src/unit_tests/health_client.rs b/rs/cli/src/unit_tests/health_client.rs index 8156b4357..71191c99c 100644 --- a/rs/cli/src/unit_tests/health_client.rs +++ b/rs/cli/src/unit_tests/health_client.rs @@ -95,7 +95,7 @@ fn health_client_tests() { let mut failed_scenarios = vec![]; for scenario in &scenarios { - println!("### Starting scenario `{}`", scenario.name); + eprintln!("### Starting scenario `{}`", scenario.name); let store = Store::new(scenario.offline).unwrap(); let network = ic_management_types::Network::new_unchecked(scenario.network.clone(), &[]).unwrap(); let cache_path = store.node_health_file_outer(&network).unwrap(); @@ -111,13 +111,13 @@ fn health_client_tests() { if scenario.should_succeed != maybe_nodes_info.is_ok() { failed_scenarios.push((maybe_nodes_info, scenario)); - println!("### Ending scenario `{}`", scenario.name); + eprintln!("### Ending scenario `{}`", scenario.name); continue; } // Scenario should fail but it is expected to fail if !scenario.should_succeed { - println!("### Ending scenario `{}`", scenario.name); + eprintln!("### Ending scenario `{}`", scenario.name); continue; } @@ -129,6 +129,6 @@ fn health_client_tests() { failed_scenarios.push((Ok(nodes_info), scenario)); } - println!("### Ending scenario `{}`", scenario.name); + eprintln!("### Ending scenario `{}`", scenario.name); } } diff --git a/rs/cli/src/unit_tests/node_labels.rs b/rs/cli/src/unit_tests/node_labels.rs index 9cfe909f7..e15c660bb 100644 --- a/rs/cli/src/unit_tests/node_labels.rs +++ b/rs/cli/src/unit_tests/node_labels.rs @@ -121,7 +121,7 @@ fn test_node_labels() { let mut failed_scenarios = vec![]; for scenario in &scenarios { - println!("### Starting scenario `{}`", scenario.name); + eprintln!("### Starting scenario `{}`", scenario.name); let network = Network::new_unchecked(scenario.network.clone(), &[]).unwrap(); let store = Store::new(scenario.offline).unwrap(); let labels_path = store.guest_labels_cache_path_outer(&network).unwrap(); @@ -141,7 +141,7 @@ fn test_node_labels() { if let Ok(labels) = maybe_labels { failed_scenarios.push((labels, scenario)); } - println!("### Ending scenario `{}`", scenario.name); + eprintln!("### Ending scenario `{}`", scenario.name); continue; } @@ -151,7 +151,7 @@ fn test_node_labels() { if !labels.eq(&labels_from_cache) { failed_scenarios.push((labels, scenario)); } - println!("### Ending scenario `{}`", scenario.name); + eprintln!("### Ending scenario `{}`", scenario.name); } assert!( diff --git a/rs/cli/src/unit_tests/update_unassigned_nodes.rs b/rs/cli/src/unit_tests/update_unassigned_nodes.rs index 8c185b9da..17c24d0ef 100644 --- a/rs/cli/src/unit_tests/update_unassigned_nodes.rs +++ b/rs/cli/src/unit_tests/update_unassigned_nodes.rs @@ -122,7 +122,7 @@ async fn should_skip_update_same_version_nns_provided() { async fn should_update_unassigned_nodes() { let mut ic_admin = MockIcAdmin::new(); // Should be called since versions differ - ic_admin.expect_simulate_proposal().once().returning(|_, _| Box::pin(async { Ok(()) })); + ic_admin.expect_simulate_proposal().once().returning(|_, _, _| Box::pin(async { Ok(()) })); ic_admin .expect_submit_proposal() .once() diff --git a/rs/cli/src/unit_tests/version.rs b/rs/cli/src/unit_tests/version.rs index 084183519..43bfe4ff6 100644 --- a/rs/cli/src/unit_tests/version.rs +++ b/rs/cli/src/unit_tests/version.rs @@ -33,7 +33,7 @@ async fn guest_os_elect_version_tests() { let captured_cmd_clone = captured_cmd.clone(); let mut ic_admin = MockIcAdmin::new(); - ic_admin.expect_simulate_proposal().returning(|_, _| Box::pin(async { Ok(()) })); + ic_admin.expect_simulate_proposal().returning(|_, _, _| Box::pin(async { Ok(()) })); let captured_cmd_clone = captured_cmd_clone.clone(); ic_admin.expect_submit_proposal().returning(move |cmd, _forum_post| { *captured_cmd_clone.write().unwrap() = Some(cmd.clone());