diff --git a/rs/cli/src/clients.rs b/rs/cli/src/clients.rs index 2a4bf79a..7798cf63 100644 --- a/rs/cli/src/clients.rs +++ b/rs/cli/src/clients.rs @@ -16,6 +16,8 @@ pub struct DashboardBackendClient { } impl DashboardBackendClient { + // Only used in tests, which should be cleaned up together with this code. + #[allow(dead_code)] pub fn new(network: Network, dev: bool) -> DashboardBackendClient { Self { url: reqwest::Url::parse(if !dev { diff --git a/rs/cli/src/main.rs b/rs/cli/src/main.rs index 40e8562f..fd92d134 100644 --- a/rs/cli/src/main.rs +++ b/rs/cli/src/main.rs @@ -198,8 +198,7 @@ async fn main() -> Result<(), anyhow::Error> { cli::Commands::Version(cmd) => { match &cmd.subcommand { Update { version, release_tag} => { - // FIXME: backend needs gitlab access to figure out RCs and versions to retire - let runner = runner::Runner::from_opts(&cli_opts).await?; + let runner = runner::Runner::new_with_network_url(cli::Cli::from_opts(&cli_opts, true).await?.into(), backend_port).await?; let (_, retire_versions) = runner.prepare_versions_to_retire(false).await?; let ic_admin: IcAdminWrapper = cli::Cli::from_opts(&cli_opts, true).await?.into(); let new_replica_info = ic_admin::IcAdminWrapper::prepare_to_propose_to_update_elected_replica_versions(version, release_tag).await?; diff --git a/rs/cli/src/runner.rs b/rs/cli/src/runner.rs index 738f41ae..8fb1df8d 100644 --- a/rs/cli/src/runner.rs +++ b/rs/cli/src/runner.rs @@ -1,7 +1,7 @@ +use crate::clients::DashboardBackendClient; +use crate::ic_admin; use crate::ic_admin::ProposeOptions; use crate::ops_subnet_node_replace; -use crate::{cli, ic_admin}; -use crate::{cli::Opts, clients::DashboardBackendClient}; use decentralization::SubnetChangeResponse; use ic_base_types::PrincipalId; use ic_management_types::requests::NodesRemoveRequest; @@ -172,13 +172,6 @@ impl Runner { .map_err(|e| anyhow::anyhow!(e)) } - pub async fn from_opts(cli_opts: &Opts) -> anyhow::Result { - Ok(Self { - ic_admin: cli::Cli::from_opts(cli_opts, false).await?.into(), - dashboard_backend_client: DashboardBackendClient::new(cli_opts.network.clone(), cli_opts.dev), - }) - } - pub async fn new_with_network_url(ic_admin: ic_admin::IcAdminWrapper, backend_port: u16) -> anyhow::Result { let dashboard_backend_client = DashboardBackendClient::new_with_network_url(format!("http://localhost:{}/", backend_port)); @@ -191,27 +184,32 @@ impl Runner { pub(crate) async fn prepare_versions_to_retire(&self, edit_summary: bool) -> anyhow::Result<(String, Vec)> { let retireable_versions = self.dashboard_backend_client.get_retireable_versions().await?; - info!("Waiting for you to pick the versions to retire in your editor"); - let template = "# In the below lines, comment out the versions that you DO NOT want to retire".to_string(); - let versions = edit::edit(format!( - "{}\n{}", - template, - retireable_versions - .into_iter() - .map(|r| format!("{} # {}", r.commit_hash, r.branch)) - .join("\n"), - ))? - .trim() - .replace("\r(\n)?", "\n") - .split('\n') - .map(|s| regex::Regex::new("#.+$").unwrap().replace_all(s, "").to_string()) - .map(|s| s.trim().to_string()) - .filter(|s| !s.is_empty()) - .collect::>(); + let versions = if retireable_versions.is_empty() { + Vec::new() + } else { + info!("Waiting for you to pick the versions to retire in your editor"); + let template = "# In the below lines, comment out the versions that you DO NOT want to retire".to_string(); + let versions = edit::edit(format!( + "{}\n{}", + template, + retireable_versions + .into_iter() + .map(|r| format!("{} # {}", r.commit_hash, r.branch)) + .join("\n"), + ))? + .trim() + .replace("\r(\n)?", "\n") + .split('\n') + .map(|s| regex::Regex::new("#.+$").unwrap().replace_all(s, "").to_string()) + .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty()) + .collect::>(); - if versions.is_empty() { - warn!("Provided empty list of versions to retire"); - } + if versions.is_empty() { + warn!("Empty list of replica versions to unelect"); + } + versions + }; let mut template = "Removing the obsolete IC replica versions from the registry, to prevent unintended version downgrades in the future" diff --git a/rs/ic-management-backend/src/git_ic_repo.rs b/rs/ic-management-backend/src/git_ic_repo.rs index aeb88ee4..6daecce1 100644 --- a/rs/ic-management-backend/src/git_ic_repo.rs +++ b/rs/ic-management-backend/src/git_ic_repo.rs @@ -8,7 +8,7 @@ use std::process::Command; use std::time::{SystemTime, UNIX_EPOCH}; use std::{collections::HashMap, fs::File}; -const DEFAULT_DEPTH: usize = 1000; +const DEFAULT_DEPTH: usize = 10000; custom_error! {IoError Io { @@ -51,6 +51,23 @@ impl IcRepo { lock_file_path, }; + if repo_path.exists() { + // If the directory exists, but git status does not return success, remove the + // directory + if !match Command::new("git") + .args(["-C", repo_path.to_str().unwrap(), "status"]) + .output() + { + Ok(output) => output.status.success(), + Err(_) => false, + } { + std::fs::remove_dir_all(&repo_path).map_err(|e| IoError::Io { + source: e, + path: repo_path.to_path_buf(), + })?; + } + } + if repo_path.exists() { info!( "Repo {} already exists, fetching new updates", diff --git a/rs/ic-management-backend/src/proposal.rs b/rs/ic-management-backend/src/proposal.rs index 58baf301..8201c05e 100644 --- a/rs/ic-management-backend/src/proposal.rs +++ b/rs/ic-management-backend/src/proposal.rs @@ -5,13 +5,16 @@ use candid::{Decode, Encode}; use futures_util::future::try_join_all; use ic_agent::Agent; +use ic_management_types::UpdateElectedReplicaVersionsProposal; use ic_management_types::{NnsFunctionProposal, TopologyChangePayload, TopologyChangeProposal}; use ic_nns_governance::pb::v1::{proposal::Action, ListProposalInfo, ListProposalInfoResponse, NnsFunction}; use ic_nns_governance::pb::v1::{ProposalInfo, ProposalStatus, Topic}; +use itertools::Itertools; use registry_canister::mutations::do_add_nodes_to_subnet::AddNodesToSubnetPayload; use registry_canister::mutations::do_change_subnet_membership::ChangeSubnetMembershipPayload; use registry_canister::mutations::do_create_subnet::CreateSubnetPayload; use registry_canister::mutations::do_remove_nodes_from_subnet::RemoveNodesFromSubnetPayload; +use registry_canister::mutations::do_update_elected_replica_versions::UpdateElectedReplicaVersionsPayload; use registry_canister::mutations::do_update_subnet_replica::UpdateSubnetReplicaVersionPayload; use registry_canister::mutations::node_management::do_remove_nodes::RemoveNodesPayload; use serde::Serialize; @@ -114,6 +117,30 @@ impl ProposalAgent { Ok(result) } + pub async fn list_open_elect_replica_proposals(&self) -> Result> { + let proposals = &self.list_proposals(vec![ProposalStatus::Open]).await?; + let open_elect_guest_proposals = + filter_map_nns_function_proposals::(proposals); + + let result = open_elect_guest_proposals + .into_iter() + .map( + |(proposal_info, proposal_payload)| UpdateElectedReplicaVersionsProposal { + proposal_id: proposal_info.id.expect("proposal should have an id").id, + version_elect: proposal_payload + .replica_version_to_elect + .expect("version elect should exist"), + + versions_unelect: proposal_payload.replica_versions_to_unelect, + }, + ) + .sorted_by_key(|p| p.proposal_id) + .rev() + .collect::>(); + + Ok(result) + } + pub async fn list_update_subnet_version_proposals(&self) -> Result> { Ok(filter_map_nns_function_proposals(&self.list_proposals(vec![]).await?) .into_iter() diff --git a/rs/ic-management-backend/src/registry.rs b/rs/ic-management-backend/src/registry.rs index b744aee9..fe04071c 100644 --- a/rs/ic-management-backend/src/registry.rs +++ b/rs/ic-management-backend/src/registry.rs @@ -12,7 +12,7 @@ use ic_base_types::{RegistryVersion, SubnetId}; use ic_interfaces_registry::{RegistryClient, RegistryValue, ZERO_REGISTRY_VERSION}; use ic_management_types::{ Datacenter, DatacenterOwner, Guest, Network, NetworkError, Node, NodeProviderDetails, NodeProvidersResponse, - Operator, Provider, ReplicaRelease, Subnet, SubnetMetadata, + Operator, Provider, ReplicaRelease, Subnet, SubnetMetadata, UpdateElectedReplicaVersionsProposal, }; use ic_protobuf::registry::crypto::v1::PublicKey; use ic_protobuf::registry::replica_version::v1::BlessedReplicaVersions; @@ -277,10 +277,11 @@ impl RegistryState { // commit is present let mut commit_to_release: HashMap = HashMap::new(); for commit_hash in blessed_versions.iter() { + info!("Looking for branches that contain git rev: {}", commit_hash); match ic_repo.get_branches_with_commit(commit_hash) { // For each commit get a list of branches that have the commit Ok(branches) => { - debug!("Commit {} ==> branches {:?}", commit_hash, branches); + debug!("Commit {} ==> branches: {}", commit_hash, branches.join(", ")); for branch in branches.into_iter().sorted() { match RE.captures(&branch) { Some(capture) => { @@ -312,10 +313,12 @@ impl RegistryState { ); } None => { - warn!( - "branch {} for git rev {} does not match RC regex", - &commit_hash, &branch - ); + if branch != "master" && branch != "HEAD" { + warn!( + "branch {} for git rev {} does not match RC regex", + &commit_hash, &branch + ); + } } }; } @@ -585,6 +588,11 @@ impl RegistryState { .collect()) } + pub async fn open_elect_replica_proposals(&self) -> Result> { + let proposal_agent = proposal::ProposalAgent::new(self.nns_url.clone()); + proposal_agent.list_open_elect_replica_proposals().await + } + pub async fn subnets_with_proposals(&self) -> Result> { let subnets = self.subnets.clone(); let proposal_agent = proposal::ProposalAgent::new(self.nns_url.clone()); @@ -610,6 +618,17 @@ impl RegistryState { } pub async fn retireable_versions(&self) -> Result> { + if self.replica_releases.is_empty() { + warn!("No replica releases found"); + } else { + info!( + "Replica versions: {}", + self.replica_releases + .iter() + .map(|r| format!("{} ({})", r.commit_hash.clone(), r.branch)) + .join("\n") + ); + }; const NUM_RELEASE_BRANCHES_TO_KEEP: usize = 2; let active_releases = self .replica_releases @@ -622,12 +641,30 @@ impl RegistryState { .collect::>(); let subnet_versions: BTreeSet = self.subnets.values().map(|s| s.replica_version.clone()).collect(); let version_on_unassigned_nodes = self.get_unassigned_nodes_version().await?; + let versions_in_proposals: BTreeSet = self + .open_elect_replica_proposals() + .await? + .iter() + .flat_map(|p| p.versions_unelect.iter()) + .cloned() + .collect(); + info!("Active releases: {}", active_releases.iter().join(", ")); + info!( + "Replica versions in use on subnets: {}", + subnet_versions.iter().join(", ") + ); + info!("Replica version on unassigned nodes: {}", version_on_unassigned_nodes); + info!( + "Replica versions in open proposals: {}", + versions_in_proposals.iter().join(", ") + ); Ok(self .replica_releases .clone() .into_iter() .filter(|rr| !active_releases.contains(&rr.branch)) .filter(|rr| !subnet_versions.contains(&rr.commit_hash) && rr.commit_hash != version_on_unassigned_nodes) + .filter(|rr| !versions_in_proposals.contains(&rr.commit_hash)) .collect()) } diff --git a/rs/ic-management-types/src/lib.rs b/rs/ic-management-types/src/lib.rs index fd73a47d..7ddb3d7d 100644 --- a/rs/ic-management-types/src/lib.rs +++ b/rs/ic-management-types/src/lib.rs @@ -13,6 +13,7 @@ use registry_canister::mutations::do_add_nodes_to_subnet::AddNodesToSubnetPayloa use registry_canister::mutations::do_change_subnet_membership::ChangeSubnetMembershipPayload; use registry_canister::mutations::do_create_subnet::CreateSubnetPayload; use registry_canister::mutations::do_remove_nodes_from_subnet::RemoveNodesFromSubnetPayload; +use registry_canister::mutations::do_update_elected_replica_versions::UpdateElectedReplicaVersionsPayload; use registry_canister::mutations::do_update_subnet_replica::UpdateSubnetReplicaVersionPayload; use registry_canister::mutations::node_management::do_remove_nodes::RemoveNodesPayload; use serde::{Deserialize, Serialize}; @@ -61,6 +62,10 @@ impl NnsFunctionProposal for RemoveNodesPayload { const TYPE: NnsFunction = NnsFunction::RemoveNodes; } +impl NnsFunctionProposal for UpdateElectedReplicaVersionsPayload { + const TYPE: NnsFunction = NnsFunction::UpdateElectedReplicaVersions; +} + pub trait TopologyChangePayload: NnsFunctionProposal { fn get_added_node_ids(&self) -> Vec; fn get_removed_node_ids(&self) -> Vec; @@ -145,6 +150,13 @@ pub struct TopologyChangeProposal { pub id: u64, } +#[derive(CandidType, Serialize, Deserialize, Clone, Debug)] +pub struct UpdateElectedReplicaVersionsProposal { + pub proposal_id: u64, + pub version_elect: String, + pub versions_unelect: Vec, +} + impl From<(ProposalInfo, T)> for TopologyChangeProposal { fn from((info, payload): (ProposalInfo, T)) -> Self { Self {