-
-
Notifications
You must be signed in to change notification settings - Fork 15
feat(stackable-operator): Add headless/metrics service name functions to RoleGroupRef #1069
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
/// Set of functions to define service names on rolegroup level. | ||
/// Headless service for cluster internal purposes only. |
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.
suggestion: Reword the doc comments.
/// Set of functions to define service names on rolegroup level. | |
/// Headless service for cluster internal purposes only. | |
/// Returns the service name used by rolegroups for cluster internal communication only. | |
/// | |
/// The internal use of of this service name is indicated by the `-headless` suffix. | |
/// This service should not be used for communication to external services or clients | |
/// and also should not be used to export metrics (like Prometheus). Metrics should be | |
/// instead exposed via a dedicated service. Use [`Self::rolegroup_metrics_service_name`] | |
/// instead. |
format!("{name}-headless", name = self.object_name()) | ||
} | ||
|
||
/// Headless metrics service exposes Prometheus endpoint only |
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.
suggestion: Reword the doc comment.
/// Headless metrics service exposes Prometheus endpoint only | |
/// Returns the service name used by rolegroups to expose metrics (like Prometheus). | |
/// | |
/// The use for metrics only is indicated by the `-metrics` suffix. This service | |
/// should not be used for any internal communication or any other external | |
/// communication. For internal communication, use [`Self::rolegroup_headless_service_name`] | |
/// instead. |
} | ||
|
||
/// Headless metrics service exposes Prometheus endpoint only | ||
pub fn rolegroup_headless_metrics_service_name(&self) -> String { |
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.
note: I feel like we should rename that function to rolegroup_metrics_service_name
instead.
pub fn rolegroup_headless_metrics_service_name(&self) -> String { | |
pub fn rolegroup_metrics_service_name(&self) -> String { |
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.
Also, please add a changelog entry for these new functions.
Description
This adds standardised functions to obey https://github.com/stackabletech/decisions/issues/54 consistently across SDP
Definition of Done Checklist
Author
Reviewer
Acceptance