fix(cli): Version unelect code fixed and cleaned up

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 {

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?;

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<Self> {
- 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<Self> {
  let dashboard_backend_client =
  DashboardBackendClient::new_with_network_url(format!("http:https://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<String>)> {
  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::<Vec<_>>();
+ 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::<Vec<_>>();
 
- 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"

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",

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<Vec<UpdateElectedReplicaVersionsProposal>> {
+ let proposals = &self.list_proposals(vec![ProposalStatus::Open]).await?;
+ let open_elect_guest_proposals =
+ filter_map_nns_function_proposals::<UpdateElectedReplicaVersionsPayload>(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::<Vec<_>>();
+
+ Ok(result)
+ }
+
  pub async fn list_update_subnet_version_proposals(&self) -> Result<Vec<SubnetUpdateProposal>> {
  Ok(filter_map_nns_function_proposals(&self.list_proposals(vec![]).await?)
  .into_iter()

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<String, ReplicaRelease> = 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<Vec<UpdateElectedReplicaVersionsProposal>> {
+ 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<BTreeMap<PrincipalId, Subnet>> {
  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<Vec<ReplicaRelease>> {
+ 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::<Vec<_>>();
  let subnet_versions: BTreeSet<String> = 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<String> = 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())
  }
 

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<PrincipalId>;
  fn get_removed_node_ids(&self) -> Vec<PrincipalId>;
@@ -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<String>,
+}
+
 impl<T: TopologyChangePayload> From<(ProposalInfo, T)> for TopologyChangeProposal {
  fn from((info, payload): (ProposalInfo, T)) -> Self {
  Self {