From 052b7a6dfb7b8021279b25ea57959fb14cb5b504 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Fri, 28 Feb 2025 10:44:53 +0100 Subject: [PATCH 1/7] feat: Support removing properties from catalogs --- CHANGELOG.md | 2 ++ deploy/helm/trino-operator/crds/crds.yaml | 11 +++++++++++ rust/operator-binary/src/catalog/config.rs | 10 ++++++++++ rust/operator-binary/src/crd/catalog/mod.rs | 13 ++++++++++++- 4 files changed, 35 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 74afe5ee..c5cfa16e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ All notable changes to this project will be documented in this file. - Run a `containerdebug` process in the background of each Trino container to collect debugging information ([#687]). - Support configuring JVM arguments ([#677]). - Aggregate emitted Kubernetes events on the CustomResources ([#677]). +- Support removing properties from catalogs. + This is helpful, because Trino fails to start in case you have any unused config properties ([#XXX]). ## Changed diff --git a/deploy/helm/trino-operator/crds/crds.yaml b/deploy/helm/trino-operator/crds/crds.yaml index 9d2c30f4..655b493c 100644 --- a/deploy/helm/trino-operator/crds/crds.yaml +++ b/deploy/helm/trino-operator/crds/crds.yaml @@ -1985,6 +1985,17 @@ spec: description: A [TPC-H](https://docs.stackable.tech/home/nightly/trino/usage-guide/catalogs/tpch) connector. type: object type: object + experimentalConfigRemovals: + default: [] + description: |- + List of config properties which should be removed. + + This is helpful, because Trino fails to start in case you have any unused config properties. The removals are executed after the `configOverrides`. + + This field is experimental, as ideally some general solution for the removal of properties is found and added to configOverrides. This mechanism would replace this field. + items: + type: string + type: array required: - connector type: object diff --git a/rust/operator-binary/src/catalog/config.rs b/rust/operator-binary/src/catalog/config.rs index a52a0533..990a39da 100644 --- a/rust/operator-binary/src/catalog/config.rs +++ b/rust/operator-binary/src/catalog/config.rs @@ -160,6 +160,16 @@ impl CatalogConfig { .properties .extend(catalog.spec.config_overrides.clone()); + for removal in &catalog.spec.config_removals { + if catalog_config.properties.remove(removal).is_none() { + tracing::warn!( + catalog.name = catalog_name, + property = removal, + "You asked to remove a non-existing config property from a catalog" + ); + } + } + Ok(catalog_config) } } diff --git a/rust/operator-binary/src/crd/catalog/mod.rs b/rust/operator-binary/src/crd/catalog/mod.rs index ac26569b..f83b9be2 100644 --- a/rust/operator-binary/src/crd/catalog/mod.rs +++ b/rust/operator-binary/src/crd/catalog/mod.rs @@ -48,11 +48,22 @@ pub mod versioned { pub struct TrinoCatalogSpec { /// The `connector` defines which connector is used. pub connector: TrinoCatalogConnector, - #[serde(default)] /// The `configOverrides` allow overriding arbitrary Trino settings. /// For example, for Hive you could add `hive.metastore.username: trino`. + #[serde(default)] pub config_overrides: HashMap, + + /// List of config properties which should be removed. + /// + /// This is helpful, because Trino fails to start in case you have any unused config + /// properties. The removals are executed after the `configOverrides`. + /// + /// This field is experimental, as ideally some general solution for the removal of + /// properties is found and added to configOverrides. This mechanism would replace this + /// field. + #[serde(default, rename = "experimentalConfigRemovals")] + pub config_removals: Vec, } } From f536ff3c3023e9018c10c8d536cba22e11991b6f Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Fri, 28 Feb 2025 11:08:18 +0100 Subject: [PATCH 2/7] changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c5cfa16e..1ae85a55 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ All notable changes to this project will be documented in this file. - Support configuring JVM arguments ([#677]). - Aggregate emitted Kubernetes events on the CustomResources ([#677]). - Support removing properties from catalogs. - This is helpful, because Trino fails to start in case you have any unused config properties ([#XXX]). + This is helpful, because Trino fails to start in case you have any unused config properties ([#713]). ## Changed @@ -25,6 +25,7 @@ All notable changes to this project will be documented in this file. [#687]: https://github.com/stackabletech/trino-operator/pull/687 [#694]: https://github.com/stackabletech/trino-operator/pull/694 [#695]: https://github.com/stackabletech/trino-operator/pull/695 +[#713]: https://github.com/stackabletech/trino-operator/pull/713 ## [24.11.1] - 2025-01-10 From 9fa6bc9df00501ef2ae9a5d67a47fcf7f8267b65 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Fri, 28 Feb 2025 11:15:09 +0100 Subject: [PATCH 3/7] Add docs --- .../pages/usage-guide/catalogs/index.adoc | 28 +++++++++++++++++-- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/docs/modules/trino/pages/usage-guide/catalogs/index.adoc b/docs/modules/trino/pages/usage-guide/catalogs/index.adoc index 5999d850..a993cede 100644 --- a/docs/modules/trino/pages/usage-guide/catalogs/index.adoc +++ b/docs/modules/trino/pages/usage-guide/catalogs/index.adoc @@ -31,9 +31,6 @@ spec: accessStyle: Path credentials: secretClass: minio-credentials -# We can use configOverrides to add arbitrary properties to the Trino catalog configuration - configOverrides: - hive.metastore.username: trino --- apiVersion: trino.stackable.tech/v1alpha1 kind: TrinoCatalog @@ -60,6 +57,31 @@ The `metadata.labels` are used by TrinoCluster to determine the link between Tri The `spec.connector.` determines which connector is used. Each connector supports a different set of attributes. +=== Config overrides and config removals + +You can use `.spec.configOverrides` to set arbitrary additional properties, which will be added to the catalog. + +There is also `.spec.experimentalConfigRemovals` to remove any properties the operator might set, but are not used by Trino. +This causes Trino to log an error message such as `Error: Configuration property 'hive.s3.aws-access-key' was not used` and refuse startup. +By removing the unneeded properties you can get Trino to start again. + +This example illustrates how to use config overrides and config removals + +[source,yaml] +---- +apiVersion: trino.stackable.tech/v1alpha1 +kind: TrinoCatalog +spec: + connector: + # Config as normal... + # Add some properties + configOverrides: + hive.metastore.username: trino + # Remove some properties + experimentalConfigRemovals: + - hive.s3.aws-access-key +---- + === Add a catalog to a Trino cluster It is necessary to specify within the TrinoCluster which catalogs it should use. From ddc5de9739b4d69cd2c4df70463ffed12e22b67d Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Fri, 28 Feb 2025 11:15:58 +0100 Subject: [PATCH 4/7] Improve docs --- docs/modules/trino/pages/usage-guide/catalogs/index.adoc | 2 -- 1 file changed, 2 deletions(-) diff --git a/docs/modules/trino/pages/usage-guide/catalogs/index.adoc b/docs/modules/trino/pages/usage-guide/catalogs/index.adoc index a993cede..0763602b 100644 --- a/docs/modules/trino/pages/usage-guide/catalogs/index.adoc +++ b/docs/modules/trino/pages/usage-guide/catalogs/index.adoc @@ -72,8 +72,6 @@ This example illustrates how to use config overrides and config removals apiVersion: trino.stackable.tech/v1alpha1 kind: TrinoCatalog spec: - connector: - # Config as normal... # Add some properties configOverrides: hive.metastore.username: trino From 4034b26a9101cf3f38948414132f502172168041 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Thu, 13 Mar 2025 09:42:34 +0100 Subject: [PATCH 5/7] fix merge conflicts --- rust/operator-binary/src/crd/catalog/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/rust/operator-binary/src/crd/catalog/mod.rs b/rust/operator-binary/src/crd/catalog/mod.rs index 9123b4da..f83b9be2 100644 --- a/rust/operator-binary/src/crd/catalog/mod.rs +++ b/rust/operator-binary/src/crd/catalog/mod.rs @@ -49,7 +49,6 @@ pub mod versioned { /// The `connector` defines which connector is used. pub connector: TrinoCatalogConnector, - #[serde(default)] /// The `configOverrides` allow overriding arbitrary Trino settings. /// For example, for Hive you could add `hive.metastore.username: trino`. #[serde(default)] From 3d03a2649b61e5f61fa8fc5a0100afe7da0f59d3 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Thu, 13 Mar 2025 12:39:38 +0100 Subject: [PATCH 6/7] Apply suggestions from code review Co-authored-by: Malte Sander --- docs/modules/trino/pages/usage-guide/catalogs/index.adoc | 2 +- rust/operator-binary/src/crd/catalog/mod.rs | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/docs/modules/trino/pages/usage-guide/catalogs/index.adoc b/docs/modules/trino/pages/usage-guide/catalogs/index.adoc index a377b6fc..6fa71c86 100644 --- a/docs/modules/trino/pages/usage-guide/catalogs/index.adoc +++ b/docs/modules/trino/pages/usage-guide/catalogs/index.adoc @@ -62,7 +62,7 @@ Each connector supports a different set of attributes. You can use `.spec.configOverrides` to set arbitrary additional properties, which will be added to the catalog. There is also `.spec.experimentalConfigRemovals` to remove any properties the operator might set, but are not used by Trino. -This causes Trino to log an error message such as `Error: Configuration property 'hive.s3.aws-access-key' was not used` and refuse startup. +This causes Trino to refuse to startup with an error message such as `Error: Configuration property 'hive.s3.aws-access-key' was not used`. By removing the unneeded properties you can get Trino to start again. This example illustrates how to use config overrides and config removals diff --git a/rust/operator-binary/src/crd/catalog/mod.rs b/rust/operator-binary/src/crd/catalog/mod.rs index f83b9be2..dc2fa10c 100644 --- a/rust/operator-binary/src/crd/catalog/mod.rs +++ b/rust/operator-binary/src/crd/catalog/mod.rs @@ -59,9 +59,7 @@ pub mod versioned { /// This is helpful, because Trino fails to start in case you have any unused config /// properties. The removals are executed after the `configOverrides`. /// - /// This field is experimental, as ideally some general solution for the removal of - /// properties is found and added to configOverrides. This mechanism would replace this - /// field. + /// This field is experimental, and might be replaced by a more generic mechanism to edit config properties #[serde(default, rename = "experimentalConfigRemovals")] pub config_removals: Vec, } From c3a86db4fba8073ce09dd5c8351422fc1727b499 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Thu, 13 Mar 2025 17:13:04 +0100 Subject: [PATCH 7/7] charts --- deploy/helm/trino-operator/crds/crds.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/deploy/helm/trino-operator/crds/crds.yaml b/deploy/helm/trino-operator/crds/crds.yaml index d189d617..698a2ea4 100644 --- a/deploy/helm/trino-operator/crds/crds.yaml +++ b/deploy/helm/trino-operator/crds/crds.yaml @@ -2034,7 +2034,7 @@ spec: This is helpful, because Trino fails to start in case you have any unused config properties. The removals are executed after the `configOverrides`. - This field is experimental, as ideally some general solution for the removal of properties is found and added to configOverrides. This mechanism would replace this field. + This field is experimental, and might be replaced by a more generic mechanism to edit config properties items: type: string type: array