use role level listener

Author: maltesander

deploy/helm/trino-operator/crds/crds.yaml

@@ -190,10 +190,6 @@ spec:
                           description: Time period Pods have to gracefully shut down, e.g. `30m`, `1h` or `2d`. Consult the operator documentation for details.
                           nullable: true
                           type: string
-                        listenerClass:
-                          description: This field controls which [ListenerClass](https://docs.stackable.tech/home/nightly/listener-operator/listenerclass.html) is used to expose the coordinator.
-                          nullable: true
-                          type: string
                         logging:
                           default:
                             containers: {}
@@ -423,11 +419,15 @@ spec:
                       x-kubernetes-preserve-unknown-fields: true
                     roleConfig:
                       default:
+                        listenerClass: cluster-internal
                         podDisruptionBudget:
                           enabled: true
                           maxUnavailable: null
                       description: This is a product-agnostic RoleConfig, which is sufficient for most of the products.
                       properties:
+                        listenerClass:
+                          default: cluster-internal
+                          type: string
                         podDisruptionBudget:
                           default:
                             enabled: true
@@ -496,10 +496,6 @@ spec:
                                 description: Time period Pods have to gracefully shut down, e.g. `30m`, `1h` or `2d`. Consult the operator documentation for details.
                                 nullable: true
                                 type: string
-                              listenerClass:
-                                description: This field controls which [ListenerClass](https://docs.stackable.tech/home/nightly/listener-operator/listenerclass.html) is used to expose the coordinator.
-                                nullable: true
-                                type: string
                               logging:
                                 default:
                                   containers: {}

rust/operator-binary/src/controller.rs

@@ -92,7 +92,7 @@ use crate::{
     },
     listener::{
         LISTENER_VOLUME_DIR, LISTENER_VOLUME_NAME, build_group_listener, build_group_listener_pvc,
-        group_listener_name, merged_listener_class,
+        group_listener_name,
     },
     operations::{
         add_graceful_shutdown_config, graceful_shutdown_config_properties, pdb::add_pdbs,
@@ -543,34 +543,26 @@ pub async fn reconcile_trino(
                         rolegroup: role_group_ref.clone(),
                     })?,
             );
+        }
 
-            if let Some(listener_class) =
-                merged_listener_class(trino, &trino_role, &role_group_ref.role_group)
-            {
-                if let Some(listener_group_name) = group_listener_name(&trino_role, &role_group_ref)
-                {
-                    let role_group_listener = build_group_listener(
-                        trino,
-                        build_recommended_labels(
-                            trino,
-                            CONTROLLER_NAME,
-                            &role_group_ref.role,
-                            &role_group_ref.role_group,
-                        ),
-                        listener_class.to_string(),
-                        listener_group_name,
-                    )
-                    .context(ListenerConfigurationSnafu)?;
+        if let Some(listener_class) = trino_role.listener_class_name(trino) {
+            if let Some(listener_group_name) = group_listener_name(trino, &trino_role) {
+                let role_group_listener = build_group_listener(
+                    trino,
+                    build_recommended_labels(trino, CONTROLLER_NAME, &trino_role_str, "none"),
+                    listener_class.to_string(),
+                    listener_group_name,
+                )
+                .context(ListenerConfigurationSnafu)?;
 
-                    cluster_resources
-                        .add(client, role_group_listener)
-                        .await
-                        .context(ApplyGroupListenerSnafu)?;
-                }
+                cluster_resources
+                    .add(client, role_group_listener)
+                    .await
+                    .context(ApplyGroupListenerSnafu)?;
             }
         }
 
-        let role_config = trino.role_config(&trino_role);
+        let role_config = trino.generic_role_config(&trino_role);
         if let Some(GenericRoleConfig {
             pod_disruption_budget: pdb,
         }) = role_config
@@ -998,7 +990,7 @@ fn build_rolegroup_statefulset(
             .build_pvc("data", Some(vec!["ReadWriteOnce"])),
     ];
     // Add listener
-    if let Some(group_listener_name) = group_listener_name(trino_role, role_group_ref) {
+    if let Some(group_listener_name) = group_listener_name(trino, trino_role) {
         cb_trino
             .add_volume_mount(LISTENER_VOLUME_NAME, LISTENER_VOLUME_DIR)
             .context(AddVolumeMountSnafu)?;
@@ -1009,7 +1001,7 @@ fn build_rolegroup_statefulset(
             // A version value is required, and we do want to use the "recommended" format for the other desired labels
             "none",
             &role_group_ref.role,
-            &role_group_ref.role_group,
+            "none",
         ))
         .context(LabelBuildSnafu)?;
 
@@ -1196,7 +1188,7 @@ fn build_rolegroup_statefulset(
                 ),
                 ..LabelSelector::default()
             },
-            service_name: Some(v1alpha1::TrinoCluster::rolegroup_headless_service_name(
+            service_name: Some(v1alpha1::TrinoCluster::rolegroup_metrics_service_name(
                 &role_group_ref.object_name(),
             )),
             template: pod_template,
@@ -1218,7 +1210,7 @@ fn build_rolegroup_service(
     Ok(Service {
         metadata: ObjectMetaBuilder::new()
             .name_and_namespace(trino)
-            .name(v1alpha1::TrinoCluster::rolegroup_headless_service_name(
+            .name(v1alpha1::TrinoCluster::rolegroup_metrics_service_name(
                 &role_group_ref.object_name(),
             ))
             .ownerreference_from_resource(trino, None, Some(true))
@@ -1236,7 +1228,7 @@ fn build_rolegroup_service(
             // Internal communication does not need to be exposed
             type_: Some("ClusterIP".to_string()),
             cluster_ip: Some("None".to_string()),
-            ports: Some(service_ports(trino)),
+            ports: Some(service_ports()),
             selector: Some(
                 Labels::role_group_selector(
                     trino,
@@ -1412,33 +1404,13 @@ fn get_random_base64() -> String {
     openssl::base64::encode_block(&buf)
 }
 
-fn service_ports(trino: &v1alpha1::TrinoCluster) -> Vec<ServicePort> {
-    let mut ports = vec![ServicePort {
+fn service_ports() -> Vec<ServicePort> {
+    vec![ServicePort {
         name: Some(METRICS_PORT_NAME.to_string()),
         port: METRICS_PORT.into(),
         protocol: Some("TCP".to_string()),
         ..ServicePort::default()
-    }];
-
-    if trino.expose_http_port() {
-        ports.push(ServicePort {
-            name: Some(HTTP_PORT_NAME.to_string()),
-            port: HTTP_PORT.into(),
-            protocol: Some("TCP".to_string()),
-            ..ServicePort::default()
-        });
-    }
-
-    if trino.expose_https_port() {
-        ports.push(ServicePort {
-            name: Some(HTTPS_PORT_NAME.to_string()),
-            port: HTTPS_PORT.into(),
-            protocol: Some("TCP".to_string()),
-            ..ServicePort::default()
-        });
-    }
-
-    ports
+    }]
 }
 
 fn container_ports(trino: &v1alpha1::TrinoCluster) -> Vec<ContainerPort> {

rust/operator-binary/src/crd/mod.rs

@@ -39,9 +39,9 @@ use stackable_operator::{
     versioned::versioned,
 };
 use strum::{Display, EnumIter, EnumString, IntoEnumIterator};
-use v1alpha1::{TrinoConfigFragment, TrinoCoordinatorConfigFragment};
+use v1alpha1::TrinoConfigFragment;
 
-use crate::crd::discovery::TrinoPodRef;
+use crate::crd::{discovery::TrinoPodRef, v1alpha1::TrinoCoordinatorRoleConfig};
 
 pub const APP_NAME: &str = "trino";
 // ports
@@ -117,10 +117,8 @@ pub const MAX_TRINO_LOG_FILES_SIZE: MemoryQuantity = MemoryQuantity {
     value: 10.0,
     unit: BinaryMultiple::Mebi,
 };
-// Headless service suffix
-// TODO(malte): Imho this should be "headless". Its metrics for consistency for now.
-// See: https://github.com/stackabletech/decisions/issues/54
-pub const HEADLESS_SERVICE_SUFFIX: &str = "metrics";
+
+pub const METRICS_SERVICE_SUFFIX: &str = "metrics";
 
 pub const JVM_HEAP_FACTOR: f32 = 0.8;
 
@@ -197,13 +195,24 @@ pub mod versioned {
         // no doc - it's in the struct.
         #[serde(default, skip_serializing_if = "Option::is_none")]
         pub coordinators:
-            Option<Role<TrinoCoordinatorConfigFragment, GenericRoleConfig, JavaCommonConfig>>,
+            Option<Role<TrinoConfigFragment, TrinoCoordinatorRoleConfig, JavaCommonConfig>>,
 
         // no doc - it's in the struct.
         #[serde(default, skip_serializing_if = "Option::is_none")]
         pub workers: Option<Role<TrinoConfigFragment, GenericRoleConfig, JavaCommonConfig>>,
     }
 
+    // TODO: move generic version to op-rs?
+    #[derive(Clone, Debug, Deserialize, JsonSchema, PartialEq, Serialize)]
+    #[serde(rename_all = "camelCase")]
+    pub struct TrinoCoordinatorRoleConfig {
+        #[serde(flatten)]
+        pub common: GenericRoleConfig,
+
+        #[serde(default = "coordinator_default_listener_class")]
+        pub listener_class: String,
+    }
+
     #[derive(Clone, Debug, Default, Fragment, JsonSchema, PartialEq)]
     #[fragment_attrs(
         derive(
@@ -242,29 +251,6 @@ pub mod versioned {
         pub requested_secret_lifetime: Option<Duration>,
     }
 
-    #[derive(Clone, Debug, Default, Fragment, JsonSchema, PartialEq)]
-    #[fragment_attrs(
-        derive(
-            Clone,
-            Debug,
-            Default,
-            Deserialize,
-            Merge,
-            JsonSchema,
-            PartialEq,
-            Serialize
-        ),
-        serde(rename_all = "camelCase")
-    )]
-    pub struct TrinoCoordinatorConfig {
-        #[fragment_attrs(serde(default, flatten))]
-        pub trino_config: TrinoConfig,
-
-        /// This field controls which [ListenerClass](DOCS_BASE_URL_PLACEHOLDER/listener-operator/listenerclass.html) is used to expose the coordinator.
-        #[serde(default)]
-        pub listener_class: String,
-    }
-
     #[derive(Clone, Debug, Deserialize, JsonSchema, PartialEq, Serialize)]
     #[serde(rename_all = "camelCase")]
     pub struct TrinoClusterConfig {
@@ -352,6 +338,19 @@ pub mod versioned {
     }
 }
 
+impl Default for v1alpha1::TrinoCoordinatorRoleConfig {
+    fn default() -> Self {
+        v1alpha1::TrinoCoordinatorRoleConfig {
+            listener_class: coordinator_default_listener_class(),
+            common: Default::default(),
+        }
+    }
+}
+
+fn coordinator_default_listener_class() -> String {
+    "cluster-internal".to_string()
+}
+
 impl Default for v1alpha1::TrinoTls {
     fn default() -> Self {
         v1alpha1::TrinoTls {
@@ -415,6 +414,17 @@ impl TrinoRole {
         }
         roles
     }
+
+    pub fn listener_class_name(&self, trino: &v1alpha1::TrinoCluster) -> Option<String> {
+        match self {
+            Self::Coordinator => trino
+                .spec
+                .coordinators
+                .to_owned()
+                .map(|coordinator| coordinator.role_config.listener_class),
+            Self::Worker => None,
+        }
+    }
 }
 
 #[derive(
@@ -764,9 +774,13 @@ impl v1alpha1::TrinoCluster {
             })
     }
 
-    pub fn role_config(&self, role: &TrinoRole) -> Option<&GenericRoleConfig> {
+    pub fn generic_role_config(&self, role: &TrinoRole) -> Option<&GenericRoleConfig> {
         match role {
-            TrinoRole::Coordinator => self.spec.coordinators.as_ref().map(|c| &c.role_config),
+            TrinoRole::Coordinator => self
+                .spec
+                .coordinators
+                .as_ref()
+                .map(|c| &c.role_config.common),
             TrinoRole::Worker => self.spec.workers.as_ref().map(|w| &w.role_config),
         }
     }
@@ -822,7 +836,7 @@ impl v1alpha1::TrinoCluster {
                 let ns = ns.clone();
                 (0..rolegroup.replicas.unwrap_or(0)).map(move |i| TrinoPodRef {
                     namespace: ns.clone(),
-                    role_group_service_name: Self::rolegroup_headless_service_name(
+                    role_group_service_name: Self::rolegroup_metrics_service_name(
                         &role_group_ref.object_name(),
                     ),
                     pod_name: format!("{}-{}", role_group_ref.object_name(), i),
@@ -831,8 +845,8 @@ impl v1alpha1::TrinoCluster {
     }
 
     /// Returns the headless rolegroup service name `simple-trino-coordinator-default-<HEADLESS_SERVICE_SUFFIX>`.
-    pub fn rolegroup_headless_service_name(role_group_ref_object_name: &str) -> String {
-        format!("{}-{}", role_group_ref_object_name, HEADLESS_SERVICE_SUFFIX)
+    pub fn rolegroup_metrics_service_name(role_group_ref_object_name: &str) -> String {
+        format!("{}-{}", role_group_ref_object_name, METRICS_SERVICE_SUFFIX)
     }
 
     /// Returns user provided authentication settings
@@ -928,18 +942,18 @@ impl v1alpha1::TrinoCluster {
 }
 
 fn extract_role_from_coordinator_config(
-    fragment: Role<TrinoCoordinatorConfigFragment, GenericRoleConfig, JavaCommonConfig>,
+    fragment: Role<TrinoConfigFragment, TrinoCoordinatorRoleConfig, JavaCommonConfig>,
 ) -> Role<TrinoConfigFragment, GenericRoleConfig, JavaCommonConfig> {
     Role {
         config: CommonConfiguration {
-            config: fragment.config.config.trino_config,
+            config: fragment.config.config,
             config_overrides: fragment.config.config_overrides,
             env_overrides: fragment.config.env_overrides,
             cli_overrides: fragment.config.cli_overrides,
             pod_overrides: fragment.config.pod_overrides,
             product_specific_common_config: fragment.config.product_specific_common_config,
         },
-        role_config: fragment.role_config,
+        role_config: fragment.role_config.common,
         role_groups: fragment
             .role_groups
             .into_iter()
@@ -948,7 +962,7 @@ fn extract_role_from_coordinator_config(
                     k,
                     RoleGroup {
                         config: CommonConfiguration {
-                            config: v.config.config.trino_config,
+                            config: v.config.config,
                             config_overrides: v.config.config_overrides,
                             env_overrides: v.config.env_overrides,
                             cli_overrides: v.config.cli_overrides,