diff --git a/CHANGELOG.md b/CHANGELOG.md index c78656cc..51d010b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -75,6 +75,9 @@ All notable changes to this project will be documented in this file. `CONTAINERDEBUG_LOG_DIRECTORY`, which containerdebug stopped reading in its version 0.2.0 when it moved to `stackable-telemetry`; the directory is now set via `FILE_LOG_DIRECTORY`. The file is rotated hourly and at most two files are kept, because rotation is opt-in ([#970]). +- A `TrinoCluster` whose name is not a valid cluster name, for example longer than 40 characters, + now fails its own reconcile with an error instead of panicking the operator, which then + crash-looped and reconciled no `TrinoCluster` at all ([#973]). ### Removed @@ -107,6 +110,7 @@ All notable changes to this project will be documented in this file. [#966]: https://github.com/stackabletech/trino-operator/pull/966 [#970]: https://github.com/stackabletech/trino-operator/pull/970 [#972]: https://github.com/stackabletech/trino-operator/pull/972 +[#973]: https://github.com/stackabletech/trino-operator/pull/973 ## [26.7.0] - 2026-07-21 diff --git a/rust/operator-binary/src/controller/validate.rs b/rust/operator-binary/src/controller/validate.rs index a642c6ed..9a190084 100644 --- a/rust/operator-binary/src/controller/validate.rs +++ b/rust/operator-binary/src/controller/validate.rs @@ -18,8 +18,12 @@ use stackable_operator::{ ValidatedContainerLogConfigChoice, VectorContainerLogConfig, validate_logging_configuration_for_container, }, + role_group_utils::ResourceNames, role_utils::{JavaCommonConfig, RoleGroup, RoleGroupConfig, with_validated_config}, - types::kubernetes::ConfigMapName, + types::{ + kubernetes::{ConfigMapName, NamespaceName}, + operator::{ClusterName, RoleName}, + }, }, }; use strum::{EnumDiscriminants, IntoStaticStr}; @@ -31,7 +35,7 @@ use crate::{ ValidatedCoordinatorRoleConfig, ValidatedTls, ValidatedTrinoConfig, ValidatedWorkerRoleConfig, dereference::DereferencedObjects, }, - crd::{Container, TrinoRole, catalog::TrinoCatalogName, v1alpha1}, + crd::{Container, TrinoRole, catalog::TrinoCatalogName, discovery::TrinoPodRef, v1alpha1}, }; #[derive(Snafu, Debug, EnumDiscriminants)] @@ -178,6 +182,7 @@ pub fn validate( operator_environment: &OperatorEnvironmentOptions, ) -> Result { let namespace = get_namespace(trino).context(GetClusterNamespaceSnafu)?; + let name = get_cluster_name(trino).context(GetClusterNameSnafu)?; let image = trino .spec @@ -290,11 +295,10 @@ pub fn validate( fault_tolerant_execution: dereferenced_objects.resolved_fte_config.clone(), client_protocol: dereferenced_objects.resolved_client_protocol_config.clone(), lineage: dereferenced_objects.resolved_lineage_config.clone(), - coordinator_pod_refs: trino.coordinator_pods(&namespace).collect(), + coordinator_pod_refs: coordinator_pods(&name, &namespace, &coordinator_role_group_configs), catalogs, }; - let name = get_cluster_name(trino).context(GetClusterNameSnafu)?; let uid = get_uid(trino).context(GetClusterUidSnafu)?; Ok(ValidatedCluster::new(ValidatedClusterParams { @@ -311,6 +315,44 @@ pub fn validate( })) } +/// The replica count lower bound used for enumerating role groups when this is not set explicitly (in such cases Kubernetes +/// runs a single pod for a `StatefulSet`). +const COORDINATOR_REPLICAS_IF_UNSET: u16 = 1; + +/// List all coordinator pods expected to form the cluster +/// +/// We try to predict the pods here rather than looking at the current cluster state in order to +/// avoid instance churn. +/// +/// A role group without an explicit `replicas` counts will be assigned a single pod by Kubernetes. Counting it as +/// zero would yield no pod refs at all for a single-role-group cluster, and the first of these +/// is what sets `discovery.uri`. +fn coordinator_pods( + cluster_name: &ClusterName, + namespace: &NamespaceName, + role_group_configs: &BTreeMap, +) -> Vec { + let role_name = RoleName::from(&TrinoRole::Coordinator); + role_group_configs + .iter() + .flat_map(|(role_group_name, role_group_config)| { + let resource_names = ResourceNames { + cluster_name: cluster_name.clone(), + role_name: role_name.clone(), + role_group_name: role_group_name.clone(), + }; + (0..role_group_config + .replicas + .unwrap_or(COORDINATOR_REPLICAS_IF_UNSET)) + .map(move |i| TrinoPodRef { + namespace: namespace.to_string(), + role_group_service_name: resource_names.headless_service_name().to_string(), + pod_name: format!("{}-{i}", resource_names.stateful_set_name()), + }) + }) + .collect() +} + /// Validates the user-provided `datasetNamespaceUri`, so that a typo fails the reconcile instead of /// only surfacing once Trino starts. /// @@ -591,6 +633,15 @@ mod tests { } } + #[test] + fn validate_rejects_cluster_name_longer_than_cluster_name_limit() { + let yaml = minimal_yaml("481").replace("simple-trino", &"a".repeat(41)); + + let err = validate_yaml(&yaml, &empty_derefs()) + .expect_err("a 41 character cluster name exceeds the ClusterName limit"); + assert!(matches!(err, Error::GetClusterName { .. })); + } + #[test] fn validate_logging_rejects_invalid_custom_config_map_name() { let yaml = r#" diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs index afc6dc4d..86bce6e4 100644 --- a/rust/operator-binary/src/crd/mod.rs +++ b/rust/operator-binary/src/crd/mod.rs @@ -5,7 +5,7 @@ pub mod client_protocol; pub mod discovery; pub mod fault_tolerant_execution; -use std::{collections::BTreeMap, ops::Deref, str::FromStr}; +use std::{ops::Deref, str::FromStr}; use affinity::get_affinity; use serde::{Deserialize, Serialize}; @@ -25,7 +25,7 @@ use stackable_operator::{ crd::{authentication::core, openlineage}, deep_merger::ObjectOverrides, k8s_openapi::apimachinery::pkg::{api::resource::Quantity, apis::meta::v1::LabelSelector}, - kube::{CustomResource, ResourceExt}, + kube::CustomResource, memory::{BinaryMultiple, MemoryQuantity}, product_logging::{self, spec::Logging}, role_utils::GenericRoleConfig, @@ -35,23 +35,19 @@ use stackable_operator::{ v2::{ builder::pod::container::EnvVarName, config_overrides::KeyValueConfigOverrides, - role_group_utils::ResourceNames, role_utils::{JavaCommonConfig, Role}, types::{ common::Port, kubernetes::{ - ConfigMapName, ContainerName, ListenerClassName, NamespaceName, SecretClassName, - SecretKey, + ConfigMapName, ContainerName, ListenerClassName, SecretClassName, SecretKey, }, - operator::{ClusterName, RoleGroupName, RoleName}, + operator::RoleName, }, }, versioned::versioned, }; use strum::{Display, EnumIter}; -use crate::crd::discovery::TrinoPodRef; - pub type TrinoCoordinatorRoleType = Role< v1alpha1::TrinoConfigFragment, v1alpha1::TrinoConfigOverrides, @@ -561,53 +557,7 @@ impl v1alpha1::TrinoConfig { } } -/// The replica count lower bound used for enumerating role groups when this is not set explicitly (in such cases Kubernetes -/// runs a single pod for a `StatefulSet`). -const COORDINATOR_REPLICAS_IF_UNSET: u16 = 1; - impl v1alpha1::TrinoCluster { - /// List all coordinator pods expected to form the cluster - /// - /// We try to predict the pods here rather than looking at the current cluster state in order to - /// avoid instance churn. - /// - /// A role group without an explicit `replicas` counts will be assigned a single pod by Kubernetes. Counting it as - /// zero would yield no pod refs at all for a single-role-group cluster, and the first of these - /// is what sets `discovery.uri`. - pub fn coordinator_pods( - &self, - namespace: &NamespaceName, - ) -> impl Iterator + '_ { - let ns = namespace.to_string(); - let cluster_name = ClusterName::from_str(&self.name_any()) - .expect("a TrinoCluster name is a valid cluster name"); - let role_name = RoleName::from_str(&TrinoRole::Coordinator.to_string()) - .expect("the coordinator role name is a valid role name"); - self.spec - .coordinators - .role_groups - .iter() - // Order rolegroups consistently, to avoid spurious downstream rewrites - .collect::>() - .into_iter() - .flat_map(move |(rolegroup_name, rolegroup)| { - let resource_names = ResourceNames { - cluster_name: cluster_name.clone(), - role_name: role_name.clone(), - role_group_name: RoleGroupName::from_str(rolegroup_name) - .expect("a role group name is a valid role group name"), - }; - let ns = ns.clone(); - (0..rolegroup.replicas.unwrap_or(COORDINATOR_REPLICAS_IF_UNSET)).map(move |i| { - TrinoPodRef { - namespace: ns.clone(), - role_group_service_name: resource_names.headless_service_name().to_string(), - pod_name: format!("{}-{i}", resource_names.stateful_set_name()), - } - }) - }) - } - /// Returns user provided authentication settings pub fn get_authentication(&self) -> &Vec { &self.spec.cluster_config.authentication