-
Notifications
You must be signed in to change notification settings - Fork 930
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Improvement] Define a general Helm chart configuration approach #6123
Comments
@pan3793 @zwangsheng @chgl Could you please have a look at it? |
Looks good to me! |
maybe just one minor detail: make sure all changed configs contribute to the checksum annotation for the statefulset so kyuubi is automatically restarted: https://github.com/apache/kyuubi/blob/master/charts/kyuubi/templates/kyuubi-statefulset.yaml#L41 |
@chgl I'm afraid it is impossible to calculate and keep updated checksum for entities in |
@dnskr , you're right of course! I hadn't considered the |
quick question, is restarting kyuubi when spark default config updated a must? |
@sudohainguyen Very good question. In the current implementation there is no Kyuubi auto restart on Spark config changes, but I assume it might be needed to stop running Spark immediately on some config changes. |
yeah I think it depends on how users expect it |
Created draft implementation #6521 |
Suggested approach and the draft implementation has an unsolved problem described below. Some properties affect both helm chart and the application. For instance, Ideally, it would be great to move such properties outside the default Any ideas? |
@dnskr Thanks for continuously working on this area. Carry my idea from the Slack channel - overriding configurations via |
@pan3793 I created improvement proposal based on your suggestion: Provide Kyuubi configuration properties with cli options. |
# 🔍 Description ## Issue References 🔗 This pull request changes Helm chart configuration approach as discussed in #6123 ## Describe Your Solution 🔧 Suggested implementation makes chart configuration more general and more flexible. It allows to configure Kyuubi (and its engines) by setting configuration file contents to chart properties or by providing configuration files through existing ConfigMaps and Secrets. Also users are not limited by predefined number of files and can put any files to configuration directories. ## Types of changes 🔖 - [ ] Bugfix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [x] Breaking change (fix or feature that would cause existing functionality to change) ## Test Plan 🧪 ### 1. Test suite "Kyuubi configuration" Property file `values-kyuubi-files.yaml` ```yaml kyuubiConf: dir: /opt/kyuubi/conf files: 'kyuubi-env.sh': | #!/usr/bin/env bash export KYUUBI_TEST=true 'kyuubi-custom.properties': | kyuubi.custom=true filesFrom: - configMap: name: kyuubi-configs ``` ConfigMap `kyuubi-configs` from `configmap-kyuubi-configs.yaml` ```yaml apiVersion: v1 kind: ConfigMap metadata: name: kyuubi-configs data: 'kyuubi-test.properties': | kyuubi.config.test=true ``` #### Rendered templates are correct ```shell $ helm template charts/kyuubi -f values-kyuubi-files.yaml -s templates/kyuubi-configmap.yaml -s templates/kyuubi-statefulset.yaml ``` #### Configuration files are in place ```shell $ kubectl create -f configmap-kyuubi-configs.yaml $ helm install kyuubi charts/kyuubi -f values-kyuubi-files.yaml $ kubectl exec kyuubi-0 -- ls conf kyuubi-custom.properties kyuubi-env.sh kyuubi-test.properties ``` ### 2. Test suite "Spark configuration" Property file `values-spark-files.yaml` ```yaml sparkConf: dir: /opt/spark/conf files: 'spark-env.sh': | #!/usr/bin/env bash export SPARK_TEST=true 'spark-custom.properties': | spark.custom=true filesFrom: - configMap: name: spark-configs ``` ConfigMap `spark-configs` from `configmap-spark-configs.yaml` ```yaml apiVersion: v1 kind: ConfigMap metadata: name: spark-configs data: 'spark-test.properties': | spark.config.test=true ``` #### Rendered templates are correct ```shell $ helm template charts/kyuubi -f values-spark-files.yaml -s templates/kyuubi-spark-configmap.yaml -s templates/kyuubi-statefulset.yaml ``` #### Configuration files are in place ```shell $ kubectl create -f configmap-spark-configs.yaml $ helm install kyuubi charts/kyuubi -f values-spark-files.yaml $ kubectl exec kyuubi-0 -- ls ../spark/conf spark-custom.properties spark-env.sh spark-test.properties ``` 3. Test suite "Custom kyuubi-defaults.conf from existing ConfigMap overwrites kyuubi-defaults.conf from values" Property file `values-kyuubi-defaults.yaml` ```yaml kyuubiConf: dir: /opt/kyuubi/conf files: 'kyuubi-defaults.conf': | custom.from.values=true filesFrom: - configMap: name: kyuubi-defaults-config ``` ConfigMap `kyuubi-defaults-config` from `configmap-kyuubi-defaults.yaml` ```yaml apiVersion: v1 kind: ConfigMap metadata: name: kyuubi-defaults-config data: 'kyuubi-defaults.conf': | custom.from.configmap=true ``` #### Rendered templates are correct ```shell $ helm template charts/kyuubi -f values-kyuubi-defaults.yaml -s templates/kyuubi-configmap.yaml -s templates/kyuubi-statefulset.yaml ``` #### Content of `kyuubi-defaults.conf` comes from ConfigMap ```shell $ kubectl create -f configmap-kyuubi-defaults.yaml $ helm install kyuubi charts/kyuubi -f values-kyuubi-defaults.yaml $ kubectl exec kyuubi-0 -- ls conf kyuubi-defaults.conf $ kubectl exec kyuubi-0 -- cat conf/kyuubi-defaults.conf custom.from.configmap=true ``` --- # Checklist 📝 - [x] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html) **Be nice. Be informative.** Closes #6521 from dnskr/implement-new-helm-chart-configuration-approach. Closes #6521 452dca3 [dnskr] Fix empty value type 14829f3 [dnskr] Revert "[REVERT BEFORE MERGE] Use 'master-snapshot' image tag" 8d90f42 [dnskr] Move default properties from 'kyuubi-defaults.conf' to --conf args 6b3c77f [dnskr] [REVERT BEFORE MERGE] Use 'master-snapshot' image tag fe7c17a [dnskr] [K8S][HELM] Implement new configuration approach Authored-by: dnskr <[email protected]> Signed-off-by: Kent Yao <[email protected]>
### Why are the changes needed? The PR adds support for Hadoop configuration files to be used by Apache Kyuubi, Apache Spark etc. The PR is continuation of PR #6521 and relates to the issue #6123. ### How was this patch tested? 1. Create `hadoop-configs.yaml` file (ConfigMap with `core-site.xml` and `hive-site.xml` entries): ```yaml apiVersion: v1 kind: ConfigMap metadata: name: hadoop-configs data: 'core-site.xml': | <?xml version="1.0"?> <?xml-stylesheet type="text/xsl" href="configuration.xsl"?> <configuration> <property> <name>hadoop.pr.test</name> <value>configmap</value> </property> </configuration> 'hive-site.xml': | <?xml version="1.0"?> <?xml-stylesheet type="text/xsl" href="configuration.xsl"?> <configuration> <property> <name>hive.pr.test</name> <value>configmap</value> </property> </configuration> ``` 2. Create ConfigMap from `hadoop-configs.yaml` file: ```shell kubectl create -f hadoop-configs.yaml ``` 3. Create custom `values-hadoop.yaml` (overwrites `core-site.xml`): ```yaml image: repository: apache/kyuubi tag: 1.10.0-spark rbac: create: true rules: - apiGroups: [""] resources: ["pods", "configmaps", "services"] verbs: ["create", "list", "delete", "watch", "deletecollection", "get"] hadoopConf: files: 'core-site.xml': | <?xml version="1.0"?> <?xml-stylesheet type="text/xsl" href="configuration.xsl"?> <configuration> <property> <name>hadoop.pr.test</name> <value>values</value> </property> </configuration> 'hdfs-site.xml': | <?xml version="1.0"?> <?xml-stylesheet type="text/xsl" href="configuration.xsl"?> <configuration> <property> <name>hdfs.pr.test</name> <value>values</value> </property> </configuration> filesFrom: - configMap: name: hadoop-configs sparkConf: files: 'spark-defaults.conf': | spark.submit.deployMode=client spark.kubernetes.container.image=apache/spark:3.5.2 spark.kubernetes.authenticate.driver.serviceAccountName=kyuubi ``` 4. Install the chart ```shell helm install kyuubi charts/kyuubi -f values-hadoop.yaml ``` 5. Check there are 3 files in the Hadoop configuration directory: ```shell kubectl exec kyuubi-0 -- ls /opt/hadoop/conf core-site.xml hdfs-site.xml hive-site.xml ``` 6. Check `/opt/hadoop/conf/core-site.xml` has content from ConfigMap: ```shell kubectl exec kyuubi-0 -- cat /opt/hadoop/conf/core-site.xml <?xml version="1.0"?> <?xml-stylesheet type="text/xsl" href="configuration.xsl"?> <configuration> <property> <name>hadoop.pr.test</name> <value>configmap</value> </property> </configuration> ``` 7. Check `/opt/hadoop/conf/hdfs-site.xml` has content from values: ```shell kubectl exec kyuubi-0 -- cat /opt/hadoop/conf/hdfs-site.xml <?xml version="1.0"?> <?xml-stylesheet type="text/xsl" href="configuration.xsl"?> <configuration> <property> <name>hdfs.pr.test</name> <value>values</value> </property> </configuration> ``` 8. Check `/opt/hadoop/conf/hive-site.xml` has content from ConfigMap: ```shell kubectl exec kyuubi-0 -- cat /opt/hadoop/conf/hive-site.xml <?xml version="1.0"?> <?xml-stylesheet type="text/xsl" href="configuration.xsl"?> <configuration> <property> <name>hive.pr.test</name> <value>configmap</value> </property> </configuration> ``` 9. Check configuration values from Spark session: ```shell kubectl exec kyuubi-0 -- ./bin/beeline -u 'jdbc:hive2://kyuubi-thrift-binary:10009' -e 'set hadoop.pr.test;' +-----------------+------------+ | key | value | +-----------------+------------+ | hadoop.pr.test | configmap | +-----------------+------------ kubectl exec kyuubi-0 -- ./bin/beeline -u 'jdbc:hive2://kyuubi-thrift-binary:10009' -e 'set hdfs.pr.test;' +---------------+---------+ | key | value | +---------------+---------+ | hdfs.pr.test | values | +---------------+--------- kubectl exec kyuubi-0 -- ./bin/beeline -u 'jdbc:hive2://kyuubi-thrift-binary:10009' -e 'set hive.pr.test;' +---------------+------------+ | key | value | +---------------+------------+ | hive.pr.test | configmap | +---------------+------------ ``` ### Was this patch authored or co-authored using generative AI tooling? No Closes #6875 from dnskr/helm-add-hadoop-configs-support. Closes #6875 8c8665f [dnskr] [K8S][HELM] Add Hadoop configuration files support Authored-by: dnskr <[email protected]> Signed-off-by: dnskr <[email protected]>
Code of Conduct
Search before asking
What would you like to be improved?
There is no clear common approach at the moment on how to configure Kyuubi deployment if the Helm chart is used.
I would like to discuss requirements, limitations and different options to choose one approach to follow and support it in the Kyuubi Helm chart configuration. The problem has been mentioned and discussed in multiple issues and PRs, so the idea is to collect all opinions in one place and make the decision.
Configuration system of the Apache Kyuubi
The configuration system allows to configure values using the following options (ordered from low to high prio):
Runtime options
JDBC Connection URLs
andSET Statements
Can be skipped in the discussion, because they used only when Kyuubi is up and running.
Runtime option
Environment variables
Configured by
{{ .Values.env }}
and{{ .Values.envFrom }}
value properties.The Helm chart users can specify environment variables to provide necessary configuration values with low effort if needed. The properties also allow to use provided (existing)
ConfigMaps
andSecrets
as the sources of environment variables, for instance:Static options
Represented by configuration files which should be located in each Kyuubi container in specific paths.
In general case, the easiest way to provide files into Kubernetes pod (container) is to mount
ConfigMap
orSecret
to a specific path.Requirements
Note: this section is subject to discuss.
ConfigMaps
under the hood from value properties ofvalue.yaml
file.Secrets
should never be created and managed by Helm chart because of security consideration!ConfigMaps
andSecrets
by resource name as a reference.ConfigMaps
andSecrets
with priority order.Multiple
ConfigMaps
andSecrets
might have key duplicates, so the implementation should clearly resolve the collision by merging keys in priority order.ConfigMaps
managed by the chart withConfigMaps
andSecrets
provided by user with priority order.The issue with key duplicates should be clearly resolved.
ConfigMaps
managed by the chart should have the lowest prio.value.yaml
file.Some configuration files might be huge and complex, so the idea is to prevent identation issues in
values.yaml
file.ConfigMaps
andSecrets
from one or many configuration files.Users might have a lot of xml, properties and other files, so the idea is to help users to create
ConfigMap
andSecret
resources in a simple way.How should we improve?
Approach
Note: this section is subject to discuss.
values.yaml
by system like Kyuubi, Hadoop, Spark, Trino etc.files
property to specify various files.Users can define files with any file name. Each entity within
files
property used as a key/value pair in the correspondingConfigMap
.filesFrom
property to specify list of existingConfigMaps
andSecrets
to be mounted to the configuration path of Kyuubi container.The implementation idea is to use Projected Volumes with
core/v1/SecretProjection
andcore/v1/ConfigMapProjection
entities.Also it will allow to merge
ConfigMap
created fromfiles
property with the entities fromfilesFrom
property.xxxConfDir
property toxxxConf
property.ConfigMap
from file or directory, see Kubernetes docs .Are you willing to submit PR?
The text was updated successfully, but these errors were encountered: