XComp commented on code in PR #24231:
URL: https://github.com/apache/flink/pull/24231#discussion_r1471262987
##########
flink-kubernetes/src/main/java/org/apache/flink/kubernetes/highavailability/KubernetesLeaderElectionDriver.java:
##########
@@ -89,11 +85,6 @@ public KubernetesLeaderElectionDriver(
kubeClient.createLeaderElector(
leaderElectionConfiguration, new
LeaderCallbackHandlerImpl());
- this.configMapLabels =
- KubernetesUtils.getConfigMapLabels(
Review Comment:
getConfigMapLabels sets some labels that are marked for deprecation
(https://github.com/apache/flink/commit/b7e8b792c086c3c445ee8429fbcfe035097a878c).
The intention was to remove the labels because Flink would internally rely on
the ConfigMap name and, therefore, reduce the k8s server API calls (see
FLINK-33598).
We might want to revert the
https://github.com/apache/flink/commit/b7e8b792c086c3c445ee8429fbcfe035097a878c
if we decide that it makes sense to keep it even if Flink is not using it
internally. Instead, we should mark this as @Public API and document it if we
want these labels to be still usable by users for external cleanup of k8s
resources.
@Myasuka WDYT? Reverting `b7e8b792` should be fine since it was just
introduced in 1.19.
##########
flink-kubernetes/src/test/java/org/apache/flink/kubernetes/kubeclient/resources/KubernetesLeaderElectorITCase.java:
##########
@@ -110,4 +112,44 @@ void testMultipleKubernetesLeaderElectors() throws
Exception {
kubernetesExtension.getFlinkKubeClient().deleteConfigMap(leaderConfigMapName).get();
}
}
+
+ @Test
+ void testClusterConfigMapLabelsAreSet() throws Exception {
+ final Configuration configuration =
kubernetesExtension.getConfiguration();
+
+ final String leaderConfigMapName =
+ LEADER_CONFIGMAP_NAME_PREFIX + System.currentTimeMillis();
Review Comment:
fyi: This part of the `KubernetesLeaderElectorITCase` was touched by
https://github.com/apache/flink/pull/24132/commits/6bf01b927c63bc6087c02b15aed156c6f3f74a8a
in the [FLINK-34007 PR](https://github.com/apache/flink/pull/24132/files). It
will move the ConfigMap lifecycle management out of the test methods. ...just
in case FLINK-34007 is going to get merged first.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
