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/07/07 16:01:36 UTC
[GitHub] [flink-kubernetes-operator] morhidi opened a new pull request, #304: [FLINK-28445] Support operator configuration from multiple configmaps
morhidi opened a new pull request, #304:
URL: https://github.com/apache/flink-kubernetes-operator/pull/304
- Added support for config overrides from an additional config folder, specified by an env variable DYNAMIC_CONF_DIR
- Since it is a relative advanced feature added a kustomize example only for the time being instead of providing Helm support
The example can be tested by
```
kubectl create ns flink
helm install flink-kubernetes-operator helm/flink-kubernetes-operator --set "watchNamespaces={default,flink}" --post-renderer examples/kustomize/dynamic-conf/render
```
--
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 a diff in pull request #304: [FLINK-28445] Support operator configuration from multiple configmaps
Posted by GitBox <gi...@apache.org>.
morhidi commented on code in PR #304:
URL: https://github.com/apache/flink-kubernetes-operator/pull/304#discussion_r916590783
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/config/FlinkConfigManager.java:
##########
@@ -211,11 +212,28 @@ protected Cache<Key, Configuration> getCache() {
return cache;
}
+ private static Configuration loadGlobalConfiguration() {
+ return loadGlobalConfiguration(EnvUtils.get(EnvUtils.ENV_DYNAMIC_CONF_DIR));
+ }
+
+ @VisibleForTesting
+ protected static Configuration loadGlobalConfiguration(Optional<String> dynamicConfigDir) {
+ if (dynamicConfigDir.isPresent()) {
+ Configuration dynamicConfigs =
+ GlobalConfiguration.loadConfiguration(dynamicConfigDir.get());
+ LOG.debug(
+ "Loading default configuration with overrides from " + dynamicConfigDir.get());
+ return GlobalConfiguration.loadConfiguration(dynamicConfigs);
+ }
Review Comment:
makes sense, will use `override` over `dynamic`
--
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 #304: [FLINK-28445] Support operator configuration from multiple configmaps
Posted by GitBox <gi...@apache.org>.
gyfora commented on code in PR #304:
URL: https://github.com/apache/flink-kubernetes-operator/pull/304#discussion_r916559172
##########
flink-kubernetes-operator/src/test/java/org/apache/flink/kubernetes/operator/config/FlinkConfigManagerTest.java:
##########
@@ -163,4 +171,19 @@ public void testConfUpdateAndCleanup() {
.getObserveConfig(deployment)
.get(KubernetesOperatorConfigOptions.OPERATOR_RECONCILE_INTERVAL));
}
+
+ @Test
+ public void testDynamicConfiguration(@TempDir Path dynamicConfigDir) throws IOException {
Review Comment:
Similarly to my previous config lets call this `confOverrideDirTest` or something like that
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/config/FlinkConfigManager.java:
##########
@@ -211,11 +212,28 @@ protected Cache<Key, Configuration> getCache() {
return cache;
}
+ private static Configuration loadGlobalConfiguration() {
+ return loadGlobalConfiguration(EnvUtils.get(EnvUtils.ENV_DYNAMIC_CONF_DIR));
+ }
+
+ @VisibleForTesting
+ protected static Configuration loadGlobalConfiguration(Optional<String> dynamicConfigDir) {
+ if (dynamicConfigDir.isPresent()) {
+ Configuration dynamicConfigs =
+ GlobalConfiguration.loadConfiguration(dynamicConfigDir.get());
+ LOG.debug(
+ "Loading default configuration with overrides from " + dynamicConfigDir.get());
+ return GlobalConfiguration.loadConfiguration(dynamicConfigs);
+ }
Review Comment:
I think we should not call this "dynamic" as it confuses things with the actual dynamic reloading of the configuraiton.
We could mayve call it `ENV_CONF_OVERRIDE_DIR` and `confOverrideDir` respectively
--
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 #304: [FLINK-28445] Support operator configuration from multiple configmaps
Posted by GitBox <gi...@apache.org>.
morhidi commented on PR #304:
URL: https://github.com/apache/flink-kubernetes-operator/pull/304#issuecomment-1178737402
> The changes look good but I think we generally want to get rid of the word `dynamic` from the naming of methods, variables, example files. This is a config override logic and it's not more dynamic than anything else in this context.
>
> In Flink the `GlobalConfiguration.loadConfiguration(dynamicConfiguraition)` naming comes from the fact that those are configs provided at job submission time "dynamically" compared to the hardocded flink conf yaml.
Thanks @gyfora got rid of it, PTAL
--
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 #304: [FLINK-28445] Support operator configuration from multiple configmaps
Posted by GitBox <gi...@apache.org>.
gyfora merged PR #304:
URL: https://github.com/apache/flink-kubernetes-operator/pull/304
--
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