fix!: Remove unused PVCs which caused problems (#769)

* fix!: Remove unused PVCs which caused problems

* changelog

* docs: Document why we need to provide an empty struct

* remove missleading leftover comment

* Update tests and docs

Author: sbernauer

CHANGELOG.md

@@ -39,6 +39,16 @@ All notable changes to this project will be documented in this file.
 
 - Use `json` file extension for log files ([#733]).
 - Fix a bug where changes to ConfigMaps that are referenced in the TrinoCluster spec didn't trigger a reconciliation ([#734]).
+- BREAKING: The PersistentVolumeClaims for coordinator and workers have been removed ([#769])
+  - They caused problems, as Trino kept it's process ID in `/stackable/data/var/run/launcher.pid`.
+    A forceful stop (e.g. OOMKilled) could result in a leftover PID in this file.
+    In this case Trino would refuse startup with `trino ERROR: already running as 21`.
+    As the PersistentVolumeClaims didn't store any actual data, they have been removed.
+  - Upgrading will result in the error message `Failed to reconcile object [...]: Forbidden: updates to   statefulset spec for fields other than [...] are forbidden`
+    as Kubernetes currently does not allow changing the `volumeClaimTemplates` field. Simply delete the mentioned StatefulSet, the operator will re-create it.
+  - You might want to clean up now useless PVCs.
+    Tip: You can list all Trino-related PVCs using `kubectl get pvc -l app.kubernetes.io/name=trino`.
+  - The `.spec.(coordinators|workers).config.resources.storage.data` field has been removed, as it's not needed anymore.
 
 ### Removed
 
@@ -58,6 +68,7 @@ All notable changes to this project will be documented in this file.
 [#755]: https://github.com/stackabletech/trino-operator/pull/755
 [#760]: https://github.com/stackabletech/trino-operator/pull/760
 [#766]: https://github.com/stackabletech/trino-operator/pull/766
+[#769]: https://github.com/stackabletech/trino-operator/pull/769
 
 ## [25.3.0] - 2025-03-21
 

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

@@ -108,36 +108,7 @@ workers:
 
 Here too, overriding properties such as `http-server.https.port` will lead to broken installations.
 
-== Resources
-
-=== Storage for data volumes
-
-You can mount a volume where data (config and logs of Trino) is stored by specifying https://kubernetes.io/docs/concepts/storage/persistent-volumes[PersistentVolumeClaims] for each individual role or role group:
-
-[source,yaml]
-----
-workers:
-  config:
-    resources:
-      storage:
-        data:
-          capacity: 2Gi
-  roleGroups:
-    default:
-      config:
-        resources:
-          storage:
-            data:
-              capacity: 3Gi
-----
-
-In the above example, all Trino workers in the default group store data (the location of the property `--data-dir`) on a `3Gi` volume.
-Additional role groups not specifying any resources inherit the config provided on the role level (`2Gi` volume).
-This works the same for memory or CPU requests.
-
-By default, in case nothing is configured in the custom resource for a certain role group, each Pod has a `2Gi` large local volume mount for the data location containing mainly logs.
-
-=== Resource Requests
+== Resource Requests
 
 include::home:concepts:stackable_resource_requests.adoc[]
 
@@ -161,9 +132,6 @@ spec:
           max: '2000m'
         memory:
           limit: '4Gi'
-        storage:
-          data:
-            capacity: '1Gi'
   workers:
     config:
       resources:
@@ -172,9 +140,6 @@ spec:
           max: '4000m'
         memory:
           limit: '4Gi'
-        storage:
-          data:
-            capacity: '1Gi'
 ----
 
 WARNING: The default values are _most likely_ not sufficient to run a proper cluster in production.

docs/modules/trino/pages/usage-guide/configuration.adoc

@@ -18,9 +18,6 @@ spec:
         replicas: 1
         config:
           resources:
-            storage:
-              data:
-                capacity: 3Gi
             cpu:
               min: 300m
               max: "2"

examples/simple-trino-cluster-resource-limits.yaml

@@ -11,8 +11,8 @@ use crate::{
     catalog::config::CatalogConfig,
     controller::{STACKABLE_LOG_CONFIG_DIR, STACKABLE_LOG_DIR},
     crd::{
-        CONFIG_DIR_NAME, Container, DATA_DIR_NAME, LOG_PROPERTIES, RW_CONFIG_DIR_NAME,
-        STACKABLE_CLIENT_TLS_DIR, STACKABLE_INTERNAL_TLS_DIR, STACKABLE_MOUNT_INTERNAL_TLS_DIR,
+        CONFIG_DIR_NAME, Container, LOG_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, STACKABLE_TLS_STORE_PASSWORD,
         SYSTEM_TRUST_STORE, SYSTEM_TRUST_STORE_PASSWORD, TrinoRole, v1alpha1,
     },
@@ -119,7 +119,7 @@ pub fn container_trino_args(
 {remove_vector_shutdown_file_command}
 prepare_signal_handlers
 containerdebug --output={STACKABLE_LOG_DIR}/containerdebug-state.json --loop &
-bin/launcher run --etc-dir={RW_CONFIG_DIR_NAME} --data-dir={DATA_DIR_NAME} &
+bin/launcher run --etc-dir={RW_CONFIG_DIR_NAME} &
 wait_for_termination $!
 {create_vector_shutdown_file_command}
 ",

rust/operator-binary/src/command.rs

@@ -78,12 +78,11 @@ 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,
-        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,
+        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,
         authentication::resolve_authentication_classes,
         catalog,
         discovery::{TrinoDiscovery, TrinoDiscoveryProtocol, TrinoPodRef},
@@ -1008,8 +1007,6 @@ fn build_rolegroup_statefulset(
             "-c".to_string(),
         ])
         .args(vec![prepare_args.join("\n")])
-        .add_volume_mount("data", DATA_DIR_NAME)
-        .context(AddVolumeMountSnafu)?
         .add_volume_mount("rwconfig", RW_CONFIG_DIR_NAME)
         .context(AddVolumeMountSnafu)?
         .add_volume_mount("log-config", STACKABLE_LOG_CONFIG_DIR)
@@ -1026,14 +1023,7 @@ 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"])),
-    ];
+    let mut persistent_volume_claims = vec![];
     // Add listener
     if let Some(group_listener_name) = group_listener_name(trino, trino_role) {
         cb_trino
@@ -1069,8 +1059,6 @@ fn build_rolegroup_statefulset(
             command::container_trino_args(trino_authentication_config, catalogs).join("\n"),
         ])
         .add_env_vars(env)
-        .add_volume_mount("data", DATA_DIR_NAME)
-        .context(AddVolumeMountSnafu)?
         .add_volume_mount("config", CONFIG_DIR_NAME)
         .context(AddVolumeMountSnafu)?
         .add_volume_mount("rwconfig", RW_CONFIG_DIR_NAME)

rust/operator-binary/src/controller.rs

@@ -16,7 +16,7 @@ use stackable_operator::{
         product_image_selection::ProductImage,
         resources::{
             CpuLimitsFragment, MemoryLimitsFragment, NoRuntimeLimits, NoRuntimeLimitsFragment,
-            PvcConfig, PvcConfigFragment, Resources, ResourcesFragment,
+            Resources, ResourcesFragment,
         },
     },
     config::{
@@ -91,7 +91,6 @@ pub const NODE_INTERNAL_ADDRESS_SOURCE_FQDN: &str = "FQDN";
 // directories
 pub const CONFIG_DIR_NAME: &str = "/stackable/config";
 pub const RW_CONFIG_DIR_NAME: &str = "/stackable/rwconfig";
-pub const DATA_DIR_NAME: &str = "/stackable/data";
 pub const STACKABLE_SERVER_TLS_DIR: &str = "/stackable/server_tls";
 pub const STACKABLE_CLIENT_TLS_DIR: &str = "/stackable/client_tls";
 pub const STACKABLE_INTERNAL_TLS_DIR: &str = "/stackable/internal_tls";
@@ -232,11 +231,17 @@ pub mod versioned {
     pub struct TrinoConfig {
         // config.properties
         pub query_max_memory: Option<String>,
+
         pub query_max_memory_per_node: Option<String>,
+
         #[fragment_attrs(serde(default))]
         pub logging: Logging<Container>,
+
+        // We need to provide *something* that implements `Fragment`, so we pick an empty struct here.
+        // Note that a unit "()" would not work, as we need something that implements `Fragment`.
         #[fragment_attrs(serde(default))]
         pub resources: Resources<TrinoStorageConfig, NoRuntimeLimits>,
+
         #[fragment_attrs(serde(default))]
         pub affinity: StackableAffinity,
 
@@ -327,10 +332,7 @@ pub mod versioned {
         ),
         serde(rename_all = "camelCase")
     )]
-    pub struct TrinoStorageConfig {
-        #[fragment_attrs(serde(default))]
-        pub data: PvcConfig,
-    }
+    pub struct TrinoStorageConfig {}
 
     #[derive(Clone, Default, Debug, Deserialize, Eq, JsonSchema, PartialEq, Serialize)]
     #[serde(rename_all = "camelCase")]
@@ -490,13 +492,7 @@ impl v1alpha1::TrinoConfig {
                     limit: Some(Quantity(memory.to_string())),
                     runtime_limits: NoRuntimeLimitsFragment {},
                 },
-                storage: v1alpha1::TrinoStorageConfigFragment {
-                    data: PvcConfigFragment {
-                        capacity: Some(Quantity("1Gi".to_owned())),
-                        storage_class: None,
-                        selectors: None,
-                    },
-                },
+                storage: v1alpha1::TrinoStorageConfigFragment {},
             },
             query_max_memory: None,
             query_max_memory_per_node: None,

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

@@ -26,17 +26,6 @@ status:
   readyReplicas: 1
   replicas: 1
 ---
-apiVersion: v1
-kind: PersistentVolumeClaim
-metadata:
-  name: data-trino-coordinator-resources-default-0
-spec:
-  accessModes:
-    - ReadWriteOnce
-  resources:
-    requests:
-      storage: 1Gi
----
 apiVersion: apps/v1
 kind: StatefulSet
 metadata:
@@ -60,17 +49,6 @@ status:
   readyReplicas: 1
   replicas: 1
 ---
-apiVersion: v1
-kind: PersistentVolumeClaim
-metadata:
-  name: data-trino-worker-resources-from-role-0
-spec:
-  accessModes:
-    - ReadWriteOnce
-  resources:
-    requests:
-      storage: 3Gi
----
 apiVersion: apps/v1
 kind: StatefulSet
 metadata:
@@ -94,17 +72,6 @@ status:
   readyReplicas: 1
   replicas: 1
 ---
-apiVersion: v1
-kind: PersistentVolumeClaim
-metadata:
-  name: data-trino-worker-resources-from-role-group-0
-spec:
-  accessModes:
-    - ReadWriteOnce
-  resources:
-    requests:
-      storage: 4Gi
----
 apiVersion: apps/v1
 kind: StatefulSet
 metadata:
@@ -127,14 +94,3 @@ spec:
 status:
   readyReplicas: 1
   replicas: 1
----
-apiVersion: v1
-kind: PersistentVolumeClaim
-metadata:
-  name: data-trino-worker-resources-from-pod-overrides-0
-spec:
-  accessModes:
-    - ReadWriteOnce
-  resources:
-    requests:
-      storage: 3Gi

tests/templates/kuttl/resources/10-assert.yaml.j2

@@ -30,9 +30,6 @@ spec:
       logging:
         enableVectorAgent: {{ lookup('env', 'VECTOR_AGGREGATOR') | length > 0 }}
       resources:
-        storage:
-          data:
-            capacity: 3Gi
         cpu:
           min: 300m
           max: 600m
@@ -45,9 +42,6 @@ spec:
         replicas: 1
         config:
           resources:
-            storage:
-              data:
-                capacity: 4Gi
             cpu:
               min: 400m
               max: 800m