Merge pull request #8795 from romayalon/romy-5.18-backports2

5.18 Backport | online upgrade | make hosts verification optional

Author: nimrod-becker

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/endpoint/endpoint.js

@@ -6,6 +6,7 @@
 require('../util/dotenv').load();
 require('../util/panic');
 require('../util/fips');
+const pkg = require('../../package.json');
 
 const dbg = require('../util/debug_module')(__filename);
 if (!dbg.get_process_name()) dbg.set_process_name('Endpoint');
@@ -294,6 +295,14 @@ function create_endpoint_handler(server_type, init_request_sdk, { virtual_hosts,
                 return fork_count_handler(req, res);
             } else if (req.url.startsWith('/endpoint_fork_id')) {
                 return endpoint_fork_id_handler(req, res);
+            } else if (req.url.startsWith('/_/')) {
+                // internals non S3 requests
+                const api = req.url.slice('/_/'.length);
+                if (api === 'version') {
+                    return version_handler(req, res);
+                } else {
+                    return internal_api_error(req, res, `Unknown API call ${api}`);
+                }
             } else {
                 return s3_rest.handler(req, res);
             }
@@ -325,28 +334,67 @@ function create_endpoint_handler(server_type, init_request_sdk, { virtual_hosts,
     }
 }
 
+///////////////////////////
+// INTERNAL API HANDLERS //
+///////////////////////////
+
+/**
+ * version_handler returns the version of noobaa package
+ * @param {EndpointRequest} req 
+ * @param {import('http').ServerResponse} res 
+ */
+function version_handler(req, res) {
+    const noobaa_package_version = pkg.version;
+    res.statusCode = 200;
+    res.setHeader('Content-Type', 'text/plain');
+    res.setHeader('Content-Length', Buffer.byteLength(noobaa_package_version));
+    res.end(noobaa_package_version);
+}
+
+/**
+ * internal_api_error returns an internal api error response
+ * @param {EndpointRequest} req 
+ * @param {import('http').ServerResponse} res 
+ * @param {string} error_message 
+ */
+function internal_api_error(req, res, error_message) {
+    const buffer = Buffer.from(JSON.stringify({ error: 'Internal Server Error', message: error_message }));
+    res.statusCode = 500;
+    res.setHeader('Content-Type', 'application/json');
+    res.setHeader('Content-Length', buffer.length);
+    res.end(buffer);
+}
+
+/**
+ * endpoint_fork_id_handler returns the worker id of the current fork
+ * @param {EndpointRequest} req 
+ * @param {import('http').ServerResponse} res 
+ */
 function endpoint_fork_id_handler(req, res) {
     let reply = {};
     if (cluster.isWorker) {
-        reply = {
-            worker_id: cluster.worker.id,
-        };
+        reply = { worker_id: cluster.worker.id };
     }
     P.delay(500);
     res.statusCode = 200;
+    const buffer = Buffer.from(JSON.stringify(reply));
     res.setHeader('Content-Type', 'application/json');
-    res.setHeader('Content-Length', Buffer.byteLength(JSON.stringify(reply)));
-    res.end(JSON.stringify(reply));
+    res.setHeader('Content-Length', buffer.length);
+    res.end(buffer);
 }
 
+/**
+ * fork_count_handler returns the total number of forks
+ * @param {EndpointRequest} req 
+ * @param {import('http').ServerResponse} res 
+ */
 function fork_count_handler(req, res) {
-    const reply = {
-        fork_count: fork_count,
-    };
+    const reply = { fork_count: fork_count };
     res.statusCode = 200;
+    const buffer = Buffer.from(JSON.stringify(reply));
     res.setHeader('Content-Type', 'application/json');
-    res.setHeader('Content-Length', Buffer.byteLength(JSON.stringify(reply)));
-    res.end(JSON.stringify(reply));
+    res.setHeader('Content-Length', buffer.length);
+    res.end(buffer);
 }
 
 /**

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) {