Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NC | S3 internal APIs gaps | Add versions CLI command | S3 internal API refactoring #8800

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/NooBaaNonContainerized/CI&Tests.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ The following is a list of `NC jest tests` files -
18. `test_cli_upgrade.test.js` - Tests of the upgrade CLI commands.
19. `test_nc_online_upgrade_cli_integrations.test.js` - Tests CLI commands during mocked config directory upgrade.
20. `test_nc_connection_cli.test.js` - Tests NooBaa CLI connection commands.
21. `test_cli_versions.test.js` - Tests NooBaa CLI versions command.

#### nc_index.js File
* The `nc_index.js` is a file that runs several NC and NSFS mocha related tests.
Expand Down
18 changes: 16 additions & 2 deletions docs/NooBaaNonContainerized/NooBaaCLI.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,9 @@
3. [Connection Status](#connection-status)
4. [List Connections][#list-connections]
5. [Delete Connection](#delete-connection)
11. [Global Options](#global-options)
12. [Examples](#examples)
11. [Fetching Versions Status](#fetching-versions-status)
12. [Global Options](#global-options)
13. [Examples](#examples)
1. [Bucket Commands Examples](#bucket-commands-examples)
2. [Account Commands Examples](#account-commands-examples)
3. [White List Server IP Command Example](#white-list-server-ip-command-example)
Expand Down Expand Up @@ -634,6 +635,19 @@ noobaa-cli connection delete --name <connection_name>
- Type: String
- Description: Specifies the name of the connection to be deleted.


## Fetching Versions Status

The `versions` command is used to print the status of the rpm_source_code_versions, host_running_service_versions and config_dir_version.
- rpm_source_code_versions consists of the package_version and the config_fs version.
- host_running_service_versions consists of the running service package_version and config_fs version.
- config_dir_version is the current config_dir_version registered in system.json.
Comment on lines +641 to +644
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add the var name style for easier reading, for example:

Suggested change
The `versions` command is used to print the status of the rpm_source_code_versions, host_running_service_versions and config_dir_version.
- rpm_source_code_versions consists of the package_version and the config_fs version.
- host_running_service_versions consists of the running service package_version and config_fs version.
- config_dir_version is the current config_dir_version registered in system.json.
The `versions` command is used to print the status of the `rpm_source_code_versions`, `host_running_service_versions`, and `config_dir_version`.
- `rpm_source_code_versions` consists of the `package_version` and the `config_fs` version.
- `host_running_service_versions` consists of the running service `package_version` and `config_fs` version.
- `config_dir_version` is the current `config_dir_version` registered in `system.json`.


#### Usage
```sh
noobaa-cli versions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to make sure - that there will be no additional actions (we could noobaa-cli versions status, but choose this way). Could you share your thoughts on this decision?

```

## Global Options

Global options used by the CLI to define the config directory settings.
Expand Down
3 changes: 3 additions & 0 deletions src/cmd/manage_nsfs.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const manage_nsfs_glacier = require('../manage_nsfs/manage_nsfs_glacier');
const manage_nsfs_logging = require('../manage_nsfs/manage_nsfs_logging');
const noobaa_cli_diagnose = require('../manage_nsfs/diagnose');
const noobaa_cli_upgrade = require('../manage_nsfs/upgrade');
const noobaa_cli_versions = require('../manage_nsfs/versions');
const { print_usage } = require('../manage_nsfs/manage_nsfs_help_utils');
const { TYPES, ACTIONS, LIST_ACCOUNT_FILTERS, LIST_BUCKET_FILTERS, GLACIER_ACTIONS } = require('../manage_nsfs/manage_nsfs_constants');
const { throw_cli_error, get_bucket_owner_account_by_name,
Expand Down Expand Up @@ -80,6 +81,8 @@ async function main(argv = minimist(process.argv.slice(2))) {
await notification_management();
} else if (type === TYPES.CONNECTION) {
await connection_management(action, user_input);
} else if (type === TYPES.VERSIONS) {
await noobaa_cli_versions.versions_management(config_fs);
} else {
throw_cli_error(ManageCLIError.InvalidType);
}
Expand Down
35 changes: 30 additions & 5 deletions src/endpoint/endpoint.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const s3_rest = require('./s3/s3_rest');
const blob_rest = require('./blob/blob_rest');
const sts_rest = require('./sts/sts_rest');
const iam_rest = require('./iam/iam_rest');
const { CONFIG_DIR_VERSION } = require('../sdk/config_fs');
const lambda_rest = require('./lambda/lambda_rest');
const endpoint_utils = require('./endpoint_utils');
const FuncSDK = require('../sdk/func_sdk');
Expand Down Expand Up @@ -60,6 +61,13 @@ const SERVICES_TYPES_ENUM = Object.freeze({
METRICS: 'METRICS'
});

const INTERNAL_APIS_OBJ = Object.freeze({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a comment about this object?
It is only used in NC deployment, and this file is mutual for all deployments.

VERSION: 'version',
CONFIG_FS_VERSION: 'config_fs_version',
ENDPOINT_FORK_ID: 'endpoint_fork_id',
TOTAL_FORK_COUNT: 'total_fork_count'
});

const new_umask = process.env.NOOBAA_ENDPOINT_UMASK || 0o000;
const old_umask = process.umask(new_umask);
let fork_count;
Expand Down Expand Up @@ -291,15 +299,17 @@ function create_endpoint_handler(server_type, init_request_sdk, { virtual_hosts,
return lambda_rest_handler(req, res);
} else if (req.headers['x-ms-version']) {
return blob_rest_handler(req, res);
} else if (req.url.startsWith('/total_fork_count')) {
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') {
if (api === INTERNAL_APIS_OBJ.VERSION) {
return version_handler(req, res);
} else if (api === INTERNAL_APIS_OBJ.CONFIG_FS_VERSION) {
return config_fs_version_handler(req, res);
} else if (api === INTERNAL_APIS_OBJ.ENDPOINT_FORK_ID) {
return endpoint_fork_id_handler(req, res);
} else if (api === INTERNAL_APIS_OBJ.TOTAL_FORK_COUNT) {
return fork_count_handler(req, res);
} else {
return internal_api_error(req, res, `Unknown API call ${api}`);
}
Expand Down Expand Up @@ -351,6 +361,21 @@ function version_handler(req, res) {
res.end(noobaa_package_version);
}

/**
* config_fs_version_handler returns the version of configFS
* this is not the actual config dir version
* this is the version that NooBaa targets for writing configuration files.
* @param {EndpointRequest} req
* @param {import('http').ServerResponse} res
*/
function config_fs_version_handler(req, res) {
const config_dir_version = CONFIG_DIR_VERSION;
res.statusCode = 200;
res.setHeader('Content-Type', 'text/plain');
res.setHeader('Content-Length', Buffer.byteLength(config_dir_version));
res.end(config_dir_version);
}

/**
* internal_api_error returns an internal api error response
* @param {EndpointRequest} req
Expand Down
4 changes: 2 additions & 2 deletions src/manage_nsfs/health.js
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ class NSFSHealth {
}

async get_endpoint_fork_response() {
let url_path = '/total_fork_count';
let url_path = '/_/total_fork_count';
const worker_ids = [];
let total_fork_count = 0;
let response;
Expand All @@ -266,7 +266,7 @@ class NSFSHealth {
}
total_fork_count = fork_count_response.fork_count;
if (total_fork_count > 0) {
url_path = '/endpoint_fork_id';
url_path = '/_/endpoint_fork_id';
await P.retry({
attempts: total_fork_count * 2,
delay_ms: 1,
Expand Down
2 changes: 1 addition & 1 deletion src/manage_nsfs/manage_nsfs_cli_errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ ManageCLIError.UnsetArgumentIsInvalid = Object.freeze({

ManageCLIError.InvalidType = Object.freeze({
code: 'InvalidType',
message: 'Invalid type, available types are account, bucket, logging, whitelist, upgrade, notification or connection.',
message: 'Invalid type, available types are account, bucket, logging, whitelist, upgrade, notification, connection, or versions.',
http_code: 400,
});

Expand Down
10 changes: 10 additions & 0 deletions src/manage_nsfs/manage_nsfs_cli_responses.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,16 @@ ManageCLIResponse.MetricsStatus = Object.freeze({
status: {}
});

///////////////////////////////
////// VERSIONS RESPONSES ////
///////////////////////////////

ManageCLIResponse.VersionsStatus = Object.freeze({
code: 'VersionsStatus',
message: 'Versions status retrieved successfully',
status: {}
});

///////////////////////////////
// IPS WHITE LIST RESPONSES ///
///////////////////////////////
Expand Down
3 changes: 2 additions & 1 deletion src/manage_nsfs/manage_nsfs_constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ const TYPES = Object.freeze({
DIAGNOSE: 'diagnose',
UPGRADE: 'upgrade',
NOTIFICATION: 'notification',
CONNECTION: 'connection'
CONNECTION: 'connection',
VERSIONS: 'versions'
});

const ACTIONS = Object.freeze({
Expand Down
82 changes: 82 additions & 0 deletions src/manage_nsfs/versions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/* Copyright (C) 2024 NooBaa */
'use strict';

const dbg = require('../util/debug_module')(__filename);
const config = require('../../config');
const pkg = require('../../package.json');
const http_utils = require('../util/http_utils');
const buffer_utils = require('../util/buffer_utils');
const { write_stdout_response } = require('./manage_nsfs_cli_utils');
const { ManageCLIResponse } = require('./manage_nsfs_cli_responses');

////////////////////////
// VERSIONS MANAGEMENT //
////////////////////////

/**
* versions_management
*/
async function versions_management(config_fs) {
const system_json = await config_fs.get_system_config_file({ silent_if_missing: true });
const versions = {
rpm_source_code_versions: {
package_version: pkg.version,
config_fs_version: config_fs.config_dir_version,
},
host_running_service_versions: await get_running_service_versions(),
config_dir_version: system_json?.config_directory.config_dir_version || 'unknown'
};

const response = { code: ManageCLIResponse.VersionsStatus, detail: versions };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@romayalon
suggestion: will it help if the rpm and host versions are different and the version status returns an error message mentioning that conflict?

write_stdout_response(response.code, response.detail);
}

/**
* get_running_service_versions returns the versions of the running service
* @returns {Promise<Object>}
*/
async function get_running_service_versions() {
const host_service_versions = {};
try {
const package_version_api = '/_/version';
const config_dir_version_api = '/_/config_fs_version';
host_service_versions.package_version = await get_version_api_response(package_version_api);
host_service_versions.config_fs_version = await get_version_api_response(config_dir_version_api);
} catch (err) {
dbg.warn('could not receive versions response', err);
}
return host_service_versions;
}

/**
* get_version_api_response runs a GET request to the given api and returns the response
* @param {string} api
* @returns
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing parameter?

*/
async function get_version_api_response(api) {
let version;
try {
const res = await http_utils.make_https_request({
hostname: 'localhost',
port: config.ENDPOINT_SSL_PORT,
path: api,
method: 'GET',
rejectUnauthorized: false
});

if (res.statusCode === 200) {
const buffer = await buffer_utils.read_stream_join(res);
version = buffer.toString('utf8');
} else if (res.statusCode >= 500) {
const buffer = await buffer_utils.read_stream_join(res);
const body = buffer.toString('utf8');
dbg.log0(`get_version_api_response received an error from ${api} api, skipping', ${body}`);
}
} catch (err) {
dbg.warn('get_version_api_response: err', err);
}
return version?.trim() || 'unknown';
}

// EXPORTS
exports.versions_management = versions_management;
1 change: 1 addition & 0 deletions src/sdk/config_fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -1425,4 +1425,5 @@ exports.JSON_SUFFIX = JSON_SUFFIX;
exports.CONFIG_SUBDIRS = CONFIG_SUBDIRS;
exports.CONFIG_TYPES = CONFIG_TYPES;
exports.CONFIG_DIR_PHASES = CONFIG_DIR_PHASES;
exports.CONFIG_DIR_VERSION = CONFIG_DIR_VERSION;
exports.ConfigFS = ConfigFS;
Loading