fix(ic-backup, CON-1035) Improve more divergence check in order to reduce flakiness

Author: zoran-dfinity

rs/backup/src/backup_helper.rs

@@ -901,7 +901,7 @@ fn last_dir_height(dir: &PathBuf, radix: u32) -> u64 {
     }
 }
 
-fn last_checkpoint(dir: &Path) -> u64 {
+pub fn last_checkpoint(dir: &Path) -> u64 {
     last_dir_height(&dir.join("checkpoints"), 16)
 }
 

rs/tests/src/orchestrator/backup_manager.rs

@@ -42,7 +42,7 @@ use crate::{
     },
     util::{block_on, get_nns_node},
 };
-use ic_backup::backup_helper::ls_path;
+use ic_backup::backup_helper::{last_checkpoint, ls_path};
 use ic_backup::config::{ColdStorage, Config, SubnetConfig};
 use ic_backup::util::sleep_secs;
 use ic_base_types::SubnetId;
@@ -77,7 +77,7 @@ pub fn config(env: TestEnv) {
 pub fn test(env: TestEnv) {
     let log = env.logger();
 
-    // Create all directories
+    info!(log, "Create all directories");
     let root_dir = tempfile::TempDir::new()
         .expect("failed to create a temporary directory")
         .path()
@@ -91,6 +91,7 @@ pub fn test(env: TestEnv) {
         .path()
         .to_path_buf();
 
+    info!(log, "Fetch the replica version");
     let nns_node = get_nns_node(&env.topology_snapshot());
     let node_ip: IpAddr = nns_node.get_ip_addr();
     let subnet_id = env.topology_snapshot().root_subnet_id();
@@ -99,6 +100,10 @@ pub fn test(env: TestEnv) {
     let initial_replica_version = ReplicaVersion::try_from(replica_version.clone())
         .expect("Assigned replica version should be valid");
 
+    info!(
+        log,
+        "Copy the binaries needed for replay of the current version"
+    );
     let backup_binaries_dir = backup_dir.join("binaries").join(&replica_version);
     fs::create_dir_all(&backup_binaries_dir).expect("failure creating backup binaries directory");
 
@@ -109,6 +114,10 @@ pub fn test(env: TestEnv) {
     copy_file(&binaries_path, &backup_binaries_dir, "sandbox_launcher");
     copy_file(&binaries_path, &backup_binaries_dir, "canister_sandbox");
 
+    info!(
+        log,
+        "Download the binaries needed for replay of the mainnet version"
+    );
     let mainnet_version = env
         .read_dependency_to_string("testnet/mainnet_nns_revision.txt")
         .expect("could not read mainnet version!");
@@ -130,6 +139,7 @@ pub fn test(env: TestEnv) {
         .expect("chmod command failed");
     chmod.wait_with_output().expect("chmod execution failed");
 
+    info!(log, "Run ECDSA signature test");
     let nns_node = env.get_first_healthy_nns_node_snapshot();
     let agent = nns_node.build_default_agent();
     let nns_canister = block_on(MessageCanister::new(
@@ -146,6 +156,7 @@ pub fn test(env: TestEnv) {
     );
     run_ecdsa_signature_test(&nns_canister, &log, key);
 
+    info!(log, "Install universal canister");
     let log2 = log.clone();
     let id = nns_node.effective_canister_id();
     let canister_id_hex: String = block_on({
@@ -155,13 +166,13 @@ pub fn test(env: TestEnv) {
         }
     });
 
-    // Update the registry with the backup key
+    info!(log, "Update the registry with the backup key");
     let payload = get_updatesubnetpayload_with_keys(subnet_id, None, Some(vec![backup_public_key]));
     block_on(update_subnet_record(nns_node.get_public_url(), payload));
     let backup_mean = AuthMean::PrivateKey(backup_private_key);
     wait_until_authentication_is_granted(&node_ip, "backup", &backup_mean);
 
-    // Fetch NNS public key
+    info!(log, "Fetch NNS public key");
     let nns_public_key = env
         .prep_dir("")
         .expect("missing NNS public key")
@@ -229,6 +240,7 @@ pub fn test(env: TestEnv) {
         &log,
     ));
 
+    info!(log, "Wait for archived checkpoint");
     let archive_dir = backup_dir.join("archive").join(subnet_id.to_string());
     // make sure we have some archive of the old version before upgrading to the new one
     loop {
@@ -305,6 +317,7 @@ pub fn test(env: TestEnv) {
     info!(log, "Modify memory file: {:?}", memory_artifact_path);
     modify_byte_in_file(memory_artifact_path).expect("Modifying a byte failed");
 
+    info!(log, "Start again the backup process in a separate thread");
     let mut command = Command::new(&ic_backup_path);
     command
         .arg("--config-file")
@@ -317,10 +330,12 @@ pub fn test(env: TestEnv) {
         .expect("Failed to start backup process");
     info!(log, "Started process: {}", child.id());
 
-    assert!(cold_storage_exists(
-        &log,
-        cold_storage_dir.join(subnet_id.to_string())
-    ));
+    if !cold_storage_exists(&log, cold_storage_dir.join(subnet_id.to_string())) {
+        info!(log, "Kill child process");
+        child.kill().expect("Error killing backup process");
+        panic!("No cold storage");
+    }
+
     info!(log, "Artifacts and states are moved to cold storage");
 
     let mut hash_mismatch = false;
@@ -365,17 +380,15 @@ fn some_checkpoint_dir(backup_dir: &Path, subnet_id: &SubnetId) -> Option<PathBu
     let dir = backup_dir
         .join("data")
         .join(subnet_id.to_string())
-        .join("ic_state")
-        .join("checkpoints");
+        .join("ic_state");
     if !dir.exists() {
         return None;
     }
-    if let Ok(mut cps) = fs::read_dir(dir) {
-        if let Some(Ok(cp)) = cps.next() {
-            return Some(cp.path());
-        }
+    let lcp = last_checkpoint(&dir);
+    if lcp == 0 {
+        return None;
     }
-    None
+    Some(dir.join(format!("checkpoints/{:016x}", lcp)))
 }
 
 fn modify_byte_in_file(file_path: PathBuf) -> std::io::Result<()> {