From 0d02b9feff3d6627a989eed32c7ba7a615637b09 Mon Sep 17 00:00:00 2001 From: Amit Prinz Setter Date: Mon, 3 Mar 2025 20:21:22 -0800 Subject: [PATCH] bucket notifications - add connection test to health (#8833) Signed-off-by: Amit Prinz Setter --- docs/NooBaaNonContainerized/Health.md | 32 ++++++- src/manage_nsfs/health.js | 104 +++++++++++++++++----- src/manage_nsfs/manage_nsfs_constants.js | 3 +- src/manage_nsfs/manage_nsfs_help_utils.js | 13 +-- src/test/unit_tests/test_nc_health.js | 23 +++++ src/util/notifications_util.js | 6 +- 6 files changed, 148 insertions(+), 33 deletions(-) diff --git a/docs/NooBaaNonContainerized/Health.md b/docs/NooBaaNonContainerized/Health.md index 08bf0c981b..e39f1907a4 100644 --- a/docs/NooBaaNonContainerized/Health.md +++ b/docs/NooBaaNonContainerized/Health.md @@ -42,6 +42,8 @@ For more details about NooBaa RPM installation, see - [Getting Started](./Gettin - checks if there is ongoing upgrade - returns error if there is no ongoing upgrade, but the config directory phase is locked - returns message if there is no ongoing upgrade and the config directory is unlocked + - `Bucket event notifications connections health` + - Sends out a test notification for each connection. * Health CLI requires root permissions. @@ -75,7 +77,12 @@ noobaa-cli diagnose health [--deployment_type][--https_port] - `all_bucket_details` - Type: Boolean - Default: false - - Description: Indicates if health output should contain valid buckets. + - Description: Indicates if health output should contain valid buckets. + +- `all_connection_details` + - Type: Boolean + - Default: false + - Description: Indicates if health output should contain connection test result. - `config_root` - Type: String @@ -143,6 +150,10 @@ The output of the Health CLI is a JSON object containing the following propertie 2. The account doesn't have RW access to its `new_buckets_path` or having invalid config JSON file. 3. The account has an invalid config JSON file. +- `invalid connections`: + - Type: [{ "name": "connection_name", "config_path": "/connection/file/path", "code": String }] + - Description: Array of connections that failed test notification. + - `valid_accounts` - Type: [{ "name": account_name, "storage_path": "/path/to/accounts/new_buckets_path" }] - Description: Array of all the valid accounts. If the all_account_details flag is set to true, valid_accounts will be included in the Health response. @@ -151,6 +162,10 @@ The output of the Health CLI is a JSON object containing the following propertie - Type: [{ "name": bucket_name, "storage_path": "/path/to/bucket/path" }] - Description: Array of all the valid buckets. If the all_bucket_details flag is set to true, valid_buckets will be included in the Health response. +- `valid_connections`: + - Type: [{ "name": "connection_name", "config_path": "/connection/file/path" }] + - Description: Array of all connections to which test notification was send successfully. + - `error_type` - Type: String - Enum: 'PERSISTENT' | 'TEMPORARY' @@ -239,6 +254,21 @@ Output: ], "error_type": "PERSISTENT" }, + "connectoins_status": { + "invalid_connections": [ + { + "name": "notif_invalid", + "config_path": "/etc/noobaa.conf.d/connections/notif_invalid.json", + "code": "ECONNREFUSED" + } + ], + "valid_connections": [ + { + "name": "notif_valid", + "config_path": "/etc/noobaa.conf.d/connections/notif_valid.json" + } + ] + }, "config_directory": { "phase": "CONFIG_DIR_UNLOCKED", "config_dir_version": "1.0.0", diff --git a/src/manage_nsfs/health.js b/src/manage_nsfs/health.js index 1a81707f68..8dc9437fb2 100644 --- a/src/manage_nsfs/health.js +++ b/src/manage_nsfs/health.js @@ -15,6 +15,7 @@ const { TYPES } = require('./manage_nsfs_constants'); const { get_boolean_or_string_value, throw_cli_error, write_stdout_response, get_bucket_owner_account_by_id } = require('./manage_nsfs_cli_utils'); const { ManageCLIResponse } = require('./manage_nsfs_cli_responses'); const ManageCLIError = require('./manage_nsfs_cli_errors').ManageCLIError; +const notifications_util = require('../util/notifications_util'); const HOSTNAME = 'localhost'; @@ -101,6 +102,7 @@ class NSFSHealth { this.https_port = options.https_port; this.all_account_details = options.all_account_details; this.all_bucket_details = options.all_bucket_details; + this.all_connection_details = options.all_connection_details; this.config_fs = options.config_fs; } @@ -128,12 +130,14 @@ class NSFSHealth { let bucket_details; let account_details; + let connection_details; const endpoint_response_code = (endpoint_state && endpoint_state.response?.response_code) || 'UNKNOWN_ERROR'; const health_check_params = { service_status, pid, endpoint_response_code, config_directory_status }; const service_health = this._calc_health_status(health_check_params); const error_code = this.get_error_code(health_check_params); if (this.all_bucket_details) bucket_details = await this.get_bucket_status(); if (this.all_account_details) account_details = await this.get_account_status(); + if (this.all_connection_details) connection_details = await this.get_connection_status(); const health = { service_name: NOOBAA_SERVICE, status: service_health, @@ -155,11 +159,13 @@ class NSFSHealth { invalid_buckets: bucket_details === undefined ? undefined : bucket_details.invalid_storages, valid_buckets: bucket_details === undefined ? undefined : bucket_details.valid_storages, error_type: health_errors_tyes.PERSISTENT, - } + }, + connections_status: connection_details } }; if (!this.all_account_details) delete health.checks.accounts_status; if (!this.all_bucket_details) delete health.checks.buckets_status; + if (!this.all_connection_details) delete health.checks.connections_status; return health; } @@ -333,6 +339,17 @@ class NSFSHealth { }; } + async validate_config_dir_exists(path, type) { + const config_root_type_exists = await this.config_fs.validate_config_dir_exists(path); + if (!config_root_type_exists) { + dbg.log1(`Config directory type - ${type} is missing, ${path}`); + return { + invalid_storages: [], + valid_storages: [] + }; + } + } + async get_bucket_status() { const bucket_details = await this.get_storage_status(TYPES.BUCKET, this.all_bucket_details); return bucket_details; @@ -343,34 +360,38 @@ class NSFSHealth { return account_details; } + async get_connection_status() { + const connection_details = await this.get_storage_status(TYPES.CONNECTION, this.all_connection_details); + //if there were in failed test notifications, mark error as temp + if (connection_details.invalid_storages && connection_details.invalid_storages.length > 0) { + connection_details.error_type = health_errors_tyes.TEMPORARY; + } + return connection_details; + } + async get_storage_status(type, all_details) { const invalid_storages = []; const valid_storages = []; //check for account and buckets dir paths - let config_root_type_exists; let config_dir_path; if (type === TYPES.BUCKET) { config_dir_path = this.config_fs.buckets_dir_path; - config_root_type_exists = await this.config_fs.validate_config_dir_exists(config_dir_path); } else if (type === TYPES.ACCOUNT) { // TODO - handle iam accounts when directory structure changes - read_account_by_id config_dir_path = this.config_fs.accounts_by_name_dir_path; - config_root_type_exists = await this.config_fs.validate_config_dir_exists(config_dir_path); - } - // TODO - this is not a good handling for that - we need to take it to an upper level - if (!config_root_type_exists) { - dbg.log1(`Config directory type - ${type} is missing, ${config_dir_path}`); - return { - invalid_storages: invalid_storages, - valid_storages: valid_storages - }; + } else { + config_dir_path = this.config_fs.connections_dir_path; } + const missing = await this.validate_config_dir_exists(config_dir_path, type); + if (missing) return missing; let config_files_names; if (type === TYPES.BUCKET) { config_files_names = await this.config_fs.list_buckets(); - } else { + } else if (type === TYPES.ACCOUNT) { config_files_names = await this.config_fs.list_accounts(); + } else { + config_files_names = await this.config_fs.list_connections(); } for (const config_file_name of config_files_names) { // config_file get data or push error @@ -386,13 +407,24 @@ class NSFSHealth { let res; const storage_path = type === TYPES.BUCKET ? config_data.path : - config_data.nsfs_account_config.new_buckets_path; + config_data.nsfs_account_config?.new_buckets_path; if (type === TYPES.ACCOUNT) { const config_file_path = this.config_fs.get_account_path_by_name(config_file_name); res = await is_new_buckets_path_valid(config_file_path, config_data, storage_path); } else if (type === TYPES.BUCKET) { res = await is_bucket_storage_and_owner_exists(this.config_fs, config_data, storage_path); + } else { + const connection_file_path = this.config_fs.get_connection_path_by_name(config_file_name); + const test_notif_err = await notifications_util.test_notifications([{ + name: config_data.name, + topic: [this.config_fs.json(config_file_name)] + }], this.config_fs.config_root); + if (test_notif_err) { + res = get_invalid_object(config_data.name, connection_file_path, undefined, test_notif_err.code); + } else { + res = get_valid_object(config_data.name, connection_file_path, undefined); + } } if (all_details && res.valid_storage) { valid_storages.push(res.valid_storage); @@ -416,16 +448,41 @@ class NSFSHealth { let config_data; let err_obj; try { - config_data = type === TYPES.BUCKET ? - await this.config_fs.get_bucket_by_name(config_file_name) : - // TODO - should be changed to id when moving to new structure for supporting iam accounts - await this.config_fs.get_account_by_name(config_file_name); + switch (type) { + case TYPES.BUCKET: { + config_data = await this.config_fs.get_bucket_by_name(config_file_name); + break; + } + case TYPES.ACCOUNT: { + // TODO - should be changed to id when moving to new structure for supporting iam accounts + config_data = await this.config_fs.get_account_by_name(config_file_name); + break; + } + case TYPES.CONNECTION: { + config_data = await this.config_fs.get_connection_by_name(config_file_name); + break; + } + default: //shouldn't happen, for js-lint + } } catch (err) { let err_code; - const config_file_path = type === TYPES.BUCKET ? - this.config_fs.get_bucket_path_by_name(config_file_name) : - // TODO - should be changed to id when moving to new structure for supporting iam accounts - this.config_fs.get_account_path_by_name(config_file_name); + let config_file_path; + switch (type) { + case TYPES.BUCKET: { + config_file_path = await this.config_fs.get_bucket_path_by_name(config_file_name); + break; + } + case TYPES.ACCOUNT: { + // TODO - should be changed to id when moving to new structure for supporting iam accounts + config_file_path = await this.config_fs.get_account_path_by_name(config_file_name); + break; + } + case TYPES.CONNECTION: { + config_file_path = await this.config_fs.get_connection_path_by_name(config_file_name); + break; + } + default: //shouldn't happen, for js-lint + } if (err.code === 'ENOENT') { dbg.log1(`Error: Config file path should be a valid path`, config_file_path, err); @@ -546,9 +603,10 @@ async function get_health_status(argv, config_fs) { const deployment_type = argv.deployment_type || 'nc'; const all_account_details = get_boolean_or_string_value(argv.all_account_details); const all_bucket_details = get_boolean_or_string_value(argv.all_bucket_details); + const all_connection_details = get_boolean_or_string_value(argv.all_connection_details); if (deployment_type === 'nc') { - const health = new NSFSHealth({ https_port, all_account_details, all_bucket_details, config_fs }); + const health = new NSFSHealth({ https_port, all_account_details, all_bucket_details, all_connection_details, config_fs }); const health_status = await health.nc_nsfs_health(); write_stdout_response(ManageCLIResponse.HealthStatus, health_status); } else { diff --git a/src/manage_nsfs/manage_nsfs_constants.js b/src/manage_nsfs/manage_nsfs_constants.js index b69d967617..4bf7340f09 100644 --- a/src/manage_nsfs/manage_nsfs_constants.js +++ b/src/manage_nsfs/manage_nsfs_constants.js @@ -74,7 +74,7 @@ const VALID_OPTIONS_GLACIER = { }; const VALID_OPTIONS_DIAGNOSE = { - 'health': new Set([ 'https_port', 'deployment_type', 'all_account_details', 'all_bucket_details', ...CLI_MUTUAL_OPTIONS]), + 'health': new Set([ 'https_port', 'deployment_type', 'all_account_details', 'all_bucket_details', 'all_connection_details', ...CLI_MUTUAL_OPTIONS]), 'gather-logs': new Set([ CONFIG_ROOT_FLAG]), 'metrics': new Set([CONFIG_ROOT_FLAG]) }; @@ -143,6 +143,7 @@ const OPTION_TYPE = { deployment_type: 'string', all_account_details: 'boolean', all_bucket_details: 'boolean', + all_connection_details: 'boolean', https_port: 'number', debug: 'number', // upgrade options diff --git a/src/manage_nsfs/manage_nsfs_help_utils.js b/src/manage_nsfs/manage_nsfs_help_utils.js index 1682d59088..e7c4fd9352 100644 --- a/src/manage_nsfs/manage_nsfs_help_utils.js +++ b/src/manage_nsfs/manage_nsfs_help_utils.js @@ -366,12 +366,13 @@ Usage: Flags: - --deployment_type (optional) Set the nsfs type for heath check.(default nc; Non Containerized) - --https_port (optional) Set the S3 endpoint listening HTTPS port to serve. (default config.ENDPOINT_SSL_PORT) - --all_account_details (optional) Set a flag for returning all account details. - --all_bucket_details (optional) Set a flag for returning all bucket details. - --debug (optional) Use for increasing the log verbosity of health cli commands. - --config_root (optional) Set Configuration files path for Noobaa standalon NSFS. (default config.NSFS_NC_DEFAULT_CONF_DIR) + --deployment_type (optional) Set the nsfs type for heath check.(default nc; Non Containerized) + --https_port (optional) Set the S3 endpoint listening HTTPS port to serve. (default config.ENDPOINT_SSL_PORT) + --all_account_details (optional) Set a flag for returning all account details. + --all_bucket_details (optional) Set a flag for returning all bucket details. + --all_connection_details (optional) Set a flag for returning all connection details. + --debug (optional) Use for increasing the log verbosity of health cli commands. + --config_root (optional) Set Configuration files path for Noobaa standalon NSFS. (default config.NSFS_NC_DEFAULT_CONF_DIR) `; diff --git a/src/test/unit_tests/test_nc_health.js b/src/test/unit_tests/test_nc_health.js index e05ce54871..016a10b846 100644 --- a/src/test/unit_tests/test_nc_health.js +++ b/src/test/unit_tests/test_nc_health.js @@ -9,6 +9,7 @@ const sinon = require('sinon'); const assert = require('assert'); const config = require('../../../config'); const pkg = require('../../../package.json'); +const fs = require('fs'); const fs_utils = require('../../util/fs_utils'); const nb_native = require('../../util/nb_native'); const { ConfigFS } = require('../../sdk/config_fs'); @@ -44,6 +45,17 @@ const get_system_config_mock_default_response = [{ } }]; const default_mock_upgrade_status = { message: 'there is no in-progress upgrade' }; +const connection_from_file_path = path.join(tmp_fs_path, 'connection_from_file'); +const connection_from_file = { + agent_request_object: { + host: "localhost", + port: 9999, + timeout: 100 + }, + notification_protocol: "http", + name: "http_notif", +}; + mocha.describe('nsfs nc health', function() { const config_root = path.join(tmp_fs_path, 'config_root_nsfs_health'); @@ -210,6 +222,17 @@ mocha.describe('nsfs nc health', function() { assert_config_dir_status(health_status, valid_system_json.config_directory); }); + mocha.it('health should report on invalid connections', async function() { + fs.writeFileSync(connection_from_file_path, JSON.stringify(connection_from_file)); + await exec_manage_cli(TYPES.CONNECTION, ACTIONS.ADD, { config_root, from_file: connection_from_file_path }); + + Health.all_connection_details = true; + const health_status = await Health.nc_nsfs_health(); + Health.all_connection_details = false; + + assert.strictEqual(health_status.checks.connections_status.invalid_storages[0].name, connection_from_file.name); + }); + mocha.it('NooBaa service is inactive', async function() { set_mock_functions(Health, { get_service_state: [{ service_status: 'inactive', pid: 0 }], diff --git a/src/util/notifications_util.js b/src/util/notifications_util.js index e2386a061b..9d1ee0f437 100644 --- a/src/util/notifications_util.js +++ b/src/util/notifications_util.js @@ -351,10 +351,12 @@ async function test_notifications(notifs, nc_config_dir) { return; } let connect_files_dir = config.NOTIFICATION_CONNECT_DIR; + let config_fs; if (nc_config_dir) { - connect_files_dir = new ConfigFS(nc_config_dir).connections_dir_path; + config_fs = new ConfigFS(nc_config_dir); + connect_files_dir = config_fs.connections_dir_path; } - const notificator = new Notificator({connect_files_dir}); + const notificator = new Notificator({connect_files_dir, nc_config_fs: config_fs}); for (const notif of notifs) { let connect; let connection;