noobaa · romayalon · Nov 7, 2024 · Oct 28, 2024
diff --git a/src/sdk/config_fs.js b/src/sdk/config_fs.js
@@ -660,7 +660,7 @@ class ConfigFS {
     async create_account_config_file(account_data) {
         await this._throw_if_config_dir_locked();
         const { _id, name, owner = undefined } = account_data;
-        const { parsed_account_data, string_account_data} = await this._prepare_for_account_schema(account_data);
+        const { parsed_account_data, string_account_data } = await this._prepare_for_account_schema(account_data);
         const account_path = this.get_identity_path_by_id(_id);
         const account_dir_path = this.get_identity_dir_path_by_id(_id);
 
@@ -762,7 +762,7 @@ class ConfigFS {
      * link_account_name_index links the access key to the relative path of the account id config file
      * @param {string} account_id
      * @param {string} account_name
-     * @param {string} owner_account_id
+     * @param {string} [owner_account_id]
      * @returns {Promise<void>} 
      */
     async link_account_name_index(account_id, account_name, owner_account_id) {
@@ -821,22 +821,22 @@ class ConfigFS {
     /**
      * unlink_access_key_index unlinks the access key from the config directory
      * 1. get the account access_key path
-     * 2. check realpath on the account access_key path to make sure it belongs to the account id we meant to delete
-     * 3. check if the account id path is the same as the account name path 
-     * 4. unlink the account name path
-     * 5. else, do nothing as the name path might already point to a new identity/deleted by concurrent calls 
+     * 2. check realpath on the account access_key path to make sure it belongs to the account id (or account_name on versions older than 5.18) we meant to delete
+     * 3. check if the account id path is the same as the account access_key path 
+     * 4. unlink the account access_key path
+     * 5. else, do nothing as the access_key path might already point to a new identity/deleted by concurrent calls 
      * @param {string} access_key
      * @returns {Promise<void>} 
      */
-    async unlink_access_key_index(access_key, account_id_config_path) {
+    async unlink_access_key_index(access_key, account_config_path) {
         const access_key_path = this.get_account_or_user_path_by_access_key(access_key);
-        const should_unlink = await this._is_symlink_pointing_to_identity(access_key_path, account_id_config_path);
+        const should_unlink = await this._is_symlink_pointing_to_identity(access_key_path, account_config_path);
         if (should_unlink) {
             try {
                 await nb_native().fs.unlink(this.fs_context, access_key_path);
             } catch (err) {
                 if (err.code === 'ENOENT') {
-                    dbg.warn(`config_fs.unlink_access_key_index: account access_key already unlinked ${access_key} ${account_id_config_path}`);
+                    dbg.warn(`config_fs.unlink_access_key_index: account access_key already unlinked ${access_key} ${account_config_path}`);
                     return;
                 }
                 throw err;

diff --git a/src/test/system_tests/test_utils.js b/src/test/system_tests/test_utils.js
@@ -18,6 +18,7 @@ const { NodeHttpHandler } = require("@smithy/node-http-handler");
 const GPFS_ROOT_PATH = process.env.GPFS_ROOT_PATH;
 const IS_GPFS = !_.isUndefined(GPFS_ROOT_PATH);
 const TMP_PATH = get_tmp_path();
+const TEST_TIMEOUT = 60 * 1000;
 
 /**
  * TMP_PATH is a path to the tmp path based on the process platform
@@ -382,7 +383,7 @@ async function create_redirect_file(config_fs, custom_config_root_path) {
  */
 async function delete_redirect_file(config_fs) {
     const redirect_file_path = path.join(config.NSFS_NC_DEFAULT_CONF_DIR, config.NSFS_NC_CONF_DIR_REDIRECT_FILE);
-    await nb_native().fs.unlink(config_fs.fs_context, redirect_file_path);
+    await fs_utils.file_delete(redirect_file_path);
 }
 
 function generate_anon_s3_client(endpoint) {
@@ -590,7 +591,7 @@ async function clean_config_dir(config_fs, custom_config_dir_path) {
     const accounts_by_name = '/accounts_by_name/';
     const access_keys_dir_name = '/access_keys/';
     const system_json = '/system.json';
-    for (const dir of [buckets_dir_name, identities_dir_name, access_keys_dir_name, accounts_by_name]) {
+    for (const dir of [buckets_dir_name, identities_dir_name, access_keys_dir_name, accounts_by_name, config.NSFS_TEMP_CONF_DIR_NAME]) {
         const default_path = path.join(config.NSFS_NC_DEFAULT_CONF_DIR, dir);
         await fs_utils.folder_delete(default_path);
         const custom_path = path.join(custom_config_dir_path, dir);
@@ -622,6 +623,7 @@ exports.generate_anon_s3_client = generate_anon_s3_client;
 exports.TMP_PATH = TMP_PATH;
 exports.IS_GPFS = IS_GPFS;
 exports.is_nc_coretest = is_nc_coretest;
+exports.TEST_TIMEOUT = TEST_TIMEOUT;
 exports.generate_nsfs_account = generate_nsfs_account;
 exports.get_new_buckets_path_by_test_env = get_new_buckets_path_by_test_env;
 exports.write_manual_config_file = write_manual_config_file;

diff --git a/src/test/unit_tests/jest_tests/test_cli_upgrade.test.js b/src/test/unit_tests/jest_tests/test_cli_upgrade.test.js
@@ -4,13 +4,13 @@
 // disabling init_rand_seed as it takes longer than the actual test execution
 process.env.DISABLE_INIT_RANDOM_SEED = 'true';
 
-
 const os = require('os');
 const path = require('path');
+const config = require('../../../../config');
 const pkg = require('../../../../package.json');
 const fs_utils = require('../../../util/fs_utils');
 const { ConfigFS } = require('../../../sdk/config_fs');
-const { TMP_PATH, exec_manage_cli } = require('../../system_tests/test_utils');
+const { TMP_PATH, exec_manage_cli, clean_config_dir, fail_test_if_default_config_dir_exists, TEST_TIMEOUT } = require('../../system_tests/test_utils');
 const { ManageCLIError } = require('../../../manage_nsfs/manage_nsfs_cli_errors');
 const { ManageCLIResponse } = require('../../../manage_nsfs/manage_nsfs_cli_responses');
 const { TYPES, UPGRADE_ACTIONS } = require('../../../manage_nsfs/manage_nsfs_constants');
@@ -242,8 +242,17 @@ const invalid_hostname_system_json = {
 };
 
 describe('noobaa cli - upgrade', () => {
+    beforeAll(async () => {
+        await fail_test_if_default_config_dir_exists('test_cli_upgrade');
+        await fs_utils.create_fresh_path(config.NSFS_NC_DEFAULT_CONF_DIR);
+    });
+
     afterEach(async () => await fs_utils.file_delete(config_fs.system_json_path));
 
+    afterAll(async () => {
+        await clean_config_dir(config_fs, config_root);
+    }, TEST_TIMEOUT);
+
     it('upgrade start - should fail on no system', async () => {
         const options = { config_root, expected_version: pkg.version, expected_hosts };
         const res = await exec_manage_cli(TYPES.UPGRADE, UPGRADE_ACTIONS.START, options, true);

diff --git a/...s/jest_tests/test_config_dir_structure.js → ...t_tests/test_config_dir_structure.test.js b/...s/jest_tests/test_config_dir_structure.js → ...t_tests/test_config_dir_structure.test.js
@@ -5,7 +5,7 @@ const path = require('path');
 const config = require('../../../../config');
 const { ConfigFS } = require('../../../sdk/config_fs');
 const { TMP_PATH, create_redirect_file, create_config_dir, clean_config_dir,
-    fail_test_if_default_config_dir_exists } = require('../../system_tests/test_utils');
+    fail_test_if_default_config_dir_exists, TEST_TIMEOUT } = require('../../system_tests/test_utils');
 const { get_process_fs_context, is_path_exists } = require('../../../util/native_fs_utils');
 
 const tmp_fs_path = path.join(TMP_PATH, 'test_config_fs');
@@ -35,7 +35,7 @@ describe('create_config_dirs_if_missing', () => {
 
     afterEach(async () => {
         await clean_config_dir(config_fs, CUSTOM_CONF_DIR_PATH);
-    });
+    }, TEST_TIMEOUT);
 
     it('create_config_dirs_if_missing() first time - nothing exists - everything should be created', async () => {
         await default_config_fs.create_config_dirs_if_missing();

diff --git a/src/test/unit_tests/jest_tests/test_config_fs_backward_compatibility.test.js b/src/test/unit_tests/jest_tests/test_config_fs_backward_compatibility.test.js
@@ -4,7 +4,7 @@
 const path = require('path');
 const fs_utils = require('../../../util/fs_utils');
 const { ConfigFS, CONFIG_TYPES } = require('../../../sdk/config_fs');
-const { TMP_PATH, write_manual_config_file, write_manual_old_account_config_file } = require('../../system_tests/test_utils');
+const { TMP_PATH, TEST_TIMEOUT, write_manual_config_file, write_manual_old_account_config_file } = require('../../system_tests/test_utils');
 
 
 const tmp_fs_path = path.join(TMP_PATH, 'test_config_fs_backward_compatibility');
@@ -38,7 +38,7 @@ describe('ConfigFS Backwards Compatibility', () => {
 
     afterAll(async () => {
         await fs_utils.folder_delete(config_root);
-    });
+    }, TEST_TIMEOUT);
 
     afterEach(async () => {
         await clean_accounts();

diff --git a/src/test/unit_tests/jest_tests/test_nc_upgrade_manager.test.js b/src/test/unit_tests/jest_tests/test_nc_upgrade_manager.test.js
@@ -16,7 +16,7 @@ const { NCUpgradeManager, DEFAULT_NC_UPGRADE_SCRIPTS_DIR, OLD_DEFAULT_PACKAGE_VE
     OLD_DEFAULT_CONFIG_DIR_VERSION, CONFIG_DIR_UNLOCKED, CONFIG_DIR_LOCKED } = require('../../../upgrade/nc_upgrade_manager');
 const { ConfigFS } = require('../../../sdk/config_fs');
 const { TMP_PATH, create_redirect_file, create_config_dir,
-    fail_test_if_default_config_dir_exists, clean_config_dir } = require('../../system_tests/test_utils');
+    fail_test_if_default_config_dir_exists, clean_config_dir, TEST_TIMEOUT } = require('../../system_tests/test_utils');
 
 const config_root = path.join(TMP_PATH, 'config_root_nc_upgrade_manager_test');
 const config_fs = new ConfigFS(config_root);
@@ -185,7 +185,7 @@ describe('nc upgrade manager - upgrade RPM', () => {
 
     afterEach(async () => {
         await clean_config_dir(config_fs, config_root);
-    });
+    }, TEST_TIMEOUT);
 
     it('upgrade rpm - nothing to upgrade - no changes in system.json', async () => {
         await config_fs.create_system_config_file(JSON.stringify(current_expected_system_json));
@@ -234,6 +234,10 @@ describe('nc upgrade manager - upgrade config directory', () => {
         await fail_test_if_default_config_dir_exists('test_config_dir_nc_upgrade_manager', config_fs);
     });
 
+    afterEach(async () => {
+        await clean_config_dir(config_fs, config_root);
+    }, TEST_TIMEOUT);
+
     describe('nc upgrade manager - config_directory_defaults', () => {
 
         it('config_directory_defaults - hostname from_version exists - 5.16.0', () => {
@@ -323,7 +327,7 @@ describe('nc upgrade manager - upgrade config directory', () => {
 
     describe('nc upgrade manager - _run_nc_upgrade_scripts', () => {
         const from_version = '0.0.0';
-        const to_version = '1.0.0';
+        const to_version = '0.0.9';
         const custom_upgrade_scripts_dir = path.join(TMP_PATH, 'custom_upgrade_scripts_dir');
         const custom_upgrade_scripts_dir_version_path = path.join(TMP_PATH, 'custom_upgrade_scripts_dir', to_version);
         const default_upgrade_script_dir_version_path = path.join(DEFAULT_NC_UPGRADE_SCRIPTS_DIR, to_version);
@@ -419,13 +423,13 @@ describe('nc upgrade manager - upgrade config directory', () => {
 
         afterEach(async () => {
             await clean_config_dir(config_fs, config_root);
-        });
+        }, TEST_TIMEOUT);
 
         it('_update_config_dir_upgrade_start - system_data doesn\'t contain old config_directory data', async () => {
             const system_data = old_expected_system_json;
             const options = {
                 config_dir_from_version: '0.0.0',
-                config_dir_to_version: '1.0.0',
+                config_dir_to_version: '0.0.9',
                 package_from_version: '5.16.0',
                 package_to_version: '5.17.0',
             };
@@ -484,7 +488,7 @@ describe('nc upgrade manager - upgrade config directory', () => {
 
         afterEach(async () => {
             await clean_config_dir(config_fs, config_root);
-        });
+        }, TEST_TIMEOUT);
 
         it('_update_config_dir_upgrade_finish - system_data doesn\'t contain old successful_upgrades data', async () => {
             const system_data = _.cloneDeep(old_expected_system_json_empty_successful_upgrades);
@@ -527,7 +531,7 @@ describe('nc upgrade manager - upgrade config directory', () => {
                 'running_host': hostname,
                 'completed_scripts': [],
                 'config_dir_from_version': '0.0.0',
-                'config_dir_to_version': '1.0.0',
+                'config_dir_to_version': '0.0.9',
                 'package_from_version': '5.17.0',
                 'package_to_version': '5.18.0',
                 'error': new Error('this is a last failure error').stack
@@ -575,7 +579,7 @@ describe('nc upgrade manager - upgrade config directory', () => {
 
         afterEach(async () => {
             await clean_config_dir(config_fs, config_root);
-        });
+        }, TEST_TIMEOUT);
 
         it('_update_config_dir_upgrade_failed - system_data doesn\'t contain old upgrade_history data', async () => {
             const system_data = _.cloneDeep(old_expected_system_json_empty_successful_upgrades);
@@ -622,7 +626,7 @@ describe('nc upgrade manager - upgrade config directory', () => {
                 'running_host': hostname,
                 'completed_scripts': [],
                 'config_dir_from_version': '0.0.0',
-                'config_dir_to_version': '1.0.0',
+                'config_dir_to_version': '0.0.9',
                 'package_from_version': '5.17.0',
                 'package_to_version': '5.18.0',
                 'error': new Error('this is a last failure error').stack

diff --git a/src/test/unit_tests/jest_tests/test_versioning_concurrency.test.js b/src/test/unit_tests/jest_tests/test_versioning_concurrency.test.js
@@ -7,11 +7,9 @@ const P = require('../../../util/promise');
 const fs_utils = require('../../../util/fs_utils');
 const NamespaceFS = require('../../../sdk/namespace_fs');
 const buffer_utils = require('../../../util/buffer_utils');
-const { TMP_PATH, IS_GPFS } = require('../../system_tests/test_utils');
+const { TMP_PATH, IS_GPFS, TEST_TIMEOUT } = require('../../system_tests/test_utils');
 const { crypto_random_string } = require('../../../util/string_utils');
 const endpoint_stats_collector = require('../../../sdk/endpoint_stats_collector');
-const SECONDS = 1000;
-const test_timeout = SECONDS * 60;
 
 function make_dummy_object_sdk(nsfs_config, uid, gid) {
     return {
@@ -71,7 +69,7 @@ describe('test versioning concurrency', () => {
         expect(failed_operations).toHaveLength(0);
         const versions = await nsfs.list_object_versions({ bucket: bucket }, DUMMY_OBJECT_SDK);
         expect(versions.objects.length).toBe(num_of_concurrency);
-    }, test_timeout);
+    }, TEST_TIMEOUT);
 
     it('multiple delete version id and key', async () => {
         const bucket = 'bucket1';
@@ -91,7 +89,7 @@ describe('test versioning concurrency', () => {
         await P.delay(1000);
         expect(successful_operations).toHaveLength(num_of_concurrency);
         expect(failed_operations).toHaveLength(0);
-    }, test_timeout);
+    }, TEST_TIMEOUT);
 
     // same as s3tests_boto3/functional/test_s3.py::test_versioning_concurrent_multi_object_delete,
     // this test has a bug, it tries to create the bucket twice and fails
@@ -133,7 +131,7 @@ describe('test versioning concurrency', () => {
         }
         const list_res = await nsfs.list_objects({ bucket: bucket }, DUMMY_OBJECT_SDK);
         expect(list_res.objects.length).toBe(0);
-    }, test_timeout);
+    }, TEST_TIMEOUT);
 
     it('concurrent delete of latest version', async () => {
         const bucket = 'bucket1';
@@ -157,7 +155,7 @@ describe('test versioning concurrency', () => {
         expect(versions.objects.length).toBe(8); // 5 versions before + 3 delete markers concurrent
         const delete_marker_arr = versions.objects.filter(object => object.delete_marker === true);
         expect(delete_marker_arr.length).toBe(3);
-    }, test_timeout);
+    }, TEST_TIMEOUT);
 
     it('concurrent put object and head object latest version', async () => {
         const bucket = 'bucket1';
@@ -185,7 +183,7 @@ describe('test versioning concurrency', () => {
         expect(successful_head_operations.length).toBe(number_of_iterations);
         const versions = await nsfs.list_object_versions({ bucket: bucket }, DUMMY_OBJECT_SDK);
         expect(versions.objects.length).toBe(number_of_iterations + 1); // 1 version before + 10 versions concurrent
-    }, test_timeout);
+    }, TEST_TIMEOUT);
 
     it('concurrent puts & delete latest objects', async () => {
         const bucket = 'bucket1';
@@ -219,7 +217,7 @@ describe('test versioning concurrency', () => {
         expect(num_of_delete_markers).toBe(num_of_concurrency);
         const num_of_latest_versions = (versions.objects.filter(version => version.is_latest === true)).length;
         expect(num_of_latest_versions).toBe(1);
-    }, test_timeout);
+    }, TEST_TIMEOUT);
 
     it('concurrent puts & delete objects by version id', async () => {
         const bucket = 'bucket1';
@@ -253,7 +251,7 @@ describe('test versioning concurrency', () => {
         expect(num_of_delete_markers).toBe(0);
         const num_of_latest_versions = (versions.objects.filter(version => version.is_latest === true)).length;
         expect(num_of_latest_versions).toBe(1);
-    }, test_timeout);
+    }, TEST_TIMEOUT);
 
     it('concurrent delete objects by version id/latest', async () => {
         const bucket = 'bucket1';
@@ -286,7 +284,7 @@ describe('test versioning concurrency', () => {
         expect(num_of_delete_markers).toBe(num_of_concurrency);
         const num_of_latest_versions = (versions.objects.filter(version => version.is_latest === true)).length;
         expect(num_of_latest_versions).toBe(1);
-    }, test_timeout);
+    }, TEST_TIMEOUT);
 
     it('nested key - concurrent delete multiple objects', async () => {
         const bucket = 'bucket1';
@@ -323,7 +321,7 @@ describe('test versioning concurrency', () => {
         }
         const list_res = await nsfs.list_objects({ bucket: bucket }, DUMMY_OBJECT_SDK);
         expect(list_res.objects).toHaveLength(0);
-    }, test_timeout);
+    }, TEST_TIMEOUT);
 
     it('nested key - concurrent puts & deletes', async () => {
         const bucket = 'bucket1';
@@ -351,7 +349,7 @@ describe('test versioning concurrency', () => {
         expect(upload_failed_operations).toHaveLength(0);
         expect(delete_successful_operations).toHaveLength(num_of_concurrency);
         expect(delete_failed_operations).toHaveLength(0);
-    }, test_timeout);
+    }, TEST_TIMEOUT);
 
     it('concurrent puts & list versions', async () => {
         const bucket = 'bucket1';
@@ -391,7 +389,7 @@ describe('test versioning concurrency', () => {
         expect(num_of_delete_markers).toBe(0);
         const num_of_latest_versions = (versions.objects.filter(version => version.is_latest === true)).length;
         expect(num_of_latest_versions).toBe(initial_num_of_objects);
-    }, test_timeout);
+    }, TEST_TIMEOUT);
 
     it('concurrent puts & list versions - version id paging', async () => {
         const bucket = 'bucket1';
@@ -434,7 +432,7 @@ describe('test versioning concurrency', () => {
         expect(num_of_delete_markers).toBe(0);
         const num_of_latest_versions = (merged_versions.filter(version => version.is_latest === true)).length;
         expect(num_of_latest_versions).toBe(initial_num_of_objects);
-    }, test_timeout);
+    }, TEST_TIMEOUT);
 
     it('multiple puts of the same key - suspended', async () => {
         const bucket = 'bucket-s';
@@ -455,7 +453,7 @@ describe('test versioning concurrency', () => {
         expect(failed_operations).toHaveLength(0);
         const versions = await nsfs.list_object_versions({ bucket: bucket }, DUMMY_OBJECT_SDK);
         expect(versions.objects.length).toBe(1); // save only the null version
-    }, test_timeout);
+    }, TEST_TIMEOUT);
 
     it('multiple puts of the same key - enabled and suspended', async () => {
         const bucket = 'bucket-es';
@@ -489,7 +487,7 @@ describe('test versioning concurrency', () => {
         expect(failed_operations1).toHaveLength(0);
         const versions = await nsfs.list_object_versions({ bucket: bucket }, DUMMY_OBJECT_SDK);
         expect(versions.objects.length).toBe(num_of_concurrency1 + 1); // num_of_concurrency1 is the number of versions uploaded when versioning was enabled + 1 null version
-    }, test_timeout);
+    }, TEST_TIMEOUT);
 
     it('multiple delete different keys', async () => {
         const bucket = 'bucket1';
@@ -523,7 +521,7 @@ describe('test versioning concurrency', () => {
         });
         expect(num_objs).toBe(num_objects - num_deletes);
 
-    }, test_timeout);
+    }, TEST_TIMEOUT);
 
     it('copy-object to same target', async () => {
         const num_copies = 5;
@@ -555,7 +553,7 @@ describe('test versioning concurrency', () => {
             }
         });
         expect(num_versions).toBe(num_copies);
-    }, test_timeout);
+    }, TEST_TIMEOUT);
 });
 
 /**