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/06/17 11:40:52 UTC
[GitHub] [flink-kubernetes-operator] gyfora commented on a diff in pull request #254: [FLINK-27812] Support Dynamic Change of Watched Namespaces
gyfora commented on code in PR #254:
URL: https://github.com/apache/flink-kubernetes-operator/pull/254#discussion_r900031472
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/config/KubernetesOperatorConfigOptions.java:
##########
@@ -187,4 +188,18 @@ public class KubernetesOperatorConfigOptions {
.withDescription(
"Interval at which periodic savepoints will be triggered. "
+ "The triggering schedule is not guaranteed, savepoints will be triggered as part of the regular reconcile loop.");
+
+ public static final ConfigOption<String> OPERATOR_WATCHED_NAMESPACES =
+ ConfigOptions.key("kubernetes.operator.watched.namespaces")
+ .stringType()
+ .defaultValue(Constants.WATCH_ALL_NAMESPACES)
+ .withDescription(
+ "Comma separated list of namespaces the operator monitors for custom resources. Defaults to "
+ + Constants.WATCH_ALL_NAMESPACES);
+
+ public static final ConfigOption<Boolean> OPERATOR_DYNAMIC_NAMESPACES_ENABLED =
+ ConfigOptions.key("kubernetes.operator.dynamic.namespaces.enabled")
+ .booleanType()
+ .defaultValue(false)
+ .withDescription("Enables dynamic change of namespaces. Defaults to false");
Review Comment:
nit: `Enables dynamic change of watched/monitored namespaces`
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/config/FlinkConfigManager.java:
##########
@@ -109,14 +117,19 @@ public Configuration getDefaultConfig() {
@VisibleForTesting
public void updateDefaultConfig(Configuration newConf) {
- if (newConf.equals(defaultConfig)) {
+ if (defaultConfig != null
+ && newConf != null
+ && defaultConfig.toMap().equals(newConf.toMap())) {
LOG.info("Default configuration did not change, nothing to do...");
return;
}
LOG.info("Updating default configuration to {}", newConf);
- this.operatorConfiguration =
- FlinkOperatorConfiguration.fromConfiguration(newConf, namespaces);
+ this.operatorConfiguration = FlinkOperatorConfiguration.fromConfiguration(newConf);
+
+ if (this.operatorConfiguration.getDynamicNamespacesEnabled()) {
Review Comment:
should we check if thhe watched namespaces actually changed?
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/config/FlinkConfigManager.java:
##########
@@ -109,14 +117,19 @@ public Configuration getDefaultConfig() {
@VisibleForTesting
public void updateDefaultConfig(Configuration newConf) {
- if (newConf.equals(defaultConfig)) {
+ if (defaultConfig != null
+ && newConf != null
Review Comment:
Do we need these nullchecks here? Why would they ever be null?
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/informer/InformerManager.java:
##########
@@ -104,4 +100,24 @@ Map<String, SharedIndexInformer<CR>> createRunnableInformer(
return informers;
}
}
+
+ public void setNameSpaces(Set<String> watchedNamespaces) {
Review Comment:
nit: `setNameSpaces` -> `setNamespaces`
--
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