[SPARK-48786] Define Config Loading Framework for Spark Operator Controller #17
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
SparkOperatorConf includes below groups / namespaces: group / namespace | notes ---|--- spark.kubernetes.operator. | general operator conf, e.g. name, namespace, watched namespaces spark.kubernetes.operator.reconciler. | reconciler behavior, e.g. parallelism, reconcile timeouts / interval, whether to enable history trimming . etc spark.kubernetes.operator.dynamicConfig. | dynamic config monitor behavior spark.kubernetes.operator.rateLimiter. | rate limiter behavior, interval, limits, multiplier . etc spark.kubernetes.operator.retry. | retry behavior, including client retry config and additional retry behavior on k8s server error / status patching spark.kubernetes.operator.metrics. | metrics behavior, e.g. whether to enable metrics sets for josdk / k8s client, metrics server port .etc spark.kubernetes.operator.health. | health check / probe behavior spark.kubernetes.operator.leaderElection. | leader election behavior for operator HA mode
| @@ -0,0 +1,49 @@ | |||
| { | |||
Member
There was a problem hiding this comment.
Shall we move this to test/resources?
| public class SparkOperatorConf { | ||
| public static final ConfigOption<String> OperatorAppName = | ||
| ConfigOption.<String>builder() | ||
| .key("spark.kubernetes.operator.name") |
Member
There was a problem hiding this comment.
Please remove all actual configuration (spark.kubernetes.operator.*) variables from this PR in order to focus on ConfigOption framework.
Member
There was a problem hiding this comment.
Specifically, let's remove from line 43 to 405 and all dependent lines.
Contributor
Author
There was a problem hiding this comment.
sure - this PR has been narrowed down to config loaders only
dongjoon-hyun
requested changes
Jul 3, 2024
Member
There was a problem hiding this comment.
Thank you for making a narrow-downed PR. We need to focus on new ConfigOption framework itself in this PR, @jiangzho . Could you address the following?
- Please remove all actual configuration (spark.kubernetes.operator.*) variables from this PR. (https://github.com/apache/spark-kubernetes-operator/pull/17/files#r1664519696)
- Let's move
EcsLayout.jsontotest/resources. (https://github.com/apache/spark-kubernetes-operator/pull/17/files#r1664512208) - Also, please revise the PR title accordingly.
jiangzho
changed the title
| public static final String LABEL_SPARK_OPERATOR_NAME = "spark.operator/name"; | ||
| public static final String LABEL_SENTINEL_RESOURCE = "spark.operator/sentinel"; | ||
| public static final String LABEL_RESOURCE_NAME = "app.kubernetes.io/name"; | ||
| public static final String LABEL_COMPONENT_NAME = "app.kubernetes.io/component"; |
Member
There was a problem hiding this comment.
Reordering this? Never mind. I found that the ordering is not consistent already.
Member
|
Could you make CI happy, @jiangzho ? |
dongjoon-hyun
changed the title
dongjoon-hyun
approved these changes
Jul 9, 2024
Member
dongjoon-hyun
left a comment
dongjoon-hyun
left a comment
There was a problem hiding this comment.
+1, LGTM (Pending CIs).
Closed
jiangzho
added a commit
to jiangzho/spark-kubernetes-operator
that referenced
this pull request
Jul 17, 2025
Refactor package name for Spark Clusters
dongjoon-hyun
added a commit
that referenced
this pull request
Oct 7, 2025
…tead of `ToStringBuilder` ### What changes were proposed in this pull request? This PR aims to improve `SentinelResourceState.toString` by JEP-280 instead of `ToStringBuilder`. ### Why are the changes needed? This is aligned with Apache Spark main repository improvement. - apache/spark#51572 Since Java 9, `String Concatenation` has been handled better by default. | ID | DESCRIPTION | | - | - | | JEP-280 | [Indify String Concatenation](https://openjdk.org/jeps/280) | For example, `SentinelResourceState.toString` is changed like the following by this PR. **BEFORE** ``` public java.lang.String toString(); Code: 0: new #43 // class org/apache/commons/lang3/builder/ToStringBuilder 3: dup 4: aload_0 5: invokespecial #45 // Method org/apache/commons/lang3/builder/ToStringBuilder."<init>":(Ljava/lang/Object;)V 8: ldc #48 // String resource 10: aload_0 11: getfield #17 // Field resource:Lorg/apache/spark/k8s/operator/BaseResource; 14: invokevirtual #49 // Method org/apache/commons/lang3/builder/ToStringBuilder.append:(Ljava/lang/String;Ljava/lang/Object;)Lorg/apache/commons/lang3/builder/ToStringBuilder; 17: ldc #53 // String previousGeneration 19: aload_0 20: getfield #39 // Field previousGeneration:J 23: invokevirtual #54 // Method org/apache/commons/lang3/builder/ToStringBuilder.append:(Ljava/lang/String;J)Lorg/apache/commons/lang3/builder/ToStringBuilder; 26: ldc #57 // String isHealthy 28: aload_0 29: getfield #13 // Field isHealthy:Z 32: invokevirtual #58 // Method org/apache/commons/lang3/builder/ToStringBuilder.append:(Ljava/lang/String;Z)Lorg/apache/commons/lang3/builder/ToStringBuilder; 35: invokevirtual #61 // Method org/apache/commons/lang3/builder/ToStringBuilder.toString:()Ljava/lang/String; 38: areturn ``` **AFTER** ``` public java.lang.String toString(); Code: 0: aload_0 1: getfield #17 // Field resource:Lorg/apache/spark/k8s/operator/BaseResource; 4: invokestatic #43 // Method java/lang/String.valueOf:(Ljava/lang/Object;)Ljava/lang/String; 7: aload_0 8: getfield #39 // Field previousGeneration:J 11: aload_0 12: getfield #13 // Field isHealthy:Z 15: invokedynamic #49, 0 // InvokeDynamic #0:makeConcatWithConstants:(Ljava/lang/String;JZ)Ljava/lang/String; 20: areturn ``` ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Pass the CIs. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #369 from dongjoon-hyun/SPARK-53818. Authored-by: Dongjoon Hyun <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
This is a breakdown PR of #12 - introducing config loading framework for controller module. It includes a config map reconciler that can watch properties updates for hot properties loading.
Why are the changes needed?
These are needed to enable user-configurable behavior of the operator.
Does this PR introduce any user-facing change?
No to existing Spark users.
How was this patch tested?
Tested via unit test coverage.
Was this patch authored or co-authored using generative AI tooling?
No.