From 6b917c723ad91d4389877a8f648fd5f00dccdf46 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Fri, 30 May 2025 11:12:15 +0200 Subject: [PATCH 01/38] add listener to roles --- deploy/helm/trino-operator/templates/roles.yaml | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/deploy/helm/trino-operator/templates/roles.yaml b/deploy/helm/trino-operator/templates/roles.yaml index 7586c26e..4a268c3a 100644 --- a/deploy/helm/trino-operator/templates/roles.yaml +++ b/deploy/helm/trino-operator/templates/roles.yaml @@ -97,11 +97,16 @@ rules: verbs: - get - apiGroups: - - apiextensions.k8s.io + - listeners.stackable.tech resources: - - customresourcedefinitions + - listeners verbs: - get + - list + - watch + - patch + - create + - delete - apiGroups: - events.k8s.io resources: From 1691220189bf22fbf07f556cb3307502ec0d0c81 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Mon, 2 Jun 2025 14:04:44 +0200 Subject: [PATCH 02/38] wip - adapt role and rolegroup access --- rust/operator-binary/src/controller.rs | 78 ++++------------ rust/operator-binary/src/crd/mod.rs | 123 ++++++++++++++++--------- 2 files changed, 93 insertions(+), 108 deletions(-) diff --git a/rust/operator-binary/src/controller.rs b/rust/operator-binary/src/controller.rs index 212fb7e9..20c594de 100644 --- a/rust/operator-binary/src/controller.rs +++ b/rust/operator-binary/src/controller.rs @@ -127,7 +127,10 @@ pub enum Error { ObjectHasNoNamespace, #[snafu(display("object defines no {role:?} role"))] - MissingTrinoRole { role: String }, + MissingTrinoRole { + source: crate::crd::Error, + role: String, + }, #[snafu(display("failed to create cluster resources"))] CreateClusterResources { @@ -459,21 +462,13 @@ pub async fn reconcile_trino( None => None, }; - let coordinator_role_service = build_coordinator_role_service(trino, &resolved_product_image)?; - - cluster_resources - .add(client, coordinator_role_service) - .await - .context(ApplyRoleServiceSnafu)?; - create_shared_internal_secret(trino, client).await?; let mut sts_cond_builder = StatefulSetConditionBuilder::default(); for (trino_role_str, role_config) in validated_config { let trino_role = TrinoRole::from_str(&trino_role_str).context(FailedToParseRoleSnafu)?; - let role: &Role = - trino.role(&trino_role).context(ReadRoleSnafu)?; + let role = trino.role(&trino_role).context(ReadRoleSnafu)?; for (role_group, config) in role_config { let rolegroup = trino_role.rolegroup_ref(trino, &role_group); @@ -485,7 +480,7 @@ pub async fn reconcile_trino( let rg_configmap = build_rolegroup_config_map( trino, &resolved_product_image, - role, + &role, &trino_role, &rolegroup, &config, @@ -576,45 +571,6 @@ pub async fn reconcile_trino( Ok(Action::await_change()) } -/// The coordinator-role service is the primary endpoint that should be used by clients that do not -/// perform internal load balancing, including targets outside of the cluster. -pub fn build_coordinator_role_service( - trino: &v1alpha1::TrinoCluster, - resolved_product_image: &ResolvedProductImage, -) -> Result { - let role = TrinoRole::Coordinator; - let role_name = role.to_string(); - let role_svc_name = trino - .role_service_name(&role) - .context(InternalOperatorFailureSnafu)?; - Ok(Service { - metadata: ObjectMetaBuilder::new() - .name_and_namespace(trino) - .name(&role_svc_name) - .ownerreference_from_resource(trino, None, Some(true)) - .context(ObjectMissingMetadataForOwnerRefSnafu)? - .with_recommended_labels(build_recommended_labels( - trino, - &resolved_product_image.app_version_label, - &role_name, - "global", - )) - .context(MetadataBuildSnafu)? - .build(), - spec: Some(ServiceSpec { - ports: Some(service_ports(trino)), - selector: Some( - Labels::role_selector(trino, APP_NAME, &role_name) - .context(LabelBuildSnafu)? - .into(), - ), - type_: Some(trino.spec.cluster_config.listener_class.k8s_service_type()), - ..ServiceSpec::default() - }), - status: None, - }) -} - /// The rolegroup [`ConfigMap`] configures the rolegroup based on the configuration given by the administrator #[allow(clippy::too_many_arguments)] fn build_rolegroup_config_map( @@ -1298,30 +1254,28 @@ fn validated_product_config( PropertyNameKind::File(ACCESS_CONTROL_PROPERTIES.to_string()), ]; + let coordinator_role = TrinoRole::Coordinator; roles.insert( - TrinoRole::Coordinator.to_string(), + coordinator_role.to_string(), ( config_files.clone(), trino - .spec - .coordinators - .clone() - .with_context(|| MissingTrinoRoleSnafu { - role: TrinoRole::Coordinator.to_string(), + .role(&coordinator_role) + .with_context(|_| MissingTrinoRoleSnafu { + role: coordinator_role.to_string(), })?, ), ); + let worker_role = TrinoRole::Worker; roles.insert( - TrinoRole::Worker.to_string(), + worker_role.to_string(), ( config_files, trino - .spec - .workers - .clone() - .with_context(|| MissingTrinoRoleSnafu { - role: TrinoRole::Worker.to_string(), + .role(&worker_role) + .with_context(|_| MissingTrinoRoleSnafu { + role: worker_role.to_string(), })?, ), ); diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs index 2465b086..4778051a 100644 --- a/rust/operator-binary/src/crd/mod.rs +++ b/rust/operator-binary/src/crd/mod.rs @@ -29,7 +29,9 @@ use stackable_operator::{ memory::{BinaryMultiple, MemoryQuantity}, product_config_utils::{Configuration, Error as ConfigError}, product_logging::{self, spec::Logging}, - role_utils::{GenericRoleConfig, JavaCommonConfig, Role, RoleGroup, RoleGroupRef}, + role_utils::{ + CommonConfiguration, GenericRoleConfig, JavaCommonConfig, Role, RoleGroup, RoleGroupRef, + }, schemars::{self, JsonSchema}, status::condition::{ClusterCondition, HasStatusCondition}, time::Duration, @@ -37,6 +39,7 @@ use stackable_operator::{ versioned::versioned, }; use strum::{Display, EnumIter, EnumString, IntoEnumIterator}; +use v1alpha1::{TrinoConfigFragment, TrinoCoordinatorConfigFragment}; use crate::crd::discovery::TrinoPodRef; @@ -189,7 +192,8 @@ pub mod versioned { // no doc - it's in the struct. #[serde(default, skip_serializing_if = "Option::is_none")] - pub coordinators: Option>, + pub coordinators: + Option>, // no doc - it's in the struct. #[serde(default, skip_serializing_if = "Option::is_none")] @@ -234,6 +238,29 @@ pub mod versioned { pub requested_secret_lifetime: Option, } + #[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 { @@ -261,20 +288,6 @@ pub mod versioned { /// to learn how to configure log aggregation with Vector. #[serde(skip_serializing_if = "Option::is_none")] pub vector_aggregator_config_map_name: Option, - - /// This field controls which type of Service the Operator creates for this TrinoCluster: - /// - /// * cluster-internal: Use a ClusterIP service - /// - /// * external-unstable: Use a NodePort service - /// - /// * external-stable: Use a LoadBalancer service - /// - /// This is a temporary solution with the goal to keep yaml manifests forward compatible. - /// In the future, this setting will control which [ListenerClass](DOCS_BASE_URL_PLACEHOLDER/listener-operator/listenerclass.html) - /// will be used to expose the service, and ListenerClass names will stay the same, allowing for a non-breaking change. - #[serde(default)] - pub listener_class: CurrentlySupportedListenerClasses, } #[derive(Clone, Debug, Deserialize, Eq, JsonSchema, PartialEq, Serialize)] @@ -335,29 +348,6 @@ pub mod versioned { } } -// TODO: Temporary solution until listener-operator is finished -#[derive(Clone, Debug, Default, Display, Deserialize, Eq, JsonSchema, PartialEq, Serialize)] -#[serde(rename_all = "PascalCase")] -pub enum CurrentlySupportedListenerClasses { - #[default] - #[serde(rename = "cluster-internal")] - ClusterInternal, - #[serde(rename = "external-unstable")] - ExternalUnstable, - #[serde(rename = "external-stable")] - ExternalStable, -} - -impl CurrentlySupportedListenerClasses { - pub fn k8s_service_type(&self) -> String { - match self { - CurrentlySupportedListenerClasses::ClusterInternal => "ClusterIP".to_string(), - CurrentlySupportedListenerClasses::ExternalUnstable => "NodePort".to_string(), - CurrentlySupportedListenerClasses::ExternalStable => "LoadBalancer".to_string(), - } - } -} - impl Default for v1alpha1::TrinoTls { fn default() -> Self { v1alpha1::TrinoTls { @@ -733,11 +723,15 @@ impl v1alpha1::TrinoCluster { pub fn role( &self, role_variant: &TrinoRole, - ) -> Result<&Role, Error> + ) -> Result, Error> { match role_variant { - TrinoRole::Coordinator => self.spec.coordinators.as_ref(), - TrinoRole::Worker => self.spec.workers.as_ref(), + TrinoRole::Coordinator => self + .spec + .coordinators + .to_owned() + .map(extract_role_from_coordinator_config), + TrinoRole::Worker => self.spec.workers.to_owned(), } .with_context(|| CannotRetrieveTrinoRoleSnafu { role: role_variant.to_string(), @@ -748,15 +742,19 @@ impl v1alpha1::TrinoCluster { pub fn rolegroup( &self, rolegroup_ref: &RoleGroupRef, - ) -> Result<&RoleGroup, Error> { - let role_variant = + ) -> Result, Error> { + let trino_role = TrinoRole::from_str(&rolegroup_ref.role).with_context(|_| UnknownTrinoRoleSnafu { role: rolegroup_ref.role.to_owned(), roles: TrinoRole::roles(), })?; - let role = self.role(&role_variant)?; - role.role_groups + + let role_variant = self.role(&trino_role)?; + + role_variant + .role_groups .get(&rolegroup_ref.role_group) + .cloned() .with_context(|| CannotRetrieveTrinoRoleGroupSnafu { role_group: rolegroup_ref.role_group.to_owned(), }) @@ -918,6 +916,39 @@ impl v1alpha1::TrinoCluster { } } +fn extract_role_from_coordinator_config( + fragment: Role, +) -> Role { + Role { + config: CommonConfiguration { + config: fragment.config.config.trino_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_groups: fragment + .role_groups + .into_iter() + .map(|(k, v)| { + (k, RoleGroup { + config: CommonConfiguration { + config: v.config.config.trino_config, + config_overrides: v.config.config_overrides, + env_overrides: v.config.env_overrides, + cli_overrides: v.config.cli_overrides, + pod_overrides: v.config.pod_overrides, + product_specific_common_config: v.config.product_specific_common_config, + }, + replicas: v.replicas, + }) + }) + .collect(), + } +} + impl HasStatusCondition for v1alpha1::TrinoCluster { fn conditions(&self) -> Vec { match &self.status { From 2dcd4fc68b79e522c092316771dec502ea426648 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Mon, 2 Jun 2025 14:06:21 +0200 Subject: [PATCH 03/38] remove unused error --- rust/operator-binary/src/controller.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/rust/operator-binary/src/controller.rs b/rust/operator-binary/src/controller.rs index 20c594de..c919ad93 100644 --- a/rust/operator-binary/src/controller.rs +++ b/rust/operator-binary/src/controller.rs @@ -341,11 +341,6 @@ pub enum Error { #[snafu(display("failed to read role"))] ReadRole { source: crate::crd::Error }, - #[snafu(display("failed to get merged jvmArgumentOverrides"))] - GetMergedJvmArgumentOverrides { - source: stackable_operator::role_utils::Error, - }, - #[snafu(display("unable to parse Trino version: {product_version:?}"))] ParseTrinoVersion { source: ParseIntError, From f0f4c5e89271214ea5f474c45491a81aa7fd3d27 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Mon, 2 Jun 2025 14:08:38 +0200 Subject: [PATCH 04/38] fmt --- rust/operator-binary/src/config/jvm.rs | 14 +- rust/operator-binary/src/controller.rs | 8 +- rust/operator-binary/src/crd/affinity.rs | 182 ++++++++++-------- .../src/crd/catalog/generic.rs | 15 +- rust/operator-binary/src/crd/mod.rs | 23 ++- rust/operator-binary/src/main.rs | 11 +- 6 files changed, 140 insertions(+), 113 deletions(-) diff --git a/rust/operator-binary/src/config/jvm.rs b/rust/operator-binary/src/config/jvm.rs index 335678e5..e10613ff 100644 --- a/rust/operator-binary/src/config/jvm.rs +++ b/rust/operator-binary/src/config/jvm.rs @@ -186,7 +186,9 @@ mod tests { "#; let jvm_config = construct_jvm_config(input); - assert_eq!(jvm_config, indoc! {" + assert_eq!( + jvm_config, + indoc! {" -server # Heap settings -Xms34406m @@ -201,7 +203,8 @@ mod tests { -Djavax.net.ssl.trustStorePassword=changeit # Recommended JVM arguments from Trino -RecommendedTrinoFlag - # Arguments from jvmArgumentOverrides"}); + # Arguments from jvmArgumentOverrides"} + ); } #[test] @@ -242,7 +245,9 @@ mod tests { "#; let jvm_config = construct_jvm_config(input); - assert_eq!(jvm_config, indoc! {" + assert_eq!( + jvm_config, + indoc! {" -server # Heap settings -Xms34406m @@ -260,7 +265,8 @@ mod tests { -Dhttps.proxyHost=proxy.my.corp -Djava.net.preferIPv4Stack=true -Xmx40000m - -Dhttps.proxyPort=1234"}); + -Dhttps.proxyPort=1234"} + ); } fn construct_jvm_config(trino_cluster: &str) -> String { diff --git a/rust/operator-binary/src/controller.rs b/rust/operator-binary/src/controller.rs index c919ad93..c802db95 100644 --- a/rust/operator-binary/src/controller.rs +++ b/rust/operator-binary/src/controller.rs @@ -548,10 +548,10 @@ pub async fn reconcile_trino( ClusterOperationsConditionBuilder::new(&trino.spec.cluster_operation); let status = v1alpha1::TrinoClusterStatus { - conditions: compute_conditions(trino, &[ - &sts_cond_builder, - &cluster_operation_cond_builder, - ]), + conditions: compute_conditions( + trino, + &[&sts_cond_builder, &cluster_operation_cond_builder], + ), }; cluster_resources diff --git a/rust/operator-binary/src/crd/affinity.rs b/rust/operator-binary/src/crd/affinity.rs index 502b1cac..77e24ff2 100644 --- a/rust/operator-binary/src/crd/affinity.rs +++ b/rust/operator-binary/src/crd/affinity.rs @@ -135,55 +135,61 @@ mod tests { .merged_config(&role, &role.rolegroup_ref(&trino, "default"), &[]) .unwrap(); - assert_eq!(merged_config.affinity, StackableAffinity { - pod_affinity: Some(PodAffinity { - preferred_during_scheduling_ignored_during_execution: Some(vec![ - WeightedPodAffinityTerm { - pod_affinity_term: PodAffinityTerm { - label_selector: Some(LabelSelector { - match_labels: Some(BTreeMap::from([ - ("app.kubernetes.io/name".to_string(), "trino".to_string()), - ( - "app.kubernetes.io/instance".to_string(), - "simple-trino".to_string(), - ), - ])), - ..LabelSelector::default() - }), - topology_key: "kubernetes.io/hostname".to_string(), - ..PodAffinityTerm::default() - }, - weight: 20, - } - ]), - required_during_scheduling_ignored_during_execution: None, - }), - pod_anti_affinity: Some(PodAntiAffinity { - preferred_during_scheduling_ignored_during_execution: Some(vec![ - WeightedPodAffinityTerm { - pod_affinity_term: PodAffinityTerm { - label_selector: Some(LabelSelector { - match_labels: Some(BTreeMap::from([ - ("app.kubernetes.io/name".to_string(), "trino".to_string(),), - ( - "app.kubernetes.io/instance".to_string(), - "simple-trino".to_string(), - ), - ("app.kubernetes.io/component".to_string(), role.to_string(),) - ])), - ..LabelSelector::default() - }), - topology_key: "kubernetes.io/hostname".to_string(), - ..PodAffinityTerm::default() - }, - weight: 70 - } - ]), - required_during_scheduling_ignored_during_execution: None, - }), - node_affinity: None, - node_selector: None, - }); + assert_eq!( + merged_config.affinity, + StackableAffinity { + pod_affinity: Some(PodAffinity { + preferred_during_scheduling_ignored_during_execution: Some(vec![ + WeightedPodAffinityTerm { + pod_affinity_term: PodAffinityTerm { + label_selector: Some(LabelSelector { + match_labels: Some(BTreeMap::from([ + ("app.kubernetes.io/name".to_string(), "trino".to_string()), + ( + "app.kubernetes.io/instance".to_string(), + "simple-trino".to_string(), + ), + ])), + ..LabelSelector::default() + }), + topology_key: "kubernetes.io/hostname".to_string(), + ..PodAffinityTerm::default() + }, + weight: 20, + } + ]), + required_during_scheduling_ignored_during_execution: None, + }), + pod_anti_affinity: Some(PodAntiAffinity { + preferred_during_scheduling_ignored_during_execution: Some(vec![ + WeightedPodAffinityTerm { + pod_affinity_term: PodAffinityTerm { + label_selector: Some(LabelSelector { + match_labels: Some(BTreeMap::from([ + ("app.kubernetes.io/name".to_string(), "trino".to_string(),), + ( + "app.kubernetes.io/instance".to_string(), + "simple-trino".to_string(), + ), + ( + "app.kubernetes.io/component".to_string(), + role.to_string(), + ) + ])), + ..LabelSelector::default() + }), + topology_key: "kubernetes.io/hostname".to_string(), + ..PodAffinityTerm::default() + }, + weight: 70 + } + ]), + required_during_scheduling_ignored_during_execution: None, + }), + node_affinity: None, + node_selector: None, + } + ); } #[rstest] @@ -268,11 +274,11 @@ mod tests { serde_yaml::with::singleton_map_recursive::deserialize(deserializer).unwrap(); let merged_config = trino - .merged_config(&role, &role.rolegroup_ref(&trino, "default"), &[ - hive_catalog_1, - tpch_catalog, - hive_catalog_2, - ]) + .merged_config( + &role, + &role.rolegroup_ref(&trino, "default"), + &[hive_catalog_1, tpch_catalog, hive_catalog_2], + ) .unwrap(); let mut expected_affinities = vec![WeightedPodAffinityTerm { @@ -363,36 +369,42 @@ mod tests { } }; - assert_eq!(merged_config.affinity, StackableAffinity { - pod_affinity: Some(PodAffinity { - preferred_during_scheduling_ignored_during_execution: Some(expected_affinities), - required_during_scheduling_ignored_during_execution: None, - }), - pod_anti_affinity: Some(PodAntiAffinity { - preferred_during_scheduling_ignored_during_execution: Some(vec![ - WeightedPodAffinityTerm { - pod_affinity_term: PodAffinityTerm { - label_selector: Some(LabelSelector { - match_labels: Some(BTreeMap::from([ - ("app.kubernetes.io/name".to_string(), "trino".to_string(),), - ( - "app.kubernetes.io/instance".to_string(), - "simple-trino".to_string(), - ), - ("app.kubernetes.io/component".to_string(), role.to_string(),) - ])), - ..LabelSelector::default() - }), - topology_key: "kubernetes.io/hostname".to_string(), - ..PodAffinityTerm::default() - }, - weight: 70 - } - ]), - required_during_scheduling_ignored_during_execution: None, - }), - node_affinity: None, - node_selector: None, - }); + assert_eq!( + merged_config.affinity, + StackableAffinity { + pod_affinity: Some(PodAffinity { + preferred_during_scheduling_ignored_during_execution: Some(expected_affinities), + required_during_scheduling_ignored_during_execution: None, + }), + pod_anti_affinity: Some(PodAntiAffinity { + preferred_during_scheduling_ignored_during_execution: Some(vec![ + WeightedPodAffinityTerm { + pod_affinity_term: PodAffinityTerm { + label_selector: Some(LabelSelector { + match_labels: Some(BTreeMap::from([ + ("app.kubernetes.io/name".to_string(), "trino".to_string(),), + ( + "app.kubernetes.io/instance".to_string(), + "simple-trino".to_string(), + ), + ( + "app.kubernetes.io/component".to_string(), + role.to_string(), + ) + ])), + ..LabelSelector::default() + }), + topology_key: "kubernetes.io/hostname".to_string(), + ..PodAffinityTerm::default() + }, + weight: 70 + } + ]), + required_during_scheduling_ignored_during_execution: None, + }), + node_affinity: None, + node_selector: None, + } + ); } } diff --git a/rust/operator-binary/src/crd/catalog/generic.rs b/rust/operator-binary/src/crd/catalog/generic.rs index 179a67ed..a83e6c78 100644 --- a/rust/operator-binary/src/crd/catalog/generic.rs +++ b/rust/operator-binary/src/crd/catalog/generic.rs @@ -78,13 +78,16 @@ mod tests { "connection-url".to_string(), Property::Value("jdbc:postgresql://example.net:5432/database".to_string()) ), - ("connection-user".to_string(), Property::ValueFromSecret { - secret_key_selector: SecretKeySelector { - key: "user".to_string(), - name: "my-postgresql-credentials-secret".to_string(), - optional: None, + ( + "connection-user".to_string(), + Property::ValueFromSecret { + secret_key_selector: SecretKeySelector { + key: "user".to_string(), + name: "my-postgresql-credentials-secret".to_string(), + optional: None, + } } - }), + ), ( "connection-password".to_string(), Property::ValueFromSecret { diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs index 4778051a..cfd4521b 100644 --- a/rust/operator-binary/src/crd/mod.rs +++ b/rust/operator-binary/src/crd/mod.rs @@ -933,17 +933,20 @@ fn extract_role_from_coordinator_config( .role_groups .into_iter() .map(|(k, v)| { - (k, RoleGroup { - config: CommonConfiguration { - config: v.config.config.trino_config, - config_overrides: v.config.config_overrides, - env_overrides: v.config.env_overrides, - cli_overrides: v.config.cli_overrides, - pod_overrides: v.config.pod_overrides, - product_specific_common_config: v.config.product_specific_common_config, + ( + k, + RoleGroup { + config: CommonConfiguration { + config: v.config.config.trino_config, + config_overrides: v.config.config_overrides, + env_overrides: v.config.env_overrides, + cli_overrides: v.config.cli_overrides, + pod_overrides: v.config.pod_overrides, + product_specific_common_config: v.config.product_specific_common_config, + }, + replicas: v.replicas, }, - replicas: v.replicas, - }) + ) }) .collect(), } diff --git a/rust/operator-binary/src/main.rs b/rust/operator-binary/src/main.rs index 97e1611b..22bd6b0e 100644 --- a/rust/operator-binary/src/main.rs +++ b/rust/operator-binary/src/main.rs @@ -93,10 +93,13 @@ async fn main() -> anyhow::Result<()> { &cluster_info_opts, ) .await?; - let event_recorder = Arc::new(Recorder::new(client.as_kube_client(), Reporter { - controller: FULL_CONTROLLER_NAME.to_string(), - instance: None, - })); + let event_recorder = Arc::new(Recorder::new( + client.as_kube_client(), + Reporter { + controller: FULL_CONTROLLER_NAME.to_string(), + instance: None, + }, + )); let cluster_controller = Controller::new( watch_namespace.get_api::>(&client), From 2dda23e9e3daf58240ee0610b6bcd9f2fbd10851 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Mon, 2 Jun 2025 18:04:05 +0200 Subject: [PATCH 05/38] fix tests --- rust/operator-binary/src/config/jvm.rs | 16 +++++----------- rust/operator-binary/src/controller.rs | 14 +++++++------- 2 files changed, 12 insertions(+), 18 deletions(-) diff --git a/rust/operator-binary/src/config/jvm.rs b/rust/operator-binary/src/config/jvm.rs index e10613ff..a5f9eec6 100644 --- a/rust/operator-binary/src/config/jvm.rs +++ b/rust/operator-binary/src/config/jvm.rs @@ -186,9 +186,7 @@ mod tests { "#; let jvm_config = construct_jvm_config(input); - assert_eq!( - jvm_config, - indoc! {" + assert_eq!(jvm_config, indoc! {" -server # Heap settings -Xms34406m @@ -203,8 +201,7 @@ mod tests { -Djavax.net.ssl.trustStorePassword=changeit # Recommended JVM arguments from Trino -RecommendedTrinoFlag - # Arguments from jvmArgumentOverrides"} - ); + # Arguments from jvmArgumentOverrides"}); } #[test] @@ -245,9 +242,7 @@ mod tests { "#; let jvm_config = construct_jvm_config(input); - assert_eq!( - jvm_config, - indoc! {" + assert_eq!(jvm_config, indoc! {" -server # Heap settings -Xms34406m @@ -265,8 +260,7 @@ mod tests { -Dhttps.proxyHost=proxy.my.corp -Djava.net.preferIPv4Stack=true -Xmx40000m - -Dhttps.proxyPort=1234"} - ); + -Dhttps.proxyPort=1234"}); } fn construct_jvm_config(trino_cluster: &str) -> String { @@ -276,7 +270,7 @@ mod tests { let role = TrinoRole::Coordinator; let rolegroup_ref = role.rolegroup_ref(&trino, "default"); let merged_config = trino.merged_config(&role, &rolegroup_ref, &[]).unwrap(); - let coordinators = trino.spec.coordinators.unwrap(); + let coordinators = trino.role(&role).unwrap(); let product_version = trino.spec.image.product_version(); diff --git a/rust/operator-binary/src/controller.rs b/rust/operator-binary/src/controller.rs index c802db95..b9ca4cb5 100644 --- a/rust/operator-binary/src/controller.rs +++ b/rust/operator-binary/src/controller.rs @@ -548,10 +548,10 @@ pub async fn reconcile_trino( ClusterOperationsConditionBuilder::new(&trino.spec.cluster_operation); let status = v1alpha1::TrinoClusterStatus { - conditions: compute_conditions( - trino, - &[&sts_cond_builder, &cluster_operation_cond_builder], - ), + conditions: compute_conditions(trino, &[ + &sts_cond_builder, + &cluster_operation_cond_builder, + ]), }; cluster_resources @@ -1690,12 +1690,12 @@ mod tests { TrinoRole::Coordinator.to_string(), ( config_files.clone(), - trino.spec.coordinators.clone().unwrap(), + trino.role(&TrinoRole::Coordinator).unwrap(), ), ), ( TrinoRole::Worker.to_string(), - (config_files, trino.spec.workers.clone().unwrap()), + (config_files, trino.role(&TrinoRole::Worker).unwrap()), ), ] .into(), @@ -1747,7 +1747,7 @@ mod tests { build_rolegroup_config_map( &trino, &resolved_product_image, - role, + &role, &trino_role, &rolegroup_ref, validated_config From 767e1c0edbb160e8573dcabc4c31c74f5cb142a5 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Tue, 3 Jun 2025 10:07:46 +0200 Subject: [PATCH 06/38] fmt --- rust/operator-binary/src/config/jvm.rs | 14 ++++++++++---- rust/operator-binary/src/controller.rs | 8 ++++---- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/rust/operator-binary/src/config/jvm.rs b/rust/operator-binary/src/config/jvm.rs index a5f9eec6..3d3fa40a 100644 --- a/rust/operator-binary/src/config/jvm.rs +++ b/rust/operator-binary/src/config/jvm.rs @@ -186,7 +186,9 @@ mod tests { "#; let jvm_config = construct_jvm_config(input); - assert_eq!(jvm_config, indoc! {" + assert_eq!( + jvm_config, + indoc! {" -server # Heap settings -Xms34406m @@ -201,7 +203,8 @@ mod tests { -Djavax.net.ssl.trustStorePassword=changeit # Recommended JVM arguments from Trino -RecommendedTrinoFlag - # Arguments from jvmArgumentOverrides"}); + # Arguments from jvmArgumentOverrides"} + ); } #[test] @@ -242,7 +245,9 @@ mod tests { "#; let jvm_config = construct_jvm_config(input); - assert_eq!(jvm_config, indoc! {" + assert_eq!( + jvm_config, + indoc! {" -server # Heap settings -Xms34406m @@ -260,7 +265,8 @@ mod tests { -Dhttps.proxyHost=proxy.my.corp -Djava.net.preferIPv4Stack=true -Xmx40000m - -Dhttps.proxyPort=1234"}); + -Dhttps.proxyPort=1234"} + ); } fn construct_jvm_config(trino_cluster: &str) -> String { diff --git a/rust/operator-binary/src/controller.rs b/rust/operator-binary/src/controller.rs index b9ca4cb5..e32a5dc6 100644 --- a/rust/operator-binary/src/controller.rs +++ b/rust/operator-binary/src/controller.rs @@ -548,10 +548,10 @@ pub async fn reconcile_trino( ClusterOperationsConditionBuilder::new(&trino.spec.cluster_operation); let status = v1alpha1::TrinoClusterStatus { - conditions: compute_conditions(trino, &[ - &sts_cond_builder, - &cluster_operation_cond_builder, - ]), + conditions: compute_conditions( + trino, + &[&sts_cond_builder, &cluster_operation_cond_builder], + ), }; cluster_resources From 7ca532cb25311048fd978bcdc520db1020a66988 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Tue, 3 Jun 2025 10:08:02 +0200 Subject: [PATCH 07/38] regenerated charts --- deploy/helm/trino-operator/crds/crds.yaml | 25 ++++++++--------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/deploy/helm/trino-operator/crds/crds.yaml b/deploy/helm/trino-operator/crds/crds.yaml index fcc5d1f3..039ed464 100644 --- a/deploy/helm/trino-operator/crds/crds.yaml +++ b/deploy/helm/trino-operator/crds/crds.yaml @@ -105,23 +105,6 @@ spec: description: matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels map is equivalent to an element of matchExpressions, whose key field is "key", the operator is "In", and the values array contains only "value". The requirements are ANDed. type: object type: object - listenerClass: - default: cluster-internal - description: |- - This field controls which type of Service the Operator creates for this TrinoCluster: - - * cluster-internal: Use a ClusterIP service - - * external-unstable: Use a NodePort service - - * external-stable: Use a LoadBalancer service - - This is a temporary solution with the goal to keep yaml manifests forward compatible. In the future, this setting will control which [ListenerClass](https://docs.stackable.tech/home/nightly/listener-operator/listenerclass.html) will be used to expose the service, and ListenerClass names will stay the same, allowing for a non-breaking change. - enum: - - cluster-internal - - external-unstable - - external-stable - type: string tls: default: internalSecretClass: tls @@ -207,6 +190,10 @@ 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: {} @@ -509,6 +496,10 @@ 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: {} From 6e607d8aeccfbf9387c4c2eb059abd941fcac9e7 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Tue, 3 Jun 2025 10:14:29 +0200 Subject: [PATCH 08/38] add docs --- .../pages/usage-guide/listenerclass.adoc | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/docs/modules/trino/pages/usage-guide/listenerclass.adoc b/docs/modules/trino/pages/usage-guide/listenerclass.adoc index 2e38886f..ddb8e0d3 100644 --- a/docs/modules/trino/pages/usage-guide/listenerclass.adoc +++ b/docs/modules/trino/pages/usage-guide/listenerclass.adoc @@ -1,17 +1,18 @@ = Service exposition with ListenerClasses +:description: Configure Trino service exposure with ListenerClasses: cluster-internal, external-unstable, or external-stable. -Trino offers a web UI and an API, both are exposed by the `connector` xref:concepts:roles-and-role-groups.adoc[role]. -The Operator deploys a service called `-connector` (where `` is the name of the TrinoCluster) through which Trino can be reached. - -This service can have three different types: `cluster-internal`, `external-unstable` and `external-stable`. -Read more about the types in the xref:concepts:service-exposition.adoc[service exposition] documentation at platform level. - -This is how the ListenerClass is configured: +The operator deploys a xref:listener-operator:listener.adoc[Listener] for the coodinator pod. +The listener defaults to only being accessible from within the Kubernetes cluster, but this can be changed by setting `.spec.coordinators.config.listenerClass`: [source,yaml] ---- spec: - clusterConfig: - listenerClass: cluster-internal # <1> + coordinators: + config: + listenerClass: external-unstable # <1> + ... + workers: + ... ---- -<1> The default `cluster-internal` setting. +<1> Specify a ListenerClass, such as `external-stable`, `external-unstable`, or `cluster-internal` (the default setting is `cluster-internal`). +This can be set only for the coordinator role. From f7a38d8c1d75960e7a556fbabde2bb2080bef7a2 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Thu, 5 Jun 2025 14:15:23 +0200 Subject: [PATCH 09/38] wip - working --- rust/operator-binary/src/controller.rs | 146 ++++++++++++++++++------- rust/operator-binary/src/crd/mod.rs | 33 +++--- rust/operator-binary/src/listener.rs | 132 ++++++++++++++++++++++ rust/operator-binary/src/main.rs | 1 + 4 files changed, 260 insertions(+), 52 deletions(-) create mode 100644 rust/operator-binary/src/listener.rs diff --git a/rust/operator-binary/src/controller.rs b/rust/operator-binary/src/controller.rs index e32a5dc6..80363173 100644 --- a/rust/operator-binary/src/controller.rs +++ b/rust/operator-binary/src/controller.rs @@ -79,9 +79,9 @@ use crate::{ command, config, crd::{ ACCESS_CONTROL_PROPERTIES, APP_NAME, CONFIG_DIR_NAME, CONFIG_PROPERTIES, Container, - DATA_DIR_NAME, DISCOVERY_URI, ENV_INTERNAL_SECRET, HTTP_PORT, HTTP_PORT_NAME, HTTPS_PORT, - HTTPS_PORT_NAME, JVM_CONFIG, JVM_SECURITY_PROPERTIES, LOG_PROPERTIES, - MAX_TRINO_LOG_FILES_SIZE, METRICS_PORT, METRICS_PORT_NAME, NODE_PROPERTIES, + DATA_DIR_NAME, DISCOVERY_URI, ENV_INTERNAL_SECRET, HEADLESS_SERVICE_SUFFIX, HTTP_PORT, + HTTP_PORT_NAME, HTTPS_PORT, HTTPS_PORT_NAME, JVM_CONFIG, JVM_SECURITY_PROPERTIES, + LOG_PROPERTIES, MAX_TRINO_LOG_FILES_SIZE, METRICS_PORT, METRICS_PORT_NAME, NODE_PROPERTIES, RW_CONFIG_DIR_NAME, STACKABLE_CLIENT_TLS_DIR, STACKABLE_INTERNAL_TLS_DIR, STACKABLE_MOUNT_INTERNAL_TLS_DIR, STACKABLE_MOUNT_SERVER_TLS_DIR, STACKABLE_SERVER_TLS_DIR, TrinoRole, @@ -90,6 +90,10 @@ use crate::{ discovery::{TrinoDiscovery, TrinoDiscoveryProtocol, TrinoPodRef}, v1alpha1, }, + listener::{ + LISTENER_VOLUME_DIR, LISTENER_VOLUME_NAME, build_group_listener, build_group_listener_pvc, + group_listener_name, merged_listener_class, + }, operations::{ add_graceful_shutdown_config, graceful_shutdown_config_properties, pdb::add_pdbs, }, @@ -346,6 +350,14 @@ pub enum Error { source: ParseIntError, product_version: String, }, + + #[snafu(display("failed to apply group listener"))] + ApplyGroupListener { + source: stackable_operator::cluster_resources::Error, + }, + + #[snafu(display("failed to configure listener"))] + ListenerConfiguration { source: crate::listener::Error }, } type Result = std::result::Result; @@ -465,19 +477,20 @@ pub async fn reconcile_trino( let trino_role = TrinoRole::from_str(&trino_role_str).context(FailedToParseRoleSnafu)?; let role = trino.role(&trino_role).context(ReadRoleSnafu)?; for (role_group, config) in role_config { - let rolegroup = trino_role.rolegroup_ref(trino, &role_group); + let role_group_ref = trino_role.rolegroup_ref(trino, &role_group); let merged_config = trino - .merged_config(&trino_role, &rolegroup, &catalog_definitions) + .merged_config(&trino_role, &role_group_ref, &catalog_definitions) .context(FailedToResolveConfigSnafu)?; - let rg_service = build_rolegroup_service(trino, &resolved_product_image, &rolegroup)?; + let rg_service = + build_rolegroup_service(trino, &resolved_product_image, &role_group_ref)?; let rg_configmap = build_rolegroup_config_map( trino, &resolved_product_image, &role, &trino_role, - &rolegroup, + &role_group_ref, &config, &merged_config, &trino_authentication_config, @@ -487,14 +500,14 @@ pub async fn reconcile_trino( let rg_catalog_configmap = build_rolegroup_catalog_config_map( trino, &resolved_product_image, - &rolegroup, + &role_group_ref, &catalogs, )?; let rg_stateful_set = build_rolegroup_statefulset( trino, &trino_role, &resolved_product_image, - &rolegroup, + &role_group_ref, &config, &merged_config, &trino_authentication_config, @@ -506,21 +519,21 @@ pub async fn reconcile_trino( .add(client, rg_service) .await .with_context(|_| ApplyRoleGroupServiceSnafu { - rolegroup: rolegroup.clone(), + rolegroup: role_group_ref.clone(), })?; cluster_resources .add(client, rg_configmap) .await .with_context(|_| ApplyRoleGroupConfigSnafu { - rolegroup: rolegroup.clone(), + rolegroup: role_group_ref.clone(), })?; cluster_resources .add(client, rg_catalog_configmap) .await .with_context(|_| ApplyRoleGroupConfigSnafu { - rolegroup: rolegroup.clone(), + rolegroup: role_group_ref.clone(), })?; sts_cond_builder.add( @@ -528,9 +541,34 @@ pub async fn reconcile_trino( .add(client, rg_stateful_set) .await .with_context(|_| ApplyRoleGroupStatefulSetSnafu { - rolegroup: rolegroup.clone(), + 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)?; + + cluster_resources + .add(client, role_group_listener) + .await + .context(ApplyGroupListenerSnafu)?; + } + } } let role_config = trino.role_config(&trino_role); @@ -548,10 +586,10 @@ pub async fn reconcile_trino( ClusterOperationsConditionBuilder::new(&trino.spec.cluster_operation); let status = v1alpha1::TrinoClusterStatus { - conditions: compute_conditions( - trino, - &[&sts_cond_builder, &cluster_operation_cond_builder], - ), + conditions: compute_conditions(trino, &[ + &sts_cond_builder, + &cluster_operation_cond_builder, + ]), }; cluster_resources @@ -806,7 +844,7 @@ fn build_rolegroup_statefulset( trino: &v1alpha1::TrinoCluster, trino_role: &TrinoRole, resolved_product_image: &ResolvedProductImage, - rolegroup_ref: &RoleGroupRef, + role_group_ref: &RoleGroupRef, config: &HashMap>, merged_config: &v1alpha1::TrinoConfig, trino_authentication_config: &TrinoAuthenticationConfig, @@ -817,7 +855,7 @@ fn build_rolegroup_statefulset( .role(trino_role) .context(InternalOperatorFailureSnafu)?; let rolegroup = trino - .rolegroup(rolegroup_ref) + .rolegroup(role_group_ref) .context(InternalOperatorFailureSnafu)?; let mut pod_builder = PodBuilder::new(); @@ -892,6 +930,7 @@ fn build_rolegroup_statefulset( let requested_secret_lifetime = merged_config .requested_secret_lifetime .context(MissingSecretLifetimeSnafu)?; + // add volume mounts depending on the client tls, internal tls, catalogs and authentication tls_volume_mounts( trino, @@ -951,6 +990,36 @@ fn build_rolegroup_statefulset( ) .build(); + // for rw config + let mut persistent_volume_claims = vec![ + merged_config + .resources + .storage + .data + .build_pvc("data", Some(vec!["ReadWriteOnce"])), + ]; + // Add listener + if let Some(group_listener_name) = group_listener_name(trino_role, role_group_ref) { + cb_trino + .add_volume_mount(LISTENER_VOLUME_NAME, LISTENER_VOLUME_DIR) + .context(AddVolumeMountSnafu)?; + + // Used for PVC templates that cannot be modified once they are deployed + let unversioned_recommended_labels = Labels::recommended(build_recommended_labels( + trino, + // 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, + )) + .context(LabelBuildSnafu)?; + + persistent_volume_claims.push( + build_group_listener_pvc(&group_listener_name, &unversioned_recommended_labels) + .context(ListenerConfigurationSnafu)?, + ); + } + let container_trino = cb_trino .image_from_product_image(resolved_product_image) .command(vec![ @@ -1010,7 +1079,7 @@ fn build_rolegroup_statefulset( .add_volume(Volume { name: "log-config".to_string(), config_map: Some(ConfigMapVolumeSource { - name: rolegroup_ref.object_name(), + name: role_group_ref.object_name(), ..ConfigMapVolumeSource::default() }), ..Volume::default() @@ -1048,8 +1117,8 @@ fn build_rolegroup_statefulset( .with_recommended_labels(build_recommended_labels( trino, &resolved_product_image.app_version_label, - &rolegroup_ref.role, - &rolegroup_ref.role_group, + &role_group_ref.role, + &role_group_ref.role_group, )) .context(MetadataBuildSnafu)? .with_annotation( @@ -1067,7 +1136,7 @@ fn build_rolegroup_statefulset( .add_volume(Volume { name: "config".to_string(), config_map: Some(ConfigMapVolumeSource { - name: rolegroup_ref.object_name(), + name: role_group_ref.object_name(), ..ConfigMapVolumeSource::default() }), ..Volume::default() @@ -1078,7 +1147,7 @@ fn build_rolegroup_statefulset( .add_volume(Volume { name: "catalog".to_string(), config_map: Some(ConfigMapVolumeSource { - name: format!("{}-catalog", rolegroup_ref.object_name()), + name: format!("{}-catalog", role_group_ref.object_name()), ..ConfigMapVolumeSource::default() }), ..Volume::default() @@ -1107,14 +1176,14 @@ fn build_rolegroup_statefulset( Ok(StatefulSet { metadata: ObjectMetaBuilder::new() .name_and_namespace(trino) - .name(rolegroup_ref.object_name()) + .name(role_group_ref.object_name()) .ownerreference_from_resource(trino, None, Some(true)) .context(ObjectMissingMetadataForOwnerRefSnafu)? .with_recommended_labels(build_recommended_labels( trino, &resolved_product_image.app_version_label, - &rolegroup_ref.role, - &rolegroup_ref.role_group, + &role_group_ref.role, + &role_group_ref.role_group, )) .context(MetadataBuildSnafu)? .build(), @@ -1126,23 +1195,20 @@ fn build_rolegroup_statefulset( Labels::role_group_selector( trino, APP_NAME, - &rolegroup_ref.role, - &rolegroup_ref.role_group, + &role_group_ref.role, + &role_group_ref.role_group, ) .context(LabelBuildSnafu)? .into(), ), ..LabelSelector::default() }, - service_name: Some(rolegroup_ref.object_name()), + service_name: Some(format!( + "{}-{HEADLESS_SERVICE_SUFFIX}", + role_group_ref.object_name() + )), template: pod_template, - volume_claim_templates: Some(vec![ - merged_config - .resources - .storage - .data - .build_pvc("data", Some(vec!["ReadWriteOnce"])), - ]), + volume_claim_templates: Some(persistent_volume_claims), ..StatefulSetSpec::default() }), status: None, @@ -1160,7 +1226,11 @@ fn build_rolegroup_service( Ok(Service { metadata: ObjectMetaBuilder::new() .name_and_namespace(trino) - .name(rolegroup_ref.object_name()) + .name(format!( + "{}-{}", + rolegroup_ref.object_name(), + HEADLESS_SERVICE_SUFFIX + )) .ownerreference_from_resource(trino, None, Some(true)) .context(ObjectMissingMetadataForOwnerRefSnafu)? .with_recommended_labels(build_recommended_labels( diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs index cfd4521b..908d2b6a 100644 --- a/rust/operator-binary/src/crd/mod.rs +++ b/rust/operator-binary/src/crd/mod.rs @@ -117,6 +117,10 @@ 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 JVM_HEAP_FACTOR: f32 = 0.8; @@ -818,7 +822,11 @@ impl v1alpha1::TrinoCluster { let ns = ns.clone(); (0..rolegroup.replicas.unwrap_or(0)).map(move |i| TrinoPodRef { namespace: ns.clone(), - role_group_service_name: rolegroup_ref.object_name(), + role_group_service_name: format!( + "{}-{}", + rolegroup_ref.object_name(), + HEADLESS_SERVICE_SUFFIX + ), pod_name: format!("{}-{}", rolegroup_ref.object_name(), i), }) })) @@ -933,20 +941,17 @@ fn extract_role_from_coordinator_config( .role_groups .into_iter() .map(|(k, v)| { - ( - k, - RoleGroup { - config: CommonConfiguration { - config: v.config.config.trino_config, - config_overrides: v.config.config_overrides, - env_overrides: v.config.env_overrides, - cli_overrides: v.config.cli_overrides, - pod_overrides: v.config.pod_overrides, - product_specific_common_config: v.config.product_specific_common_config, - }, - replicas: v.replicas, + (k, RoleGroup { + config: CommonConfiguration { + config: v.config.config.trino_config, + config_overrides: v.config.config_overrides, + env_overrides: v.config.env_overrides, + cli_overrides: v.config.cli_overrides, + pod_overrides: v.config.pod_overrides, + product_specific_common_config: v.config.product_specific_common_config, }, - ) + replicas: v.replicas, + }) }) .collect(), } diff --git a/rust/operator-binary/src/listener.rs b/rust/operator-binary/src/listener.rs new file mode 100644 index 00000000..08d3acc6 --- /dev/null +++ b/rust/operator-binary/src/listener.rs @@ -0,0 +1,132 @@ +use snafu::{ResultExt, Snafu}; +use stackable_operator::{ + builder::{ + meta::ObjectMetaBuilder, + pod::volume::{ + ListenerOperatorVolumeSourceBuilder, ListenerOperatorVolumeSourceBuilderError, + ListenerReference, + }, + }, + config::merge::Merge, + crd::listener::v1alpha1::{Listener, ListenerPort, ListenerSpec}, + k8s_openapi::api::core::v1::PersistentVolumeClaim, + kvp::{Labels, ObjectLabels}, + role_utils::RoleGroupRef, +}; + +use crate::crd::{TrinoRole, v1alpha1}; + +pub const LISTENER_VOLUME_NAME: &str = "listener"; +pub const LISTENER_VOLUME_DIR: &str = "/stackable/listener"; + +#[derive(Snafu, Debug)] +pub enum Error { + #[snafu(display("listener object is missing metadata to build owner reference"))] + ObjectMissingMetadataForOwnerRef { + source: stackable_operator::builder::meta::Error, + }, + + #[snafu(display("failed to build listener object meta data"))] + BuildObjectMeta { + source: stackable_operator::builder::meta::Error, + }, + + #[snafu(display("failed to build listener volume"))] + BuildListenerPersistentVolume { + source: ListenerOperatorVolumeSourceBuilderError, + }, +} + +pub fn build_group_listener( + trino: &v1alpha1::TrinoCluster, + object_labels: ObjectLabels, + listener_class: String, + listener_group_name: String, +) -> Result { + Ok(Listener { + metadata: ObjectMetaBuilder::new() + .name_and_namespace(trino) + .name(listener_group_name) + .ownerreference_from_resource(trino, None, Some(true)) + .context(ObjectMissingMetadataForOwnerRefSnafu)? + .with_recommended_labels(object_labels) + .context(BuildObjectMetaSnafu)? + .build(), + spec: ListenerSpec { + class_name: Some(listener_class), + ports: Some(listener_ports(trino)), + ..ListenerSpec::default() + }, + status: None, + }) +} + +pub fn build_group_listener_pvc( + group_listener_name: &str, + unversioned_recommended_labels: &Labels, +) -> Result { + ListenerOperatorVolumeSourceBuilder::new( + &ListenerReference::ListenerName(group_listener_name.to_string()), + unversioned_recommended_labels, + ) + .context(BuildListenerPersistentVolumeSnafu)? + .build_pvc(LISTENER_VOLUME_NAME.to_string()) + .context(BuildListenerPersistentVolumeSnafu) +} + +/// The name of the group-listener provided for a specific role-group. +/// Coordinator(s) will use this group listener so that only one load balancer +/// is needed (per role group). +pub fn group_listener_name( + role: &TrinoRole, + role_group_ref: &RoleGroupRef, +) -> Option { + match role { + TrinoRole::Coordinator => Some(role_group_ref.object_name()), + TrinoRole::Worker => None, + } +} + +pub fn merged_listener_class( + trino: &v1alpha1::TrinoCluster, + role: &TrinoRole, + rolegroup_name: &String, +) -> Option { + if role == &TrinoRole::Coordinator { + if let Some(coordinators) = trino.spec.coordinators.as_ref() { + let conf_defaults = Some("cluster-internal".to_string()); + let mut conf_role = coordinators.config.config.listener_class.to_owned(); + let mut conf_rolegroup = coordinators + .role_groups + .get(rolegroup_name) + .map(|rg| rg.config.config.listener_class.clone()) + // TODO: Discuss in review + // This should always be Some() due to the reconcile loop (role, role_group) in controller.rs + // We do not want to use expect() here since its only a 99.99%. E.g. probably not possible to apply + // and empty string / null via yaml, but could be via json - not sure? + .unwrap_or_default(); + + conf_role.merge(&conf_defaults); + conf_rolegroup.merge(&conf_role); + + tracing::debug!("Merged listener-class: {:?}", conf_rolegroup); + conf_rolegroup + } else { + None + } + } else { + None + } +} + +/// We only use the http/https port here and intentionally omit the metrics one. +fn listener_ports(trino: &v1alpha1::TrinoCluster) -> Vec { + let name = trino.exposed_protocol().to_string(); + let port = trino.exposed_port().into(); + + vec![ListenerPort { + name, + port, + protocol: Some("TCP".to_string()), + }] +} diff --git a/rust/operator-binary/src/main.rs b/rust/operator-binary/src/main.rs index 22bd6b0e..25a44ba4 100644 --- a/rust/operator-binary/src/main.rs +++ b/rust/operator-binary/src/main.rs @@ -5,6 +5,7 @@ mod command; mod config; mod controller; mod crd; +mod listener; mod operations; mod product_logging; From ab6d4c95c931eac039d58a2a8b9028e76c74af2b Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Thu, 5 Jun 2025 14:28:10 +0200 Subject: [PATCH 10/38] make headless service name a function --- rust/operator-binary/src/controller.rs | 27 ++++++++---------- rust/operator-binary/src/crd/mod.rs | 38 +++++++++++++++----------- 2 files changed, 34 insertions(+), 31 deletions(-) diff --git a/rust/operator-binary/src/controller.rs b/rust/operator-binary/src/controller.rs index 80363173..235f22ff 100644 --- a/rust/operator-binary/src/controller.rs +++ b/rust/operator-binary/src/controller.rs @@ -79,9 +79,9 @@ use crate::{ command, config, crd::{ ACCESS_CONTROL_PROPERTIES, APP_NAME, CONFIG_DIR_NAME, CONFIG_PROPERTIES, Container, - DATA_DIR_NAME, DISCOVERY_URI, ENV_INTERNAL_SECRET, HEADLESS_SERVICE_SUFFIX, HTTP_PORT, - HTTP_PORT_NAME, HTTPS_PORT, HTTPS_PORT_NAME, JVM_CONFIG, JVM_SECURITY_PROPERTIES, - LOG_PROPERTIES, MAX_TRINO_LOG_FILES_SIZE, METRICS_PORT, METRICS_PORT_NAME, NODE_PROPERTIES, + DATA_DIR_NAME, DISCOVERY_URI, ENV_INTERNAL_SECRET, HTTP_PORT, HTTP_PORT_NAME, HTTPS_PORT, + HTTPS_PORT_NAME, JVM_CONFIG, JVM_SECURITY_PROPERTIES, LOG_PROPERTIES, + MAX_TRINO_LOG_FILES_SIZE, METRICS_PORT, METRICS_PORT_NAME, NODE_PROPERTIES, RW_CONFIG_DIR_NAME, STACKABLE_CLIENT_TLS_DIR, STACKABLE_INTERNAL_TLS_DIR, STACKABLE_MOUNT_INTERNAL_TLS_DIR, STACKABLE_MOUNT_SERVER_TLS_DIR, STACKABLE_SERVER_TLS_DIR, TrinoRole, @@ -1203,9 +1203,8 @@ fn build_rolegroup_statefulset( ), ..LabelSelector::default() }, - service_name: Some(format!( - "{}-{HEADLESS_SERVICE_SUFFIX}", - role_group_ref.object_name() + service_name: Some(v1alpha1::TrinoCluster::rolegroup_headless_service_name( + &role_group_ref.object_name(), )), template: pod_template, volume_claim_templates: Some(persistent_volume_claims), @@ -1221,23 +1220,21 @@ fn build_rolegroup_statefulset( fn build_rolegroup_service( trino: &v1alpha1::TrinoCluster, resolved_product_image: &ResolvedProductImage, - rolegroup_ref: &RoleGroupRef, + role_group_ref: &RoleGroupRef, ) -> Result { Ok(Service { metadata: ObjectMetaBuilder::new() .name_and_namespace(trino) - .name(format!( - "{}-{}", - rolegroup_ref.object_name(), - HEADLESS_SERVICE_SUFFIX + .name(v1alpha1::TrinoCluster::rolegroup_headless_service_name( + &role_group_ref.object_name(), )) .ownerreference_from_resource(trino, None, Some(true)) .context(ObjectMissingMetadataForOwnerRefSnafu)? .with_recommended_labels(build_recommended_labels( trino, &resolved_product_image.app_version_label, - &rolegroup_ref.role, - &rolegroup_ref.role_group, + &role_group_ref.role, + &role_group_ref.role_group, )) .context(MetadataBuildSnafu)? .with_label(Label::try_from(("prometheus.io/scrape", "true")).context(LabelBuildSnafu)?) @@ -1251,8 +1248,8 @@ fn build_rolegroup_service( Labels::role_group_selector( trino, APP_NAME, - &rolegroup_ref.role, - &rolegroup_ref.role_group, + &role_group_ref.role, + &role_group_ref.role_group, ) .context(LabelBuildSnafu)? .into(), diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs index 908d2b6a..da4cc09c 100644 --- a/rust/operator-binary/src/crd/mod.rs +++ b/rust/operator-binary/src/crd/mod.rs @@ -818,20 +818,23 @@ impl v1alpha1::TrinoCluster { .collect::>() .into_iter() .flat_map(move |(rolegroup_name, rolegroup)| { - let rolegroup_ref = TrinoRole::Coordinator.rolegroup_ref(self, rolegroup_name); + let role_group_ref = TrinoRole::Coordinator.rolegroup_ref(self, rolegroup_name); let ns = ns.clone(); (0..rolegroup.replicas.unwrap_or(0)).map(move |i| TrinoPodRef { namespace: ns.clone(), - role_group_service_name: format!( - "{}-{}", - rolegroup_ref.object_name(), - HEADLESS_SERVICE_SUFFIX + role_group_service_name: Self::rolegroup_headless_service_name( + &role_group_ref.object_name(), ), - pod_name: format!("{}-{}", rolegroup_ref.object_name(), i), + pod_name: format!("{}-{}", role_group_ref.object_name(), i), }) })) } + /// Returns the headless rolegroup service name `simple-trino-coordinator-default-`. + pub fn rolegroup_headless_service_name(role_group_ref_object_name: &str) -> String { + format!("{}-{}", role_group_ref_object_name, HEADLESS_SERVICE_SUFFIX) + } + /// Returns user provided authentication settings pub fn get_authentication(&self) -> &Vec { &self.spec.cluster_config.authentication @@ -941,17 +944,20 @@ fn extract_role_from_coordinator_config( .role_groups .into_iter() .map(|(k, v)| { - (k, RoleGroup { - config: CommonConfiguration { - config: v.config.config.trino_config, - config_overrides: v.config.config_overrides, - env_overrides: v.config.env_overrides, - cli_overrides: v.config.cli_overrides, - pod_overrides: v.config.pod_overrides, - product_specific_common_config: v.config.product_specific_common_config, + ( + k, + RoleGroup { + config: CommonConfiguration { + config: v.config.config.trino_config, + config_overrides: v.config.config_overrides, + env_overrides: v.config.env_overrides, + cli_overrides: v.config.cli_overrides, + pod_overrides: v.config.pod_overrides, + product_specific_common_config: v.config.product_specific_common_config, + }, + replicas: v.replicas, }, - replicas: v.replicas, - }) + ) }) .collect(), } From 0f80bf3609f30af96f79aed6f097dee68c0f17cd Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Thu, 5 Jun 2025 15:43:15 +0200 Subject: [PATCH 11/38] added tests --- .../kuttl/listener/00-assert.yaml.j2 | 10 ++++ ...tor-aggregator-discovery-configmap.yaml.j2 | 9 +++ .../kuttl/listener/00-patch-ns.yaml.j2 | 9 +++ .../templates/kuttl/listener/00-rbac.yaml.j2 | 29 +++++++++ tests/templates/kuttl/listener/10-assert.yaml | 59 +++++++++++++++++++ .../kuttl/listener/10-install-trino.yaml.j2 | 56 ++++++++++++++++++ tests/templates/kuttl/listener/20-assert.yaml | 12 ++++ .../kuttl/listener/20-install-check.yaml | 29 +++++++++ tests/templates/kuttl/listener/21-assert.yaml | 6 ++ .../kuttl/listener/21-copy-scripts.yaml | 5 ++ .../kuttl/listener/check-active-workers.py | 51 ++++++++++++++++ tests/test-definition.yaml | 4 ++ 12 files changed, 279 insertions(+) create mode 100644 tests/templates/kuttl/listener/00-assert.yaml.j2 create mode 100644 tests/templates/kuttl/listener/00-install-vector-aggregator-discovery-configmap.yaml.j2 create mode 100644 tests/templates/kuttl/listener/00-patch-ns.yaml.j2 create mode 100644 tests/templates/kuttl/listener/00-rbac.yaml.j2 create mode 100644 tests/templates/kuttl/listener/10-assert.yaml create mode 100644 tests/templates/kuttl/listener/10-install-trino.yaml.j2 create mode 100644 tests/templates/kuttl/listener/20-assert.yaml create mode 100644 tests/templates/kuttl/listener/20-install-check.yaml create mode 100644 tests/templates/kuttl/listener/21-assert.yaml create mode 100644 tests/templates/kuttl/listener/21-copy-scripts.yaml create mode 100755 tests/templates/kuttl/listener/check-active-workers.py diff --git a/tests/templates/kuttl/listener/00-assert.yaml.j2 b/tests/templates/kuttl/listener/00-assert.yaml.j2 new file mode 100644 index 00000000..50b1d4c3 --- /dev/null +++ b/tests/templates/kuttl/listener/00-assert.yaml.j2 @@ -0,0 +1,10 @@ +--- +apiVersion: kuttl.dev/v1beta1 +kind: TestAssert +{% if lookup('env', 'VECTOR_AGGREGATOR') %} +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: vector-aggregator-discovery +{% endif %} diff --git a/tests/templates/kuttl/listener/00-install-vector-aggregator-discovery-configmap.yaml.j2 b/tests/templates/kuttl/listener/00-install-vector-aggregator-discovery-configmap.yaml.j2 new file mode 100644 index 00000000..2d6a0df5 --- /dev/null +++ b/tests/templates/kuttl/listener/00-install-vector-aggregator-discovery-configmap.yaml.j2 @@ -0,0 +1,9 @@ +{% if lookup('env', 'VECTOR_AGGREGATOR') %} +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: vector-aggregator-discovery +data: + ADDRESS: {{ lookup('env', 'VECTOR_AGGREGATOR') }} +{% endif %} diff --git a/tests/templates/kuttl/listener/00-patch-ns.yaml.j2 b/tests/templates/kuttl/listener/00-patch-ns.yaml.j2 new file mode 100644 index 00000000..67185acf --- /dev/null +++ b/tests/templates/kuttl/listener/00-patch-ns.yaml.j2 @@ -0,0 +1,9 @@ +{% if test_scenario['values']['openshift'] == 'true' %} +# see https://github.com/stackabletech/issues/issues/566 +--- +apiVersion: kuttl.dev/v1beta1 +kind: TestStep +commands: + - script: kubectl patch namespace $NAMESPACE -p '{"metadata":{"labels":{"pod-security.kubernetes.io/enforce":"privileged"}}}' + timeout: 120 +{% endif %} diff --git a/tests/templates/kuttl/listener/00-rbac.yaml.j2 b/tests/templates/kuttl/listener/00-rbac.yaml.j2 new file mode 100644 index 00000000..9cbf0351 --- /dev/null +++ b/tests/templates/kuttl/listener/00-rbac.yaml.j2 @@ -0,0 +1,29 @@ +--- +kind: Role +apiVersion: rbac.authorization.k8s.io/v1 +metadata: + name: use-integration-tests-scc +rules: +{% if test_scenario['values']['openshift'] == "true" %} + - apiGroups: ["security.openshift.io"] + resources: ["securitycontextconstraints"] + resourceNames: ["privileged"] + verbs: ["use"] +{% endif %} +--- +apiVersion: v1 +kind: ServiceAccount +metadata: + name: integration-tests-sa +--- +kind: RoleBinding +apiVersion: rbac.authorization.k8s.io/v1 +metadata: + name: use-integration-tests-scc +subjects: + - kind: ServiceAccount + name: integration-tests-sa +roleRef: + kind: Role + name: use-integration-tests-scc + apiGroup: rbac.authorization.k8s.io diff --git a/tests/templates/kuttl/listener/10-assert.yaml b/tests/templates/kuttl/listener/10-assert.yaml new file mode 100644 index 00000000..3705c330 --- /dev/null +++ b/tests/templates/kuttl/listener/10-assert.yaml @@ -0,0 +1,59 @@ +--- +apiVersion: kuttl.dev/v1beta1 +kind: TestAssert +timeout: 600 +--- +apiVersion: apps/v1 +kind: StatefulSet +metadata: + name: test-trino-coordinator-default +status: + readyReplicas: 1 + replicas: 1 +--- +apiVersion: apps/v1 +kind: StatefulSet +metadata: + name: test-trino-worker-default +status: + readyReplicas: 1 + replicas: 1 +--- +apiVersion: policy/v1 +kind: PodDisruptionBudget +metadata: + name: test-trino-coordinator +status: + expectedPods: 1 + currentHealthy: 1 + disruptionsAllowed: 1 +--- +apiVersion: policy/v1 +kind: PodDisruptionBudget +metadata: + name: test-trino-worker +status: + expectedPods: 1 + currentHealthy: 1 + disruptionsAllowed: 1 +--- +apiVersion: v1 +kind: Service +metadata: + name: test-trino-coordinator-default +spec: + type: NodePort # external-unstable - by listener op +--- +apiVersion: v1 +kind: Service +metadata: + name: test-trino-coordinator-default-metrics +spec: + type: ClusterIP # cluster-internal - by trino op +--- +apiVersion: v1 +kind: Service +metadata: + name: test-trino-worker-default-metrics +spec: + type: ClusterIP # cluster-internal - by trino op diff --git a/tests/templates/kuttl/listener/10-install-trino.yaml.j2 b/tests/templates/kuttl/listener/10-install-trino.yaml.j2 new file mode 100644 index 00000000..989c03ba --- /dev/null +++ b/tests/templates/kuttl/listener/10-install-trino.yaml.j2 @@ -0,0 +1,56 @@ +--- +apiVersion: authentication.stackable.tech/v1alpha1 +kind: AuthenticationClass +metadata: + name: password +spec: + provider: + static: + userCredentialsSecret: + name: test-users +--- +apiVersion: v1 +kind: Secret +metadata: + name: test-users +stringData: + admin: admin +--- +apiVersion: trino.stackable.tech/v1alpha1 +kind: TrinoCluster +metadata: + name: test-trino +spec: + image: +{% if test_scenario['values']['trino'].find(",") > 0 %} + custom: "{{ test_scenario['values']['trino'].split(',')[1] }}" + productVersion: "{{ test_scenario['values']['trino'].split(',')[0] }}" +{% else %} + productVersion: "{{ test_scenario['values']['trino'] }}" +{% endif %} + pullPolicy: IfNotPresent + clusterConfig: + authentication: + - authenticationClass: password + catalogLabelSelector: {} +{% if lookup('env', 'VECTOR_AGGREGATOR') %} + vectorAggregatorConfigMapName: vector-aggregator-discovery +{% endif %} + coordinators: + config: + listenerClass: cluster-internal + gracefulShutdownTimeout: 5s # Let the test run faster + roleGroups: + default: + replicas: 1 + config: + # test merge -> we expect node port service + listenerClass: external-unstable + workers: + config: + gracefulShutdownTimeout: 5s # Let the test run faster + roleGroups: + default: + replicas: 1 + + diff --git a/tests/templates/kuttl/listener/20-assert.yaml b/tests/templates/kuttl/listener/20-assert.yaml new file mode 100644 index 00000000..168d5d8f --- /dev/null +++ b/tests/templates/kuttl/listener/20-assert.yaml @@ -0,0 +1,12 @@ +--- +apiVersion: kuttl.dev/v1beta1 +kind: TestAssert +timeout: 300 +--- +apiVersion: apps/v1 +kind: StatefulSet +metadata: + name: trino-test-helper +status: + readyReplicas: 1 + replicas: 1 diff --git a/tests/templates/kuttl/listener/20-install-check.yaml b/tests/templates/kuttl/listener/20-install-check.yaml new file mode 100644 index 00000000..cbd9fb4c --- /dev/null +++ b/tests/templates/kuttl/listener/20-install-check.yaml @@ -0,0 +1,29 @@ +--- +apiVersion: apps/v1 +kind: StatefulSet +metadata: + name: trino-test-helper + labels: + app: trino-test-helper +spec: + replicas: 1 + selector: + matchLabels: + app: trino-test-helper + template: + metadata: + labels: + app: trino-test-helper + spec: + serviceAccount: integration-tests-sa + containers: + - name: trino-test-helper + image: oci.stackable.tech/sdp/testing-tools:0.2.0-stackable0.0.0-dev + command: ["sleep", "infinity"] + resources: + requests: + cpu: "250m" + memory: "64Mi" + limits: + cpu: "500m" + memory: "64Mi" diff --git a/tests/templates/kuttl/listener/21-assert.yaml b/tests/templates/kuttl/listener/21-assert.yaml new file mode 100644 index 00000000..353eb1d7 --- /dev/null +++ b/tests/templates/kuttl/listener/21-assert.yaml @@ -0,0 +1,6 @@ +--- +apiVersion: kuttl.dev/v1beta1 +kind: TestAssert +timeout: 300 +commands: + - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-active-workers.py -u admin -p admin -w 1 diff --git a/tests/templates/kuttl/listener/21-copy-scripts.yaml b/tests/templates/kuttl/listener/21-copy-scripts.yaml new file mode 100644 index 00000000..7b581302 --- /dev/null +++ b/tests/templates/kuttl/listener/21-copy-scripts.yaml @@ -0,0 +1,5 @@ +--- +apiVersion: kuttl.dev/v1beta1 +kind: TestStep +commands: + - script: kubectl cp -n $NAMESPACE ./check-active-workers.py trino-test-helper-0:/tmp || true diff --git a/tests/templates/kuttl/listener/check-active-workers.py b/tests/templates/kuttl/listener/check-active-workers.py new file mode 100755 index 00000000..b1b388cc --- /dev/null +++ b/tests/templates/kuttl/listener/check-active-workers.py @@ -0,0 +1,51 @@ +#!/usr/bin/env python +import trino +import argparse +import sys + +if not sys.warnoptions: + import warnings +warnings.simplefilter("ignore") + + +def get_connection(username, password): + host = 'test-trino-coordinator-default' + conn = trino.dbapi.connect( + host=host, + port=8443, + user=username, + http_scheme='https', + auth=trino.auth.BasicAuthentication(username, password), + session_properties={"query_max_execution_time": "60s"}, + ) + conn._http_session.verify = False + return conn + + +if __name__ == '__main__': + # Construct an argument parser + all_args = argparse.ArgumentParser() + + # Add arguments to the parser + all_args.add_argument("-u", "--user", required=True, + help="Username to connect as") + all_args.add_argument("-p", "--password", required=True, + help="Password for the user") + all_args.add_argument("-w", "--workers", required=True, + help="Expected amount of workers to be present") + + args = vars(all_args.parse_args()) + + expected_workers = args['workers'] + conn = get_connection(args['user'], args['password']) + + cursor = conn.cursor() + cursor.execute("SELECT COUNT(*) as nodes FROM system.runtime.nodes WHERE coordinator=false AND state='active'") + + (active_workers,) = cursor.fetchone() + + if int(active_workers) != int(expected_workers): + print("Missmatch: [expected/active] workers [" + str(expected_workers) + "/" + str(active_workers) + "]") + exit(-1) + + print("Test check-active-workers.py succeeded!") diff --git a/tests/test-definition.yaml b/tests/test-definition.yaml index 9a86f260..e71a68ce 100644 --- a/tests/test-definition.yaml +++ b/tests/test-definition.yaml @@ -111,6 +111,10 @@ tests: - opa - keycloak - openshift + - name: listener + dimensions: + - trino + - openshift suites: - name: nightly # Run with the latest product versions and tls true and false to cover different tls code paths. From ecb48b5699d23da3e242fb0126b90ac5bd1c66fb Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Thu, 5 Jun 2025 15:48:28 +0200 Subject: [PATCH 12/38] adapted changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d4e6d40..6e917ae5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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`. +- Adds Listener integration for Trino ([#753]). ### Changed @@ -45,6 +46,7 @@ All notable changes to this project will be documented in this file. [#745]: https://github.com/stackabletech/trino-operator/pull/745 [#748]: https://github.com/stackabletech/trino-operator/pull/748 [#752]: https://github.com/stackabletech/trino-operator/pull/752 +[#753]: https://github.com/stackabletech/trino-operator/pull/753 ## [25.3.0] - 2025-03-21 From b36582d6ef4105d902bdc1a28ec11c0082b074d6 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Thu, 5 Jun 2025 17:04:37 +0200 Subject: [PATCH 13/38] move scripts to commons folder, fix tests --- .../kuttl/authentication/20-assert.yaml | 10 +- .../kuttl/authentication/20-test-trino.yaml | 4 +- .../authentication/check-active-workers.py | 52 ------ .../check-active-workers.py | 9 +- .../kuttl/{smoke_aws => commons}/check-opa.py | 27 ++-- .../kuttl/{smoke_aws => commons}/check-s3.py | 27 ++-- tests/templates/kuttl/listener/21-assert.yaml | 2 +- .../kuttl/listener/21-copy-scripts.yaml | 3 +- tests/templates/kuttl/smoke/21-assert.yaml | 6 +- .../kuttl/smoke/21-copy-scripts.yaml | 6 +- tests/templates/kuttl/smoke/31-assert.yaml | 6 +- .../kuttl/smoke/check-active-workers.py | 53 ------- tests/templates/kuttl/smoke/check-opa.py | 77 --------- tests/templates/kuttl/smoke/check-s3.py | 149 ------------------ .../templates/kuttl/smoke_aws/21-assert.yaml | 6 +- .../kuttl/smoke_aws/21-copy-scripts.yaml | 6 +- .../kuttl/smoke_aws/check-active-workers.py | 70 -------- 17 files changed, 50 insertions(+), 463 deletions(-) delete mode 100755 tests/templates/kuttl/authentication/check-active-workers.py rename tests/templates/kuttl/{listener => commons}/check-active-workers.py (83%) rename tests/templates/kuttl/{smoke_aws => commons}/check-opa.py (67%) rename tests/templates/kuttl/{smoke_aws => commons}/check-s3.py (92%) delete mode 100755 tests/templates/kuttl/smoke/check-active-workers.py delete mode 100755 tests/templates/kuttl/smoke/check-opa.py delete mode 100755 tests/templates/kuttl/smoke/check-s3.py delete mode 100755 tests/templates/kuttl/smoke_aws/check-active-workers.py diff --git a/tests/templates/kuttl/authentication/20-assert.yaml b/tests/templates/kuttl/authentication/20-assert.yaml index 0ed9b5ae..ffcb4611 100644 --- a/tests/templates/kuttl/authentication/20-assert.yaml +++ b/tests/templates/kuttl/authentication/20-assert.yaml @@ -4,10 +4,10 @@ kind: TestAssert timeout: 300 commands: # file - - script: kubectl exec -n $NAMESPACE test-trino-0 -- python /tmp/check-active-workers.py -u test_user_1 -p test_user_1 -n $NAMESPACE -w 1 - - script: kubectl exec -n $NAMESPACE test-trino-0 -- python /tmp/check-active-workers.py -u test_user_2_other -p test_user_2_other -n $NAMESPACE -w 1 + - script: kubectl exec -n $NAMESPACE test-trino-0 -- python /tmp/check-active-workers.py -u test_user_1 -p test_user_1 -c trino-coordinator-default -w 1 + - script: kubectl exec -n $NAMESPACE test-trino-0 -- python /tmp/check-active-workers.py -u test_user_2_other -p test_user_2_other -c trino-coordinator-default -w 1 # ldap - - script: kubectl exec -n $NAMESPACE test-trino-0 -- python /tmp/check-active-workers.py -u integrationtest -p integrationtest -n $NAMESPACE -w 1 - - script: kubectl exec -n $NAMESPACE test-trino-0 -- python /tmp/check-active-workers.py -u integrationtest-other -p integrationtest-other -n $NAMESPACE -w 1 + - script: kubectl exec -n $NAMESPACE test-trino-0 -- python /tmp/check-active-workers.py -u integrationtest -p integrationtest -c trino-coordinator-default -w 1 + - script: kubectl exec -n $NAMESPACE test-trino-0 -- python /tmp/check-active-workers.py -u integrationtest-other -p integrationtest-other -c trino-coordinator-default -w 1 # oidc/oauth2 - - script: kubectl exec -n $NAMESPACE test-trino-0 -- python /tmp/check-oauth-login.py https://trino-coordinator-default.$NAMESPACE.svc.cluster.local:8443/ui/ + - script: kubectl exec -n $NAMESPACE test-trino-0 -- python /tmp/check-oauth-login.py https://trino-coordinator-default:8443/ui/ diff --git a/tests/templates/kuttl/authentication/20-test-trino.yaml b/tests/templates/kuttl/authentication/20-test-trino.yaml index 75643e64..ccafc5e6 100644 --- a/tests/templates/kuttl/authentication/20-test-trino.yaml +++ b/tests/templates/kuttl/authentication/20-test-trino.yaml @@ -2,5 +2,5 @@ apiVersion: kuttl.dev/v1beta1 kind: TestStep commands: - - script: kubectl cp -n $NAMESPACE ./check-active-workers.py test-trino-0:/tmp - - script: kubectl cp -n $NAMESPACE ./check-oauth-login.py test-trino-0:/tmp + - script: kubectl cp -n $NAMESPACE ../../../../templates/kuttl/commons/check-active-workers.py trino-test-helper-0:/tmp || true + - script: kubectl cp -n $NAMESPACE ./check-oauth-login.py test-trino-0:/tmp diff --git a/tests/templates/kuttl/authentication/check-active-workers.py b/tests/templates/kuttl/authentication/check-active-workers.py deleted file mode 100755 index 993f4baf..00000000 --- a/tests/templates/kuttl/authentication/check-active-workers.py +++ /dev/null @@ -1,52 +0,0 @@ -#!/usr/bin/env python -import trino -import argparse -import sys - -if not sys.warnoptions: - import warnings -warnings.simplefilter("ignore") - - -def get_connection(username, password, namespace): - host = 'trino-coordinator-default-0.trino-coordinator-default.' + namespace + '.svc.cluster.local' - conn = trino.dbapi.connect( - host=host, - port=8443, - user=username, - http_scheme='https', - auth=trino.auth.BasicAuthentication(username, password), - ) - conn._http_session.verify = False - return conn - - -if __name__ == '__main__': - # Construct an argument parser - all_args = argparse.ArgumentParser() - - # Add arguments to the parser - all_args.add_argument("-u", "--user", required=True, - help="Username to connect as") - all_args.add_argument("-p", "--password", required=True, - help="Password for the user") - all_args.add_argument("-n", "--namespace", required=True, - help="Namespace the test is running in") - all_args.add_argument("-w", "--workers", required=True, - help="Expected amount of workers to be present") - - args = vars(all_args.parse_args()) - - expected_workers = args['workers'] - conn = get_connection(args['user'], args['password'], args['namespace']) - - cursor = conn.cursor() - cursor.execute("SELECT COUNT(*) as nodes FROM system.runtime.nodes WHERE coordinator=false AND state='active'") - - (active_workers,) = cursor.fetchone() - - if int(active_workers) != int(expected_workers): - print("Missmatch: [expected/active] workers [" + str(expected_workers) + "/" + str(active_workers) + "]") - exit(-1) - - print("Test check-active-workers.py succeeded!") diff --git a/tests/templates/kuttl/listener/check-active-workers.py b/tests/templates/kuttl/commons/check-active-workers.py similarity index 83% rename from tests/templates/kuttl/listener/check-active-workers.py rename to tests/templates/kuttl/commons/check-active-workers.py index b1b388cc..a3eb43e5 100755 --- a/tests/templates/kuttl/listener/check-active-workers.py +++ b/tests/templates/kuttl/commons/check-active-workers.py @@ -8,10 +8,9 @@ warnings.simplefilter("ignore") -def get_connection(username, password): - host = 'test-trino-coordinator-default' +def get_connection(username, password, coordinator): conn = trino.dbapi.connect( - host=host, + host=coordinator, port=8443, user=username, http_scheme='https', @@ -31,13 +30,15 @@ def get_connection(username, password): help="Username to connect as") all_args.add_argument("-p", "--password", required=True, help="Password for the user") + all_args.add_argument("-c", "--coordinator", required=True, + help="Trino Coordinator Host to connect to") all_args.add_argument("-w", "--workers", required=True, help="Expected amount of workers to be present") args = vars(all_args.parse_args()) expected_workers = args['workers'] - conn = get_connection(args['user'], args['password']) + conn = get_connection(args['user'], args['password'], args['coordinator']) cursor = conn.cursor() cursor.execute("SELECT COUNT(*) as nodes FROM system.runtime.nodes WHERE coordinator=false AND state='active'") diff --git a/tests/templates/kuttl/smoke_aws/check-opa.py b/tests/templates/kuttl/commons/check-opa.py similarity index 67% rename from tests/templates/kuttl/smoke_aws/check-opa.py rename to tests/templates/kuttl/commons/check-opa.py index e45e8511..6a7c07b2 100755 --- a/tests/templates/kuttl/smoke_aws/check-opa.py +++ b/tests/templates/kuttl/commons/check-opa.py @@ -10,14 +10,9 @@ warnings.simplefilter("ignore") -def get_connection(username, password, namespace): - host = ( - "trino-coordinator-default-0.trino-coordinator-default." - + namespace - + ".svc.cluster.local" - ) +def get_connection(username, password, coordinator): conn = trino.dbapi.connect( - host=host, + host=coordinator, port=8443, user=username, http_scheme="https", @@ -31,8 +26,8 @@ def get_connection(username, password, namespace): return conn -def test_user(user, password, namespace, query): - conn = get_connection(user, password, namespace) +def test_user(user, password, coordinator, query): + conn = get_connection(user, password, coordinator) cursor = conn.cursor() try: cursor.execute(query) @@ -46,27 +41,25 @@ def main(): # Construct an argument parser all_args = argparse.ArgumentParser() # Add arguments to the parser - all_args.add_argument( - "-n", "--namespace", required=True, help="Namespace the test is running in" - ) + all_args.add_argument("-c", "--coordinator", required=True, help="Trino Coordinator Host to connect to") args = vars(all_args.parse_args()) - namespace = args["namespace"] + coordinator = args["coordinator"] # We expect the admin user query to pass - if not test_user("admin", "admin", namespace, "SHOW CATALOGS"): + if not test_user("admin", "admin", coordinator, "SHOW CATALOGS"): print("User admin cannot show catalogs!") sys.exit(-1) # We expect the admin user query to pass - if not test_user("admin", "admin", namespace, "SHOW SCHEMAS FROM system"): + if not test_user("admin", "admin", coordinator, "SHOW SCHEMAS FROM system"): print("User admin cannot select schemas from system") sys.exit(-1) # We expect the bob query for catalogs to pass - if not test_user("bob", "bob", namespace, "SHOW CATALOGS"): + if not test_user("bob", "bob", coordinator, "SHOW CATALOGS"): print("User bob cannot show catalogs!") sys.exit(-1) # We expect the bob query for schemas to fail - if test_user("bob", "bob", namespace, "SHOW SCHEMAS FROM system"): + if test_user("bob", "bob", coordinator, "SHOW SCHEMAS FROM system"): print("User bob can show schemas from system. This should not be happening!") sys.exit(-1) diff --git a/tests/templates/kuttl/smoke_aws/check-s3.py b/tests/templates/kuttl/commons/check-s3.py similarity index 92% rename from tests/templates/kuttl/smoke_aws/check-s3.py rename to tests/templates/kuttl/commons/check-s3.py index e73bbc1d..3a0c5cd7 100755 --- a/tests/templates/kuttl/smoke_aws/check-s3.py +++ b/tests/templates/kuttl/commons/check-s3.py @@ -8,18 +8,12 @@ warnings.simplefilter("ignore") -def get_connection(username, password, namespace): - host = ( - "trino-coordinator-default-0.trino-coordinator-default." - + namespace - + ".svc.cluster.local" - ) +def get_connection(username, password, coordinator): # If you want to debug this locally use # kubectl -n kuttl-test-XXX port-forward svc/trino-coordinator-default 8443 # host = '127.0.0.1' - conn = trino.dbapi.connect( - host=host, + host=coordinator, port=8443, user=username, http_scheme="https", @@ -41,15 +35,15 @@ def run_query(connection, query): # Construct an argument parser all_args = argparse.ArgumentParser() # Add arguments to the parser - all_args.add_argument( - "-n", "--namespace", required=True, help="Namespace the test is running in" - ) + all_args.add_argument("-c", "--coordinator", required=True, help="Trino Coordinator Host to connect to") + all_args.add_argument("-b", "--bucket", required=True, help="The S3 bucket name to use") args = vars(all_args.parse_args()) - namespace = args["namespace"] + coordinator = args["coordinator"] + bucket_name = args["bucket"] print("Starting S3 tests...") - connection = get_connection("admin", "admin", namespace) + connection = get_connection("admin", "admin", coordinator) trino_version = run_query( connection, @@ -61,10 +55,9 @@ def run_query(connection, query): assert trino_version.isnumeric() assert trino_version == run_query(connection, "select version()")[0][0] - # WARNING (@NickLarsenNZ): Hard-coded bucket run_query( connection, - "CREATE SCHEMA IF NOT EXISTS hive.s3 WITH (location = 's3a://my-bucket/')", + f"CREATE SCHEMA IF NOT EXISTS hive.s3 WITH (location = 's3a://{bucket_name}/')", ) run_query(connection, "DROP TABLE IF EXISTS hive.s3.taxi_data") @@ -75,7 +68,7 @@ def run_query(connection, query): run_query( connection, - """ + f""" CREATE TABLE IF NOT EXISTS hive.s3.taxi_data ( vendor_id VARCHAR, tpep_pickup_datetime VARCHAR, @@ -84,7 +77,7 @@ def run_query(connection, query): trip_distance VARCHAR, ratecode_id VARCHAR ) WITH ( - external_location = 's3a://sble-s3-smoke-bucket-1/taxi-data/', + external_location = 's3a://{bucket_name}/taxi-data/', format = 'csv', skip_header_line_count = 1 ) diff --git a/tests/templates/kuttl/listener/21-assert.yaml b/tests/templates/kuttl/listener/21-assert.yaml index 353eb1d7..d0a89c69 100644 --- a/tests/templates/kuttl/listener/21-assert.yaml +++ b/tests/templates/kuttl/listener/21-assert.yaml @@ -3,4 +3,4 @@ apiVersion: kuttl.dev/v1beta1 kind: TestAssert timeout: 300 commands: - - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-active-workers.py -u admin -p admin -w 1 + - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-active-workers.py -u admin -p admin -c test-trino-coordinator-default -w 1 diff --git a/tests/templates/kuttl/listener/21-copy-scripts.yaml b/tests/templates/kuttl/listener/21-copy-scripts.yaml index 7b581302..52eb1beb 100644 --- a/tests/templates/kuttl/listener/21-copy-scripts.yaml +++ b/tests/templates/kuttl/listener/21-copy-scripts.yaml @@ -2,4 +2,5 @@ apiVersion: kuttl.dev/v1beta1 kind: TestStep commands: - - script: kubectl cp -n $NAMESPACE ./check-active-workers.py trino-test-helper-0:/tmp || true + - script: kubectl cp -n $NAMESPACE ../../../../templates/kuttl/commons/check-active-workers.py trino-test-helper-0:/tmp || true + \ No newline at end of file diff --git a/tests/templates/kuttl/smoke/21-assert.yaml b/tests/templates/kuttl/smoke/21-assert.yaml index 600736ce..e1f04235 100644 --- a/tests/templates/kuttl/smoke/21-assert.yaml +++ b/tests/templates/kuttl/smoke/21-assert.yaml @@ -3,6 +3,6 @@ apiVersion: kuttl.dev/v1beta1 kind: TestAssert timeout: 300 commands: - - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-active-workers.py -u admin -p admin -n $NAMESPACE -w 1 - - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-opa.py -n $NAMESPACE - - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-s3.py -n $NAMESPACE + - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-active-workers.py -u admin -p admin -c trino-coordinator-default -w 1 + - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-opa.py -c trino-coordinator-default + - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-s3.py -c trino-coordinator-default -b trino diff --git a/tests/templates/kuttl/smoke/21-copy-scripts.yaml b/tests/templates/kuttl/smoke/21-copy-scripts.yaml index f38f3274..7b879178 100644 --- a/tests/templates/kuttl/smoke/21-copy-scripts.yaml +++ b/tests/templates/kuttl/smoke/21-copy-scripts.yaml @@ -2,6 +2,6 @@ apiVersion: kuttl.dev/v1beta1 kind: TestStep commands: - - script: kubectl cp -n $NAMESPACE ./check-active-workers.py trino-test-helper-0:/tmp || true - - script: kubectl cp -n $NAMESPACE ./check-opa.py trino-test-helper-0:/tmp || true - - script: kubectl cp -n $NAMESPACE ./check-s3.py trino-test-helper-0:/tmp || true + - script: kubectl cp -n $NAMESPACE ../../../../templates/kuttl/commons/check-active-workers.py trino-test-helper-0:/tmp || true + - script: kubectl cp -n $NAMESPACE ../../../../templates/kuttl/commons/check-opa.py trino-test-helper-0:/tmp || true + - script: kubectl cp -n $NAMESPACE ../../../../templates/kuttl/commons/check-s3.py trino-test-helper-0:/tmp || true diff --git a/tests/templates/kuttl/smoke/31-assert.yaml b/tests/templates/kuttl/smoke/31-assert.yaml index 0690b385..b0e33099 100644 --- a/tests/templates/kuttl/smoke/31-assert.yaml +++ b/tests/templates/kuttl/smoke/31-assert.yaml @@ -3,6 +3,6 @@ apiVersion: kuttl.dev/v1beta1 kind: TestAssert timeout: 600 commands: - - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-active-workers.py -u admin -p admin -n $NAMESPACE -w 2 - - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-opa.py -n $NAMESPACE - - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-s3.py -n $NAMESPACE + - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-active-workers.py -u admin -p admin -c trino-coordinator-default -w 2 + - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-opa.py -c trino-coordinator-default + - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-s3.py -c trino-coordinator-default -b trino diff --git a/tests/templates/kuttl/smoke/check-active-workers.py b/tests/templates/kuttl/smoke/check-active-workers.py deleted file mode 100755 index e78e36b6..00000000 --- a/tests/templates/kuttl/smoke/check-active-workers.py +++ /dev/null @@ -1,53 +0,0 @@ -#!/usr/bin/env python -import trino -import argparse -import sys - -if not sys.warnoptions: - import warnings -warnings.simplefilter("ignore") - - -def get_connection(username, password, namespace): - host = 'trino-coordinator-default-0.trino-coordinator-default.' + namespace + '.svc.cluster.local' - conn = trino.dbapi.connect( - host=host, - port=8443, - user=username, - http_scheme='https', - auth=trino.auth.BasicAuthentication(username, password), - session_properties={"query_max_execution_time": "60s"}, - ) - conn._http_session.verify = False - return conn - - -if __name__ == '__main__': - # Construct an argument parser - all_args = argparse.ArgumentParser() - - # Add arguments to the parser - all_args.add_argument("-u", "--user", required=True, - help="Username to connect as") - all_args.add_argument("-p", "--password", required=True, - help="Password for the user") - all_args.add_argument("-n", "--namespace", required=True, - help="Namespace the test is running in") - all_args.add_argument("-w", "--workers", required=True, - help="Expected amount of workers to be present") - - args = vars(all_args.parse_args()) - - expected_workers = args['workers'] - conn = get_connection(args['user'], args['password'], args['namespace']) - - cursor = conn.cursor() - cursor.execute("SELECT COUNT(*) as nodes FROM system.runtime.nodes WHERE coordinator=false AND state='active'") - - (active_workers,) = cursor.fetchone() - - if int(active_workers) != int(expected_workers): - print("Missmatch: [expected/active] workers [" + str(expected_workers) + "/" + str(active_workers) + "]") - exit(-1) - - print("Test check-active-workers.py succeeded!") diff --git a/tests/templates/kuttl/smoke/check-opa.py b/tests/templates/kuttl/smoke/check-opa.py deleted file mode 100755 index e45e8511..00000000 --- a/tests/templates/kuttl/smoke/check-opa.py +++ /dev/null @@ -1,77 +0,0 @@ -#!/usr/bin/env python -import argparse -import sys - -import trino -import trino.exceptions as trino_ex - -if not sys.warnoptions: - import warnings -warnings.simplefilter("ignore") - - -def get_connection(username, password, namespace): - host = ( - "trino-coordinator-default-0.trino-coordinator-default." - + namespace - + ".svc.cluster.local" - ) - conn = trino.dbapi.connect( - host=host, - port=8443, - user=username, - http_scheme="https", - auth=trino.auth.BasicAuthentication(username, password), - verify=False, - # Commented out because it apparently breaks the OPA rules. - # With this enabled, the script fails to validate that user bob can - # show catalogs. - # session_properties={"query_max_execution_time": "60s"}, - ) - return conn - - -def test_user(user, password, namespace, query): - conn = get_connection(user, password, namespace) - cursor = conn.cursor() - try: - cursor.execute(query) - cursor.fetchone() - return True - except trino_ex.Error: - return False - - -def main(): - # Construct an argument parser - all_args = argparse.ArgumentParser() - # Add arguments to the parser - all_args.add_argument( - "-n", "--namespace", required=True, help="Namespace the test is running in" - ) - - args = vars(all_args.parse_args()) - namespace = args["namespace"] - - # We expect the admin user query to pass - if not test_user("admin", "admin", namespace, "SHOW CATALOGS"): - print("User admin cannot show catalogs!") - sys.exit(-1) - # We expect the admin user query to pass - if not test_user("admin", "admin", namespace, "SHOW SCHEMAS FROM system"): - print("User admin cannot select schemas from system") - sys.exit(-1) - # We expect the bob query for catalogs to pass - if not test_user("bob", "bob", namespace, "SHOW CATALOGS"): - print("User bob cannot show catalogs!") - sys.exit(-1) - # We expect the bob query for schemas to fail - if test_user("bob", "bob", namespace, "SHOW SCHEMAS FROM system"): - print("User bob can show schemas from system. This should not be happening!") - sys.exit(-1) - - print("Test check-opa.py succeeded!") - - -if __name__ == "__main__": - main() diff --git a/tests/templates/kuttl/smoke/check-s3.py b/tests/templates/kuttl/smoke/check-s3.py deleted file mode 100755 index 310d0870..00000000 --- a/tests/templates/kuttl/smoke/check-s3.py +++ /dev/null @@ -1,149 +0,0 @@ -#!/usr/bin/env python -import trino -import argparse -import sys - -if not sys.warnoptions: - import warnings -warnings.simplefilter("ignore") - - -def get_connection(username, password, namespace): - host = 'trino-coordinator-default-0.trino-coordinator-default.' + namespace + '.svc.cluster.local' - # If you want to debug this locally use - # kubectl -n kuttl-test-XXX port-forward svc/trino-coordinator-default 8443 - # host = '127.0.0.1' - - conn = trino.dbapi.connect( - host=host, - port=8443, - user=username, - http_scheme='https', - auth=trino.auth.BasicAuthentication(username, password), - session_properties={"query_max_execution_time": "60s"}, - ) - conn._http_session.verify = False - return conn - - -def run_query(connection, query): - print(f"[DEBUG] Executing query {query}") - cursor = connection.cursor() - cursor.execute(query) - return cursor.fetchall() - - -if __name__ == '__main__': - # Construct an argument parser - all_args = argparse.ArgumentParser() - # Add arguments to the parser - all_args.add_argument("-n", "--namespace", required=True, help="Namespace the test is running in") - - args = vars(all_args.parse_args()) - namespace = args["namespace"] - - print("Starting S3 tests...") - connection = get_connection("admin", "admin", namespace) - - trino_version = run_query(connection, "select node_version from system.runtime.nodes where coordinator = true and state = 'active'")[0][0] - print(f"[INFO] Testing against Trino version \"{trino_version}\"") - - assert len(trino_version) >= 3 - assert trino_version.isnumeric() - assert trino_version == run_query(connection, "select version()")[0][0] - - run_query(connection, "CREATE SCHEMA IF NOT EXISTS hive.minio WITH (location = 's3a://trino/')") - - run_query(connection, "DROP TABLE IF EXISTS hive.minio.taxi_data") - run_query(connection, "DROP TABLE IF EXISTS hive.minio.taxi_data_copy") - run_query(connection, "DROP TABLE IF EXISTS hive.minio.taxi_data_transformed") - run_query(connection, "DROP TABLE IF EXISTS hive.hdfs.taxi_data_copy") - run_query(connection, "DROP TABLE IF EXISTS iceberg.minio.taxi_data_copy_iceberg") - - run_query(connection, """ -CREATE TABLE IF NOT EXISTS hive.minio.taxi_data ( - vendor_id VARCHAR, - tpep_pickup_datetime VARCHAR, - tpep_dropoff_datetime VARCHAR, - passenger_count VARCHAR, - trip_distance VARCHAR, - ratecode_id VARCHAR -) WITH ( - external_location = 's3a://trino/taxi-data/', - format = 'csv', - skip_header_line_count = 1 -) - """) - assert run_query(connection, "SELECT COUNT(*) FROM hive.minio.taxi_data")[0][0] == 5000 - rows_written = run_query(connection, "CREATE TABLE IF NOT EXISTS hive.minio.taxi_data_copy AS SELECT * FROM hive.minio.taxi_data")[0][0] - assert rows_written == 5000 or rows_written == 0 - assert run_query(connection, "SELECT COUNT(*) FROM hive.minio.taxi_data_copy")[0][0] == 5000 - - rows_written = run_query(connection, """ -CREATE TABLE IF NOT EXISTS hive.minio.taxi_data_transformed AS -SELECT - CAST(vendor_id as BIGINT) as vendor_id, - tpep_pickup_datetime, - tpep_dropoff_datetime, - CAST(passenger_count as BIGINT) as passenger_count, - CAST(trip_distance as DOUBLE) as trip_distance, - CAST(ratecode_id as BIGINT) as ratecode_id -FROM hive.minio.taxi_data -""")[0][0] - assert rows_written == 5000 or rows_written == 0 - assert run_query(connection, "SELECT COUNT(*) FROM hive.minio.taxi_data_transformed")[0][0] == 5000 - - print("[INFO] Testing HDFS") - - run_query(connection, "CREATE SCHEMA IF NOT EXISTS hive.hdfs WITH (location = 'hdfs://hdfs/trino/')") - rows_written = run_query(connection, "CREATE TABLE IF NOT EXISTS hive.hdfs.taxi_data_copy AS SELECT * FROM hive.minio.taxi_data")[0][0] - assert rows_written == 5000 or rows_written == 0 - assert run_query(connection, "SELECT COUNT(*) FROM hive.hdfs.taxi_data_copy")[0][0] == 5000 - - print("[INFO] Testing Iceberg") - run_query(connection, "DROP TABLE IF EXISTS iceberg.minio.taxi_data_copy_iceberg") # Clean up table to don't fail an second run - assert run_query(connection, """ -CREATE TABLE IF NOT EXISTS iceberg.minio.taxi_data_copy_iceberg -WITH (partitioning = ARRAY['vendor_id', 'passenger_count'], format = 'parquet') -AS SELECT * FROM hive.minio.taxi_data -""")[0][0] == 5000 - # Check current count - assert run_query(connection, "SELECT COUNT(*) FROM iceberg.minio.taxi_data_copy_iceberg")[0][0] == 5000 - assert run_query(connection, 'SELECT COUNT(*) FROM iceberg.minio."taxi_data_copy_iceberg$snapshots"')[0][0] == 1 - assert run_query(connection, 'SELECT COUNT(*) FROM iceberg.minio."taxi_data_copy_iceberg$partitions"')[0][0] == 12 - assert run_query(connection, 'SELECT COUNT(*) FROM iceberg.minio."taxi_data_copy_iceberg$files"')[0][0] == 12 - - assert run_query(connection, "INSERT INTO iceberg.minio.taxi_data_copy_iceberg SELECT * FROM hive.minio.taxi_data")[0][0] == 5000 - - # Check current count - assert run_query(connection, "SELECT COUNT(*) FROM iceberg.minio.taxi_data_copy_iceberg")[0][0] == 10000 - assert run_query(connection, 'SELECT COUNT(*) FROM iceberg.minio."taxi_data_copy_iceberg$snapshots"')[0][0] == 2 - assert run_query(connection, 'SELECT COUNT(*) FROM iceberg.minio."taxi_data_copy_iceberg$partitions"')[0][0] == 12 - assert run_query(connection, 'SELECT COUNT(*) FROM iceberg.minio."taxi_data_copy_iceberg$files"')[0][0] == 24 - - if trino_version == '377': - # io.trino.spi.TrinoException: This connector [iceberg] does not support versioned tables - print("[INFO] Skipping the Iceberg tests reading versioned tables for trino version 377 as it does not support versioned tables") - else: - # Check count for first snapshot - first_snapshot = run_query(connection, 'select snapshot_id from iceberg.minio."taxi_data_copy_iceberg$snapshots" order by committed_at limit 1')[0][0] - assert run_query(connection, f"SELECT COUNT(*) FROM iceberg.minio.taxi_data_copy_iceberg FOR VERSION AS OF {first_snapshot}")[0][0] == 5000 - - # Compact files - run_query(connection, "ALTER TABLE iceberg.minio.taxi_data_copy_iceberg EXECUTE optimize") - - # Check current count - assert run_query(connection, "SELECT COUNT(*) FROM iceberg.minio.taxi_data_copy_iceberg")[0][0] == 10000 - assert run_query(connection, 'SELECT COUNT(*) FROM iceberg.minio."taxi_data_copy_iceberg$snapshots"')[0][0] == 3 - assert run_query(connection, 'SELECT COUNT(*) FROM iceberg.minio."taxi_data_copy_iceberg$partitions"')[0][0] == 12 - assert run_query(connection, 'SELECT COUNT(*) FROM iceberg.minio."taxi_data_copy_iceberg$files"')[0][0] == 12 # Compaction yeah :) - - # Test could be improved by also testing update and deletes - - # Test postgres connection - run_query(connection, 'SHOW SCHEMAS IN postgresgeneric') - run_query(connection, 'CREATE SCHEMA IF NOT EXISTS postgresgeneric.tpch') - run_query(connection, 'CREATE TABLE IF NOT EXISTS postgresgeneric.tpch.nation AS SELECT * FROM tpch.tiny.nation') - assert run_query(connection, "SELECT COUNT(*) FROM postgresgeneric.tpch.nation")[0][0] == 25 - - print("[SUCCESS] All tests in check-s3.py succeeded!") diff --git a/tests/templates/kuttl/smoke_aws/21-assert.yaml b/tests/templates/kuttl/smoke_aws/21-assert.yaml index 600736ce..f4abadd3 100644 --- a/tests/templates/kuttl/smoke_aws/21-assert.yaml +++ b/tests/templates/kuttl/smoke_aws/21-assert.yaml @@ -3,6 +3,6 @@ apiVersion: kuttl.dev/v1beta1 kind: TestAssert timeout: 300 commands: - - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-active-workers.py -u admin -p admin -n $NAMESPACE -w 1 - - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-opa.py -n $NAMESPACE - - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-s3.py -n $NAMESPACE + - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-active-workers.py -u admin -p admin -c trino-coordinator-default -w 1 + - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-opa.py -c trino-coordinator-default + - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-s3.py -c trino-coordinator-default -b my-bucket \ No newline at end of file diff --git a/tests/templates/kuttl/smoke_aws/21-copy-scripts.yaml b/tests/templates/kuttl/smoke_aws/21-copy-scripts.yaml index f38f3274..f3310a72 100644 --- a/tests/templates/kuttl/smoke_aws/21-copy-scripts.yaml +++ b/tests/templates/kuttl/smoke_aws/21-copy-scripts.yaml @@ -2,6 +2,6 @@ apiVersion: kuttl.dev/v1beta1 kind: TestStep commands: - - script: kubectl cp -n $NAMESPACE ./check-active-workers.py trino-test-helper-0:/tmp || true - - script: kubectl cp -n $NAMESPACE ./check-opa.py trino-test-helper-0:/tmp || true - - script: kubectl cp -n $NAMESPACE ./check-s3.py trino-test-helper-0:/tmp || true + - script: kubectl cp -n $NAMESPACE ../../../../templates/kuttl/commons/check-active-workers.py trino-test-helper-0:/tmp || true + - script: kubectl cp -n $NAMESPACE ../../../../templates/kuttl/commons/check-opa.py trino-test-helper-0:/tmp || true + - script: kubectl cp -n $NAMESPACE ../../../../templates/kuttl/commons/check-s3.py trino-test-helper-0:/tmp || true \ No newline at end of file diff --git a/tests/templates/kuttl/smoke_aws/check-active-workers.py b/tests/templates/kuttl/smoke_aws/check-active-workers.py deleted file mode 100755 index 1367e1d7..00000000 --- a/tests/templates/kuttl/smoke_aws/check-active-workers.py +++ /dev/null @@ -1,70 +0,0 @@ -#!/usr/bin/env python -import trino -import argparse -import sys - -if not sys.warnoptions: - import warnings -warnings.simplefilter("ignore") - - -def get_connection(username, password, namespace): - host = ( - "trino-coordinator-default-0.trino-coordinator-default." - + namespace - + ".svc.cluster.local" - ) - conn = trino.dbapi.connect( - host=host, - port=8443, - user=username, - http_scheme="https", - auth=trino.auth.BasicAuthentication(username, password), - session_properties={"query_max_execution_time": "60s"}, - ) - conn._http_session.verify = False - return conn - - -if __name__ == "__main__": - # Construct an argument parser - all_args = argparse.ArgumentParser() - - # Add arguments to the parser - all_args.add_argument("-u", "--user", required=True, help="Username to connect as") - all_args.add_argument( - "-p", "--password", required=True, help="Password for the user" - ) - all_args.add_argument( - "-n", "--namespace", required=True, help="Namespace the test is running in" - ) - all_args.add_argument( - "-w", - "--workers", - required=True, - help="Expected amount of workers to be present", - ) - - args = vars(all_args.parse_args()) - - expected_workers = args["workers"] - conn = get_connection(args["user"], args["password"], args["namespace"]) - - cursor = conn.cursor() - cursor.execute( - "SELECT COUNT(*) as nodes FROM system.runtime.nodes WHERE coordinator=false AND state='active'" - ) - - (active_workers,) = cursor.fetchone() - - if int(active_workers) != int(expected_workers): - print( - "Missmatch: [expected/active] workers [" - + str(expected_workers) - + "/" - + str(active_workers) - + "]" - ) - exit(-1) - - print("Test check-active-workers.py succeeded!") From 6061ae998e21efe13c51b6d9b9fcf9a650df2a1c Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Thu, 5 Jun 2025 17:11:21 +0200 Subject: [PATCH 14/38] new lines --- tests/templates/kuttl/listener/21-copy-scripts.yaml | 1 - tests/templates/kuttl/smoke_aws/21-assert.yaml | 2 +- tests/templates/kuttl/smoke_aws/21-copy-scripts.yaml | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/templates/kuttl/listener/21-copy-scripts.yaml b/tests/templates/kuttl/listener/21-copy-scripts.yaml index 52eb1beb..9a67ea90 100644 --- a/tests/templates/kuttl/listener/21-copy-scripts.yaml +++ b/tests/templates/kuttl/listener/21-copy-scripts.yaml @@ -3,4 +3,3 @@ apiVersion: kuttl.dev/v1beta1 kind: TestStep commands: - script: kubectl cp -n $NAMESPACE ../../../../templates/kuttl/commons/check-active-workers.py trino-test-helper-0:/tmp || true - \ No newline at end of file diff --git a/tests/templates/kuttl/smoke_aws/21-assert.yaml b/tests/templates/kuttl/smoke_aws/21-assert.yaml index f4abadd3..52a1abb6 100644 --- a/tests/templates/kuttl/smoke_aws/21-assert.yaml +++ b/tests/templates/kuttl/smoke_aws/21-assert.yaml @@ -5,4 +5,4 @@ timeout: 300 commands: - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-active-workers.py -u admin -p admin -c trino-coordinator-default -w 1 - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-opa.py -c trino-coordinator-default - - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-s3.py -c trino-coordinator-default -b my-bucket \ No newline at end of file + - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-s3.py -c trino-coordinator-default -b my-bucket diff --git a/tests/templates/kuttl/smoke_aws/21-copy-scripts.yaml b/tests/templates/kuttl/smoke_aws/21-copy-scripts.yaml index f3310a72..7b879178 100644 --- a/tests/templates/kuttl/smoke_aws/21-copy-scripts.yaml +++ b/tests/templates/kuttl/smoke_aws/21-copy-scripts.yaml @@ -4,4 +4,4 @@ kind: TestStep commands: - script: kubectl cp -n $NAMESPACE ../../../../templates/kuttl/commons/check-active-workers.py trino-test-helper-0:/tmp || true - script: kubectl cp -n $NAMESPACE ../../../../templates/kuttl/commons/check-opa.py trino-test-helper-0:/tmp || true - - script: kubectl cp -n $NAMESPACE ../../../../templates/kuttl/commons/check-s3.py trino-test-helper-0:/tmp || true \ No newline at end of file + - script: kubectl cp -n $NAMESPACE ../../../../templates/kuttl/commons/check-s3.py trino-test-helper-0:/tmp || true From 7d39acbccd67bc6fb344f579ae88a0843943fca1 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Thu, 5 Jun 2025 17:33:13 +0200 Subject: [PATCH 15/38] fix tls test --- tests/templates/kuttl/tls/10-assert.yaml.j2 | 6 +----- tests/templates/kuttl/tls/check-tls.py | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/templates/kuttl/tls/10-assert.yaml.j2 b/tests/templates/kuttl/tls/10-assert.yaml.j2 index b19c14f9..e7680d87 100644 --- a/tests/templates/kuttl/tls/10-assert.yaml.j2 +++ b/tests/templates/kuttl/tls/10-assert.yaml.j2 @@ -22,13 +22,9 @@ status: apiVersion: v1 kind: Service metadata: - name: trino-coordinator + name: trino-coordinator-default spec: ports: - - name: metrics - port: 8081 - protocol: TCP - targetPort: 8081 {% if test_scenario['values']['use-tls'] == 'false' %} - name: http port: 8080 diff --git a/tests/templates/kuttl/tls/check-tls.py b/tests/templates/kuttl/tls/check-tls.py index 4c88edea..d35226a8 100755 --- a/tests/templates/kuttl/tls/check-tls.py +++ b/tests/templates/kuttl/tls/check-tls.py @@ -65,7 +65,7 @@ def read_json(config_path): namespace = args["namespace"] conf = read_json("/tmp/test-config.json") # config file to indicate our test script if auth / tls is used or not - coordinator_host = 'trino-coordinator-default-0.trino-coordinator-default.' + namespace + '.svc.cluster.local' + coordinator_host = 'trino-coordinator-default-metrics.' + namespace + '.svc.cluster.local' trusted_ca = "/stackable/trusted/ca.crt" # will be mounted from secret op untrusted_ca = "/stackable/untrusted-cert.crt" # some random CA query = "SHOW CATALOGS" From 09c7fb42545b1f8d1497fb029dae4f59a06ef386 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Thu, 5 Jun 2025 18:07:27 +0200 Subject: [PATCH 16/38] fix authentication test --- tests/templates/kuttl/authentication/05-assert.yaml | 2 +- .../authentication/05-install-test-trino.yaml.j2 | 12 ++++++------ tests/templates/kuttl/authentication/20-assert.yaml | 10 +++++----- .../kuttl/authentication/20-test-trino.yaml | 2 +- tests/templates/kuttl/authentication/31-assert.yaml | 2 +- tests/templates/kuttl/authentication/33-assert.yaml | 2 +- .../kuttl/listener/10-install-trino.yaml.j2 | 2 -- 7 files changed, 15 insertions(+), 17 deletions(-) diff --git a/tests/templates/kuttl/authentication/05-assert.yaml b/tests/templates/kuttl/authentication/05-assert.yaml index 1257ec56..168d5d8f 100644 --- a/tests/templates/kuttl/authentication/05-assert.yaml +++ b/tests/templates/kuttl/authentication/05-assert.yaml @@ -6,7 +6,7 @@ timeout: 300 apiVersion: apps/v1 kind: StatefulSet metadata: - name: test-trino + name: trino-test-helper status: readyReplicas: 1 replicas: 1 diff --git a/tests/templates/kuttl/authentication/05-install-test-trino.yaml.j2 b/tests/templates/kuttl/authentication/05-install-test-trino.yaml.j2 index 41d2f209..d8fba82c 100644 --- a/tests/templates/kuttl/authentication/05-install-test-trino.yaml.j2 +++ b/tests/templates/kuttl/authentication/05-install-test-trino.yaml.j2 @@ -2,18 +2,18 @@ apiVersion: apps/v1 kind: StatefulSet metadata: - name: test-trino + name: trino-test-helper labels: - app: test-trino + app: trino-test-helper spec: replicas: 1 selector: matchLabels: - app: test-trino + app: trino-test-helper template: metadata: labels: - app: test-trino + app: trino-test-helper spec: serviceAccount: integration-tests-sa {% if test_scenario['values']['openshift'] == 'false' %} @@ -21,7 +21,7 @@ spec: fsGroup: 1000 {% endif %} containers: - - name: test-trino + - name: trino-test-helper image: oci.stackable.tech/sdp/testing-tools:0.2.0-stackable0.0.0-dev command: ["sleep", "infinity"] volumeMounts: @@ -36,4 +36,4 @@ spec: driver: secrets.stackable.tech volumeAttributes: secrets.stackable.tech/class: tls - secrets.stackable.tech/scope: pod + secrets.stackable.tech/scope: pod,node diff --git a/tests/templates/kuttl/authentication/20-assert.yaml b/tests/templates/kuttl/authentication/20-assert.yaml index ffcb4611..8f38517c 100644 --- a/tests/templates/kuttl/authentication/20-assert.yaml +++ b/tests/templates/kuttl/authentication/20-assert.yaml @@ -4,10 +4,10 @@ kind: TestAssert timeout: 300 commands: # file - - script: kubectl exec -n $NAMESPACE test-trino-0 -- python /tmp/check-active-workers.py -u test_user_1 -p test_user_1 -c trino-coordinator-default -w 1 - - script: kubectl exec -n $NAMESPACE test-trino-0 -- python /tmp/check-active-workers.py -u test_user_2_other -p test_user_2_other -c trino-coordinator-default -w 1 + - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-active-workers.py -u test_user_1 -p test_user_1 -c trino-coordinator-default-metrics.$NAMESPACE.svc.cluster.local -w 1 + - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-active-workers.py -u test_user_2_other -p test_user_2_other -c trino-coordinator-default-metrics.$NAMESPACE.svc.cluster.local -w 1 # ldap - - script: kubectl exec -n $NAMESPACE test-trino-0 -- python /tmp/check-active-workers.py -u integrationtest -p integrationtest -c trino-coordinator-default -w 1 - - script: kubectl exec -n $NAMESPACE test-trino-0 -- python /tmp/check-active-workers.py -u integrationtest-other -p integrationtest-other -c trino-coordinator-default -w 1 + - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-active-workers.py -u integrationtest -p integrationtest -c trino-coordinator-default-metrics.$NAMESPACE.svc.cluster.local -w 1 + - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-active-workers.py -u integrationtest-other -p integrationtest-other -c trino-coordinator-default-metrics.$NAMESPACE.svc.cluster.local -w 1 # oidc/oauth2 - - script: kubectl exec -n $NAMESPACE test-trino-0 -- python /tmp/check-oauth-login.py https://trino-coordinator-default:8443/ui/ + - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-oauth-login.py https://trino-coordinator-default-metrics.$NAMESPACE.svc.cluster.local:8443/ui/ diff --git a/tests/templates/kuttl/authentication/20-test-trino.yaml b/tests/templates/kuttl/authentication/20-test-trino.yaml index ccafc5e6..f215bfb9 100644 --- a/tests/templates/kuttl/authentication/20-test-trino.yaml +++ b/tests/templates/kuttl/authentication/20-test-trino.yaml @@ -3,4 +3,4 @@ apiVersion: kuttl.dev/v1beta1 kind: TestStep commands: - script: kubectl cp -n $NAMESPACE ../../../../templates/kuttl/commons/check-active-workers.py trino-test-helper-0:/tmp || true - - script: kubectl cp -n $NAMESPACE ./check-oauth-login.py test-trino-0:/tmp + - script: kubectl cp -n $NAMESPACE ./check-oauth-login.py trino-test-helper-0:/tmp diff --git a/tests/templates/kuttl/authentication/31-assert.yaml b/tests/templates/kuttl/authentication/31-assert.yaml index f3aa92eb..5b2948fa 100644 --- a/tests/templates/kuttl/authentication/31-assert.yaml +++ b/tests/templates/kuttl/authentication/31-assert.yaml @@ -5,4 +5,4 @@ timeout: 600 commands: # file # new user? - - script: kubectl exec -n $NAMESPACE test-trino-0 -- python /tmp/check-active-workers.py -u hot_reloaded -p hot_reloaded -n $NAMESPACE -w 1 + - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-active-workers.py -u hot_reloaded -p hot_reloaded -c trino-coordinator-default-metrics.$NAMESPACE.svc.cluster.local -w 1 diff --git a/tests/templates/kuttl/authentication/33-assert.yaml b/tests/templates/kuttl/authentication/33-assert.yaml index 7a84cfde..5794c83e 100644 --- a/tests/templates/kuttl/authentication/33-assert.yaml +++ b/tests/templates/kuttl/authentication/33-assert.yaml @@ -5,4 +5,4 @@ timeout: 600 commands: # We use the check-active-workers script for the login. Since we do want to wait until we cannot log in anymore # we flip the return value in the end. - - script: kubectl exec -n $NAMESPACE test-trino-0 -- python /tmp/check-active-workers.py -u hot_reloaded -p hot_reloaded -n $NAMESPACE -w 1; if [ $? -eq 0 ]; then exit 1; fi + - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-active-workers.py -u hot_reloaded -p hot_reloaded -c trino-coordinator-default-metrics.$NAMESPACE.svc.cluster.local -w 1; if [ $? -eq 0 ]; then exit 1; fi diff --git a/tests/templates/kuttl/listener/10-install-trino.yaml.j2 b/tests/templates/kuttl/listener/10-install-trino.yaml.j2 index 989c03ba..6ee1f9e8 100644 --- a/tests/templates/kuttl/listener/10-install-trino.yaml.j2 +++ b/tests/templates/kuttl/listener/10-install-trino.yaml.j2 @@ -52,5 +52,3 @@ spec: roleGroups: default: replicas: 1 - - From 3d4e983499cb46e65c0e76ddfe0e1712db055d4a Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Fri, 6 Jun 2025 09:42:42 +0200 Subject: [PATCH 17/38] fix opa test --- tests/templates/kuttl/opa-authorization/check-opa.py.j2 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/templates/kuttl/opa-authorization/check-opa.py.j2 b/tests/templates/kuttl/opa-authorization/check-opa.py.j2 index bafd398e..4c8fe731 100755 --- a/tests/templates/kuttl/opa-authorization/check-opa.py.j2 +++ b/tests/templates/kuttl/opa-authorization/check-opa.py.j2 @@ -515,7 +515,7 @@ class TestOpa: @staticmethod def get_connection(username, password, namespace, impersonation=None): connection = trino.dbapi.connect( - host="trino-coordinator.{0}.svc.cluster.local".format(namespace), + host="trino-coordinator-default.{0}.svc.cluster.local".format(namespace), port=8443, user=impersonation if impersonation is not None else username, http_scheme="https", From 0f8e2cbcaeec172bb32af9903c8ebee18144a34f Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Fri, 6 Jun 2025 12:18:53 +0200 Subject: [PATCH 18/38] attempt to fix --- .../pages/usage-guide/listenerclass.adoc | 2 +- .../kuttl/authentication/check-oauth-login.py | 15 ++-- .../kuttl/commons/check-active-workers.py | 44 ++++++++---- tests/templates/kuttl/commons/check-opa.py | 7 +- tests/templates/kuttl/commons/check-s3.py | 11 ++- .../kuttl/logging/test_log_aggregation.py | 33 ++++----- tests/templates/kuttl/tls/check-tls.py | 70 +++++++++++++------ 7 files changed, 118 insertions(+), 64 deletions(-) diff --git a/docs/modules/trino/pages/usage-guide/listenerclass.adoc b/docs/modules/trino/pages/usage-guide/listenerclass.adoc index ddb8e0d3..5faa9286 100644 --- a/docs/modules/trino/pages/usage-guide/listenerclass.adoc +++ b/docs/modules/trino/pages/usage-guide/listenerclass.adoc @@ -10,7 +10,7 @@ spec: coordinators: config: listenerClass: external-unstable # <1> - ... + ... workers: ... ---- diff --git a/tests/templates/kuttl/authentication/check-oauth-login.py b/tests/templates/kuttl/authentication/check-oauth-login.py index d2c5ceb8..7729f0fa 100644 --- a/tests/templates/kuttl/authentication/check-oauth-login.py +++ b/tests/templates/kuttl/authentication/check-oauth-login.py @@ -5,6 +5,7 @@ of the Keycloak page and posts the credentials of a test user to it. Finally it tests that Keycloak redirects back to the original page. """ + import logging import requests import sys @@ -19,17 +20,15 @@ def test_login_flow(login_url): result.raise_for_status() - html = BeautifulSoup(result.text, 'html.parser') - authenticate_url = html.form['action'] - result = session.post(authenticate_url, data={ - 'username': "test", - 'password': "test" - }) + html = BeautifulSoup(result.text, "html.parser") + authenticate_url = html.form["action"] + result = session.post( + authenticate_url, data={"username": "test", "password": "test"} + ) result.raise_for_status() - assert result.url == login_url, \ - "Redirection to the Trino UI expected" + assert result.url == login_url, "Redirection to the Trino UI expected" def main(): diff --git a/tests/templates/kuttl/commons/check-active-workers.py b/tests/templates/kuttl/commons/check-active-workers.py index a3eb43e5..431d2dee 100755 --- a/tests/templates/kuttl/commons/check-active-workers.py +++ b/tests/templates/kuttl/commons/check-active-workers.py @@ -13,7 +13,7 @@ def get_connection(username, password, coordinator): host=coordinator, port=8443, user=username, - http_scheme='https', + http_scheme="https", auth=trino.auth.BasicAuthentication(username, password), session_properties={"query_max_execution_time": "60s"}, ) @@ -21,32 +21,48 @@ def get_connection(username, password, coordinator): return conn -if __name__ == '__main__': +if __name__ == "__main__": # Construct an argument parser all_args = argparse.ArgumentParser() # Add arguments to the parser - all_args.add_argument("-u", "--user", required=True, - help="Username to connect as") - all_args.add_argument("-p", "--password", required=True, - help="Password for the user") - all_args.add_argument("-c", "--coordinator", required=True, - help="Trino Coordinator Host to connect to") - all_args.add_argument("-w", "--workers", required=True, - help="Expected amount of workers to be present") + all_args.add_argument("-u", "--user", required=True, help="Username to connect as") + all_args.add_argument( + "-p", "--password", required=True, help="Password for the user" + ) + all_args.add_argument( + "-c", + "--coordinator", + required=True, + help="Trino Coordinator Host to connect to", + ) + all_args.add_argument( + "-w", + "--workers", + required=True, + help="Expected amount of workers to be present", + ) args = vars(all_args.parse_args()) - expected_workers = args['workers'] - conn = get_connection(args['user'], args['password'], args['coordinator']) + expected_workers = args["workers"] + conn = get_connection(args["user"], args["password"], args["coordinator"]) cursor = conn.cursor() - cursor.execute("SELECT COUNT(*) as nodes FROM system.runtime.nodes WHERE coordinator=false AND state='active'") + cursor.execute( + "SELECT COUNT(*) as nodes FROM system.runtime.nodes WHERE coordinator=false AND state='active'" + ) (active_workers,) = cursor.fetchone() if int(active_workers) != int(expected_workers): - print("Missmatch: [expected/active] workers [" + str(expected_workers) + "/" + str(active_workers) + "]") + print( + "Missmatch: [expected/active] workers [" + + str(expected_workers) + + "/" + + str(active_workers) + + "]" + ) exit(-1) print("Test check-active-workers.py succeeded!") diff --git a/tests/templates/kuttl/commons/check-opa.py b/tests/templates/kuttl/commons/check-opa.py index 6a7c07b2..9cec9621 100755 --- a/tests/templates/kuttl/commons/check-opa.py +++ b/tests/templates/kuttl/commons/check-opa.py @@ -41,7 +41,12 @@ def main(): # Construct an argument parser all_args = argparse.ArgumentParser() # Add arguments to the parser - all_args.add_argument("-c", "--coordinator", required=True, help="Trino Coordinator Host to connect to") + all_args.add_argument( + "-c", + "--coordinator", + required=True, + help="Trino Coordinator Host to connect to", + ) args = vars(all_args.parse_args()) coordinator = args["coordinator"] diff --git a/tests/templates/kuttl/commons/check-s3.py b/tests/templates/kuttl/commons/check-s3.py index 3a0c5cd7..a3d26122 100755 --- a/tests/templates/kuttl/commons/check-s3.py +++ b/tests/templates/kuttl/commons/check-s3.py @@ -35,8 +35,15 @@ def run_query(connection, query): # Construct an argument parser all_args = argparse.ArgumentParser() # Add arguments to the parser - all_args.add_argument("-c", "--coordinator", required=True, help="Trino Coordinator Host to connect to") - all_args.add_argument("-b", "--bucket", required=True, help="The S3 bucket name to use") + all_args.add_argument( + "-c", + "--coordinator", + required=True, + help="Trino Coordinator Host to connect to", + ) + all_args.add_argument( + "-b", "--bucket", required=True, help="The S3 bucket name to use" + ) args = vars(all_args.parse_args()) coordinator = args["coordinator"] diff --git a/tests/templates/kuttl/logging/test_log_aggregation.py b/tests/templates/kuttl/logging/test_log_aggregation.py index 83d653cd..811af20b 100755 --- a/tests/templates/kuttl/logging/test_log_aggregation.py +++ b/tests/templates/kuttl/logging/test_log_aggregation.py @@ -4,9 +4,9 @@ def check_sent_events(): response = requests.post( - 'http://trino-vector-aggregator:8686/graphql', + "http://trino-vector-aggregator:8686/graphql", json={ - 'query': """ + "query": """ { transforms(first:100) { nodes { @@ -20,29 +20,30 @@ def check_sent_events(): } } """ - } + }, ) - assert response.status_code == 200, \ - 'Cannot access the API of the vector aggregator.' + assert response.status_code == 200, ( + "Cannot access the API of the vector aggregator." + ) result = response.json() - transforms = result['data']['transforms']['nodes'] + transforms = result["data"]["transforms"]["nodes"] for transform in transforms: - sentEvents = transform['metrics']['sentEventsTotal'] - componentId = transform['componentId'] + sentEvents = transform["metrics"]["sentEventsTotal"] + componentId = transform["componentId"] - if componentId == 'filteredInvalidEvents': - assert sentEvents is None or \ - sentEvents['sentEventsTotal'] == 0, \ - 'Invalid log events were sent.' + if componentId == "filteredInvalidEvents": + assert sentEvents is None or sentEvents["sentEventsTotal"] == 0, ( + "Invalid log events were sent." + ) else: - assert sentEvents is not None and \ - sentEvents['sentEventsTotal'] > 0, \ + assert sentEvents is not None and sentEvents["sentEventsTotal"] > 0, ( f'No events were sent in "{componentId}".' + ) -if __name__ == '__main__': +if __name__ == "__main__": check_sent_events() - print('Test successful!') + print("Test successful!") diff --git a/tests/templates/kuttl/tls/check-tls.py b/tests/templates/kuttl/tls/check-tls.py index d35226a8..dc7e7515 100755 --- a/tests/templates/kuttl/tls/check-tls.py +++ b/tests/templates/kuttl/tls/check-tls.py @@ -2,7 +2,6 @@ import trino import argparse import json -import requests def get_http_connection(host, user): @@ -10,17 +9,13 @@ def get_http_connection(host, user): host=host, port=8080, user=user, - http_scheme='http', + http_scheme="http", ) def get_https_connection(host, user, verify): return trino.dbapi.connect( - host=host, - port=8443, - user=user, - http_scheme='https', - verify=verify + host=host, port=8443, user=user, http_scheme="https", verify=verify ) @@ -29,9 +24,9 @@ def get_authenticated_https_connection(host, user, password, verify): host=host, port=8443, user=user, - http_scheme='https', + http_scheme="https", auth=trino.auth.BasicAuthentication(user, password), - verify=verify + verify=verify, ) @@ -50,29 +45,37 @@ def test_query_failure(conn, query, expected_error, failure_message): def read_json(config_path): - with open(config_path, 'r') as stream: + with open(config_path, "r") as stream: config = json.load(stream) return config -if __name__ == '__main__': +if __name__ == "__main__": # Construct an argument parser all_args = argparse.ArgumentParser() # Add arguments to the parser - all_args.add_argument("-n", "--namespace", required=True, help="Namespace the test is running in") + all_args.add_argument( + "-n", "--namespace", required=True, help="Namespace the test is running in" + ) args = vars(all_args.parse_args()) namespace = args["namespace"] - conf = read_json("/tmp/test-config.json") # config file to indicate our test script if auth / tls is used or not - coordinator_host = 'trino-coordinator-default-metrics.' + namespace + '.svc.cluster.local' + conf = read_json( + "/tmp/test-config.json" + ) # config file to indicate our test script if auth / tls is used or not + coordinator_host = ( + "trino-coordinator-default-metrics." + namespace + ".svc.cluster.local" + ) trusted_ca = "/stackable/trusted/ca.crt" # will be mounted from secret op untrusted_ca = "/stackable/untrusted-cert.crt" # some random CA query = "SHOW CATALOGS" # We expect these to work if conf["useAuthentication"]: - conn = get_authenticated_https_connection(coordinator_host, "admin", "admin", trusted_ca) + conn = get_authenticated_https_connection( + coordinator_host, "admin", "admin", trusted_ca + ) test_query(conn, query) elif conf["useTls"]: conn = get_https_connection(coordinator_host, "admin", trusted_ca) @@ -83,14 +86,37 @@ def read_json(config_path): # We expect these to fail if conf["useAuthentication"]: - conn = get_authenticated_https_connection(coordinator_host, "admin", "admin", untrusted_ca) - test_query_failure(conn, query, trino.exceptions.TrinoConnectionError, "Could connect with wrong certificate") - conn = get_authenticated_https_connection(coordinator_host, "admin", "wrong_password", trusted_ca) - test_query_failure(conn, query, trino.exceptions.HttpError, "Could connect with wrong password") - conn = get_authenticated_https_connection(coordinator_host, "wrong_user", "wrong_password", trusted_ca) - test_query_failure(conn, query, trino.exceptions.HttpError, "Could connect with wrong user and password") + conn = get_authenticated_https_connection( + coordinator_host, "admin", "admin", untrusted_ca + ) + test_query_failure( + conn, + query, + trino.exceptions.TrinoConnectionError, + "Could connect with wrong certificate", + ) + conn = get_authenticated_https_connection( + coordinator_host, "admin", "wrong_password", trusted_ca + ) + test_query_failure( + conn, query, trino.exceptions.HttpError, "Could connect with wrong password" + ) + conn = get_authenticated_https_connection( + coordinator_host, "wrong_user", "wrong_password", trusted_ca + ) + test_query_failure( + conn, + query, + trino.exceptions.HttpError, + "Could connect with wrong user and password", + ) elif conf["useTls"]: conn = get_https_connection(coordinator_host, "admin", untrusted_ca) - test_query_failure(conn, query, trino.exceptions.TrinoConnectionError, "Could connect with wrong certificate") + test_query_failure( + conn, + query, + trino.exceptions.TrinoConnectionError, + "Could connect with wrong certificate", + ) print("All TLS tests finished successfully!") From fcc228697d61821366109583e97ba97ea8f544bc Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Fri, 6 Jun 2025 14:39:58 +0200 Subject: [PATCH 19/38] adapt getting started to listener --- docs/modules/trino/examples/getting_started/code/trino.yaml | 3 ++- docs/modules/trino/examples/getting_started/code/trino.yaml.j2 | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/modules/trino/examples/getting_started/code/trino.yaml b/docs/modules/trino/examples/getting_started/code/trino.yaml index d187bf61..b9bb4644 100644 --- a/docs/modules/trino/examples/getting_started/code/trino.yaml +++ b/docs/modules/trino/examples/getting_started/code/trino.yaml @@ -10,8 +10,9 @@ spec: catalogLabelSelector: matchLabels: trino: simple-trino - listenerClass: external-unstable coordinators: + config: + listenerClass: external-unstable roleGroups: default: replicas: 1 diff --git a/docs/modules/trino/examples/getting_started/code/trino.yaml.j2 b/docs/modules/trino/examples/getting_started/code/trino.yaml.j2 index d187bf61..b9bb4644 100644 --- a/docs/modules/trino/examples/getting_started/code/trino.yaml.j2 +++ b/docs/modules/trino/examples/getting_started/code/trino.yaml.j2 @@ -10,8 +10,9 @@ spec: catalogLabelSelector: matchLabels: trino: simple-trino - listenerClass: external-unstable coordinators: + config: + listenerClass: external-unstable roleGroups: default: replicas: 1 From 0c24393fe206290f99101ca174a20847094dbba1 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Tue, 17 Jun 2025 16:54:13 +0200 Subject: [PATCH 20/38] use role level listener --- deploy/helm/trino-operator/crds/crds.yaml | 12 +-- rust/operator-binary/src/controller.rs | 76 +++++---------- rust/operator-binary/src/crd/mod.rs | 92 +++++++++++-------- rust/operator-binary/src/listener.rs | 45 ++------- tests/templates/kuttl/listener/10-assert.yaml | 2 +- .../kuttl/listener/10-install-trino.yaml.j2 | 6 +- tests/templates/kuttl/listener/21-assert.yaml | 2 +- 7 files changed, 91 insertions(+), 144 deletions(-) diff --git a/deploy/helm/trino-operator/crds/crds.yaml b/deploy/helm/trino-operator/crds/crds.yaml index 039ed464..1d2ec122 100644 --- a/deploy/helm/trino-operator/crds/crds.yaml +++ b/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: {} diff --git a/rust/operator-binary/src/controller.rs b/rust/operator-binary/src/controller.rs index c17b3fc3..b1d40a58 100644 --- a/rust/operator-binary/src/controller.rs +++ b/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 { - let mut ports = vec![ServicePort { +fn service_ports() -> Vec { + 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 { diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs index da4cc09c..183ae87c 100644 --- a/rust/operator-binary/src/crd/mod.rs +++ b/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>, + Option>, // no doc - it's in the struct. #[serde(default, skip_serializing_if = "Option::is_none")] pub workers: Option>, } + // 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, } - #[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 { + 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-`. - 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, + fragment: Role, ) -> Role { 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, diff --git a/rust/operator-binary/src/listener.rs b/rust/operator-binary/src/listener.rs index 08d3acc6..e324bf01 100644 --- a/rust/operator-binary/src/listener.rs +++ b/rust/operator-binary/src/listener.rs @@ -7,11 +7,10 @@ use stackable_operator::{ ListenerReference, }, }, - config::merge::Merge, crd::listener::v1alpha1::{Listener, ListenerPort, ListenerSpec}, k8s_openapi::api::core::v1::PersistentVolumeClaim, + kube::ResourceExt, kvp::{Labels, ObjectLabels}, - role_utils::RoleGroupRef, }; use crate::crd::{TrinoRole, v1alpha1}; @@ -77,48 +76,16 @@ pub fn build_group_listener_pvc( /// The name of the group-listener provided for a specific role-group. /// Coordinator(s) will use this group listener so that only one load balancer /// is needed (per role group). -pub fn group_listener_name( - role: &TrinoRole, - role_group_ref: &RoleGroupRef, -) -> Option { +pub fn group_listener_name(trino: &v1alpha1::TrinoCluster, role: &TrinoRole) -> Option { match role { - TrinoRole::Coordinator => Some(role_group_ref.object_name()), + TrinoRole::Coordinator => Some(format!( + "{cluster_name}-{role}", + cluster_name = trino.name_any() + )), TrinoRole::Worker => None, } } -pub fn merged_listener_class( - trino: &v1alpha1::TrinoCluster, - role: &TrinoRole, - rolegroup_name: &String, -) -> Option { - if role == &TrinoRole::Coordinator { - if let Some(coordinators) = trino.spec.coordinators.as_ref() { - let conf_defaults = Some("cluster-internal".to_string()); - let mut conf_role = coordinators.config.config.listener_class.to_owned(); - let mut conf_rolegroup = coordinators - .role_groups - .get(rolegroup_name) - .map(|rg| rg.config.config.listener_class.clone()) - // TODO: Discuss in review - // This should always be Some() due to the reconcile loop (role, role_group) in controller.rs - // We do not want to use expect() here since its only a 99.99%. E.g. probably not possible to apply - // and empty string / null via yaml, but could be via json - not sure? - .unwrap_or_default(); - - conf_role.merge(&conf_defaults); - conf_rolegroup.merge(&conf_role); - - tracing::debug!("Merged listener-class: {:?}", conf_rolegroup); - conf_rolegroup - } else { - None - } - } else { - None - } -} - /// We only use the http/https port here and intentionally omit the metrics one. fn listener_ports(trino: &v1alpha1::TrinoCluster) -> Vec { let name = trino.exposed_protocol().to_string(); diff --git a/tests/templates/kuttl/listener/10-assert.yaml b/tests/templates/kuttl/listener/10-assert.yaml index 3705c330..24ae199a 100644 --- a/tests/templates/kuttl/listener/10-assert.yaml +++ b/tests/templates/kuttl/listener/10-assert.yaml @@ -40,7 +40,7 @@ status: apiVersion: v1 kind: Service metadata: - name: test-trino-coordinator-default + name: test-trino-coordinator spec: type: NodePort # external-unstable - by listener op --- diff --git a/tests/templates/kuttl/listener/10-install-trino.yaml.j2 b/tests/templates/kuttl/listener/10-install-trino.yaml.j2 index 6ee1f9e8..c16fd689 100644 --- a/tests/templates/kuttl/listener/10-install-trino.yaml.j2 +++ b/tests/templates/kuttl/listener/10-install-trino.yaml.j2 @@ -37,15 +37,13 @@ spec: vectorAggregatorConfigMapName: vector-aggregator-discovery {% endif %} coordinators: + roleConfig: + listenerClass: external-unstable config: - listenerClass: cluster-internal gracefulShutdownTimeout: 5s # Let the test run faster roleGroups: default: replicas: 1 - config: - # test merge -> we expect node port service - listenerClass: external-unstable workers: config: gracefulShutdownTimeout: 5s # Let the test run faster diff --git a/tests/templates/kuttl/listener/21-assert.yaml b/tests/templates/kuttl/listener/21-assert.yaml index d0a89c69..d9f9be94 100644 --- a/tests/templates/kuttl/listener/21-assert.yaml +++ b/tests/templates/kuttl/listener/21-assert.yaml @@ -3,4 +3,4 @@ apiVersion: kuttl.dev/v1beta1 kind: TestAssert timeout: 300 commands: - - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-active-workers.py -u admin -p admin -c test-trino-coordinator-default -w 1 + - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-active-workers.py -u admin -p admin -c test-trino-coordinator -w 1 From 4a909525d5bb8fb235b56df1460af00823847c24 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Tue, 17 Jun 2025 16:59:15 +0200 Subject: [PATCH 21/38] add lost comment --- rust/operator-binary/src/crd/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs index 183ae87c..7737680d 100644 --- a/rust/operator-binary/src/crd/mod.rs +++ b/rust/operator-binary/src/crd/mod.rs @@ -209,6 +209,7 @@ pub mod versioned { #[serde(flatten)] pub common: GenericRoleConfig, + /// This field controls which [ListenerClass](DOCS_BASE_URL_PLACEHOLDER/listener-operator/listenerclass.html) is used to expose the coordinator. #[serde(default = "coordinator_default_listener_class")] pub listener_class: String, } From e9ce6e2b00a789b0a8f88bd9fd1a525d4a93f291 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Wed, 18 Jun 2025 11:54:50 +0200 Subject: [PATCH 22/38] fix listener examples / comments --- docs/modules/trino/examples/getting_started/code/trino.yaml | 2 +- .../modules/trino/examples/getting_started/code/trino.yaml.j2 | 2 +- docs/modules/trino/pages/usage-guide/listenerclass.adoc | 4 ++-- rust/operator-binary/src/crd/mod.rs | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/modules/trino/examples/getting_started/code/trino.yaml b/docs/modules/trino/examples/getting_started/code/trino.yaml index b9bb4644..aa95cf83 100644 --- a/docs/modules/trino/examples/getting_started/code/trino.yaml +++ b/docs/modules/trino/examples/getting_started/code/trino.yaml @@ -11,7 +11,7 @@ spec: matchLabels: trino: simple-trino coordinators: - config: + roleConfig: listenerClass: external-unstable roleGroups: default: diff --git a/docs/modules/trino/examples/getting_started/code/trino.yaml.j2 b/docs/modules/trino/examples/getting_started/code/trino.yaml.j2 index b9bb4644..aa95cf83 100644 --- a/docs/modules/trino/examples/getting_started/code/trino.yaml.j2 +++ b/docs/modules/trino/examples/getting_started/code/trino.yaml.j2 @@ -11,7 +11,7 @@ spec: matchLabels: trino: simple-trino coordinators: - config: + roleConfig: listenerClass: external-unstable roleGroups: default: diff --git a/docs/modules/trino/pages/usage-guide/listenerclass.adoc b/docs/modules/trino/pages/usage-guide/listenerclass.adoc index 5faa9286..148513fd 100644 --- a/docs/modules/trino/pages/usage-guide/listenerclass.adoc +++ b/docs/modules/trino/pages/usage-guide/listenerclass.adoc @@ -2,13 +2,13 @@ :description: Configure Trino service exposure with ListenerClasses: cluster-internal, external-unstable, or external-stable. The operator deploys a xref:listener-operator:listener.adoc[Listener] for the coodinator pod. -The listener defaults to only being accessible from within the Kubernetes cluster, but this can be changed by setting `.spec.coordinators.config.listenerClass`: +The listener defaults to only being accessible from within the Kubernetes cluster, but this can be changed by setting `.spec.coordinators.roleConfig.listenerClass`: [source,yaml] ---- spec: coordinators: - config: + roleConfig: listenerClass: external-unstable # <1> ... workers: diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs index 7737680d..d8e384bf 100644 --- a/rust/operator-binary/src/crd/mod.rs +++ b/rust/operator-binary/src/crd/mod.rs @@ -845,7 +845,7 @@ impl v1alpha1::TrinoCluster { })) } - /// Returns the headless rolegroup service name `simple-trino-coordinator-default-`. + /// Returns the metrics rolegroup service name `simple-trino-coordinator-default-`. pub fn rolegroup_metrics_service_name(role_group_ref_object_name: &str) -> String { format!("{}-{}", role_group_ref_object_name, METRICS_SERVICE_SUFFIX) } From dd900e277a517bc6a1dc8a8969f17ab23755d487 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Wed, 18 Jun 2025 11:56:16 +0200 Subject: [PATCH 23/38] Apply suggestions from code review Co-authored-by: Nick <10092581+NickLarsenNZ@users.noreply.github.com> --- CHANGELOG.md | 2 +- rust/operator-binary/src/listener.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e917ae5..6c7bda74 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +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`. -- Adds Listener integration for Trino ([#753]). +- Add Listener integration for Trino ([#753]). ### Changed diff --git a/rust/operator-binary/src/listener.rs b/rust/operator-binary/src/listener.rs index e324bf01..3edb60af 100644 --- a/rust/operator-binary/src/listener.rs +++ b/rust/operator-binary/src/listener.rs @@ -32,7 +32,7 @@ pub enum Error { #[snafu(display("failed to build listener volume"))] BuildListenerPersistentVolume { - source: ListenerOperatorVolumeSourceBuilderError, + source: stackable_operator::builder::pod::volume::ListenerOperatorVolumeSourceBuilderError, }, } From 01e86d1a71f2219bf98c39e74f2695479cfea005 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Wed, 18 Jun 2025 11:58:50 +0200 Subject: [PATCH 24/38] clippy --- deploy/helm/trino-operator/crds/crds.yaml | 59 ++++++++++++----------- rust/operator-binary/src/listener.rs | 5 +- 2 files changed, 31 insertions(+), 33 deletions(-) diff --git a/deploy/helm/trino-operator/crds/crds.yaml b/deploy/helm/trino-operator/crds/crds.yaml index 1d2ec122..fb4ed30b 100644 --- a/deploy/helm/trino-operator/crds/crds.yaml +++ b/deploy/helm/trino-operator/crds/crds.yaml @@ -76,7 +76,7 @@ spec: type: object type: object catalogLabelSelector: - description: '[LabelSelector](https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#label-selectors) selecting the Catalogs to include in the Trino instance.' + description: "[LabelSelector](https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#label-selectors) selecting the Catalogs to include in the Trino instance." properties: matchExpressions: description: matchExpressions is a list of label selector requirements. The requirements are ANDed. @@ -113,12 +113,12 @@ spec: properties: internalSecretClass: default: tls - description: 'Only affects internal communication. Use mutual verification between Trino nodes This setting controls: - Which cert the servers should use to authenticate themselves against other servers - Which ca.crt to use when validating the other server' + description: "Only affects internal communication. Use mutual verification between Trino nodes This setting controls: - Which cert the servers should use to authenticate themselves against other servers - Which ca.crt to use when validating the other server" nullable: true type: string serverSecretClass: default: tls - description: 'Only affects client connections. This setting controls: - If TLS encryption is used at all - Which cert the servers should use to authenticate themselves against the client' + description: "Only affects client connections. This setting controls: - If TLS encryption is used at all - Which cert the servers should use to authenticate themselves against the client" nullable: true type: string type: object @@ -133,7 +133,7 @@ spec: default: reconciliationPaused: false stopped: false - description: '[Cluster operations](https://docs.stackable.tech/home/nightly/concepts/operations/cluster_operations) properties, allow stopping the product instance as well as pausing reconciliation.' + description: "[Cluster operations](https://docs.stackable.tech/home/nightly/concepts/operations/cluster_operations) properties, allow stopping the product instance as well as pausing reconciliation." properties: reconciliationPaused: default: false @@ -317,7 +317,7 @@ spec: memory: properties: limit: - description: 'The maximum amount of memory that should be available to the Pod. Specified as a byte [Quantity](https://kubernetes.io/docs/reference/kubernetes-api/common-definitions/quantity/), which means these suffixes are supported: E, P, T, G, M, k. You can also use the power-of-two equivalents: Ei, Pi, Ti, Gi, Mi, Ki. For example, the following represent roughly the same value: `128974848, 129e6, 129M, 128974848000m, 123Mi`' + description: "The maximum amount of memory that should be available to the Pod. Specified as a byte [Quantity](https://kubernetes.io/docs/reference/kubernetes-api/common-definitions/quantity/), which means these suffixes are supported: E, P, T, G, M, k. You can also use the power-of-two equivalents: Ei, Pi, Ti, Gi, Mi, Ki. For example, the following represent roughly the same value: `128974848, 129e6, 129M, 128974848000m, 123Mi`" nullable: true type: string runtimeLimits: @@ -384,7 +384,7 @@ spec: additionalProperties: type: string default: {} - description: '`envOverrides` configure environment variables to be set in the Pods. It is a map from strings to strings - environment variables and the value to set. Read the [environment variable overrides documentation](https://docs.stackable.tech/home/nightly/concepts/overrides#env-overrides) for more information and consult the operator specific usage guide to find out about the product specific environment variables that are available.' + description: "`envOverrides` configure environment variables to be set in the Pods. It is a map from strings to strings - environment variables and the value to set. Read the [environment variable overrides documentation](https://docs.stackable.tech/home/nightly/concepts/overrides#env-overrides) for more information and consult the operator specific usage guide to find out about the product specific environment variables that are available." type: object jvmArgumentOverrides: default: @@ -427,6 +427,7 @@ spec: properties: listenerClass: default: cluster-internal + description: This field controls which [ListenerClass](https://docs.stackable.tech/home/nightly/listener-operator/listenerclass.html) is used to expose the coordinator. type: string podDisruptionBudget: default: @@ -623,7 +624,7 @@ spec: memory: properties: limit: - description: 'The maximum amount of memory that should be available to the Pod. Specified as a byte [Quantity](https://kubernetes.io/docs/reference/kubernetes-api/common-definitions/quantity/), which means these suffixes are supported: E, P, T, G, M, k. You can also use the power-of-two equivalents: Ei, Pi, Ti, Gi, Mi, Ki. For example, the following represent roughly the same value: `128974848, 129e6, 129M, 128974848000m, 123Mi`' + description: "The maximum amount of memory that should be available to the Pod. Specified as a byte [Quantity](https://kubernetes.io/docs/reference/kubernetes-api/common-definitions/quantity/), which means these suffixes are supported: E, P, T, G, M, k. You can also use the power-of-two equivalents: Ei, Pi, Ti, Gi, Mi, Ki. For example, the following represent roughly the same value: `128974848, 129e6, 129M, 128974848000m, 123Mi`" nullable: true type: string runtimeLimits: @@ -690,7 +691,7 @@ spec: additionalProperties: type: string default: {} - description: '`envOverrides` configure environment variables to be set in the Pods. It is a map from strings to strings - environment variables and the value to set. Read the [environment variable overrides documentation](https://docs.stackable.tech/home/nightly/concepts/overrides#env-overrides) for more information and consult the operator specific usage guide to find out about the product specific environment variables that are available.' + description: "`envOverrides` configure environment variables to be set in the Pods. It is a map from strings to strings - environment variables and the value to set. Read the [environment variable overrides documentation](https://docs.stackable.tech/home/nightly/concepts/overrides#env-overrides) for more information and consult the operator specific usage guide to find out about the product specific environment variables that are available." type: object jvmArgumentOverrides: default: @@ -753,19 +754,19 @@ spec: type: string pullPolicy: default: Always - description: '[Pull policy](https://kubernetes.io/docs/concepts/containers/images/#image-pull-policy) used when pulling the image.' + description: "[Pull policy](https://kubernetes.io/docs/concepts/containers/images/#image-pull-policy) used when pulling the image." enum: - IfNotPresent - Always - Never type: string pullSecrets: - description: '[Image pull secrets](https://kubernetes.io/docs/concepts/containers/images/#specifying-imagepullsecrets-on-a-pod) to pull images from a private registry.' + description: "[Image pull secrets](https://kubernetes.io/docs/concepts/containers/images/#specifying-imagepullsecrets-on-a-pod) to pull images from a private registry." items: description: LocalObjectReference contains enough information to let you locate the referenced object inside the same namespace. properties: name: - description: 'Name of the referent. This field is effectively required, but due to backwards compatibility is allowed to be empty. Instances of this type with an empty value here are almost certainly wrong. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names' + description: "Name of the referent. This field is effectively required, but due to backwards compatibility is allowed to be empty. Instances of this type with an empty value here are almost certainly wrong. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names" type: string required: - name @@ -954,7 +955,7 @@ spec: memory: properties: limit: - description: 'The maximum amount of memory that should be available to the Pod. Specified as a byte [Quantity](https://kubernetes.io/docs/reference/kubernetes-api/common-definitions/quantity/), which means these suffixes are supported: E, P, T, G, M, k. You can also use the power-of-two equivalents: Ei, Pi, Ti, Gi, Mi, Ki. For example, the following represent roughly the same value: `128974848, 129e6, 129M, 128974848000m, 123Mi`' + description: "The maximum amount of memory that should be available to the Pod. Specified as a byte [Quantity](https://kubernetes.io/docs/reference/kubernetes-api/common-definitions/quantity/), which means these suffixes are supported: E, P, T, G, M, k. You can also use the power-of-two equivalents: Ei, Pi, Ti, Gi, Mi, Ki. For example, the following represent roughly the same value: `128974848, 129e6, 129M, 128974848000m, 123Mi`" nullable: true type: string runtimeLimits: @@ -1021,7 +1022,7 @@ spec: additionalProperties: type: string default: {} - description: '`envOverrides` configure environment variables to be set in the Pods. It is a map from strings to strings - environment variables and the value to set. Read the [environment variable overrides documentation](https://docs.stackable.tech/home/nightly/concepts/overrides#env-overrides) for more information and consult the operator specific usage guide to find out about the product specific environment variables that are available.' + description: "`envOverrides` configure environment variables to be set in the Pods. It is a map from strings to strings - environment variables and the value to set. Read the [environment variable overrides documentation](https://docs.stackable.tech/home/nightly/concepts/overrides#env-overrides) for more information and consult the operator specific usage guide to find out about the product specific environment variables that are available." type: object jvmArgumentOverrides: default: @@ -1256,7 +1257,7 @@ spec: memory: properties: limit: - description: 'The maximum amount of memory that should be available to the Pod. Specified as a byte [Quantity](https://kubernetes.io/docs/reference/kubernetes-api/common-definitions/quantity/), which means these suffixes are supported: E, P, T, G, M, k. You can also use the power-of-two equivalents: Ei, Pi, Ti, Gi, Mi, Ki. For example, the following represent roughly the same value: `128974848, 129e6, 129M, 128974848000m, 123Mi`' + description: "The maximum amount of memory that should be available to the Pod. Specified as a byte [Quantity](https://kubernetes.io/docs/reference/kubernetes-api/common-definitions/quantity/), which means these suffixes are supported: E, P, T, G, M, k. You can also use the power-of-two equivalents: Ei, Pi, Ti, Gi, Mi, Ki. For example, the following represent roughly the same value: `128974848, 129e6, 129M, 128974848000m, 123Mi`" nullable: true type: string runtimeLimits: @@ -1323,7 +1324,7 @@ spec: additionalProperties: type: string default: {} - description: '`envOverrides` configure environment variables to be set in the Pods. It is a map from strings to strings - environment variables and the value to set. Read the [environment variable overrides documentation](https://docs.stackable.tech/home/nightly/concepts/overrides#env-overrides) for more information and consult the operator specific usage guide to find out about the product specific environment variables that are available.' + description: "`envOverrides` configure environment variables to be set in the Pods. It is a map from strings to strings - environment variables and the value to set. Read the [environment variable overrides documentation](https://docs.stackable.tech/home/nightly/concepts/overrides#env-overrides) for more information and consult the operator specific usage guide to find out about the product specific environment variables that are available." type: object jvmArgumentOverrides: default: @@ -1398,8 +1399,8 @@ spec: status: description: Status of the condition, one of True, False, Unknown. enum: - - 'True' - - 'False' + - "True" + - "False" - Unknown type: string type: @@ -1455,7 +1456,7 @@ spec: additionalProperties: type: string default: {} - description: 'The `configOverrides` allow overriding arbitrary Trino settings. For example, for Hive you could add `hive.metastore.username: trino`.' + description: "The `configOverrides` allow overriding arbitrary Trino settings. For example, for Hive you could add `hive.metastore.username: trino`." type: object connector: description: The `connector` defines which connector is used. @@ -1526,7 +1527,7 @@ spec: nullable: true properties: scope: - description: '[Scope](https://docs.stackable.tech/home/nightly/secret-operator/scope) of the [SecretClass](https://docs.stackable.tech/home/nightly/secret-operator/secretclass).' + description: "[Scope](https://docs.stackable.tech/home/nightly/secret-operator/scope) of the [SecretClass](https://docs.stackable.tech/home/nightly/secret-operator/secretclass)." nullable: true properties: listenerVolumes: @@ -1551,13 +1552,13 @@ spec: type: array type: object secretClass: - description: '[SecretClass](https://docs.stackable.tech/home/nightly/secret-operator/secretclass) containing the LDAP bind credentials.' + description: "[SecretClass](https://docs.stackable.tech/home/nightly/secret-operator/secretclass) containing the LDAP bind credentials." type: string required: - secretClass type: object host: - description: 'Host of the S3 server without any protocol or port. For example: `west1.my-cloud.com`.' + description: "Host of the S3 server without any protocol or port. For example: `west1.my-cloud.com`." type: string port: description: Port the S3 server listens on. If not specified the product will determine the port to use. @@ -1653,7 +1654,7 @@ spec: description: The key to select. type: string name: - description: 'Name of the referent. This field is effectively required, but due to backwards compatibility is allowed to be empty. Instances of this type with an empty value here are almost certainly wrong. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names' + description: "Name of the referent. This field is effectively required, but due to backwards compatibility is allowed to be empty. Instances of this type with an empty value here are almost certainly wrong. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names" type: string optional: description: Specify whether the ConfigMap or its key must be defined @@ -1669,7 +1670,7 @@ spec: description: The key of the secret to select from. Must be a valid secret key. type: string name: - description: 'Name of the referent. This field is effectively required, but due to backwards compatibility is allowed to be empty. Instances of this type with an empty value here are almost certainly wrong. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names' + description: "Name of the referent. This field is effectively required, but due to backwards compatibility is allowed to be empty. Instances of this type with an empty value here are almost certainly wrong. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names" type: string optional: description: Specify whether the Secret or its key must be defined @@ -1757,7 +1758,7 @@ spec: nullable: true properties: scope: - description: '[Scope](https://docs.stackable.tech/home/nightly/secret-operator/scope) of the [SecretClass](https://docs.stackable.tech/home/nightly/secret-operator/secretclass).' + description: "[Scope](https://docs.stackable.tech/home/nightly/secret-operator/scope) of the [SecretClass](https://docs.stackable.tech/home/nightly/secret-operator/secretclass)." nullable: true properties: listenerVolumes: @@ -1782,13 +1783,13 @@ spec: type: array type: object secretClass: - description: '[SecretClass](https://docs.stackable.tech/home/nightly/secret-operator/secretclass) containing the LDAP bind credentials.' + description: "[SecretClass](https://docs.stackable.tech/home/nightly/secret-operator/secretclass) containing the LDAP bind credentials." type: string required: - secretClass type: object host: - description: 'Host of the S3 server without any protocol or port. For example: `west1.my-cloud.com`.' + description: "Host of the S3 server without any protocol or port. For example: `west1.my-cloud.com`." type: string port: description: Port the S3 server listens on. If not specified the product will determine the port to use. @@ -1905,7 +1906,7 @@ spec: nullable: true properties: scope: - description: '[Scope](https://docs.stackable.tech/home/nightly/secret-operator/scope) of the [SecretClass](https://docs.stackable.tech/home/nightly/secret-operator/secretclass).' + description: "[Scope](https://docs.stackable.tech/home/nightly/secret-operator/scope) of the [SecretClass](https://docs.stackable.tech/home/nightly/secret-operator/secretclass)." nullable: true properties: listenerVolumes: @@ -1930,13 +1931,13 @@ spec: type: array type: object secretClass: - description: '[SecretClass](https://docs.stackable.tech/home/nightly/secret-operator/secretclass) containing the LDAP bind credentials.' + description: "[SecretClass](https://docs.stackable.tech/home/nightly/secret-operator/secretclass) containing the LDAP bind credentials." type: string required: - secretClass type: object host: - description: 'Host of the S3 server without any protocol or port. For example: `west1.my-cloud.com`.' + description: "Host of the S3 server without any protocol or port. For example: `west1.my-cloud.com`." type: string port: description: Port the S3 server listens on. If not specified the product will determine the port to use. diff --git a/rust/operator-binary/src/listener.rs b/rust/operator-binary/src/listener.rs index 3edb60af..443a7a45 100644 --- a/rust/operator-binary/src/listener.rs +++ b/rust/operator-binary/src/listener.rs @@ -2,10 +2,7 @@ use snafu::{ResultExt, Snafu}; use stackable_operator::{ builder::{ meta::ObjectMetaBuilder, - pod::volume::{ - ListenerOperatorVolumeSourceBuilder, ListenerOperatorVolumeSourceBuilderError, - ListenerReference, - }, + pod::volume::{ListenerOperatorVolumeSourceBuilder, ListenerReference}, }, crd::listener::v1alpha1::{Listener, ListenerPort, ListenerSpec}, k8s_openapi::api::core::v1::PersistentVolumeClaim, From dc397c1f34764746322286cf4b725c9443dcdeb2 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Wed, 18 Jun 2025 12:24:30 +0200 Subject: [PATCH 25/38] fix smoke test --- tests/templates/kuttl/smoke/21-assert.yaml | 6 +++--- tests/templates/kuttl/smoke/31-assert.yaml | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/templates/kuttl/smoke/21-assert.yaml b/tests/templates/kuttl/smoke/21-assert.yaml index e1f04235..dd34d78c 100644 --- a/tests/templates/kuttl/smoke/21-assert.yaml +++ b/tests/templates/kuttl/smoke/21-assert.yaml @@ -3,6 +3,6 @@ apiVersion: kuttl.dev/v1beta1 kind: TestAssert timeout: 300 commands: - - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-active-workers.py -u admin -p admin -c trino-coordinator-default -w 1 - - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-opa.py -c trino-coordinator-default - - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-s3.py -c trino-coordinator-default -b trino + - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-active-workers.py -u admin -p admin -c trino-coordinator -w 1 + - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-opa.py -c trino-coordinator + - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-s3.py -c trino-coordinator -b trino diff --git a/tests/templates/kuttl/smoke/31-assert.yaml b/tests/templates/kuttl/smoke/31-assert.yaml index b0e33099..4dedc4d1 100644 --- a/tests/templates/kuttl/smoke/31-assert.yaml +++ b/tests/templates/kuttl/smoke/31-assert.yaml @@ -3,6 +3,6 @@ apiVersion: kuttl.dev/v1beta1 kind: TestAssert timeout: 600 commands: - - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-active-workers.py -u admin -p admin -c trino-coordinator-default -w 2 - - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-opa.py -c trino-coordinator-default - - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-s3.py -c trino-coordinator-default -b trino + - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-active-workers.py -u admin -p admin -c trino-coordinator -w 2 + - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-opa.py -c trino-coordinator + - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-s3.py -c trino-coordinator -b trino From d3684d1e6b537ccc1da16cb19d72c406c13061b0 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Wed, 18 Jun 2025 12:24:46 +0200 Subject: [PATCH 26/38] fix tls test --- tests/templates/kuttl/tls/10-assert.yaml.j2 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/templates/kuttl/tls/10-assert.yaml.j2 b/tests/templates/kuttl/tls/10-assert.yaml.j2 index e7680d87..f4348ded 100644 --- a/tests/templates/kuttl/tls/10-assert.yaml.j2 +++ b/tests/templates/kuttl/tls/10-assert.yaml.j2 @@ -22,7 +22,7 @@ status: apiVersion: v1 kind: Service metadata: - name: trino-coordinator-default + name: trino-coordinator spec: ports: {% if test_scenario['values']['use-tls'] == 'false' %} From 665a4e7fcf539c3eb4079b21839dfc66bac83148 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Wed, 18 Jun 2025 12:53:20 +0200 Subject: [PATCH 27/38] fix authorization test --- tests/templates/kuttl/opa-authorization/check-opa.py.j2 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/templates/kuttl/opa-authorization/check-opa.py.j2 b/tests/templates/kuttl/opa-authorization/check-opa.py.j2 index 4c8fe731..bafd398e 100755 --- a/tests/templates/kuttl/opa-authorization/check-opa.py.j2 +++ b/tests/templates/kuttl/opa-authorization/check-opa.py.j2 @@ -515,7 +515,7 @@ class TestOpa: @staticmethod def get_connection(username, password, namespace, impersonation=None): connection = trino.dbapi.connect( - host="trino-coordinator-default.{0}.svc.cluster.local".format(namespace), + host="trino-coordinator.{0}.svc.cluster.local".format(namespace), port=8443, user=impersonation if impersonation is not None else username, http_scheme="https", From 186aa3f5a3dcada117be14317c6b9ad2c16852e9 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Wed, 18 Jun 2025 12:57:04 +0200 Subject: [PATCH 28/38] regenerate charts --- deploy/helm/trino-operator/crds/crds.yaml | 58 +++++++++++------------ 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/deploy/helm/trino-operator/crds/crds.yaml b/deploy/helm/trino-operator/crds/crds.yaml index fb4ed30b..887ababd 100644 --- a/deploy/helm/trino-operator/crds/crds.yaml +++ b/deploy/helm/trino-operator/crds/crds.yaml @@ -76,7 +76,7 @@ spec: type: object type: object catalogLabelSelector: - description: "[LabelSelector](https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#label-selectors) selecting the Catalogs to include in the Trino instance." + description: '[LabelSelector](https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#label-selectors) selecting the Catalogs to include in the Trino instance.' properties: matchExpressions: description: matchExpressions is a list of label selector requirements. The requirements are ANDed. @@ -113,12 +113,12 @@ spec: properties: internalSecretClass: default: tls - description: "Only affects internal communication. Use mutual verification between Trino nodes This setting controls: - Which cert the servers should use to authenticate themselves against other servers - Which ca.crt to use when validating the other server" + description: 'Only affects internal communication. Use mutual verification between Trino nodes This setting controls: - Which cert the servers should use to authenticate themselves against other servers - Which ca.crt to use when validating the other server' nullable: true type: string serverSecretClass: default: tls - description: "Only affects client connections. This setting controls: - If TLS encryption is used at all - Which cert the servers should use to authenticate themselves against the client" + description: 'Only affects client connections. This setting controls: - If TLS encryption is used at all - Which cert the servers should use to authenticate themselves against the client' nullable: true type: string type: object @@ -133,7 +133,7 @@ spec: default: reconciliationPaused: false stopped: false - description: "[Cluster operations](https://docs.stackable.tech/home/nightly/concepts/operations/cluster_operations) properties, allow stopping the product instance as well as pausing reconciliation." + description: '[Cluster operations](https://docs.stackable.tech/home/nightly/concepts/operations/cluster_operations) properties, allow stopping the product instance as well as pausing reconciliation.' properties: reconciliationPaused: default: false @@ -317,7 +317,7 @@ spec: memory: properties: limit: - description: "The maximum amount of memory that should be available to the Pod. Specified as a byte [Quantity](https://kubernetes.io/docs/reference/kubernetes-api/common-definitions/quantity/), which means these suffixes are supported: E, P, T, G, M, k. You can also use the power-of-two equivalents: Ei, Pi, Ti, Gi, Mi, Ki. For example, the following represent roughly the same value: `128974848, 129e6, 129M, 128974848000m, 123Mi`" + description: 'The maximum amount of memory that should be available to the Pod. Specified as a byte [Quantity](https://kubernetes.io/docs/reference/kubernetes-api/common-definitions/quantity/), which means these suffixes are supported: E, P, T, G, M, k. You can also use the power-of-two equivalents: Ei, Pi, Ti, Gi, Mi, Ki. For example, the following represent roughly the same value: `128974848, 129e6, 129M, 128974848000m, 123Mi`' nullable: true type: string runtimeLimits: @@ -384,7 +384,7 @@ spec: additionalProperties: type: string default: {} - description: "`envOverrides` configure environment variables to be set in the Pods. It is a map from strings to strings - environment variables and the value to set. Read the [environment variable overrides documentation](https://docs.stackable.tech/home/nightly/concepts/overrides#env-overrides) for more information and consult the operator specific usage guide to find out about the product specific environment variables that are available." + description: '`envOverrides` configure environment variables to be set in the Pods. It is a map from strings to strings - environment variables and the value to set. Read the [environment variable overrides documentation](https://docs.stackable.tech/home/nightly/concepts/overrides#env-overrides) for more information and consult the operator specific usage guide to find out about the product specific environment variables that are available.' type: object jvmArgumentOverrides: default: @@ -624,7 +624,7 @@ spec: memory: properties: limit: - description: "The maximum amount of memory that should be available to the Pod. Specified as a byte [Quantity](https://kubernetes.io/docs/reference/kubernetes-api/common-definitions/quantity/), which means these suffixes are supported: E, P, T, G, M, k. You can also use the power-of-two equivalents: Ei, Pi, Ti, Gi, Mi, Ki. For example, the following represent roughly the same value: `128974848, 129e6, 129M, 128974848000m, 123Mi`" + description: 'The maximum amount of memory that should be available to the Pod. Specified as a byte [Quantity](https://kubernetes.io/docs/reference/kubernetes-api/common-definitions/quantity/), which means these suffixes are supported: E, P, T, G, M, k. You can also use the power-of-two equivalents: Ei, Pi, Ti, Gi, Mi, Ki. For example, the following represent roughly the same value: `128974848, 129e6, 129M, 128974848000m, 123Mi`' nullable: true type: string runtimeLimits: @@ -691,7 +691,7 @@ spec: additionalProperties: type: string default: {} - description: "`envOverrides` configure environment variables to be set in the Pods. It is a map from strings to strings - environment variables and the value to set. Read the [environment variable overrides documentation](https://docs.stackable.tech/home/nightly/concepts/overrides#env-overrides) for more information and consult the operator specific usage guide to find out about the product specific environment variables that are available." + description: '`envOverrides` configure environment variables to be set in the Pods. It is a map from strings to strings - environment variables and the value to set. Read the [environment variable overrides documentation](https://docs.stackable.tech/home/nightly/concepts/overrides#env-overrides) for more information and consult the operator specific usage guide to find out about the product specific environment variables that are available.' type: object jvmArgumentOverrides: default: @@ -754,19 +754,19 @@ spec: type: string pullPolicy: default: Always - description: "[Pull policy](https://kubernetes.io/docs/concepts/containers/images/#image-pull-policy) used when pulling the image." + description: '[Pull policy](https://kubernetes.io/docs/concepts/containers/images/#image-pull-policy) used when pulling the image.' enum: - IfNotPresent - Always - Never type: string pullSecrets: - description: "[Image pull secrets](https://kubernetes.io/docs/concepts/containers/images/#specifying-imagepullsecrets-on-a-pod) to pull images from a private registry." + description: '[Image pull secrets](https://kubernetes.io/docs/concepts/containers/images/#specifying-imagepullsecrets-on-a-pod) to pull images from a private registry.' items: description: LocalObjectReference contains enough information to let you locate the referenced object inside the same namespace. properties: name: - description: "Name of the referent. This field is effectively required, but due to backwards compatibility is allowed to be empty. Instances of this type with an empty value here are almost certainly wrong. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names" + description: 'Name of the referent. This field is effectively required, but due to backwards compatibility is allowed to be empty. Instances of this type with an empty value here are almost certainly wrong. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names' type: string required: - name @@ -955,7 +955,7 @@ spec: memory: properties: limit: - description: "The maximum amount of memory that should be available to the Pod. Specified as a byte [Quantity](https://kubernetes.io/docs/reference/kubernetes-api/common-definitions/quantity/), which means these suffixes are supported: E, P, T, G, M, k. You can also use the power-of-two equivalents: Ei, Pi, Ti, Gi, Mi, Ki. For example, the following represent roughly the same value: `128974848, 129e6, 129M, 128974848000m, 123Mi`" + description: 'The maximum amount of memory that should be available to the Pod. Specified as a byte [Quantity](https://kubernetes.io/docs/reference/kubernetes-api/common-definitions/quantity/), which means these suffixes are supported: E, P, T, G, M, k. You can also use the power-of-two equivalents: Ei, Pi, Ti, Gi, Mi, Ki. For example, the following represent roughly the same value: `128974848, 129e6, 129M, 128974848000m, 123Mi`' nullable: true type: string runtimeLimits: @@ -1022,7 +1022,7 @@ spec: additionalProperties: type: string default: {} - description: "`envOverrides` configure environment variables to be set in the Pods. It is a map from strings to strings - environment variables and the value to set. Read the [environment variable overrides documentation](https://docs.stackable.tech/home/nightly/concepts/overrides#env-overrides) for more information and consult the operator specific usage guide to find out about the product specific environment variables that are available." + description: '`envOverrides` configure environment variables to be set in the Pods. It is a map from strings to strings - environment variables and the value to set. Read the [environment variable overrides documentation](https://docs.stackable.tech/home/nightly/concepts/overrides#env-overrides) for more information and consult the operator specific usage guide to find out about the product specific environment variables that are available.' type: object jvmArgumentOverrides: default: @@ -1257,7 +1257,7 @@ spec: memory: properties: limit: - description: "The maximum amount of memory that should be available to the Pod. Specified as a byte [Quantity](https://kubernetes.io/docs/reference/kubernetes-api/common-definitions/quantity/), which means these suffixes are supported: E, P, T, G, M, k. You can also use the power-of-two equivalents: Ei, Pi, Ti, Gi, Mi, Ki. For example, the following represent roughly the same value: `128974848, 129e6, 129M, 128974848000m, 123Mi`" + description: 'The maximum amount of memory that should be available to the Pod. Specified as a byte [Quantity](https://kubernetes.io/docs/reference/kubernetes-api/common-definitions/quantity/), which means these suffixes are supported: E, P, T, G, M, k. You can also use the power-of-two equivalents: Ei, Pi, Ti, Gi, Mi, Ki. For example, the following represent roughly the same value: `128974848, 129e6, 129M, 128974848000m, 123Mi`' nullable: true type: string runtimeLimits: @@ -1324,7 +1324,7 @@ spec: additionalProperties: type: string default: {} - description: "`envOverrides` configure environment variables to be set in the Pods. It is a map from strings to strings - environment variables and the value to set. Read the [environment variable overrides documentation](https://docs.stackable.tech/home/nightly/concepts/overrides#env-overrides) for more information and consult the operator specific usage guide to find out about the product specific environment variables that are available." + description: '`envOverrides` configure environment variables to be set in the Pods. It is a map from strings to strings - environment variables and the value to set. Read the [environment variable overrides documentation](https://docs.stackable.tech/home/nightly/concepts/overrides#env-overrides) for more information and consult the operator specific usage guide to find out about the product specific environment variables that are available.' type: object jvmArgumentOverrides: default: @@ -1399,8 +1399,8 @@ spec: status: description: Status of the condition, one of True, False, Unknown. enum: - - "True" - - "False" + - 'True' + - 'False' - Unknown type: string type: @@ -1456,7 +1456,7 @@ spec: additionalProperties: type: string default: {} - description: "The `configOverrides` allow overriding arbitrary Trino settings. For example, for Hive you could add `hive.metastore.username: trino`." + description: 'The `configOverrides` allow overriding arbitrary Trino settings. For example, for Hive you could add `hive.metastore.username: trino`.' type: object connector: description: The `connector` defines which connector is used. @@ -1527,7 +1527,7 @@ spec: nullable: true properties: scope: - description: "[Scope](https://docs.stackable.tech/home/nightly/secret-operator/scope) of the [SecretClass](https://docs.stackable.tech/home/nightly/secret-operator/secretclass)." + description: '[Scope](https://docs.stackable.tech/home/nightly/secret-operator/scope) of the [SecretClass](https://docs.stackable.tech/home/nightly/secret-operator/secretclass).' nullable: true properties: listenerVolumes: @@ -1552,13 +1552,13 @@ spec: type: array type: object secretClass: - description: "[SecretClass](https://docs.stackable.tech/home/nightly/secret-operator/secretclass) containing the LDAP bind credentials." + description: '[SecretClass](https://docs.stackable.tech/home/nightly/secret-operator/secretclass) containing the LDAP bind credentials.' type: string required: - secretClass type: object host: - description: "Host of the S3 server without any protocol or port. For example: `west1.my-cloud.com`." + description: 'Host of the S3 server without any protocol or port. For example: `west1.my-cloud.com`.' type: string port: description: Port the S3 server listens on. If not specified the product will determine the port to use. @@ -1654,7 +1654,7 @@ spec: description: The key to select. type: string name: - description: "Name of the referent. This field is effectively required, but due to backwards compatibility is allowed to be empty. Instances of this type with an empty value here are almost certainly wrong. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names" + description: 'Name of the referent. This field is effectively required, but due to backwards compatibility is allowed to be empty. Instances of this type with an empty value here are almost certainly wrong. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names' type: string optional: description: Specify whether the ConfigMap or its key must be defined @@ -1670,7 +1670,7 @@ spec: description: The key of the secret to select from. Must be a valid secret key. type: string name: - description: "Name of the referent. This field is effectively required, but due to backwards compatibility is allowed to be empty. Instances of this type with an empty value here are almost certainly wrong. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names" + description: 'Name of the referent. This field is effectively required, but due to backwards compatibility is allowed to be empty. Instances of this type with an empty value here are almost certainly wrong. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names' type: string optional: description: Specify whether the Secret or its key must be defined @@ -1758,7 +1758,7 @@ spec: nullable: true properties: scope: - description: "[Scope](https://docs.stackable.tech/home/nightly/secret-operator/scope) of the [SecretClass](https://docs.stackable.tech/home/nightly/secret-operator/secretclass)." + description: '[Scope](https://docs.stackable.tech/home/nightly/secret-operator/scope) of the [SecretClass](https://docs.stackable.tech/home/nightly/secret-operator/secretclass).' nullable: true properties: listenerVolumes: @@ -1783,13 +1783,13 @@ spec: type: array type: object secretClass: - description: "[SecretClass](https://docs.stackable.tech/home/nightly/secret-operator/secretclass) containing the LDAP bind credentials." + description: '[SecretClass](https://docs.stackable.tech/home/nightly/secret-operator/secretclass) containing the LDAP bind credentials.' type: string required: - secretClass type: object host: - description: "Host of the S3 server without any protocol or port. For example: `west1.my-cloud.com`." + description: 'Host of the S3 server without any protocol or port. For example: `west1.my-cloud.com`.' type: string port: description: Port the S3 server listens on. If not specified the product will determine the port to use. @@ -1906,7 +1906,7 @@ spec: nullable: true properties: scope: - description: "[Scope](https://docs.stackable.tech/home/nightly/secret-operator/scope) of the [SecretClass](https://docs.stackable.tech/home/nightly/secret-operator/secretclass)." + description: '[Scope](https://docs.stackable.tech/home/nightly/secret-operator/scope) of the [SecretClass](https://docs.stackable.tech/home/nightly/secret-operator/secretclass).' nullable: true properties: listenerVolumes: @@ -1931,13 +1931,13 @@ spec: type: array type: object secretClass: - description: "[SecretClass](https://docs.stackable.tech/home/nightly/secret-operator/secretclass) containing the LDAP bind credentials." + description: '[SecretClass](https://docs.stackable.tech/home/nightly/secret-operator/secretclass) containing the LDAP bind credentials.' type: string required: - secretClass type: object host: - description: "Host of the S3 server without any protocol or port. For example: `west1.my-cloud.com`." + description: 'Host of the S3 server without any protocol or port. For example: `west1.my-cloud.com`.' type: string port: description: Port the S3 server listens on. If not specified the product will determine the port to use. From 5e3aa75e6f3f37b4b63087ec07197c8d402ab56a Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Mon, 23 Jun 2025 12:38:52 +0200 Subject: [PATCH 29/38] fix metrics test for lsitener changes --- tests/templates/kuttl/commons/check-metrics.py | 15 +++++++-------- tests/templates/kuttl/smoke/21-assert.yaml | 2 +- tests/templates/kuttl/smoke_aws/21-assert.yaml | 2 +- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/tests/templates/kuttl/commons/check-metrics.py b/tests/templates/kuttl/commons/check-metrics.py index 9a4a42c5..7fd11bda 100644 --- a/tests/templates/kuttl/commons/check-metrics.py +++ b/tests/templates/kuttl/commons/check-metrics.py @@ -68,17 +68,16 @@ def check_monitoring(hosts): if __name__ == "__main__": all_args = argparse.ArgumentParser(description="Test Trino metrics.") all_args.add_argument( - "-n", "--namespace", help="The namespace to run in", required=True + "-c", "--coordinator", help="The coordinator service name", required=True ) - args = vars(all_args.parse_args()) - namespace = args["namespace"] - - host_coordinator_0 = f"trino-coordinator-default-0.trino-coordinator-default.{namespace}.svc.cluster.local" - host_worker_0 = ( - f"trino-worker-default-0.trino-worker-default.{namespace}.svc.cluster.local" + all_args.add_argument( + "-w", "--worker", help="The worker service name", required=True ) + args = vars(all_args.parse_args()) + service_coordinator = args["coordinator"] + service_worker = args["worker"] - hosts = [host_coordinator_0, host_worker_0] + hosts = [service_coordinator, service_worker] check_monitoring(hosts) diff --git a/tests/templates/kuttl/smoke/21-assert.yaml b/tests/templates/kuttl/smoke/21-assert.yaml index 1fde20fd..1619480f 100644 --- a/tests/templates/kuttl/smoke/21-assert.yaml +++ b/tests/templates/kuttl/smoke/21-assert.yaml @@ -6,4 +6,4 @@ commands: - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-active-workers.py -u admin -p admin -c trino-coordinator -w 1 - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-opa.py -c trino-coordinator - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-s3.py -c trino-coordinator -b trino - - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-metrics.py -n $NAMESPACE + - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-metrics.py -c trino-coordinator-default-metrics -w trino-worker-default-metrics diff --git a/tests/templates/kuttl/smoke_aws/21-assert.yaml b/tests/templates/kuttl/smoke_aws/21-assert.yaml index 0ad6d229..db05f096 100644 --- a/tests/templates/kuttl/smoke_aws/21-assert.yaml +++ b/tests/templates/kuttl/smoke_aws/21-assert.yaml @@ -6,4 +6,4 @@ commands: - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-active-workers.py -u admin -p admin -c trino-coordinator-default -w 1 - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-opa.py -c trino-coordinator-default - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-s3.py -c trino-coordinator-default -b my-bucket - - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-metrics.py -n $NAMESPACE + - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-metrics.py -c trino-coordinator-default-metrics -w trino-worker-default-metrics From 1e22614dfefb605527f1ec8e106b7e7e5bd5017c Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Mon, 23 Jun 2025 12:53:37 +0200 Subject: [PATCH 30/38] fix test --- tests/templates/kuttl/smoke/31-assert.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/templates/kuttl/smoke/31-assert.yaml b/tests/templates/kuttl/smoke/31-assert.yaml index a31898b7..b70ce2a1 100644 --- a/tests/templates/kuttl/smoke/31-assert.yaml +++ b/tests/templates/kuttl/smoke/31-assert.yaml @@ -6,4 +6,4 @@ commands: - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-active-workers.py -u admin -p admin -c trino-coordinator -w 2 - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-opa.py -c trino-coordinator - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-s3.py -c trino-coordinator -b trino - - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-metrics.py -n $NAMESPACE + - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-metrics.py -c trino-coordinator-default-metrics -w trino-worker-default-metrics From f44b8f8560a76c5e3749d7c156a9efb7a26c93e6 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Mon, 23 Jun 2025 14:04:02 +0200 Subject: [PATCH 31/38] Update rust/operator-binary/src/crd/mod.rs Co-authored-by: Nick <10092581+NickLarsenNZ@users.noreply.github.com> --- rust/operator-binary/src/crd/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs index d8e384bf..1ecce36e 100644 --- a/rust/operator-binary/src/crd/mod.rs +++ b/rust/operator-binary/src/crd/mod.rs @@ -847,7 +847,7 @@ impl v1alpha1::TrinoCluster { /// Returns the metrics rolegroup service name `simple-trino-coordinator-default-`. pub fn rolegroup_metrics_service_name(role_group_ref_object_name: &str) -> String { - format!("{}-{}", role_group_ref_object_name, METRICS_SERVICE_SUFFIX) + format!("{role_group_ref_object_name}-{METRICS_SERVICE_SUFFIX}") } /// Returns user provided authentication settings From ac5a5a51b7aa85a1d415340c9452741735aae6aa Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Mon, 23 Jun 2025 14:50:22 +0200 Subject: [PATCH 32/38] Update rust/operator-binary/src/crd/mod.rs Co-authored-by: Nick <10092581+NickLarsenNZ@users.noreply.github.com> --- rust/operator-binary/src/crd/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs index 1ecce36e..db1b4edc 100644 --- a/rust/operator-binary/src/crd/mod.rs +++ b/rust/operator-binary/src/crd/mod.rs @@ -840,7 +840,7 @@ impl v1alpha1::TrinoCluster { role_group_service_name: Self::rolegroup_metrics_service_name( &role_group_ref.object_name(), ), - pod_name: format!("{}-{}", role_group_ref.object_name(), i), + pod_name: format!("{role_group}-{i}", role_group = role_group_ref.object_name()), }) })) } From 37bcd4f1154450e20c1686545bf571476469fc5a Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Mon, 23 Jun 2025 14:57:06 +0200 Subject: [PATCH 33/38] fmt --- rust/operator-binary/src/crd/mod.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs index db1b4edc..61bc9fe6 100644 --- a/rust/operator-binary/src/crd/mod.rs +++ b/rust/operator-binary/src/crd/mod.rs @@ -840,7 +840,10 @@ impl v1alpha1::TrinoCluster { role_group_service_name: Self::rolegroup_metrics_service_name( &role_group_ref.object_name(), ), - pod_name: format!("{role_group}-{i}", role_group = role_group_ref.object_name()), + pod_name: format!( + "{role_group}-{i}", + role_group = role_group_ref.object_name() + ), }) })) } From 7921ba0111ece490baeee4bc2f0d70c328afed24 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Tue, 24 Jun 2025 11:19:13 +0200 Subject: [PATCH 34/38] fix app version --- rust/operator-binary/src/controller.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/rust/operator-binary/src/controller.rs b/rust/operator-binary/src/controller.rs index b1d40a58..59c017a1 100644 --- a/rust/operator-binary/src/controller.rs +++ b/rust/operator-binary/src/controller.rs @@ -549,7 +549,12 @@ pub async fn reconcile_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"), + build_recommended_labels( + trino, + &resolved_product_image.app_version_label, + &trino_role_str, + "none", + ), listener_class.to_string(), listener_group_name, ) @@ -1001,7 +1006,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, - "none", + &role_group_ref.role_group, )) .context(LabelBuildSnafu)?; From 29184236da80065ca009a65ad92c4595619d4165 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Tue, 24 Jun 2025 11:52:13 +0200 Subject: [PATCH 35/38] remove merge artifact --- tests/templates/kuttl/smoke/check-s3.py | 295 ------------------------ 1 file changed, 295 deletions(-) delete mode 100755 tests/templates/kuttl/smoke/check-s3.py diff --git a/tests/templates/kuttl/smoke/check-s3.py b/tests/templates/kuttl/smoke/check-s3.py deleted file mode 100755 index ff461c1c..00000000 --- a/tests/templates/kuttl/smoke/check-s3.py +++ /dev/null @@ -1,295 +0,0 @@ -#!/usr/bin/env python -import trino -import argparse -import sys -import re - -if not sys.warnoptions: - import warnings -warnings.simplefilter("ignore") - - -def get_connection(username, password, namespace): - host = ( - "trino-coordinator-default-0.trino-coordinator-default." - + namespace - + ".svc.cluster.local" - ) - # If you want to debug this locally use - # kubectl -n kuttl-test-XXX port-forward svc/trino-coordinator-default 8443 - # host = '127.0.0.1' - - conn = trino.dbapi.connect( - host=host, - port=8443, - user=username, - http_scheme="https", - auth=trino.auth.BasicAuthentication(username, password), - session_properties={"query_max_execution_time": "60s"}, - ) - conn._http_session.verify = False - return conn - - -def run_query(connection, query): - print(f"[DEBUG] Executing query {query}") - cursor = connection.cursor() - cursor.execute(query) - return cursor.fetchall() - - -if __name__ == "__main__": - # Construct an argument parser - all_args = argparse.ArgumentParser() - # Add arguments to the parser - all_args.add_argument( - "-n", "--namespace", required=True, help="Namespace the test is running in" - ) - - args = vars(all_args.parse_args()) - namespace = args["namespace"] - - print("Starting S3 tests...") - connection = get_connection("admin", "admin", namespace) - - trino_version = run_query( - connection, - "select node_version from system.runtime.nodes where coordinator = true and state = 'active'", - )[0][0] - print(f'[INFO] Testing against Trino version "{trino_version}"') - - # Strip SDP release suffix from the version string - trino_product_version = re.split(r"-stackable", trino_version, maxsplit=1)[0] - - assert len(trino_product_version) >= 3 - assert trino_product_version.isnumeric() - assert trino_version == run_query(connection, "select version()")[0][0] - - run_query( - connection, - "CREATE SCHEMA IF NOT EXISTS hive.minio WITH (location = 's3a://trino/')", - ) - - run_query(connection, "DROP TABLE IF EXISTS hive.minio.taxi_data") - run_query(connection, "DROP TABLE IF EXISTS hive.minio.taxi_data_copy") - run_query(connection, "DROP TABLE IF EXISTS hive.minio.taxi_data_transformed") - run_query(connection, "DROP TABLE IF EXISTS hive.hdfs.taxi_data_copy") - run_query(connection, "DROP TABLE IF EXISTS iceberg.minio.taxi_data_copy_iceberg") - - run_query( - connection, - """ -CREATE TABLE IF NOT EXISTS hive.minio.taxi_data ( - vendor_id VARCHAR, - tpep_pickup_datetime VARCHAR, - tpep_dropoff_datetime VARCHAR, - passenger_count VARCHAR, - trip_distance VARCHAR, - ratecode_id VARCHAR -) WITH ( - external_location = 's3a://trino/taxi-data/', - format = 'csv', - skip_header_line_count = 1 -) - """, - ) - assert ( - run_query(connection, "SELECT COUNT(*) FROM hive.minio.taxi_data")[0][0] == 5000 - ) - rows_written = run_query( - connection, - "CREATE TABLE IF NOT EXISTS hive.minio.taxi_data_copy AS SELECT * FROM hive.minio.taxi_data", - )[0][0] - assert rows_written == 5000 or rows_written == 0 - assert ( - run_query(connection, "SELECT COUNT(*) FROM hive.minio.taxi_data_copy")[0][0] - == 5000 - ) - - rows_written = run_query( - connection, - """ -CREATE TABLE IF NOT EXISTS hive.minio.taxi_data_transformed AS -SELECT - CAST(vendor_id as BIGINT) as vendor_id, - tpep_pickup_datetime, - tpep_dropoff_datetime, - CAST(passenger_count as BIGINT) as passenger_count, - CAST(trip_distance as DOUBLE) as trip_distance, - CAST(ratecode_id as BIGINT) as ratecode_id -FROM hive.minio.taxi_data -""", - )[0][0] - assert rows_written == 5000 or rows_written == 0 - assert ( - run_query(connection, "SELECT COUNT(*) FROM hive.minio.taxi_data_transformed")[ - 0 - ][0] - == 5000 - ) - - print("[INFO] Testing HDFS") - - run_query( - connection, - "CREATE SCHEMA IF NOT EXISTS hive.hdfs WITH (location = 'hdfs://hdfs/trino/')", - ) - rows_written = run_query( - connection, - "CREATE TABLE IF NOT EXISTS hive.hdfs.taxi_data_copy AS SELECT * FROM hive.minio.taxi_data", - )[0][0] - assert rows_written == 5000 or rows_written == 0 - assert ( - run_query(connection, "SELECT COUNT(*) FROM hive.hdfs.taxi_data_copy")[0][0] - == 5000 - ) - - print("[INFO] Testing Iceberg") - run_query( - connection, "DROP TABLE IF EXISTS iceberg.minio.taxi_data_copy_iceberg" - ) # Clean up table to don't fail an second run - assert ( - run_query( - connection, - """ -CREATE TABLE IF NOT EXISTS iceberg.minio.taxi_data_copy_iceberg -WITH (partitioning = ARRAY['vendor_id', 'passenger_count'], format = 'parquet') -AS SELECT * FROM hive.minio.taxi_data -""", - )[0][0] - == 5000 - ) - # Check current count - assert ( - run_query( - connection, "SELECT COUNT(*) FROM iceberg.minio.taxi_data_copy_iceberg" - )[0][0] - == 5000 - ) - assert ( - run_query( - connection, - 'SELECT COUNT(*) FROM iceberg.minio."taxi_data_copy_iceberg$snapshots"', - )[0][0] - == 1 - ) - assert ( - run_query( - connection, - 'SELECT COUNT(*) FROM iceberg.minio."taxi_data_copy_iceberg$partitions"', - )[0][0] - == 12 - ) - assert ( - run_query( - connection, - 'SELECT COUNT(*) FROM iceberg.minio."taxi_data_copy_iceberg$files"', - )[0][0] - == 12 - ) - - assert ( - run_query( - connection, - "INSERT INTO iceberg.minio.taxi_data_copy_iceberg SELECT * FROM hive.minio.taxi_data", - )[0][0] - == 5000 - ) - - # Check current count - assert ( - run_query( - connection, "SELECT COUNT(*) FROM iceberg.minio.taxi_data_copy_iceberg" - )[0][0] - == 10000 - ) - assert ( - run_query( - connection, - 'SELECT COUNT(*) FROM iceberg.minio."taxi_data_copy_iceberg$snapshots"', - )[0][0] - == 2 - ) - assert ( - run_query( - connection, - 'SELECT COUNT(*) FROM iceberg.minio."taxi_data_copy_iceberg$partitions"', - )[0][0] - == 12 - ) - assert ( - run_query( - connection, - 'SELECT COUNT(*) FROM iceberg.minio."taxi_data_copy_iceberg$files"', - )[0][0] - == 24 - ) - - if trino_version == "377": - # io.trino.spi.TrinoException: This connector [iceberg] does not support versioned tables - print( - "[INFO] Skipping the Iceberg tests reading versioned tables for trino version 377 as it does not support versioned tables" - ) - else: - # Check count for first snapshot - first_snapshot = run_query( - connection, - 'select snapshot_id from iceberg.minio."taxi_data_copy_iceberg$snapshots" order by committed_at limit 1', - )[0][0] - assert ( - run_query( - connection, - f"SELECT COUNT(*) FROM iceberg.minio.taxi_data_copy_iceberg FOR VERSION AS OF {first_snapshot}", - )[0][0] - == 5000 - ) - - # Compact files - run_query( - connection, "ALTER TABLE iceberg.minio.taxi_data_copy_iceberg EXECUTE optimize" - ) - - # Check current count - assert ( - run_query( - connection, "SELECT COUNT(*) FROM iceberg.minio.taxi_data_copy_iceberg" - )[0][0] - == 10000 - ) - assert ( - run_query( - connection, - 'SELECT COUNT(*) FROM iceberg.minio."taxi_data_copy_iceberg$snapshots"', - )[0][0] - == 3 - ) - assert ( - run_query( - connection, - 'SELECT COUNT(*) FROM iceberg.minio."taxi_data_copy_iceberg$partitions"', - )[0][0] - == 12 - ) - assert ( - run_query( - connection, - 'SELECT COUNT(*) FROM iceberg.minio."taxi_data_copy_iceberg$files"', - )[0][0] - == 12 - ) # Compaction yeah :) - - # Test could be improved by also testing update and deletes - - # Test postgres connection - run_query(connection, "SHOW SCHEMAS IN postgresgeneric") - run_query(connection, "CREATE SCHEMA IF NOT EXISTS postgresgeneric.tpch") - run_query( - connection, - "CREATE TABLE IF NOT EXISTS postgresgeneric.tpch.nation AS SELECT * FROM tpch.tiny.nation", - ) - assert ( - run_query(connection, "SELECT COUNT(*) FROM postgresgeneric.tpch.nation")[0][0] - == 25 - ) - - print("[SUCCESS] All tests in check-s3.py succeeded!") From 04953d8aa8de141bf86d2214a79834f6511aec98 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Tue, 24 Jun 2025 11:57:06 +0200 Subject: [PATCH 36/38] adapt readme for smoke aws test --- tests/templates/kuttl/smoke_aws/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/templates/kuttl/smoke_aws/README.md b/tests/templates/kuttl/smoke_aws/README.md index 5bcbea75..a1343e64 100644 --- a/tests/templates/kuttl/smoke_aws/README.md +++ b/tests/templates/kuttl/smoke_aws/README.md @@ -12,7 +12,7 @@ aws s3api create-bucket --bucket ${BUCKET_NAME} --region eu-central-1 --create-b aws s3 cp yellow_tripdata_2021-07.csv s3://${BUCKET_NAME}/taxi-data/ ``` -You will need to update the bucket name in [check-s3.py](check-s3.py). +You will need to update the bucket name in the assert call for [check-s3.py](../commons/check-s3.py). ## Add AWS credentials From 4fd00f1599e34731ca763a225073016f0850917e Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Tue, 24 Jun 2025 15:58:47 +0200 Subject: [PATCH 37/38] move rolegroup_metrics_service_name out of cluster impl --- rust/operator-binary/src/controller.rs | 6 +++--- rust/operator-binary/src/crd/mod.rs | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/rust/operator-binary/src/controller.rs b/rust/operator-binary/src/controller.rs index 59c017a1..2b610599 100644 --- a/rust/operator-binary/src/controller.rs +++ b/rust/operator-binary/src/controller.rs @@ -88,7 +88,7 @@ use crate::{ authentication::resolve_authentication_classes, catalog, discovery::{TrinoDiscovery, TrinoDiscoveryProtocol, TrinoPodRef}, - v1alpha1, + rolegroup_metrics_service_name, v1alpha1, }, listener::{ LISTENER_VOLUME_DIR, LISTENER_VOLUME_NAME, build_group_listener, build_group_listener_pvc, @@ -1193,7 +1193,7 @@ fn build_rolegroup_statefulset( ), ..LabelSelector::default() }, - service_name: Some(v1alpha1::TrinoCluster::rolegroup_metrics_service_name( + service_name: Some(rolegroup_metrics_service_name( &role_group_ref.object_name(), )), template: pod_template, @@ -1215,7 +1215,7 @@ fn build_rolegroup_service( Ok(Service { metadata: ObjectMetaBuilder::new() .name_and_namespace(trino) - .name(v1alpha1::TrinoCluster::rolegroup_metrics_service_name( + .name(rolegroup_metrics_service_name( &role_group_ref.object_name(), )) .ownerreference_from_resource(trino, None, Some(true)) diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs index 61bc9fe6..d159ba19 100644 --- a/rust/operator-binary/src/crd/mod.rs +++ b/rust/operator-binary/src/crd/mod.rs @@ -837,7 +837,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_metrics_service_name( + role_group_service_name: rolegroup_metrics_service_name( &role_group_ref.object_name(), ), pod_name: format!( @@ -848,11 +848,6 @@ impl v1alpha1::TrinoCluster { })) } - /// Returns the metrics rolegroup service name `simple-trino-coordinator-default-`. - 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 pub fn get_authentication(&self) -> &Vec { &self.spec.cluster_config.authentication @@ -945,6 +940,11 @@ impl v1alpha1::TrinoCluster { } } +/// Returns the metrics rolegroup service name `---`. +pub fn rolegroup_metrics_service_name(role_group_ref_object_name: &str) -> String { + format!("{role_group_ref_object_name}-{METRICS_SERVICE_SUFFIX}") +} + fn extract_role_from_coordinator_config( fragment: Role, ) -> Role { From 186aa8ed06122e68783cbb82a48f7800713dc47b Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Tue, 24 Jun 2025 15:59:49 +0200 Subject: [PATCH 38/38] typo --- rust/operator-binary/src/crd/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs index d159ba19..a68bfddc 100644 --- a/rust/operator-binary/src/crd/mod.rs +++ b/rust/operator-binary/src/crd/mod.rs @@ -940,7 +940,7 @@ impl v1alpha1::TrinoCluster { } } -/// Returns the metrics rolegroup service name `---`. +/// Returns the metrics rolegroup service name `---`. pub fn rolegroup_metrics_service_name(role_group_ref_object_name: &str) -> String { format!("{role_group_ref_object_name}-{METRICS_SERVICE_SUFFIX}") }