You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "jiangzho (via GitHub)" <gi...@apache.org> on 2026/01/07 00:14:40 UTC
[PR] [SPARK-54926] Improve Helm chart for adding labels and annotations for operator config map [spark-kubernetes-operator]
jiangzho opened a new pull request, #446:
URL: https://github.com/apache/spark-kubernetes-operator/pull/446
## What changes were proposed in this pull request?
This PR allows user to config the labels and annotations for the operator configuration `ConfigMaps`, which hold operator configuration properties files.
Also added schema validation for the new fields in `values.schema.json`.
### Why are the changes needed?
Users may need to add custom labels and annotations to the operator configuration ConfigMaps for various operational purposes, or need toolings that rely on labels or annotations on resources.
Currently, the Helm chart doesn't provide a good way to customize labels and annotations for the operator `ConfigMap` - it only support annotations for dynamic `ConfigMap`.
### Does this PR introduce _any_ user-facing change?
Users can now specify custom labels and annotations for the operator configuration ConfigMap in their `values.yaml`:
```yaml
operatorConfiguration:
configMap:
annotations:
example.com/annotation: "value"
labels:
example.com/label: "value"
```
### How was this patch tested?
Self-contained linters, also validated manually.
Without explicitly configured labels or annotations, the `ConfigMap` renders as
```yaml
# Source: spark-kubernetes-operator/templates/spark-operator.yaml apiVersion: v1
kind: ConfigMap
metadata:
name: spark-kubernetes-operator-configuration
namespace: spark-demo
labels:
app.kubernetes.io/name: spark-kubernetes-operator
app.kubernetes.io/component: "operator-config"
app.kubernetes.io/version: "0.7.0-SNAPSHOT"
app.kubernetes.io/managed-by: Helm
helm.sh/chart: spark-kubernetes-operator-1.5.0-dev
spark-role: operator
data:
log4j2.properties: |+
# ...
```
With value override
```yaml
operatorConfiguration:
configMap:
annotations:
foo1: bar1
foo2: bar2
labels:
foo3: bar3
```
The `ConfigMap` renders as
```yaml
# Source: spark-kubernetes-operator/templates/spark-operator.yaml apiVersion: v1
kind: ConfigMap
metadata:
name: spark-kubernetes-operator-configuration
namespace: spark-demo
labels:
app.kubernetes.io/name: spark-kubernetes-operator
app.kubernetes.io/component: "operator-config"
app.kubernetes.io/version: "0.7.0-SNAPSHOT"
app.kubernetes.io/managed-by: Helm
helm.sh/chart: spark-kubernetes-operator-1.5.0-dev
spark-role: operator
foo3: bar3
annotations:
foo1: bar1
foo2: bar2
data:
log4j2.properties: |+
# ...
```
### Was this patch authored or co-authored using generative AI tooling?
No
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54926] Improve Helm chart for adding labels and annotations for operator config map [spark-kubernetes-operator]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #446:
URL: https://github.com/apache/spark-kubernetes-operator/pull/446#discussion_r2670463663
##########
.github/workflows/build_and_test.yml:
##########
@@ -177,6 +177,47 @@ jobs:
if: matrix.mode == 'selector'
run: |
chainsaw test --test-dir ./tests/e2e/${{ matrix.test-group }} --parallel 1
+ helm-tests:
+ name: "Helm Tests"
Review Comment:
Can we extend `k8s-integration-tests` instead of adding `helm-tests` because we already have `helm test` here.
https://github.com/apache/spark-kubernetes-operator/blob/43473c7ff12c98be35a7292b00bc9dcc31665f0d/.github/workflows/build_and_test.yml#L145
##########
.github/workflows/build_and_test.yml:
##########
@@ -177,6 +177,47 @@ jobs:
if: matrix.mode == 'selector'
run: |
chainsaw test --test-dir ./tests/e2e/${{ matrix.test-group }} --parallel 1
+ helm-tests:
+ name: "Helm Tests"
Review Comment:
Can we extend `k8s-integration-tests` instead of adding `helm-tests` because we already have `helm test` here?
https://github.com/apache/spark-kubernetes-operator/blob/43473c7ff12c98be35a7292b00bc9dcc31665f0d/.github/workflows/build_and_test.yml#L145
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54926] Improve Helm chart for adding labels and annotations for operator config map [spark-kubernetes-operator]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #446:
URL: https://github.com/apache/spark-kubernetes-operator/pull/446#discussion_r2670471057
##########
tests/e2e/helm/helm-test-values/configmap-metadata/values.yaml:
##########
@@ -0,0 +1,35 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# Test values for validating ConfigMap labels and annotations feature
+#
+# Usage:
+# helm install spark-operator . -f tests/test-values-configmap-metadata.yaml
+# helm test spark-operator
+#
+# This file demonstrates the ConfigMap labels and annotations feature
+# and can be used to validate the Helm test for this functionality.
+
+operatorConfiguration:
+ append: true
+ configMap:
Review Comment:
I have two questions.
- What happens when there is a conflict between the user-defined labels and annotations and the existing ones?
- Can we control extra `labels` and `annotations` for other resources of `Spark K8s Operator`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54926] Improve Helm chart for adding labels and annotations for operator config map [spark-kubernetes-operator]
Posted by "jiangzho (via GitHub)" <gi...@apache.org>.
jiangzho commented on code in PR #446:
URL: https://github.com/apache/spark-kubernetes-operator/pull/446#discussion_r2674119332
##########
.github/workflows/build_and_test.yml:
##########
@@ -177,6 +177,47 @@ jobs:
if: matrix.mode == 'selector'
run: |
chainsaw test --test-dir ./tests/e2e/${{ matrix.test-group }} --parallel 1
+ helm-tests:
+ name: "Helm Tests"
Review Comment:
It is technically feasible - this line runs helm test with the value override in this particular scenario.
This new job helps to reduce the size of `exclude` in `k8s-integration-tests ` job - even if later we add more helm tests with custom value overrides.
If we reuse the same job, the helm tests need to exclude integration test groups & integration test modes need to exclude helm tests. This `exclude` block would have a size increase of (num of integration test groups) * (num of helm tests). That makes it less elegant an harder to maintain. Other than that I do not have concerns to merge them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54926] Improve Helm chart for adding labels and annotations for operator config map [spark-kubernetes-operator]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun closed pull request #446: [SPARK-54926] Improve Helm chart for adding labels and annotations for operator config map
URL: https://github.com/apache/spark-kubernetes-operator/pull/446
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54926] Improve Helm chart for adding labels and annotations for operator config map [spark-kubernetes-operator]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #446:
URL: https://github.com/apache/spark-kubernetes-operator/pull/446#issuecomment-3731814830
To @viirya , @peter-toth , @jiangzho , I'm going to prepare Apache Spark K8s Operator v0.7 via creating branch-0.7 in order to deliver the latest updates along with Apache Spark 4.1.x.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54926] Improve Helm chart for adding labels and annotations for operator config map [spark-kubernetes-operator]
Posted by "jiangzho (via GitHub)" <gi...@apache.org>.
jiangzho commented on code in PR #446:
URL: https://github.com/apache/spark-kubernetes-operator/pull/446#discussion_r2674131175
##########
tests/e2e/helm/helm-test-values/configmap-metadata/values.yaml:
##########
@@ -0,0 +1,35 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# Test values for validating ConfigMap labels and annotations feature
+#
+# Usage:
+# helm install spark-operator . -f tests/test-values-configmap-metadata.yaml
+# helm test spark-operator
+#
+# This file demonstrates the ConfigMap labels and annotations feature
+# and can be used to validate the Helm test for this functionality.
+
+operatorConfiguration:
+ append: true
+ configMap:
Review Comment:
> What happens when there is a conflict between the user-defined labels and annotations and the existing ones?
In this approach, the user-provided ones take higher precedence as they are appended to default ones.
> Can we control extra labels and annotations for other resources of Spark K8s Operator?
As of now, we support configure these for operator pod, operator & workload rbac (service accounts, roles .etc to run Spark and the operator). ConfigMap is a missing piece here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54926] Improve Helm chart for adding labels and annotations for operator config map [spark-kubernetes-operator]
Posted by "jiangzho (via GitHub)" <gi...@apache.org>.
jiangzho commented on PR #446:
URL: https://github.com/apache/spark-kubernetes-operator/pull/446#issuecomment-3721334404
Thanks @dongjoon-hyun ! Updated the commit to decouple dynamic config & added a helm test job for this
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54926] Improve Helm chart for adding labels and annotations for operator config map [spark-kubernetes-operator]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #446:
URL: https://github.com/apache/spark-kubernetes-operator/pull/446#discussion_r2666693371
##########
build-tools/helm/spark-kubernetes-operator/templates/spark-operator.yaml:
##########
@@ -210,6 +219,9 @@ metadata:
namespace: {{ .Release.Namespace }}
labels:
{{- include "spark-operator.dynamicConfigLabels" . | nindent 4 }}
+ {{- if .Values.operatorConfiguration.dynamicConfig.labels }}
Review Comment:
Can we spin off `dynamicConfig.labels` into a separate PR because it supports `annotations` already, @jiangzho ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org