-
-
Notifications
You must be signed in to change notification settings - Fork 4
fix: rename hbase-site.xml keys for restserver advertisement #716
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
razvan
reviewed
Nov 18, 2025
adwk67
reviewed
Nov 18, 2025
Co-authored-by: Andrew Kenworthy <1712947+adwk67@users.noreply.github.com>
1 task
1 task
razvan
previously approved these changes
Nov 20, 2025
Member
razvan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one suggestion otherwise it lgtm.
Co-authored-by: Razvan-Daniel Mihai <84674+razvan@users.noreply.github.com>
razvan
approved these changes
Nov 20, 2025
Member
Author
Release NoteAdd |
20 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
release-note
Denotes a PR that will be considered when it comes time to generate release notes.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Follow up to #708
The previous usage of
hbase.rest.portby HBase defaulted to port8080, this is the correct port HBase restserver should listen on internally on the container/Pod. By overriding it in #708 the restserver was listening on the port that was provided by the listener. But this new port should be used for external access not internally, so we can't use thehbase.rest.portkey to advertise the restserver to the outside.This approach was possible in
masterandregionserverroles, because those two had a patch https://github.com/stackabletech/docker-images/blob/main/hbase/hbase/stackable/patches/2.6.3/0005-Allow-overriding-ipc-bind-port-and-use-alternative-p.patch changing the behavior of thehbase.[master/regionserver].portkeys, which are now used for advertising andhbase.[master/regionserver].ipc.portfor internal port listening.Using the same approach for restserver would require adding a similar patch for the restserver as well. @adwk67 and me discussed that it wouldn't make too much sense to put that much work into it before discussing Discovery 2.0, which might change the whole approach anyway.
Instead this PR changes the previously added
hbase-site.xmlkeys so a singlehbase.rest.endpointone, providing the full advertised restserver address and not touchhbase.rest.portdefault behavior.Integrationtests passed
🟢 https://testing.stackable.tech/view/02%20Operator%20Tests%20(custom)/job/hbase-operator-it-custom/72/
Definition of Done Checklist
Author
Reviewer
Acceptance
type/deprecationlabel & add to the deprecation scheduletype/experimentallabel & add to the experimental features tracker