HDDS-15690. Support Node-ID for DiskBalancer Commands#10755
HDDS-15690. Support Node-ID for DiskBalancer Commands#10755Gargi-jais11 wants to merge 2 commits into
Conversation
|
@Gargi-jais11 thanks for the patch! |
There was a problem hiding this comment.
Pull request overview
Updates the ozone admin datanode diskbalancer CLI to accept datanode UUIDs (Node IDs) in addition to host/IP forms, resolving UUIDs via SCM before issuing DiskBalancer RPCs and showing UUIDs in the display output. This improves CLI usability/consistency with other admin commands and reduces user confusion when supplying node identifiers.
Changes:
- Add UUID detection + UUID→CLIENT_RPC address resolution via SCM for DiskBalancer subcommands.
- Include UUID in human/JSON output display strings (
hostname (ip:port / uuid)), and document the new accepted identifier forms. - Add/extend unit tests covering UUID resolution, mixed valid/invalid inputs, JSON output, and state-reset behavior across invocations.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/AbstractDiskBalancerSubCommand.java | Resolves UUID args via SCM, tracks resolution failures, resets per-invocation state, and formats display names via SCM metadata. |
| hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/DiskBalancerSubCommandUtil.java | Adds UUID detection/resolution helpers, CLIENT_RPC address helper, and appends UUID to display formatting. |
| hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/DiskBalancerStatusSubcommand.java | Clears cached per-run status state and widens output formatting to accommodate UUID in display. |
| hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/DiskBalancerReportSubcommand.java | Clears cached per-run report state to prevent cross-invocation leakage. |
| hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/DiskBalancerCommands.java | Updates CLI usage docs to describe UUID-capable positional args and output format changes. |
| hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/DatanodeParameters.java | Updates positional parameter label and help text to reflect UUID support. |
| hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/hdds/scm/cli/datanode/TestDiskBalancerSubCommandUtil.java | New unit tests for UUID detection, UUID resolution, and UUID-inclusive display formatting. |
| hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/hdds/scm/cli/datanode/TestDiskBalancerSubCommands.java | Adds tests for UUID arguments across status/start/stop/update, mixed validity, JSON output, and state reset. |
| hadoop-hdds/docs/content/feature/DiskBalancer.md | Documents UUID support in the DiskBalancer CLI user docs. |
| hadoop-hdds/docs/content/feature/DiskBalancer.zh.md | Same as above for the Chinese user docs. |
| hadoop-hdds/docs/content/design/diskbalancer.md | Updates design docs to reflect SCM usage for UUID resolution in addition to batch discovery. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
slfan1989
left a comment
There was a problem hiding this comment.
Thanks for working on this! The overall approach looks good to me, and the added test coverage is helpful. I left one minor comment about preserving non-UUID targets when SCM resolution is unavailable, but it does not look like a blocker to me.
|
@ChenSammi Please take a look again. I have updated the code to support --node-id for uuid. |
rich7420
left a comment
There was a problem hiding this comment.
LGTM! thanks for the update!
What changes were proposed in this pull request?
ozone admin datanode diskbalancer status command fails when passing DataNode ID instead of address
Minor (Usability / CLI Consistency)
The ozone admin datanode diskbalancer status command currently only accepts a DataNode's network address/host string. If a user attempts to pass a valid datanodeUuid (DataNode ID), the command fails or throws an unsupported/invalid argument error.
For better usability and alignment with other ozone admin commands (which typically support both UUIDs and addresses to identify cluster nodes), this command should be updated to resolve and accept the DataNode ID.
DiskBalancer output:-
PR Changes:
hostname/ipAddressor--in-service-datanodeis given then datanode is displayed inhostname(ipAddpress:port)formatelse when uuid is given it simply shows uuid at datanode place.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-15690
How was this patch tested?
Added unit tests.