From 77dc74096adb9d4bc156635b89dc55f8980fb895 Mon Sep 17 00:00:00 2001 From: Romy <35330373+romayalon@users.noreply.github.com> Date: Wed, 25 Dec 2024 14:32:34 +0200 Subject: [PATCH] NC | Online Upgrade | add host config dir version | Health - add blocked hosts check Signed-off-by: Romy <35330373+romayalon@users.noreply.github.com> --- src/cmd/nsfs.js | 2 +- src/manage_nsfs/health.js | 47 +++++++++++- src/sdk/config_fs.js | 45 +++++++++--- .../jest_tests/test_config_fs.test.js | 42 +++++++++++ .../test_nc_upgrade_manager.test.js | 72 ++++++++++--------- src/test/unit_tests/test_nc_health.js | 60 +++++++++++----- src/upgrade/nc_upgrade_manager.js | 4 +- 7 files changed, 209 insertions(+), 63 deletions(-) diff --git a/src/cmd/nsfs.js b/src/cmd/nsfs.js index 7a49fe5190..80f09e74a9 100644 --- a/src/cmd/nsfs.js +++ b/src/cmd/nsfs.js @@ -330,7 +330,7 @@ async function main(argv = minimist(process.argv.slice(2))) { const nc_upgrade_manager = new NCUpgradeManager(config_fs); await nc_upgrade_manager.update_rpm_upgrade(); } else { - system_data = await config_fs.init_nc_system(); + system_data = await config_fs.register_hostname_in_system_json(); } } diff --git a/src/manage_nsfs/health.js b/src/manage_nsfs/health.js index 3c02a908c2..1a81707f68 100644 --- a/src/manage_nsfs/health.js +++ b/src/manage_nsfs/health.js @@ -450,15 +450,17 @@ class NSFSHealth { */ _get_config_dir_status(system_data) { if (!system_data) return { error: 'system data is missing' }; + const blocked_hosts_status = this._get_blocked_hosts_status(this.config_fs, system_data); const config_dir_data = system_data.config_directory; - if (!config_dir_data) return { error: 'config directory data is missing, must upgrade config directory' }; + if (!config_dir_data) return { error: 'config directory data is missing, must upgrade config directory', blocked_hosts: blocked_hosts_status }; const config_dir_upgrade_status = this._get_config_dir_upgrade_status(config_dir_data); return { phase: config_dir_data.phase, config_dir_version: config_dir_data.config_dir_version, upgrade_package_version: config_dir_data.upgrade_package_version, upgrade_status: config_dir_upgrade_status, - error: config_dir_upgrade_status.error || undefined + error: config_dir_upgrade_status.error || undefined, + blocked_hosts: blocked_hosts_status }; } @@ -480,6 +482,47 @@ class NSFSHealth { } } + /** + * _get_blocked_hosts_status checks if there are any hosts blocked for updating the config directory + * 1. if only config dir was upgraded (>=5.18.0) - host's config_dir_version does not exist (<5.18.0) and system's config_dir_version exists (>=5.18.0) + * it means it's not blocked because the source code won't include _throw_if_config_dir_locked() but it still can create invalid config files, therefore including it in the blocked list + * 2. if system's config_dir_version wasn't upgraded yet and hosts's config_dir_version exist (>= 5.18.0) + * it means updates to the config directory from this host are blocked + * 3. if system's config dir version does not match the hosts's config_dir_version - updates to the config directory from this host are blocked + * @param {import('../sdk/config_fs').ConfigFS} config_fs + * @param {Object} system_data + * @returns {Object} + */ + _get_blocked_hosts_status(config_fs, system_data) { + const system_config_dir_version = system_data.config_directory?.config_dir_version; + const hosts_data = config_fs.get_hosts_data(system_data); + let res; + for (const host_name of Object.keys(hosts_data)) { + const host_data = hosts_data[host_name]; + let version_compare_err; + const only_config_dir_upgraded = !host_data.config_dir_version && system_config_dir_version; + const only_host_upgraded = host_data.config_dir_version && !system_config_dir_version; + if (only_config_dir_upgraded) { + version_compare_err = `host's config_dir_version is undefined, system's config_dir_version already upgraded to ${system_config_dir_version}, updates to the config directory via the host will result with invalid config_dir files until the host source code upgrade`; + } else if (only_host_upgraded) { + version_compare_err = `host's config_dir_version is ${host_data.config_dir_version}, system's config_dir_version is undefined, updates to the config directory will be blocked until the config dir upgrade`; + } else { + version_compare_err = host_data.config_dir_version && system_config_dir_version && + config_fs.compare_host_and_config_dir_version(host_data.config_dir_version, system_config_dir_version); + } + if (version_compare_err !== undefined) { + res = Object.assign(res || {}, { + [host_name]: { + host_version: host_data.current_version, + host_config_dir_version: host_data.config_dir_version, + error: version_compare_err + } + }); + } + } + return res; + } + /** * _calc_health_status calcs the overall health status of NooBaa NC * @param {{service_status: String, diff --git a/src/sdk/config_fs.js b/src/sdk/config_fs.js index 03130301fa..914c790ed0 100644 --- a/src/sdk/config_fs.js +++ b/src/sdk/config_fs.js @@ -1070,15 +1070,15 @@ class ConfigFS { } /** - * init_nc_system creates/updates system.json file + * register_hostname_in_system_json creates/updates system.json file * if system.json does not exist (a new system) - host and config dir data will be set on the newly created file * else - * 1. if the host data already exist in system.json - return - * 2. set the host data on system.json data and update the file + * 2. update the host data on system.json * Note - config directory data on upgraded systems will be set by nc_upgrade_manager - * @returns + * @returns {Promise} */ - async init_nc_system() { + async register_hostname_in_system_json() { const system_data = await this.get_system_config_file({silent_if_missing: true}); let updated_system_json = system_data || {}; @@ -1176,21 +1176,40 @@ class ConfigFS { const system_data = await this.get_system_config_file({ silent_if_missing: true }); // if system was never created, currently we allow using the CLI without creating system // we should consider changing it to throw on this scenario as well + // https://github.com/noobaa/noobaa-core/issues/8468 if (!system_data) return; if (!system_data.config_directory) { throw new RpcError('CONFIG_DIR_VERSION_MISMATCH', `config_directory data is missing in system.json, any updates to the config directory are blocked until the config dir upgrade`); } const running_code_config_dir_version = this.config_dir_version; const system_config_dir_version = system_data.config_directory.config_dir_version; + const ver_comparison_err = this.compare_host_and_config_dir_version(running_code_config_dir_version, system_config_dir_version); + if (ver_comparison_err !== undefined) { + throw new RpcError('CONFIG_DIR_VERSION_MISMATCH', ver_comparison_err); + } + } + + /** + * compare_host_and_config_dir_version compares the version of the config dir in the system.json file + * with the config dir version of the running host + * if compare result is 0 - undefined will be returned + * else - an appropriate error string will be returned + * @param {String} running_code_config_dir_version + * @param {String} system_config_dir_version + * @returns {String | Undefined} + */ + compare_host_and_config_dir_version(running_code_config_dir_version, system_config_dir_version) { const ver_comparison = version_compare(running_code_config_dir_version, system_config_dir_version); + dbg.log0(`config_fs.compare_host_and_config_dir_version: ver_comparison ${ver_comparison} running_code_config_dir_version ${running_code_config_dir_version} system_config_dir_version ${system_config_dir_version}`); if (ver_comparison > 0) { - throw new RpcError('CONFIG_DIR_VERSION_MISMATCH', `running code config_dir_version=${running_code_config_dir_version} is higher than the config dir version` + - `mentioned in system.json =${system_config_dir_version}, any updates to the config directory are blocked until the config dir upgrade`); + return `running code config_dir_version=${running_code_config_dir_version} is higher than the config dir version ` + + `mentioned in system.json=${system_config_dir_version}, any updates to the config directory are blocked until the config dir upgrade`; } if (ver_comparison < 0) { - throw new RpcError('CONFIG_DIR_VERSION_MISMATCH', `running code config_dir_version=${running_code_config_dir_version} is lower than the config dir version` + - `mentioned in system.json =${system_config_dir_version}, any updates to the config directory are blocked until the source code upgrade`); + return `running code config_dir_version=${running_code_config_dir_version} is lower than the config dir version ` + + `mentioned in system.json=${system_config_dir_version}, any updates to the config directory are blocked until the source code upgrade`; } + return undefined; } /** @@ -1201,6 +1220,7 @@ class ConfigFS { return { [os.hostname()]: { current_version: pkg.version, + config_dir_version: this.config_dir_version, upgrade_history: { successful_upgrades: [] }, @@ -1226,6 +1246,15 @@ class ConfigFS { } }; } + + /** + * get_hosts_data recieves system_data and returns only the hosts data + * @param {Object} system_data + * @returns {Object} + */ + get_hosts_data(system_data) { + return _.omit(system_data, 'config_directory'); + } } // EXPORTS diff --git a/src/test/unit_tests/jest_tests/test_config_fs.test.js b/src/test/unit_tests/jest_tests/test_config_fs.test.js index 48410f54b8..2dbf4aa525 100644 --- a/src/test/unit_tests/jest_tests/test_config_fs.test.js +++ b/src/test/unit_tests/jest_tests/test_config_fs.test.js @@ -1,8 +1,10 @@ /* Copyright (C) 2024 NooBaa */ 'use strict'; +const os = require('os'); const path = require('path'); const config = require('../../../../config'); +const pkg = require('../../../../package.json'); const { TMP_PATH } = require('../../system_tests/test_utils'); const { get_process_fs_context } = require('../../../util/native_fs_utils'); const { ConfigFS } = require('../../../sdk/config_fs'); @@ -29,3 +31,43 @@ describe('adjust_bucket_with_schema_updates', () => { expect(bucket).not.toHaveProperty('bucket_owner'); }); }); + +describe('_get_new_hostname_data', () => { + it('_get_new_hostname_data - happy path', () => { + const new_hostname_data = config_fs._get_new_hostname_data(); + expect(new_hostname_data).toStrictEqual({ + [os.hostname()]: { + current_version: pkg.version, + config_dir_version: config_fs.config_dir_version, + upgrade_history: { + successful_upgrades: [] + }, + } + }); + }); +}); + +describe('compare_host_and_config_dir_version', () => { + it('running code config_dir_version equals to system.json config_dir_version', () => { + const running_code_config_dir_version = '0.0.0'; + const system_config_dir_version = '0.0.0'; + const ver_compare_err = config_fs.compare_host_and_config_dir_version(running_code_config_dir_version, system_config_dir_version); + expect(ver_compare_err).toBeUndefined(); + }); + + it('running code config_dir_version higher than system.json config_dir_version', () => { + const running_code_config_dir_version = '1.0.0'; + const system_config_dir_version = '0.0.0'; + const ver_compare_err = config_fs.compare_host_and_config_dir_version(running_code_config_dir_version, system_config_dir_version); + expect(ver_compare_err).toBe(`running code config_dir_version=${running_code_config_dir_version} is higher than the config dir version ` + + `mentioned in system.json=${system_config_dir_version}, any updates to the config directory are blocked until the config dir upgrade`); + }); + + it('running code config_dir_version lower than system.json config_dir_version', () => { + const running_code_config_dir_version = '0.0.0'; + const system_config_dir_version = '1.0.0'; + const ver_compare_err = config_fs.compare_host_and_config_dir_version(running_code_config_dir_version, system_config_dir_version); + expect(ver_compare_err).toBe(`running code config_dir_version=${running_code_config_dir_version} is lower than the config dir version ` + + `mentioned in system.json=${system_config_dir_version}, any updates to the config directory are blocked until the source code upgrade`); + }); +}); 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 index f73956e696..7c628cefb8 100644 --- 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 @@ -59,12 +59,12 @@ module.exports = { `; const old_expected_system_json = { [hostname]: { - 'current_version': '5.17.0', - 'upgrade_history': { - 'successful_upgrades': [{ - 'timestamp': 1724687496424, - 'from_version': '5.16.0', - 'to_version': '5.17.0' + current_version: '5.17.0', + upgrade_history: { + successful_upgrades: [{ + timestamp: 1724687496424, + from_version: '5.16.0', + to_version: '5.17.0' }] }, } @@ -72,25 +72,25 @@ const old_expected_system_json = { const old_expected_system_json_has_config_directory = { [hostname]: { - 'current_version': '5.18.1', - 'upgrade_history': { - 'successful_upgrades': [{ - 'timestamp': 1724687496424, - 'from_version': '5.18.0', - 'to_version': '5.18.1' + current_version: '5.18.1', + upgrade_history: { + successful_upgrades: [{ + timestamp: 1724687496424, + from_version: '5.18.0', + to_version: '5.18.1' }] }, }, config_directory: { - 'config_dir_version': '1.0.0', - 'upgrade_package_version': '5.18.0', - 'phase': CONFIG_DIR_PHASES.CONFIG_DIR_UNLOCKED, - 'upgrade_history': { - 'successful_upgrades': [{ - 'timestamp': 1724687496424, - 'completed_scripts': [], - 'package_from_version': '5.17.0', - 'package_to_version': '5.18.0' + config_dir_version: '1.0.0', + upgrade_package_version: '5.18.0', + phase: CONFIG_DIR_PHASES.CONFIG_DIR_UNLOCKED, + upgrade_history: { + successful_upgrades: [{ + timestamp: 1724687496424, + completed_scripts: [], + package_from_version: '5.17.0', + package_to_version: '5.18.0' }] } } @@ -98,21 +98,22 @@ const old_expected_system_json_has_config_directory = { const old_expected_system_json_no_successful_upgrades = { [hostname]: { - 'current_version': '5.17.0', - 'upgrade_history': { - 'successful_upgrades': [] + current_version: '5.17.0', + upgrade_history: { + successful_upgrades: [] }, } }; const current_expected_system_json = { [hostname]: { - 'current_version': pkg.version, - 'upgrade_history': { - 'successful_upgrades': [{ - 'timestamp': 1724687496424, - 'from_version': '5.17.0', - 'to_version': pkg.version + current_version: pkg.version, + config_dir_version: config_fs.config_dir_version, + upgrade_history: { + successful_upgrades: [{ + timestamp: 1724687496424, + from_version: '5.17.0', + to_version: pkg.version }] }, } @@ -121,9 +122,10 @@ const current_expected_system_json = { const current_expected_system_json_no_successful_upgrades = { [hostname]: { - 'current_version': pkg.version, - 'upgrade_history': { - 'successful_upgrades': [] + current_version: pkg.version, + config_dir_version: config_fs.config_dir_version, + upgrade_history: { + successful_upgrades: [] }, } }; @@ -207,8 +209,10 @@ describe('nc upgrade manager - upgrade RPM', () => { await nc_upgrade_manager.update_rpm_upgrade(config_fs); const system_data_after_upgrade_run = await config_fs.get_system_config_file(); const new_version = pkg.version; + const new_config_dir_version = config_fs.config_dir_version; const host_data_after_upgrade = system_data_after_upgrade_run[hostname]; expect(host_data_after_upgrade.current_version).toStrictEqual(new_version); + expect(host_data_after_upgrade.config_dir_version).toStrictEqual(new_config_dir_version); expect(host_data_after_upgrade.upgrade_history.successful_upgrades[0].from_version).toStrictEqual( old_expected_system_json[hostname].current_version); expect(host_data_after_upgrade.upgrade_history.successful_upgrades[0].to_version).toStrictEqual(new_version); @@ -219,8 +223,10 @@ describe('nc upgrade manager - upgrade RPM', () => { await nc_upgrade_manager.update_rpm_upgrade(config_fs); const system_data_after_upgrade_run = await config_fs.get_system_config_file(); const new_version = pkg.version; + const new_config_dir_version = config_fs.config_dir_version; const host_data_after_upgrade = system_data_after_upgrade_run[hostname]; expect(host_data_after_upgrade.current_version).toStrictEqual(new_version); + expect(host_data_after_upgrade.config_dir_version).toStrictEqual(new_config_dir_version); expect(host_data_after_upgrade.upgrade_history.successful_upgrades[0].from_version).toStrictEqual( old_expected_system_json_no_successful_upgrades[hostname].current_version); expect(host_data_after_upgrade.upgrade_history.successful_upgrades[0].to_version).toStrictEqual(new_version); diff --git a/src/test/unit_tests/test_nc_health.js b/src/test/unit_tests/test_nc_health.js index 69fb47a4c4..e05ce54871 100644 --- a/src/test/unit_tests/test_nc_health.js +++ b/src/test/unit_tests/test_nc_health.js @@ -30,8 +30,8 @@ const hostname = os.hostname(); const valid_system_json = { [hostname]: { - 'current_version': pkg.version, - 'upgrade_history': { 'successful_upgrades': [] }, + current_version: pkg.version, + upgrade_history: { successful_upgrades: [] }, }, }; @@ -186,10 +186,12 @@ mocha.describe('nsfs nc health', function() { mocha.it('Health all condition is success', async function() { valid_system_json.config_directory = { - 'config_dir_version': config_fs.config_dir_version, - 'upgrade_package_version': pkg.version, - 'phase': CONFIG_DIR_PHASES.CONFIG_DIR_UNLOCKED + config_dir_version: config_fs.config_dir_version, + upgrade_package_version: pkg.version, + phase: CONFIG_DIR_PHASES.CONFIG_DIR_UNLOCKED, + upgrade_status: default_mock_upgrade_status }; + valid_system_json[hostname].config_dir_version = config_fs.config_dir_version; await Health.config_fs.create_system_config_file(JSON.stringify(valid_system_json)); set_mock_functions(Health, { get_service_state: get_service_state_mock_default_response, @@ -606,14 +608,16 @@ mocha.describe('nsfs nc health', function() { mocha.it('Health all condition - failed config directory upgrade status', async function() { valid_system_json.config_directory = { - 'config_dir_version': config_fs.config_dir_version, - 'upgrade_package_version': pkg.version, - 'phase': CONFIG_DIR_PHASES.CONFIG_DIR_LOCKED, + config_dir_version: config_fs.config_dir_version, + upgrade_package_version: pkg.version, + phase: CONFIG_DIR_PHASES.CONFIG_DIR_LOCKED, upgrade_history: { successful_upgrades: [], last_failure: { error: 'mock error'} - } + }, + error: 'last_upgrade_failed' }; + valid_system_json[hostname].config_dir_version = config_fs.config_dir_version; await Health.config_fs.create_system_config_file(JSON.stringify(valid_system_json)); const health_status = await Health.nc_nsfs_health(); await fs_utils.file_delete(Health.config_fs.system_json_path); @@ -635,6 +639,24 @@ mocha.describe('nsfs nc health', function() { await fs_utils.file_delete(Health.config_fs.system_json_path); assert_config_dir_status(health_status, valid_system_json.config_directory); }); + + mocha.it('health should report on blocked hosts - config directory data is missing, host is blocked for updates', async function() { + valid_system_json.config_directory = undefined; + valid_system_json[hostname].config_dir_version = config_fs.config_dir_version; + await Health.config_fs.create_system_config_file(JSON.stringify(valid_system_json)); + const health_status = await Health.nc_nsfs_health(); + await fs_utils.file_delete(Health.config_fs.system_json_path); + assert_config_dir_status(health_status, { + error: 'config directory data is missing, must upgrade config directory', + blocked_hosts: { + [hostname]: { + host_version: valid_system_json[hostname].current_version, + host_config_dir_version: valid_system_json[hostname].config_dir_version, + error: `host's config_dir_version is ${valid_system_json[hostname].config_dir_version}, system's config_dir_version is undefined, updates to the config directory will be blocked until the config dir upgrade` + } + } + }); + }); }); }); @@ -645,23 +667,27 @@ mocha.describe('nsfs nc health', function() { */ function assert_config_dir_status(health_status, expected_config_dir) { const actual_config_dir_health = health_status.checks.config_directory_status; + assert.strictEqual(actual_config_dir_health.error, expected_config_dir.error); + assert.deepStrictEqual(actual_config_dir_health.blocked_hosts, expected_config_dir.blocked_hosts); assert.strictEqual(actual_config_dir_health.phase, expected_config_dir.phase); assert.strictEqual(actual_config_dir_health.config_dir_version, expected_config_dir.config_dir_version); assert.strictEqual(actual_config_dir_health.upgrade_package_version, expected_config_dir.upgrade_package_version); - const expected_upgrade_status = expected_config_dir.in_progress_upgrade || expected_config_dir.upgrade_history?.last_failure; - assert_upgrade_status(actual_config_dir_health.upgrade_status, expected_upgrade_status); + assert_upgrade_status(actual_config_dir_health.upgrade_status, expected_config_dir); } /** * assert_upgrade_status asserts there the actual and expected upgrade_status * @param {Object} actual_upgrade_status - * @param {Object} [expected_upgrade_status] + * @param {Object} expected_upgrade_status */ -function assert_upgrade_status(actual_upgrade_status, expected_upgrade_status = default_mock_upgrade_status.message) { - const actual_upgrade_status_res = actual_upgrade_status.in_progress_upgrade || - actual_upgrade_status.last_failure || - actual_upgrade_status.message; - assert.deepStrictEqual(actual_upgrade_status_res, expected_upgrade_status); +function assert_upgrade_status(actual_upgrade_status, expected_upgrade_status) { + const actual_upgrade_status_res = actual_upgrade_status?.in_progress_upgrade || + actual_upgrade_status?.last_failure || + actual_upgrade_status?.message; + const expected_upgrade_status_res = expected_upgrade_status?.in_progress_upgrade || + expected_upgrade_status?.upgrade_history?.last_failure || + expected_upgrade_status?.upgrade_status?.message; + assert.deepStrictEqual(actual_upgrade_status_res, expected_upgrade_status_res); } /** diff --git a/src/upgrade/nc_upgrade_manager.js b/src/upgrade/nc_upgrade_manager.js index fdc3150171..93b2c83a8f 100644 --- a/src/upgrade/nc_upgrade_manager.js +++ b/src/upgrade/nc_upgrade_manager.js @@ -2,7 +2,6 @@ "use strict"; const os = require('os'); -const _ = require('lodash'); const path = require('path'); const util = require('util'); const pkg = require('../../package.json'); @@ -74,6 +73,7 @@ class NCUpgradeManager { const this_upgrade = { timestamp: Date.now(), from_version, to_version }; system_data[hostname].current_version = to_version; + system_data[hostname].config_dir_version = this.config_fs.config_dir_version; system_data[hostname]?.upgrade_history?.successful_upgrades.unshift(this_upgrade); await this.config_fs.update_system_json_with_retries(JSON.stringify(system_data)); } @@ -164,7 +164,7 @@ class NCUpgradeManager { */ async _verify_config_dir_upgrade(system_data, expected_version, expected_hosts) { const new_version = this.package_version; - const hosts_data = _.omit(system_data, 'config_directory'); + const hosts_data = this.config_fs.get_hosts_data(system_data); let err_message; if (expected_version !== new_version) { err_message = `config dir upgrade can not be started - the host's package version=${new_version} does not match the user's expected version=${expected_version}`;