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

romayalon · 2025-02-16T18:15:20Z

Explain the changes

Added a new CLI command called versions that returns the following versions status -

> noobaa-cli versions
{
  "response": {
    "code": "VersionsStatus",
    "message": "Versions status retrieved successfully",
    "reply": {
      "rpm_source_code_versions": {
        "package_version": "5.19.0",
        "config_fs_version": "1.0.0"
      },
      "host_running_service_versions": {
        "package_version": "5.19.0",
        "config_fs_version": "1.0.0"
      },
      "config_dir_version": "1.0.0"
    }
  }
}

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.

added endpoint_fork_id and total_fork_count to be under the internal endpoint apis prefix - /_/ and did some refactoring there.
Added unit tests to the new CLI command.

Issues: Fixed #xxx / Gap #xxx

Fixed gaps mentioned in Add http://{endpoint_address}:{s3_port}/_/version api to the endpoint #8786
Open question - is rpm_source_code_versions fetched correctly.

Testing Instructions:

sudo jest --testRegex=jest_tests/test_cli_versions
Manual -

Service is up -
a. Tab 1 - start noobaa service - sudo node noobaa-core/src/cmd/nsfs.js --debug
b. Tab 2 - run noobaa-cli versions command - noobaa-cli versions

Service is down -
a. Tab 1 - start noobaa service - sudo node noobaa-core/src/cmd/nsfs.js --debug, stop noobaa service - Ctrl + C
b. Tab 1 - run noobaa-cli versions command - noobaa-cli versions

system.json was never created -
a. Tab 1 - run noobaa-cli versions command - noobaa-cli versions

Doc added/updated
Tests added

shirady

LGTM
I added minor comments and questions

shirady · 2025-02-17T14:47:13Z

docs/NooBaaNonContainerized/NooBaaCLI.md

+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.


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`.

shirady · 2025-02-17T14:51:20Z

src/endpoint/endpoint.js

@@ -60,6 +61,13 @@ const SERVICES_TYPES_ENUM = Object.freeze({
    METRICS: 'METRICS'
 });

+const INTERNAL_APIS_OBJ = Object.freeze({


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

shirady · 2025-02-17T14:55:32Z

src/manage_nsfs/versions.js

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


missing parameter?

shirady · 2025-02-17T14:58:04Z

src/test/unit_tests/jest_tests/test_cli_versions.js

@@ -0,0 +1,143 @@
+/* Copyright (C) 2016 NooBaa */


You can update the year

Suggested change

/* Copyright (C) 2016 NooBaa */

/* Copyright (C) 2025 NooBaa */

shirady · 2025-02-17T15:03:32Z

src/test/unit_tests/jest_tests/test_cli_versions.js

+                    package_version: pkg.version,
+                    config_fs_version: CONFIG_DIR_VERSION
+                },
+                expected_config_dir_version: 'unknown'


To help others understand the flow better - in this case, I would add a comment that we see the 'unknown' because there is no system.json file, and in the other cases I would just point to the reason (I understand that it is written in the test title, but I think it can help).

shirady · 2025-02-17T15:10:47Z

docs/NooBaaNonContainerized/NooBaaCLI.md

+
+#### Usage
+```sh
+noobaa-cli versions


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?

shirady · 2025-02-17T15:12:22Z

@romayalon if you can attach or add in the manual testing the expected result I think it can be easier to understand parts of the cases.

naveenpaul1 · 2025-02-18T04:46:37Z

src/manage_nsfs/versions.js

+        config_dir_version: system_json?.config_directory.config_dir_version || 'unknown'
+    };
+
+    const response = { code: ManageCLIResponse.VersionsStatus, detail: versions };


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

Signed-off-by: Romy <[email protected]>

pull-request-size bot added the size/L label Feb 16, 2025

romayalon force-pushed the romy-s3-internal-apis branch 3 times, most recently from ff1877a to cb95c81 Compare February 17, 2025 12:25

romayalon requested review from shirady and naveenpaul1 February 17, 2025 13:59

shirady approved these changes Feb 17, 2025

View reviewed changes

naveenpaul1 reviewed Feb 18, 2025

View reviewed changes

S3 internal APIs gaps

a165994

Signed-off-by: Romy <[email protected]>

romayalon force-pushed the romy-s3-internal-apis branch from 07a7878 to a165994 Compare February 18, 2025 16:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

romayalon commented Feb 16, 2025 •

edited

Loading

shirady left a comment

shirady Feb 17, 2025

shirady Feb 17, 2025

shirady Feb 17, 2025

shirady Feb 17, 2025

shirady Feb 17, 2025

shirady Feb 17, 2025

shirady commented Feb 17, 2025

naveenpaul1 Feb 18, 2025

	/* Copyright (C) 2016 NooBaa */
	/* Copyright (C) 2025 NooBaa */

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

Are you sure you want to change the base?

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

Conversation

romayalon commented Feb 16, 2025 • edited Loading

Explain the changes

Issues: Fixed #xxx / Gap #xxx

Testing Instructions:

shirady left a comment

Choose a reason for hiding this comment

shirady Feb 17, 2025

Choose a reason for hiding this comment

shirady Feb 17, 2025

Choose a reason for hiding this comment

shirady Feb 17, 2025

Choose a reason for hiding this comment

shirady Feb 17, 2025

Choose a reason for hiding this comment

shirady Feb 17, 2025

Choose a reason for hiding this comment

shirady Feb 17, 2025

Choose a reason for hiding this comment

shirady commented Feb 17, 2025

naveenpaul1 Feb 18, 2025

Choose a reason for hiding this comment

romayalon commented Feb 16, 2025 •

edited

Loading