-
-
Notifications
You must be signed in to change notification settings - Fork 1
feat: Hive listener integration #605
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
|
I might have run into stackabletech/hdfs-operator#686 during development. I recognized that I can have an empty string in my discovery configMap from time to time. Behaviour appears to be flaky, but yet more often then not the emtpy string appears: Expected
Flaky faulty one
|
…tackabletech/hive-operator into feat/hive-listener-integration
🟢
|
@@ -737,7 +770,7 @@ fn build_rolegroup_service( | |||
Ok(Service { | |||
metadata: ObjectMetaBuilder::new() | |||
.name_and_namespace(hive) | |||
.name(rolegroup.object_name()) | |||
.name(format!("{name}-metrics", name = rolegroup.object_name())) |
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.
This is used multiple times, can we extract the name creation into a method?
@@ -10,6 +10,7 @@ All notable changes to this project will be documented in this file. | |||
- Use `--file-log-max-files` (or `FILE_LOG_MAX_FILES`) to limit the number of log files kept. | |||
- Use `--file-log-rotation-period` (or `FILE_LOG_ROTATION_PERIOD`) to configure the frequency of rotation. | |||
- Use `--console-log-format` (or `CONSOLE_LOG_FORMAT`) to set the format to `plain` (default) or `json`. | |||
- BREAKING: Added listener support for Hive ([#605]) |
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.
- BREAKING: Added listener support for Hive ([#605]) | |
- BREAKING: Added Listener support for Hive ([#605]) |
@@ -1093,8 +1145,12 @@ fn build_metastore_rolegroup_statefulset( | |||
), | |||
..LabelSelector::default() | |||
}, | |||
service_name: Some(rolegroup_ref.object_name()), | |||
service_name: Some(format!( | |||
"{name}-metrics", |
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.
hint: use method here
NoServicePort { | ||
port_name: String, | ||
obj_ref: ObjectRef<Service>, | ||
//obj_ref: ObjectRef<Service>, |
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.
Remove or use the rolegroup provided in the build_listener_connection_string
method.
kube::{Resource, runtime::reflector::ObjectRef}, | ||
}; | ||
|
||
use crate::{ | ||
controller::build_recommended_labels, | ||
crd::{HIVE_PORT, HIVE_PORT_NAME, HiveRole, ServiceType, v1alpha1}, | ||
crd::{HIVE_PORT_NAME, HiveRole, v1alpha1}, | ||
}; | ||
|
||
#[derive(Snafu, Debug)] |
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.
There are a couple of unused errors now, e.g. NoName
or NoNamespace
.
@@ -105,6 +105,10 @@ tests: | |||
dimensions: | |||
- hive-latest | |||
- openshift | |||
- name: external-access | |||
dimensions: | |||
- hive-latest |
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.
Should be tested for all versions?
{% if lookup('env', 'VECTOR_AGGREGATOR') %} | ||
vectorAggregatorConfigMapName: vector-aggregator-discovery | ||
{% endif %} |
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.
status: | ||
readyReplicas: 2 | ||
replicas: 2 | ||
|
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 role_svc_name = hive | ||
.metastore_role_service_name() | ||
.context(GlobalServiceNameNotFoundSnafu)?; |
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.
Error GlobalServiceNameNotFound
not used anymore.
chroot: Option<&str>, | ||
listener_refs: BTreeMap<&String, Listener>, |
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.
We should explicitly mention this in the release notes / breaking changes that the discovery key changed. Or we keep the HIVE
key for backwards compatibility?
Trino wont work anymore etc.
Description
Adds listener Support
Definition of Done Checklist
Author
Reviewer
Acceptance