You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/11/09 14:45:47 UTC
[GitHub] [flink-kubernetes-operator] gyfora opened a new pull request, #434: [FLINK-29919] Support operator leader election
gyfora opened a new pull request, #434:
URL: https://github.com/apache/flink-kubernetes-operator/pull/434
## What is the purpose of the change
Support enabling and configuring leader election for the kubernetes operator to make it possible to run standby operator instances.
## Brief change log
- *Add configs to enable and configure the leader election service*
- *Add tests*
- *Update helm chart*
## Verifying this change
1. Extended the `FlinkOperatorTest` to cover the leader election config.
2. Manually tested on minikube
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): yes
- The public API, i.e., is any changes to the `CustomResourceDescriptors`: no
- Core observer or reconciler logic that is regularly executed: no
## Documentation
- Does this pull request introduce a new feature? yes
- If yes, how is the feature documented? docs
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] gyfora merged pull request #434: [FLINK-29919] Support operator leader election
Posted by GitBox <gi...@apache.org>.
gyfora merged PR #434:
URL: https://github.com/apache/flink-kubernetes-operator/pull/434
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] gaborgsomogyi commented on a diff in pull request #434: [FLINK-29919] Support operator leader election
Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on code in PR #434:
URL: https://github.com/apache/flink-kubernetes-operator/pull/434#discussion_r1019246303
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/config/FlinkOperatorConfiguration.java:
##########
@@ -60,6 +62,7 @@ public class FlinkOperatorConfiguration {
int exceptionFieldLengthThreshold;
int exceptionThrowableCountThreshold;
String labelSelector;
+ LeaderElectionConfiguration leaderElectionConfiguration;
Review Comment:
Nit: any reason why these are not private?
##########
flink-kubernetes-operator/src/test/java/org/apache/flink/kubernetes/operator/FlinkOperatorTest.java:
##########
@@ -74,4 +93,16 @@ public void testConfigurationPassedToJOSDK() {
Assertions.assertThrows(IllegalStateException.class, () -> new FlinkOperator(secondConfig));
}
+
+ @Test
+ public void testLeaderElectionConfig() {
+ var operatorConfig = new Configuration();
+ operatorConfig.set(KubernetesOperatorConfigOptions.OPERATOR_LEADER_ELECTION_ENABLED, true);
+
+ try {
+ new FlinkOperator(operatorConfig);
+ } catch (IllegalConfigurationException ice) {
+ assertTrue(ice.getMessage().startsWith("Lease name must be defined"));
Review Comment:
Nit: Maybe we can mention either the feature (leader election) or config name. Such case one don't need to dig for conf name just copy-paste.
##########
pom.xml:
##########
@@ -69,7 +69,7 @@ under the License.
<maven-resources-plugin.version>3.2.0</maven-resources-plugin.version>
<git-commit-id-maven-plugin.version>5.0.0</git-commit-id-maven-plugin.version>
- <operator.sdk.version>4.1.0</operator.sdk.version>
+ <operator.sdk.version>4.1.1</operator.sdk.version>
Review Comment:
Just for my own understanding, is this needed for `KUBERNETES_NAMESPACE_SYSTEM_PROPERTY`?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] gyfora commented on pull request #434: [FLINK-29919] Support operator leader election
Posted by GitBox <gi...@apache.org>.
gyfora commented on PR #434:
URL: https://github.com/apache/flink-kubernetes-operator/pull/434#issuecomment-1310458568
> +1 LGTM overall. Some e2e would be nice. Is this going to be the recommended prod setup or we're not sure yet?
I think the recommended setup is without leader election as it's a very optional feature. I think most prod settings do not require this and it only adds complication
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] gyfora commented on a diff in pull request #434: [FLINK-29919] Support operator leader election
Posted by GitBox <gi...@apache.org>.
gyfora commented on code in PR #434:
URL: https://github.com/apache/flink-kubernetes-operator/pull/434#discussion_r1019288321
##########
pom.xml:
##########
@@ -69,7 +69,7 @@ under the License.
<maven-resources-plugin.version>3.2.0</maven-resources-plugin.version>
<git-commit-id-maven-plugin.version>5.0.0</git-commit-id-maven-plugin.version>
- <operator.sdk.version>4.1.0</operator.sdk.version>
+ <operator.sdk.version>4.1.1</operator.sdk.version>
Review Comment:
No this is just so we have the latest josdk version and some fixes
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] gyfora commented on pull request #434: [FLINK-29919] Support operator leader election
Posted by GitBox <gi...@apache.org>.
gyfora commented on PR #434:
URL: https://github.com/apache/flink-kubernetes-operator/pull/434#issuecomment-1308873419
cc @morhidi
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] gyfora commented on a diff in pull request #434: [FLINK-29919] Support operator leader election
Posted by GitBox <gi...@apache.org>.
gyfora commented on code in PR #434:
URL: https://github.com/apache/flink-kubernetes-operator/pull/434#discussion_r1019438277
##########
flink-kubernetes-operator/src/test/java/org/apache/flink/kubernetes/operator/FlinkOperatorTest.java:
##########
@@ -74,4 +93,16 @@ public void testConfigurationPassedToJOSDK() {
Assertions.assertThrows(IllegalStateException.class, () -> new FlinkOperator(secondConfig));
}
+
+ @Test
+ public void testLeaderElectionConfig() {
+ var operatorConfig = new Configuration();
+ operatorConfig.set(KubernetesOperatorConfigOptions.OPERATOR_LEADER_ELECTION_ENABLED, true);
+
+ try {
+ new FlinkOperator(operatorConfig);
+ } catch (IllegalConfigurationException ice) {
+ assertTrue(ice.getMessage().startsWith("Lease name must be defined"));
Review Comment:
Good idea, I will add 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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] gyfora commented on a diff in pull request #434: [FLINK-29919] Support operator leader election
Posted by GitBox <gi...@apache.org>.
gyfora commented on code in PR #434:
URL: https://github.com/apache/flink-kubernetes-operator/pull/434#discussion_r1019287141
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/config/FlinkOperatorConfiguration.java:
##########
@@ -60,6 +62,7 @@ public class FlinkOperatorConfiguration {
int exceptionFieldLengthThreshold;
int exceptionThrowableCountThreshold;
String labelSelector;
+ LeaderElectionConfiguration leaderElectionConfiguration;
Review Comment:
Lombok @Value makes them private
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] gaborgsomogyi commented on a diff in pull request #434: [FLINK-29919] Support operator leader election
Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on code in PR #434:
URL: https://github.com/apache/flink-kubernetes-operator/pull/434#discussion_r1019258482
##########
flink-kubernetes-operator/src/test/java/org/apache/flink/kubernetes/operator/FlinkOperatorTest.java:
##########
@@ -74,4 +93,16 @@ public void testConfigurationPassedToJOSDK() {
Assertions.assertThrows(IllegalStateException.class, () -> new FlinkOperator(secondConfig));
}
+
+ @Test
+ public void testLeaderElectionConfig() {
+ var operatorConfig = new Configuration();
+ operatorConfig.set(KubernetesOperatorConfigOptions.OPERATOR_LEADER_ELECTION_ENABLED, true);
+
+ try {
+ new FlinkOperator(operatorConfig);
+ } catch (IllegalConfigurationException ice) {
+ assertTrue(ice.getMessage().startsWith("Lease name must be defined"));
Review Comment:
Oh, now see that the feature later mentioned. Maybe adding the key would provide more convenience.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] morhidi commented on pull request #434: [FLINK-29919] Support operator leader election
Posted by GitBox <gi...@apache.org>.
morhidi commented on PR #434:
URL: https://github.com/apache/flink-kubernetes-operator/pull/434#issuecomment-1310438255
+1 LGTM overall. Some e2e would be nice. Is this going to be the recommended prod setup or we're not sure yet?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org