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 | Health fix cli arguments validation and some small fixes #8052

Merged
merged 1 commit into from
Jun 16, 2024

Conversation

romayalon
Copy link
Contributor

@romayalon romayalon commented May 19, 2024

Explain the changes

  1. Health.js -
  • Added CLI arguments type validation (validate_input_types).
  • Called get_boolean_or_string_value(arg) on every boolean argument.
  • Added try catch when calling exec(), and changed ignore_rc to - when ignore_rc=true service_status/pid/memory are undefined, when ignore_rc=false, setting them as missing so the user will know the command failed and didn't succeed.
  1. Manage_nsfs_constants.js - Added Health related type and options.
  2. Manage_nsfs_validations.js - Added Health options to be validated.
  3. test_utils.js - added exec_health_cli.js function.
  4. test_nc_nsfs_health.js - added relevant tests and fixed failing.

Issues: Fixed #xxx / Gap #xxx

  1. Fixed https://bugzilla.redhat.com/show_bug.cgi?id=2262777
  2. all_account_details/all_bucket_details defined check Uday mentioned on slack channel.

Testing Instructions:

  1. sudo node --trace-warnings ./node_modules/mocha/bin/mocha /Users/romy/github/noobaa-core/src/test/unit_tests/test_nc_nsfs_health.js
  2. Gap - a follow-up Refactoring PR
  • Doc added/updated
  • Tests added

@romayalon romayalon force-pushed the romy-nc-health-fixes branch 2 times, most recently from 476576b to 7c899a8 Compare May 19, 2024 13:23
src/cmd/health.js Outdated Show resolved Hide resolved
src/cmd/health.js Outdated Show resolved Hide resolved
src/manage_nsfs/manage_nsfs_constants.js Outdated Show resolved Hide resolved
src/manage_nsfs/manage_nsfs_validations.js Outdated Show resolved Hide resolved
src/cmd/health.js Show resolved Hide resolved
src/cmd/health.js Outdated Show resolved Hide resolved
src/cmd/health.js Outdated Show resolved Hide resolved
src/cmd/health.js Outdated Show resolved Hide resolved
src/test/unit_tests/test_nc_nsfs_health.js Outdated Show resolved Hide resolved
src/test/unit_tests/test_nc_nsfs_health.js Outdated Show resolved Hide resolved
src/test/unit_tests/test_nc_nsfs_health.js Outdated Show resolved Hide resolved
@naveenpaul1
Copy link
Contributor

@romayalon could you update the doc with new changes, Other than that its LGTM

@romayalon romayalon force-pushed the romy-nc-health-fixes branch 4 times, most recently from 6890b96 to 2af30b6 Compare May 29, 2024 15:39
@romayalon
Copy link
Contributor Author

This PR is depenedant on validations changes, therefore it can not be backported to 5.15.4

@romayalon romayalon force-pushed the romy-nc-health-fixes branch 3 times, most recently from a88934e to ab2bada Compare June 10, 2024 09:30
Signed-off-by: Romy <35330373+romayalon@users.noreply.github.com>
@romayalon romayalon merged commit 33d7471 into noobaa:master Jun 16, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants