-
-
Notifications
You must be signed in to change notification settings - Fork 4
feat: HBase Listener integration #639
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
base: main
Are you sure you want to change the base?
Conversation
Can you please run |
…g through as as argument
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.
Tests / Demos look good so far!
Co-authored-by: Malte Sander <[email protected]>
Co-authored-by: Malte Sander <[email protected]>
🟢 Re-run local tests Tests
|
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.
LGTM! Will wait with approval for on stackabletech/docker-images#1159
🟢 Re-run local tests (changed to use the 2.6.1 image) Tests
|
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.
(Not a full review just something that stood out to me.)
template: pod_template, | ||
volume_claim_templates: Some(vec![pvc]), |
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.
Sorry, missed this until now!
Master and regionservers should probably use ephemeral listener volumes instead (Pod.spec.volumes[].ephemeral.volumeClaimTemplate
), since clients don't actually care about address persistence (they just pull the latest address from ZooKeeper!).
@@ -929,6 +981,15 @@ fn build_rolegroup_statefulset( | |||
.service_account_name(service_account.name_any()) | |||
.security_context(PodSecurityContextBuilder::new().fs_group(1000).build()); | |||
|
|||
// externally-reachable listener endpoints should use a pvc volume... | |||
let pvc = ListenerOperatorVolumeSourceBuilder::new( |
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.
let pvc = ListenerOperatorVolumeSourceBuilder::new( | |
let listener_pvc = ListenerOperatorVolumeSourceBuilder::new( |
Description
fixes: #618
Warning
Merge this at the same time as stackabletech/docker-images#1159, otherwise tests will break!
How to test
bake -p hbase
from the branch for this PRkind load docker-image oci.stackable.tech/sdp/hbase:2.6.1-stackable0.0.0-dev --name stackable-data-platform
stackable op in hbase=0.0.0-pr639
./scripts/run-tests --skip-operator hbase --test ...
Implementation notes
Output from the HBase demo:
The Localities indicate that the serving region is the same node as the HDFS data:
CRD change
Note
There does not need to be a decision on this (discussed briefly in planning 09.04.2024) as the implementation is in line with existing implementations for HDFs and Kafka.
old:
new:
(this is inline with the implementations for HDFS and Kafka)
Tests (OpenShift)
Definition of Done Checklist
Author
Reviewer
Acceptance