CR

Signed-off-by: Romy <[email protected]>

Author: romayalon

src/manage_nsfs/health.js

@@ -450,17 +450,17 @@ class NSFSHealth {
      */
     _get_config_dir_status(system_data) {
         if (!system_data) return { error: 'system data is missing' };
-        const blocked_hosts = this._get_blocked_hosts_status(this.config_fs, system_data);
+        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', blocked_hosts };
+        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,
-            blocked_hosts
+            blocked_hosts: blocked_hosts_status
         };
     }
 
@@ -484,26 +484,28 @@ class NSFSHealth {
 
     /**
      * _get_blocked_hosts_status checks if there are any hosts blocked for updating the config directory
-     * 1. when host's config_dir_version older than 5.18.0 and system's config_dit_version is 5.18.0 and higher it means it's not blocked 
-     * because the source code won't include _throw_if_config_dir_locked() but it's still can cause invalid config files, therefore including it in the blocked list
-     * 2. when system's config_dir_version older than 5.18.0 and hosts's config_dir_version is 5.18.0 and higher
-     * it means updated to the config directory from this host are blocked
+     * 1. if only config dir was upgraded (>=5.18.0) - host's config_dir_version doesn 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 = _.omit(system_data, 'config_directory');
+        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`;
+                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`;
+                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);

src/sdk/config_fs.js

@@ -1074,9 +1074,9 @@ class ConfigFS {
      * 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<Object>}
      */
     async register_hostname_in_system_json() {
         const system_data = await this.get_system_config_file({silent_if_missing: true});
@@ -1176,6 +1176,7 @@ 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`);
@@ -1191,13 +1192,15 @@ class ConfigFS {
     /**
      * 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', running_code_config_dir_version, system_config_dir_version, ver_comparison);
+        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) {
             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`;
@@ -1243,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

src/test/unit_tests/test_nc_health.js

@@ -186,9 +186,9 @@ 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;
@@ -640,7 +640,7 @@ mocha.describe('nsfs nc health', function() {
             assert_config_dir_status(health_status, valid_system_json.config_directory);
         });
 
-        mocha.it('invalid blocked hosts', async function() {
+        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));
@@ -652,7 +652,7 @@ mocha.describe('nsfs nc health', function() {
                     [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`
+                        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`
                     }
                 }
             });

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');
@@ -165,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}`;