From bc70e01ec0195b3c415b91d5e97052286c76878f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20M=C3=BCller?= Date: Mon, 10 Aug 2026 08:46:20 +0200 Subject: [PATCH 1/3] fix: Application config takes precedence over Template --- CHANGELOG.md | 11 + .../pages/usage-guide/app_templates.adoc | 4 + .../src/crd/template_merger.rs | 218 ++++++++++++++---- 3 files changed, 194 insertions(+), 39 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 612d19bc..41ff861a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,9 +13,20 @@ All notable changes to this project will be documented in this file. - The RBAC ServiceAccounts and RoleBindings of the history and connect servers are now built with the operator-rs `v2::rbac` functions and carry the recommended labels ([#727]). +### Fixed + +- BREAKING (behaviour): The `config` and `cliOverrides` of a SparkApplication role now take + precedence over the ones of a referenced SparkApplicationTemplate. Previously the template + value won and the value set on the SparkApplication was silently discarded ([#745]). +- BREAKING (behaviour): The `jvmArgumentOverrides` of a SparkApplication role are no longer + discarded when the application references a SparkApplicationTemplate. The overrides of the + template are now applied first and the ones of the SparkApplication on top of them, so an + application can also remove a JVM argument that one of its templates added ([#745]). + [#721]: https://github.com/stackabletech/spark-k8s-operator/pull/721 [#727]: https://github.com/stackabletech/spark-k8s-operator/pull/727 [#732]: https://github.com/stackabletech/spark-k8s-operator/pull/732 +[#745]: https://github.com/stackabletech/spark-k8s-operator/pull/745 ## [26.7.0] - 2026-07-21 diff --git a/docs/modules/spark-k8s/pages/usage-guide/app_templates.adoc b/docs/modules/spark-k8s/pages/usage-guide/app_templates.adoc index 1e3fc716..a10884cd 100644 --- a/docs/modules/spark-k8s/pages/usage-guide/app_templates.adoc +++ b/docs/modules/spark-k8s/pages/usage-guide/app_templates.adoc @@ -12,6 +12,10 @@ Application templates are available for the `v1alpha1` version of the SparkAppli 4. Application template references are immutable in the sense that once applied to an application they cannot be changed again. Currently templates are applied upon the creation of the application, and any changes to the template references after that will be ignored. 5. Application and template CRDs must have the exact same versions. Currently only `v1alpha1` is supported. +NOTE: The `jvmArgumentOverrides` are an exception to the precedence rule above. +They are not overridden but applied in sequence: the overrides of the templates are applied first, in the order the templates are referenced, and the ones of the SparkApplication last. +An application can therefore also remove a JVM argument that one of its templates added. + IMPORTANT: Application templates were cluster-scoped when they were first released in SDP 26.3, and are namespace-scoped from SDP 26.7 onwards. Upgrading across that change requires manual steps, because a CustomResourceDefinition cannot change its scope in place. See xref:usage-guide/upgrade.adoc[] before upgrading. diff --git a/rust/operator-binary/src/crd/template_merger.rs b/rust/operator-binary/src/crd/template_merger.rs index 7f8afbba..8f5e83f1 100644 --- a/rust/operator-binary/src/crd/template_merger.rs +++ b/rust/operator-binary/src/crd/template_merger.rs @@ -70,10 +70,13 @@ pub fn deep_merge(base: &SparkApplication, overlay: &SparkApplication) -> SparkA spark_image: overlay.spec.spark_image.clone(), // Merge job configuration - job: merge_common_config(base.spec.job.as_ref(), overlay.spec.job.as_ref()), + job: merge_optional_common_config(base.spec.job.as_ref(), overlay.spec.job.as_ref()), // Merge driver configuration - driver: merge_common_config(base.spec.driver.as_ref(), overlay.spec.driver.as_ref()), + driver: merge_optional_common_config( + base.spec.driver.as_ref(), + overlay.spec.driver.as_ref(), + ), // Merge executor configuration (RoleGroup) executor: merge_role_group(base.spec.executor.as_ref(), overlay.spec.executor.as_ref()), @@ -174,69 +177,77 @@ fn merge_vec(base: &[T], overlay: &[T]) -> Vec { merged } -/// Merge CommonConfiguration using the Merge trait -fn merge_common_config( +/// Merge two optional CommonConfigurations, see [`merge_common_config`] for the precedence rules. +fn merge_optional_common_config( base: Option<&CommonConfiguration>, overlay: Option<&CommonConfiguration>, ) -> Option> where Config: Clone + Merge, - CommonConfig: Clone, + CommonConfig: Clone + Merge, ConfigOverrides: Clone + Merge, { match (base, overlay) { (None, None) => None, (Some(b), None) => Some(b.clone()), (None, Some(o)) => Some(o.clone()), - (Some(b), Some(o)) => { - // Clone the base and merge the overlay config into it - let mut merged = b.clone(); - merged.config.merge(&o.config); - // Merge with overlay precedence: keep overlay values for conflicts, - // fill missing keys from base. - let mut config_overrides = o.config_overrides.clone(); - config_overrides.merge(&b.config_overrides); - merged.config_overrides = config_overrides; - merged.env_overrides = merge_hashmap(&b.env_overrides, &o.env_overrides); - merged.pod_overrides = merge_pod_template_spec(&b.pod_overrides, &o.pod_overrides); - Some(merged) - } + (Some(b), Some(o)) => Some(merge_common_config(b, o)), } } -/// Merge RoleGroup +/// Overlay values win conflicts and values missing from the overlay are filled in from the base. +fn merge_common_config( + base: &CommonConfiguration, + overlay: &CommonConfiguration, +) -> CommonConfiguration +where + Config: Clone + Merge, + CommonConfig: Clone + Merge, + ConfigOverrides: Clone + Merge, +{ + let mut config = overlay.config.clone(); + config.merge(&base.config); + + let mut config_overrides = overlay.config_overrides.clone(); + config_overrides.merge(&base.config_overrides); + + let mut cli_overrides = base.cli_overrides.clone(); + cli_overrides.extend(overlay.cli_overrides.clone()); + + // Note that this does not overwrite anything for `JavaCommonConfig`, which is what all roles + // of a SparkApplication use: merging registers the JVM argument overrides of the base as + // preceding ones, so that they are applied first and the ones of the overlay on top of them. + let mut product_specific_common_config = overlay.product_specific_common_config.clone(); + product_specific_common_config.merge(&base.product_specific_common_config); + + CommonConfiguration { + config, + config_overrides, + env_overrides: merge_hashmap(&base.env_overrides, &overlay.env_overrides), + cli_overrides, + pod_overrides: merge_pod_template_spec(&base.pod_overrides, &overlay.pod_overrides), + product_specific_common_config, + } +} + +/// Merge two optional RoleGroups, see [`merge_common_config`] for the precedence rules. fn merge_role_group( base: Option<&RoleGroup>, overlay: Option<&RoleGroup>, ) -> Option> where Config: Clone + Merge, - CommonConfig: Clone, + CommonConfig: Clone + Merge, ConfigOverrides: Clone + Merge, { match (base, overlay) { (None, None) => None, (Some(b), None) => Some(b.clone()), (None, Some(o)) => Some(o.clone()), - (Some(b), Some(o)) => { - // Clone the base and merge overlay - let mut merged = b.clone(); - merged.config.config.merge(&o.config.config); - // Merge with overlay precedence: keep overlay values for conflicts, - // fill missing keys from base. - let mut config_overrides = o.config.config_overrides.clone(); - config_overrides.merge(&b.config.config_overrides); - merged.config.config_overrides = config_overrides; - merged.config.env_overrides = - merge_hashmap(&b.config.env_overrides, &o.config.env_overrides); - merged.config.pod_overrides = - merge_pod_template_spec(&b.config.pod_overrides, &o.config.pod_overrides); - // Use overlay replicas if present - if o.replicas.is_some() { - merged.replicas = o.replicas; - } - Some(merged) - } + (Some(b), Some(o)) => Some(RoleGroup { + config: merge_common_config(&b.config, &o.config), + replicas: o.replicas.or(b.replicas), + }), } } @@ -256,6 +267,7 @@ fn merge_deps( #[cfg(test)] mod tests { use indoc::indoc; + use stackable_operator::k8s_openapi::apimachinery::pkg::api::resource::Quantity; use super::*; @@ -1569,4 +1581,132 @@ mod tests { .map(|term| term.weight) ); } + + #[test] + fn test_merge_template_role_config_into_spark_application() { + let template = serde_yaml::from_str::< + crate::crd::template_spec::v1alpha1::SparkApplicationTemplate, + >(indoc! {r#" + --- + apiVersion: spark.stackable.tech/v1alpha1 + kind: SparkApplicationTemplate + metadata: + name: template-with-resources + spec: + mode: cluster + mainApplicationFile: local:///template.jar + sparkImage: + productVersion: "3.5.8" + driver: + config: + resources: + cpu: + max: "2" + memory: + limit: 1Gi + jvmArgumentOverrides: + add: + - -Dfrom=template + - -Dremoved=by-application + executor: + config: + resources: + cpu: + max: "2" + memory: + limit: 1Gi + jvmArgumentOverrides: + add: + - -Dfrom=template + - -Dremoved=by-application + "#}) + .unwrap(); + + let spark_app = serde_yaml::from_str::(indoc! {r#" + --- + apiVersion: spark.stackable.tech/v1alpha1 + kind: SparkApplication + metadata: + name: my-spark-app + spec: + mode: cluster + mainApplicationFile: local:///app.jar + sparkImage: + productVersion: "3.5.8" + driver: + config: + resources: + cpu: + max: "4" + jvmArgumentOverrides: + remove: + - -Dremoved=by-application + add: + - -Dfrom=application + executor: + config: + resources: + cpu: + max: "4" + jvmArgumentOverrides: + remove: + - -Dremoved=by-application + add: + - -Dfrom=application + "#}) + .unwrap(); + + let app_from_template = crate::crd::v1alpha1::SparkApplication::from(template); + let merged = deep_merge(&app_from_template, &spark_app); + + let driver = merged.spec.driver.unwrap(); + assert_eq!( + driver.config.resources.cpu.max, + Some(Quantity("4".to_string())), + "the driver cpu of the spark application should take precedence" + ); + assert_eq!( + driver.config.resources.memory.limit, + Some(Quantity("1Gi".to_string())), + "the driver memory should be filled in from the template" + ); + // The application removes `-Dremoved=by-application`, which only the template adds. The + // argument can only disappear if the overrides are applied in sequence, so this also + // rules out a plain concatenation of the `add` lists. + assert_eq!( + driver + .product_specific_common_config + .jvm_argument_overrides + .apply_to(Vec::new()), + vec![ + "-Dfrom=template".to_string(), + "-Dfrom=application".to_string() + ], + "the driver jvm arguments of the template should be applied before the ones of the spark application" + ); + + let executor = merged.spec.executor.unwrap(); + assert_eq!( + executor.config.config.resources.cpu.max, + Some(Quantity("4".to_string())), + "the executor cpu of the spark application should take precedence" + ); + assert_eq!( + executor.config.config.resources.memory.limit, + Some(Quantity("1Gi".to_string())), + "the executor memory should be filled in from the template" + ); + assert_eq!( + executor + .config + .product_specific_common_config + .jvm_argument_overrides + .apply_to(Vec::new()), + vec![ + "-Dfrom=template".to_string(), + "-Dfrom=application".to_string() + ], + "the executor jvm arguments of the template should be applied before the ones of the spark application" + ); + } } From c3d97850674e6b4ae72bae41caf8932eb66ae5d0 Mon Sep 17 00:00:00 2001 From: Razvan-Daniel Mihai <84674+razvan@users.noreply.github.com> Date: Mon, 10 Aug 2026 12:08:59 +0200 Subject: [PATCH 2/3] Update CHANGELOG.md remove double fixed header --- CHANGELOG.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 357e47a6..7dea8222 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,9 +19,6 @@ All notable changes to this project will be documented in this file. - Fix a longstanding problem of including empty `categories`, `shortNames` and `additionalPrinterColumns` in the CRDs, which could cause problems with GitOps tools (e.g. ArgoCD) reporting a diff in the custom resources. See [our internal issue](https://github.com/stackabletech/hdfs-operator/issues/626) and [the fix](https://github.com/kube-rs/kube/pull/2042) for details ([#744]). - -### Fixed - - BREAKING (behaviour): The `config` and `cliOverrides` of a SparkApplication role now take precedence over the ones of a referenced SparkApplicationTemplate. Previously the template value won and the value set on the SparkApplication was silently discarded ([#745]). From 79b7011193c651c7acfcf79a94643d6981cc2818 Mon Sep 17 00:00:00 2001 From: Razvan-Daniel Mihai <84674+razvan@users.noreply.github.com> Date: Mon, 10 Aug 2026 12:16:29 +0200 Subject: [PATCH 3/3] Update CHANGELOG.md delete empty line --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7dea8222..cb27b0b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,7 +33,6 @@ All notable changes to this project will be documented in this file. [#744]: https://github.com/stackabletech/spark-k8s-operator/pull/744 [#745]: https://github.com/stackabletech/spark-k8s-operator/pull/745 - ## [26.7.0] - 2026-07-21 ## [26.7.0-rc1] - 2026-07-16