Skip to content

HDDS-15677. Update client side version checks to use isSupportedBy.#10611

Open
errose28 wants to merge 4 commits into
apache:HDDS-14496-zdufrom
errose28:om-version-client-cleanup
Open

HDDS-15677. Update client side version checks to use isSupportedBy.#10611
errose28 wants to merge 4 commits into
apache:HDDS-14496-zdufrom
errose28:om-version-client-cleanup

Conversation

@errose28

@errose28 errose28 commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Replace client side version comparison that was previously using compareTo with the new and more readable isSupportedBy. No functional change is expected. compareTo cannot be removed since it is built into the enum, and it provides correct results so there is no harm other than readability.

What is the link to the Apache JIRA

HDDS-15677

How was this patch tested?

New coverage was added for the client side version parsing which was previously missing. These tests were generated by Claude Code and reviewed by me.

Green run on my fork

@errose28 errose28 added the zdu Pull requests for Zero Downtime Upgrade (ZDU) https://issues.apache.org/jira/browse/HDDS-14496 label Jun 25, 2026
@errose28 errose28 requested review from dombizita and sodonnel June 25, 2026 21:31
@errose28 errose28 marked this pull request as ready for review June 26, 2026 17:43
@errose28

Copy link
Copy Markdown
Contributor Author

Compat test failure isn't related to this PR, despite the suite it occurred in:

ERROR: Test execution of xcompat/test-old.sh is FAILED!!!!
Unable to find image 'ghcr.io/apache/ozone-runner:20260206-2-jdk21' locally

@ayushtkn ayushtkn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

Comment on lines +297 to +300
return node(HddsProtos.NodeType.OM, version);
}

private static ServiceInfo node(HddsProtos.NodeType nodeType, OzoneManagerVersion version) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this method is called only by

 private static ServiceInfo om(OzoneManagerVersion version) {

maybe we could have implemented there itself rather than having two methods

@dombizita dombizita left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for this change @errose28, one minor logging comment.

}
LOG.trace("Ozone Manager version is {}", version.name());
return version;
LOG.trace("Ozone Manager version is {}", minOMVersion);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Isn't that missing?

Suggested change
LOG.trace("Ozone Manager version is {}", minOMVersion);
LOG.trace("Ozone Manager version is {}", minOMVersion.name());

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

zdu Pull requests for Zero Downtime Upgrade (ZDU) https://issues.apache.org/jira/browse/HDDS-14496

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants