Skip to content

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions crates/stackable-operator/src/role_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,17 @@ impl<K: Resource> RoleGroupRef<K> {
pub fn object_name(&self) -> String {
format!("{}-{}-{}", self.cluster.name, self.role, self.role_group)
}

/// Set of functions to define service names on rolegroup level.
/// Headless service for cluster internal purposes only.
Comment on lines +476 to +477
Copy link
Member

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.

Suggested change
/// 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.

pub fn rolegroup_headless_service_name(&self) -> String {
format!("{name}-headless", name = self.object_name())
}

/// Headless metrics service exposes Prometheus endpoint only
Copy link
Member

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.

Suggested change
/// 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.

pub fn rolegroup_headless_metrics_service_name(&self) -> String {
Copy link
Member

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.

Suggested change
pub fn rolegroup_headless_metrics_service_name(&self) -> String {
pub fn rolegroup_metrics_service_name(&self) -> String {

format!("{name}-metrics", name = self.object_name())
}
}

impl<K: Resource> Display for RoleGroupRef<K> {
Expand Down