Merge pull request #8790 from romayalon/romy-expected-hosts-optional

NC | Online upgrade | convert --expected_hosts flag from required to optional

Author: romayalon

docs/NooBaaNonContainerized/NooBaaCLI.md

@@ -483,7 +483,7 @@ The `upgrade start` command is used to start a config directory upgrade run.
 
 #### Usage
 ```sh
-noobaa-cli upgrade start --expected_version <expected-version> --expected_hosts <expected-hosts> [--skip-verification] [--custom_upgrade_scripts_dir]
+noobaa-cli upgrade start --expected_version <expected-version> [--expected_hosts] <expected-hosts> [--skip-verification] [--custom_upgrade_scripts_dir]
 ```
 
 #### Flags -
@@ -492,7 +492,7 @@ noobaa-cli upgrade start --expected_version <expected-version> --expected_hosts
     - Description: Specifies the upgrade's expected target version.
     - Example - `--expected_version 5.18.0`
 
-- `expected_hosts` (Required)
+- `expected_hosts`
     - Type: String
     - Description: Specifies the upgrade's expected hosts. String of hostnames separated by comma (,). 
     - Example - `--expected_hosts hostname1,hostname2,hostname3`

docs/NooBaaNonContainerized/Upgrade.md

@@ -103,7 +103,9 @@ Online Upgrade Algorithm commands examples -
 2. Stop NooBaa service - `systemctl stop noobaa`
 3. RPM upgrade on a specific node - `rpm -Uvh /path/to/new_noobaa_rpm_version.rpm`
 4. Restart NooBaa service - `systemctl restart noobaa`
-5. `noobaa-cli upgrade start --expected_version=5.18.0 --expected_hosts=hostname1,hostname2,hostname3` 
+5. Check that each node has NooBaa service running with the new code - `curl -k https://localhost:6443/_/version` or `curl http://${host_address}:6001/_/version`
+6. `noobaa-cli upgrade start --expected_version=5.18.0`
+
 
 ### Additional Upgrade Properties of `system.json`
 

src/manage_nsfs/manage_nsfs_help_utils.js

@@ -437,7 +437,7 @@ Usage:
 Flags:
 
     --expected_version              <string>                            The expected target version of the upgrade
-    --expected_hosts                <string>                            The expected hosts running NooBaa NC, a string of hosts separated by ,
+    --expected_hosts                <string>         (optional)         The expected hosts running NooBaa NC, a string of hosts separated by ,
     --skip_verification             <boolean>        (optional)         skip upgrade verification
                                                                         WARNING: can cause corrupted config dir files created by hosts running old code.
                                                                         this should generally not be used and is intended exclusively for NooBaa team support. 

src/manage_nsfs/upgrade.js

@@ -49,7 +49,7 @@ async function start_config_dir_upgrade(user_input, config_fs) {
         new NoobaaEvent(NoobaaEvent.CONFIG_DIR_UPGRADE_STARTED).create_event(undefined, { expected_version, expected_hosts }, undefined);
 
         if (!expected_version) throw new Error('expected_version flag is required');
-        if (!expected_hosts) throw new Error('expected_hosts flag is required');
+        if (!expected_hosts) dbg.warn('expected_hosts flag is empty, config dir upgrade will be performed without hosts version verification');
 
         const nc_upgrade_manager = new NCUpgradeManager(config_fs, { custom_upgrade_scripts_dir });
         const upgrade_res = await nc_upgrade_manager.upgrade_config_dir(expected_version, expected_hosts, { skip_verification });

src/test/unit_tests/jest_tests/test_cli_upgrade.test.js

@@ -270,12 +270,11 @@ describe('noobaa cli - upgrade', () => {
         expect(parsed_res.error.cause).toContain('expected_version flag is required');
     });
 
-    it('upgrade start - should fail on no expected_hosts', async () => {
+    it('upgrade start - should succeed although missing expected hosts', async () => {
         await fs_utils.replace_file(config_fs.system_json_path, JSON.stringify(old_rpm_expected_system_json));
         const res = await exec_manage_cli(TYPES.UPGRADE, UPGRADE_ACTIONS.START, { config_root, expected_version: pkg.version }, true);
-        const parsed_res = JSON.parse(res.stdout);
-        expect(parsed_res.error.message).toBe(ManageCLIError.UpgradeFailed.message);
-        expect(parsed_res.error.cause).toContain('expected_hosts flag is required');
+        const parsed_res = JSON.parse(res);
+        expect(parsed_res.response.code).toBe(ManageCLIResponse.UpgradeSuccessful.code);
     });
 
     it('upgrade start - should fail on missing expected_hosts in system.json', async () => {

src/upgrade/nc_upgrade_manager.js

@@ -93,7 +93,7 @@ class NCUpgradeManager {
      *    4.4. moves the current upgrade from in_progress_upgrade to the upgrade_history.successful_upgrades 
      *    4.5. is 5.17.0 is a good default for from_version?
      * @param {string} expected_version
-     * @param {[string]} hosts_list
+     * @param {[string]} [hosts_list]
      * @param {{skip_verification?: boolean}} [options]
      * @returns {Promise<Object>}
      */
@@ -159,7 +159,7 @@ class NCUpgradeManager {
      * therefore we can not treat the system.json as the source of truth of the hosts information
      * @param {Object} system_data
      * @param {String} expected_version
-     * @param {[String]} expected_hosts
+     * @param {[String]} [expected_hosts]
      * @returns {Promise<Void>}
      */
     async _verify_config_dir_upgrade(system_data, expected_version, expected_hosts) {
@@ -175,23 +175,27 @@ class NCUpgradeManager {
             err_message = `config dir upgrade can not be started missing hosts_data hosts_data=${util.inspect(hosts_data)}`;
         }
 
-        const all_hostnames_exist_in_expected_hosts = hostnames.every(item => expected_hosts.includes(item));
-        if (!all_hostnames_exist_in_expected_hosts) {
-            dbg.warn(`_verify_config_dir_upgrade - system.json contains one or more hosts info that are not specified in expected_hosts: hosts_data=${util.inspect(hosts_data)} expected_hosts=${util.inspect(expected_hosts)}`);
-        }
+        if (expected_hosts) {
+            const all_hostnames_exist_in_expected_hosts = hostnames.every(item => expected_hosts.includes(item));
+            if (!all_hostnames_exist_in_expected_hosts) {
+                dbg.warn(`_verify_config_dir_upgrade - system.json contains one or more hosts info that are not specified in expected_hosts: hosts_data=${util.inspect(hosts_data)} expected_hosts=${util.inspect(expected_hosts)}`);
+            }
 
-        const all_expected_hosts_exist_in_system_json = expected_hosts.every(item => hostnames.includes(item));
-        if (!err_message && !all_expected_hosts_exist_in_system_json) {
-            err_message = `config dir upgrade can not be started - expected_hosts contains one or more hosts that are not specified in system.json hosts_data=${util.inspect(hosts_data)} expected_hosts=${util.inspect(expected_hosts)}`;
-        }
+            const all_expected_hosts_exist_in_system_json = expected_hosts.every(item => hostnames.includes(item));
+            if (!err_message && !all_expected_hosts_exist_in_system_json) {
+                err_message = `config dir upgrade can not be started - expected_hosts contains one or more hosts that are not specified in system.json hosts_data=${util.inspect(hosts_data)} expected_hosts=${util.inspect(expected_hosts)}`;
+            }
 
-        if (!err_message) {
-            for (const cur_hostname of expected_hosts) {
-                const host_data = hosts_data[cur_hostname];
-                if (!host_data.current_version || version_compare(host_data.current_version, new_version) !== 0) {
-                    err_message = `config dir upgrade can not be started until all expected hosts have the expected version=${new_version}, host=${cur_hostname} host's current_version=${host_data.current_version}`;
+            if (!err_message) {
+                for (const cur_hostname of expected_hosts) {
+                    const host_data = hosts_data[cur_hostname];
+                    if (!host_data.current_version || version_compare(host_data.current_version, new_version) !== 0) {
+                        err_message = `config dir upgrade can not be started until all expected hosts have the expected version=${new_version}, host=${cur_hostname} host's current_version=${host_data.current_version}`;
+                    }
                 }
             }
+        } else {
+            dbg.warn('expected_hosts is empty, config dir upgrade will be performed without hosts version verification');
         }
 
         if (!err_message && system_data.config_directory?.in_progress_upgrade) {