From 3f9122eb80a7874d6def5718bb9099e9f2de456a Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Wed, 15 Jan 2025 09:46:35 +0100 Subject: [PATCH 1/3] chore: Increase coordinator temporary credentials lifetime --- rust/crd/src/lib.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/rust/crd/src/lib.rs b/rust/crd/src/lib.rs index 5665b82d..5863d181 100644 --- a/rust/crd/src/lib.rs +++ b/rust/crd/src/lib.rs @@ -440,7 +440,6 @@ pub struct TrinoConfig { } impl TrinoConfig { - const DEFAULT_SECRET_LIFETIME: Duration = Duration::from_days_unchecked(1); fn default_config( cluster_name: &str, role: &TrinoRole, @@ -454,6 +453,13 @@ impl TrinoConfig { TrinoRole::Coordinator => DEFAULT_COORDINATOR_GRACEFUL_SHUTDOWN_TIMEOUT, TrinoRole::Worker => DEFAULT_WORKER_GRACEFUL_SHUTDOWN_TIMEOUT, }; + let requested_secret_lifetime = match role { + // TODO: Once Trino supports a HA setup for coordinators we should decrease this! + // See https://github.com/stackabletech/trino-operator/issues/693 + // and https://github.com/stackabletech/decisions/issues/38 for details + TrinoRole::Coordinator => Duration::from_days_unchecked(15), + TrinoRole::Worker => Duration::from_days_unchecked(1), + }; TrinoConfigFragment { logging: product_logging::spec::default_logging(), @@ -478,7 +484,7 @@ impl TrinoConfig { query_max_memory: None, query_max_memory_per_node: None, graceful_shutdown_timeout: Some(graceful_shutdown_timeout), - requested_secret_lifetime: Some(Self::DEFAULT_SECRET_LIFETIME), + requested_secret_lifetime: Some(requested_secret_lifetime), } } } From 058a804d9ee5ecbc26a49899ea48f1024b5a7fc6 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Wed, 15 Jan 2025 09:48:20 +0100 Subject: [PATCH 2/3] changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6dbdf22a..d8d4317b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,11 @@ All notable changes to this project will be documented in this file. config property `requestedSecretLifetime`. This helps reduce frequent Pod restarts ([#676]). - Run a `containerdebug` process in the background of each Trino container to collect debugging information ([#687]). +## Changed + +- Increased the default temporary secret lifetime for coordinators from 1 day to 15 days. + This is because Trino currently does not offer a HA setup for them, a restart kills all running queries ([#694]). + ### Fixed - Fix OIDC endpoint construction in case the `rootPath` does have a trailing slash ([#673]). @@ -21,6 +26,7 @@ All notable changes to this project will be documented in this file. [#673]: https://github.com/stackabletech/trino-operator/pull/673 [#676]: https://github.com/stackabletech/trino-operator/pull/676 [#687]: https://github.com/stackabletech/trino-operator/pull/687 +[#694]: https://github.com/stackabletech/trino-operator/pull/694 ## [24.11.0] - 2024-11-18 From 030c4eced7309de65dd7e063773e371765e8ab6e Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Wed, 22 Jan 2025 09:30:27 +0100 Subject: [PATCH 3/3] docs: Document lifetimes --- deploy/helm/trino-operator/crds/crds.yaml | 20 ++++++++++++++++---- rust/crd/src/lib.rs | 3 +++ 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/deploy/helm/trino-operator/crds/crds.yaml b/deploy/helm/trino-operator/crds/crds.yaml index bf803700..8f3d2358 100644 --- a/deploy/helm/trino-operator/crds/crds.yaml +++ b/deploy/helm/trino-operator/crds/crds.yaml @@ -298,7 +298,10 @@ spec: nullable: true type: string requestedSecretLifetime: - description: Request secret (currently only autoTls certificates) lifetime from the secret operator, e.g. `7d`, or `30d`. This can be shortened by the `maxCertificateLifetime` setting on the SecretClass issuing the TLS certificate. + description: |- + Request secret (currently only autoTls certificates) lifetime from the secret operator, e.g. `7d`, or `30d`. This can be shortened by the `maxCertificateLifetime` setting on the SecretClass issuing the TLS certificate. + + Defaults to `15d` for coordinators (as currently a restart kills all running queries) and `1d` for workers. nullable: true type: string resources: @@ -571,7 +574,10 @@ spec: nullable: true type: string requestedSecretLifetime: - description: Request secret (currently only autoTls certificates) lifetime from the secret operator, e.g. `7d`, or `30d`. This can be shortened by the `maxCertificateLifetime` setting on the SecretClass issuing the TLS certificate. + description: |- + Request secret (currently only autoTls certificates) lifetime from the secret operator, e.g. `7d`, or `30d`. This can be shortened by the `maxCertificateLifetime` setting on the SecretClass issuing the TLS certificate. + + Defaults to `15d` for coordinators (as currently a restart kills all running queries) and `1d` for workers. nullable: true type: string resources: @@ -873,7 +879,10 @@ spec: nullable: true type: string requestedSecretLifetime: - description: Request secret (currently only autoTls certificates) lifetime from the secret operator, e.g. `7d`, or `30d`. This can be shortened by the `maxCertificateLifetime` setting on the SecretClass issuing the TLS certificate. + description: |- + Request secret (currently only autoTls certificates) lifetime from the secret operator, e.g. `7d`, or `30d`. This can be shortened by the `maxCertificateLifetime` setting on the SecretClass issuing the TLS certificate. + + Defaults to `15d` for coordinators (as currently a restart kills all running queries) and `1d` for workers. nullable: true type: string resources: @@ -1146,7 +1155,10 @@ spec: nullable: true type: string requestedSecretLifetime: - description: Request secret (currently only autoTls certificates) lifetime from the secret operator, e.g. `7d`, or `30d`. This can be shortened by the `maxCertificateLifetime` setting on the SecretClass issuing the TLS certificate. + description: |- + Request secret (currently only autoTls certificates) lifetime from the secret operator, e.g. `7d`, or `30d`. This can be shortened by the `maxCertificateLifetime` setting on the SecretClass issuing the TLS certificate. + + Defaults to `15d` for coordinators (as currently a restart kills all running queries) and `1d` for workers. nullable: true type: string resources: diff --git a/rust/crd/src/lib.rs b/rust/crd/src/lib.rs index 5863d181..b7659a10 100644 --- a/rust/crd/src/lib.rs +++ b/rust/crd/src/lib.rs @@ -435,6 +435,9 @@ pub struct TrinoConfig { /// Request secret (currently only autoTls certificates) lifetime from the secret operator, e.g. `7d`, or `30d`. /// This can be shortened by the `maxCertificateLifetime` setting on the SecretClass issuing the TLS certificate. + /// + /// Defaults to `15d` for coordinators (as currently a restart kills all running queries) + /// and `1d` for workers. #[fragment_attrs(serde(default))] pub requested_secret_lifetime: Option, }