Skip to content

Commit 7458fab

Browse files
jackyalbojackyalbo
committed
Remove error on No upgrade required
Signed-off-by: jackyalbo <jacky.albo@gmail.com>
1 parent 828d450 commit 7458fab

File tree

5 files changed

+97
-55
lines changed

5 files changed

+97
-55
lines changed

‎src/manage_nsfs/manage_nsfs_cli_responses.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,12 @@ ManageCLIResponse.UpgradeSuccessful = Object.freeze({
154154
status: {}
155155
});
156156

157+
ManageCLIResponse.NoUpgradeRequired = Object.freeze({
158+
code: 'NoUpgradeRequired',
159+
message: 'Config directory upgrade not required',
160+
status: {}
161+
});
162+
157163
ManageCLIResponse.UpgradeStatus = Object.freeze({
158164
code: 'UpgradeStatus',
159165
message: 'Config directory upgrade status retrieved successfully',

‎src/manage_nsfs/upgrade.js‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,10 @@ async function start_config_dir_upgrade(user_input, config_fs) {
5555
const nc_upgrade_manager = new NCUpgradeManager(config_fs, { custom_upgrade_scripts_dir });
5656
const upgrade_res = await nc_upgrade_manager.upgrade_config_dir(expected_version, expected_hosts, { skip_verification });
5757
if (!upgrade_res) throw new Error('Upgrade config directory failed', { cause: upgrade_res });
58-
write_stdout_response(ManageCLIResponse.UpgradeSuccessful, upgrade_res, { expected_version, expected_hosts });
58+
write_stdout_response(
59+
upgrade_res.code || ManageCLIResponse.UpgradeSuccessful,
60+
upgrade_res.upgrade_info, { expected_version, expected_hosts }
61+
);
5962
} catch (err) {
6063
if (err instanceof ManageCLIError) {
6164
// those are errors that the admin made in the CLI, the upgrade didn't start

‎src/test/integration_tests/nc/cli/test_cli_upgrade.test.js‎

Lines changed: 26 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -76,15 +76,14 @@ const old_expected_system_json2 = {
7676
'upgrade_package_version': '5.18.0',
7777
'phase': CONFIG_DIR_PHASES.CONFIG_DIR_UNLOCKED,
7878
'upgrade_history': {
79-
'successful_upgrades': [
80-
{
81-
'timestamp': 1724687496424,
82-
'running_host': hostname,
83-
'package_from_version': '5.17.0',
84-
'package_to_version': '5.18.0',
85-
'config_dir_from_version': '0.0.0',
86-
'config_dir_to_version': '1.0.0'
87-
}]
79+
'successful_upgrades': [{
80+
'timestamp': 1724687496424,
81+
'running_host': hostname,
82+
'package_from_version': '5.17.0',
83+
'package_to_version': '5.18.0',
84+
'config_dir_from_version': '0.0.0',
85+
'config_dir_to_version': '1.0.0'
86+
}]
8887
}
8988
}
9089
};
@@ -105,15 +104,14 @@ const old_expected_system_json5 = {
105104
'upgrade_package_version': '5.17.0',
106105
'phase': CONFIG_DIR_PHASES.CONFIG_DIR_UNLOCKED,
107106
'upgrade_history': {
108-
'successful_upgrades': [
109-
{
110-
'timestamp': 1724687496424,
111-
'running_host': hostname,
112-
'package_from_version': '5.16.0',
113-
'package_to_version': '5.17.0',
114-
'config_dir_from_version': '-1.0.0',
115-
'config_dir_to_version': '0.0.0'
116-
}]
107+
'successful_upgrades': [{
108+
'timestamp': 1724687496424,
109+
'running_host': hostname,
110+
'package_from_version': '5.16.0',
111+
'package_to_version': '5.17.0',
112+
'config_dir_from_version': '-1.0.0',
113+
'config_dir_to_version': '0.0.0'
114+
}]
117115
}
118116
}
119117
};
@@ -133,8 +131,7 @@ const new_expected_system_json = {
133131
'config_dir_version': '1.0.0',
134132
'phase': CONFIG_DIR_PHASES.CONFIG_DIR_UNLOCKED,
135133
'upgrade_history': {
136-
'successful_upgrades': [
137-
{
134+
'successful_upgrades': [{
138135
'timestamp': 1724687496424,
139136
'running_host': hostname,
140137
'package_from_version': '5.17.0',
@@ -143,11 +140,12 @@ const new_expected_system_json = {
143140
'config_dir_to_version': '1.0.0'
144141
},
145142
{
146-
'timestamp': 1724687496424,
147-
'completed_scripts': [],
148-
'from_version': '5.16.0',
149-
'to_version': '5.17.0'
150-
}]
143+
'timestamp': 1724687496424,
144+
'completed_scripts': [],
145+
'from_version': '5.16.0',
146+
'to_version': '5.17.0'
147+
}
148+
]
151149
}
152150
}
153151
};
@@ -368,9 +366,9 @@ describe('noobaa cli - upgrade', () => {
368366
const system_data_before_upgrade = await config_fs.get_system_config_file();
369367
const options = { config_root, expected_version: pkg.version, expected_hosts };
370368
const res = await exec_manage_cli(TYPES.UPGRADE, UPGRADE_ACTIONS.START, options, true);
371-
const parsed_res = JSON.parse(res.stdout);
372-
expect(parsed_res.error.message).toBe(ManageCLIError.UpgradeFailed.message);
373-
expect(parsed_res.error.cause).toContain(`config_dir_version on system.json and config_fs.config_dir_version match, nothing to upgrade`);
369+
const parsed_res = JSON.parse(res);
370+
expect(parsed_res.response.code).toBe(ManageCLIResponse.NoUpgradeRequired.code);
371+
expect(parsed_res.response.reply.detail).toContain(`config_dir_version on system.json and config_fs.config_dir_version match, nothing to upgrade`);
374372
const system_data_after_upgrade = await config_fs.get_system_config_file();
375373
// check that in the hostname section nothing changed
376374
expect(system_data_before_upgrade[hostname]).toStrictEqual(system_data_after_upgrade[hostname]);

‎src/test/unit_tests/nc/configuration/test_nc_upgrade_manager.test.js‎

Lines changed: 48 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -13,19 +13,29 @@ const path = require('path');
1313
const fs_utils = require('../../../../util/fs_utils');
1414
const config = require('../../../../../config');
1515
const pkg = require('../../../../../package.json');
16-
const { NCUpgradeManager, DEFAULT_NC_UPGRADE_SCRIPTS_DIR, OLD_DEFAULT_PACKAGE_VERSION,
17-
OLD_DEFAULT_CONFIG_DIR_VERSION } = require('../../../../upgrade/nc_upgrade_manager');
16+
const {
17+
NCUpgradeManager,
18+
DEFAULT_NC_UPGRADE_SCRIPTS_DIR,
19+
OLD_DEFAULT_PACKAGE_VERSION,
20+
OLD_DEFAULT_CONFIG_DIR_VERSION
21+
} = require('../../../../upgrade/nc_upgrade_manager');
1822
const { ConfigFS, CONFIG_DIR_PHASES } = require('../../../../sdk/config_fs');
19-
const { TMP_PATH, create_redirect_file, create_config_dir,
20-
fail_test_if_default_config_dir_exists, clean_config_dir, TEST_TIMEOUT } = require('../../../system_tests/test_utils');
23+
const {
24+
TMP_PATH,
25+
create_redirect_file,
26+
create_config_dir,
27+
fail_test_if_default_config_dir_exists,
28+
clean_config_dir,
29+
TEST_TIMEOUT
30+
} = require('../../../system_tests/test_utils');
2131

2232
const config_root = path.join(TMP_PATH, 'config_root_nc_upgrade_manager_test');
2333
const config_fs = new ConfigFS(config_root);
2434
const hostname = os.hostname();
2535
const mock_higher_version_than_pkg_version = pkg.version + '.1';
2636

2737
const dummy_upgrade_script_1 =
28-
`/* Copyright (C) 2024 NooBaa */
38+
`/* Copyright (C) 2024 NooBaa */
2939
'use strict';
3040
async function run() {
3141
console.log('script number 1');
@@ -36,7 +46,7 @@ module.exports = {
3646
};
3747
`;
3848
const dummy_upgrade_script_2 =
39-
`/* Copyright (C) 2024 NooBaa */
49+
`/* Copyright (C) 2024 NooBaa */
4050
'use strict';
4151
async function run() {
4252
console.log('script number 2');
@@ -47,7 +57,7 @@ module.exports = {
4757
};`;
4858

4959
const dummy_failing_upgrade_script_3 =
50-
`/* Copyright (C) 2024 NooBaa */
60+
`/* Copyright (C) 2024 NooBaa */
5161
'use strict';
5262
async function run() {
5363
console.log('script number 3');
@@ -353,23 +363,31 @@ describe('nc upgrade manager - upgrade config directory', () => {
353363
});
354364

355365
it('_verify_config_dir_upgrade - empty host current_version', async () => {
356-
const system_data = { [hostname]: []};
366+
const system_data = {
367+
[hostname]: []
368+
};
357369
const expected_err_msg = `config dir upgrade can not be started until all expected hosts have the expected version=${pkg.version}, host=${hostname} host's current_version=undefined`;
358370
await expect(nc_upgrade_manager._verify_config_dir_upgrade(system_data, pkg.version, [hostname]))
359371
.rejects.toThrow(expected_err_msg);
360372
});
361373

362374
it('_verify_config_dir_upgrade - host current_version < new_version should upgrade RPM', async () => {
363375
const old_version = '5.16.0';
364-
const system_data = { [hostname]: { current_version: old_version }, other_hostname: { current_version: pkg.version } };
376+
const system_data = {
377+
[hostname]: { current_version: old_version },
378+
other_hostname: { current_version: pkg.version }
379+
};
365380
const expected_err_msg = `config dir upgrade can not be started until all expected hosts have the expected version=${pkg.version}, host=${hostname} host's current_version=${old_version}`;
366381
await expect(nc_upgrade_manager._verify_config_dir_upgrade(system_data, pkg.version, [hostname, 'other_hostname']))
367382
.rejects.toThrow(expected_err_msg);
368383
});
369384

370385
it('_verify_config_dir_upgrade - host current_version > new_version should upgrade RPM', async () => {
371386
const newer_version = mock_higher_version_than_pkg_version;
372-
const system_data = { [hostname]: { current_version: newer_version }, other_hostname: { current_version: pkg.version } };
387+
const system_data = {
388+
[hostname]: { current_version: newer_version },
389+
other_hostname: { current_version: pkg.version }
390+
};
373391
const expected_err_msg = `config dir upgrade can not be started until all expected hosts have the expected version=${pkg.version}, host=${hostname} host's current_version=${newer_version}`;
374392
await expect(nc_upgrade_manager._verify_config_dir_upgrade(system_data, pkg.version, [hostname, 'other_hostname']))
375393
.rejects.toThrow(expected_err_msg);
@@ -385,7 +403,9 @@ describe('nc upgrade manager - upgrade config directory', () => {
385403

386404
it('_verify_config_dir_upgrade - fail on mismatch expected_version', async () => {
387405
const expected_version = mock_higher_version_than_pkg_version;
388-
const system_data = { [hostname]: { current_version: pkg.version, other_hostname: { current_version: pkg.version } }};
406+
const system_data = {
407+
[hostname]: { current_version: pkg.version, other_hostname: { current_version: pkg.version } }
408+
};
389409
const nc_upgrade_manager_higher_version = new NCUpgradeManager(config_fs);
390410
const expected_err_msg = `config dir upgrade can not be started - the host's package version=${pkg.version} does not match the user's expected version=${expected_version}`;
391411
await expect(nc_upgrade_manager_higher_version._verify_config_dir_upgrade(system_data, expected_version, [hostname]))
@@ -413,7 +433,8 @@ describe('nc upgrade manager - upgrade config directory', () => {
413433
const successful_upgrade_scripts_obj = { dummy_upgrade_script_1, dummy_upgrade_script_2 };
414434
const failing_upgrade_scripts_obj = { ...successful_upgrade_scripts_obj, dummy_failing_upgrade_script_3 };
415435
const default_this_upgrade = Object.freeze({
416-
config_dir_from_version: from_version, config_dir_to_version: to_version,
436+
config_dir_from_version: from_version,
437+
config_dir_to_version: to_version,
417438
completed_scripts: []
418439
});
419440
const default_upgrade_script_paths = Object.keys(successful_upgrade_scripts_obj).map(script_name =>
@@ -546,7 +567,7 @@ describe('nc upgrade manager - upgrade config directory', () => {
546567
system_data.config_directory.in_progress_upgrade = {
547568
completed_scripts: [],
548569
running_host: hostname,
549-
...options
570+
...options
550571
};
551572
assert_upgrade_start_data(upgrade_start_data, system_data);
552573
const system_json_data = await config_fs.get_system_config_file();
@@ -773,7 +794,7 @@ describe('nc upgrade manager - upgrade config directory', () => {
773794
config_dir_version: config_fs.config_dir_version,
774795
upgrade_package_version: expected_version,
775796
upgrade_history: {
776-
successful_upgrades: [res]
797+
successful_upgrades: [res.upgrade_info]
777798
}
778799
}
779800
};
@@ -785,9 +806,9 @@ describe('nc upgrade manager - upgrade config directory', () => {
785806
const expected_version = pkg.version;
786807
const hosts_list = [hostname];
787808
await config_fs.create_system_config_file(JSON.stringify(system_data));
788-
const expected_err_msg = 'config_dir_version on system.json and config_fs.config_dir_version match, nothing to upgrade';
789-
await expect(nc_upgrade_manager.upgrade_config_dir(expected_version, hosts_list))
790-
.rejects.toThrow(expected_err_msg);
809+
const expected_msg = 'config_dir_version on system.json and config_fs.config_dir_version match, nothing to upgrade';
810+
const res = await nc_upgrade_manager.upgrade_config_dir(expected_version, hosts_list);
811+
expect(res.upgrade_info.detail).toStrictEqual(expected_msg);
791812
});
792813

793814
it('upgrade_config_dir - config_dir_version in system.json is newer than the hosts current config_dir_version', async () => {
@@ -825,7 +846,7 @@ describe('nc upgrade manager - upgrade config directory', () => {
825846
upgrade_package_version: expected_version,
826847
upgrade_history: {
827848
successful_upgrades: [
828-
res,
849+
res.upgrade_info,
829850
...failed_expected_system_json_has_config_directory.config_directory.upgrade_history.successful_upgrades,
830851
]
831852
}
@@ -845,7 +866,7 @@ describe('nc upgrade manager - upgrade config directory', () => {
845866
*/
846867
function assert_upgrade_start_data(actual_upgrade_start, expected_system_data) {
847868
const { config_dir_version, upgrade_package_version, phase, upgrade_history, in_progress_upgrade, current_version } =
848-
actual_upgrade_start.config_directory;
869+
actual_upgrade_start.config_directory;
849870
const expected_in_progress_upgrade = expected_system_data.config_directory?.in_progress_upgrade;
850871

851872
expect(phase).toBe(CONFIG_DIR_PHASES.CONFIG_DIR_LOCKED);
@@ -871,8 +892,14 @@ function assert_upgrade_start_data(actual_upgrade_start, expected_system_data) {
871892
* @param {Object} expected_system_data
872893
*/
873894
function assert_upgrade_finish_or_fail_data(actual_system_data, expected_system_data) {
874-
const { config_dir_version, upgrade_package_version, phase, upgrade_history, in_progress_upgrade, current_version } =
875-
actual_system_data.config_directory;
895+
const {
896+
config_dir_version,
897+
upgrade_package_version,
898+
phase,
899+
upgrade_history,
900+
in_progress_upgrade,
901+
current_version
902+
} = actual_system_data.config_directory;
876903

877904
expect(phase).toBe(expected_system_data.config_directory?.phase);
878905
expect(config_dir_version).toBe(expected_system_data.config_directory?.config_dir_version);

‎src/upgrade/nc_upgrade_manager.js‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ const dbg = require('../util/debug_module')(__filename);
99
const { CONFIG_DIR_PHASES } = require('../sdk/config_fs');
1010
const { should_upgrade, run_upgrade_scripts } = require('./upgrade_utils');
1111
const { version_compare } = require('../util/versions_utils');
12+
const { ManageCLIResponse } = require('../manage_nsfs/manage_nsfs_cli_responses');
1213

1314
const hostname = os.hostname();
1415
// prior to 5.18.0 - there is no config dir version, the config dir version to be used on the first upgrade is 0.0.0 (5.17.0 -> 5.18.0)
@@ -110,9 +111,14 @@ class NCUpgradeManager {
110111
const this_upgrade_versions = { config_dir_from_version, config_dir_to_version, package_from_version, package_to_version };
111112

112113
if (!should_upgrade(config_dir_from_version, config_dir_to_version)) {
113-
const err_message = 'config_dir_version on system.json and config_fs.config_dir_version match, nothing to upgrade';
114-
dbg.error(`upgrade_config_dir: ${err_message}`);
115-
throw new Error(err_message);
114+
const message = 'config_dir_version on system.json and config_fs.config_dir_version match, nothing to upgrade';
115+
dbg.warn(`upgrade_config_dir: ${message}`);
116+
return {
117+
code: ManageCLIResponse.NoUpgradeRequired,
118+
upgrade_info: {
119+
detail: message
120+
}
121+
};
116122
}
117123

118124
if (!skip_verification) await this._verify_config_dir_upgrade(system_data, expected_version, hosts_list);
@@ -128,7 +134,9 @@ class NCUpgradeManager {
128134
}
129135

130136
await this._update_config_dir_upgrade_finish(system_data, this_upgrade);
131-
return this_upgrade;
137+
return {
138+
upgrade_info: this_upgrade
139+
};
132140
}
133141

134142
//////////////////////////////
@@ -230,7 +238,7 @@ class NCUpgradeManager {
230238
* @param {Object} system_data
231239
* @param {{ config_dir_from_version: string; config_dir_to_version: string; package_from_version: string; package_to_version: string; }} options
232240
* @returns {Promise<Object>}
233-
*/
241+
*/
234242
async _update_config_dir_upgrade_start(system_data, options) {
235243
const updated_config_directory = {
236244
...system_data.config_directory,

0 commit comments

Comments
 (0)