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/12/02 13:51:06 UTC
[GitHub] [flink-kubernetes-operator] mxm opened a new pull request, #466: [FLINK-30280] Default operator logging configuration is broken
mxm opened a new pull request, #466:
URL: https://github.com/apache/flink-kubernetes-operator/pull/466
The default logging configuration is set here: https://github.com/apache/flink-kubernetes-operator/blob/ea01e294cf1b68d597244d0a11b3c81822a163e7/helm/flink-kubernetes-operator/templates/flink-operator.yaml#L89
However, this file is not available in the official Docker image. It's best to not set this value and rely on the included logging configuration in the operator JAR. Users can define overrides on the deployment in https://github.com/apache/flink-kubernetes-operator/blob/ea01e294cf1b68d597244d0a11b3c81822a163e7/helm/flink-kubernetes-operator/values.yaml#L135 or re-add the environment variable pointing to a valid file.
--
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] mbalassi closed pull request #466: [FLINK-30280] Default operator logging configuration is broken
Posted by GitBox <gi...@apache.org>.
mbalassi closed pull request #466: [FLINK-30280] Default operator logging configuration is broken
URL: https://github.com/apache/flink-kubernetes-operator/pull/466
--
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 #466: [FLINK-30280] Default operator logging configuration is broken
Posted by GitBox <gi...@apache.org>.
morhidi commented on PR #466:
URL: https://github.com/apache/flink-kubernetes-operator/pull/466#issuecomment-1335281369
The Helm generates the file and refers it in the template. It doesn't have to be backed in the image.
--
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 #466: [FLINK-30280] Default operator logging configuration is broken
Posted by GitBox <gi...@apache.org>.
gyfora commented on PR #466:
URL: https://github.com/apache/flink-kubernetes-operator/pull/466#issuecomment-1335593501
I agree with @morhidi , the current logic has been working very robustly for basically every new user so far when installing with Helm.
--
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] mxm commented on pull request #466: [FLINK-30280] Default operator logging configuration is broken
Posted by GitBox <gi...@apache.org>.
mxm commented on PR #466:
URL: https://github.com/apache/flink-kubernetes-operator/pull/466#issuecomment-1335473057
Another caveat is that `append` applies to flink config, Flink logging config, and operator logging config.
--
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] mxm commented on pull request #466: [FLINK-30280] Default operator logging configuration is broken
Posted by GitBox <gi...@apache.org>.
mxm commented on PR #466:
URL: https://github.com/apache/flink-kubernetes-operator/pull/466#issuecomment-1335270930
Have you checked the contents of this file? It is empty, hence the logging breaks. Unless we duplicate the default logging file which is already included in the operator jar, I'd suggest to not set the LOG_CONFIG environment variable.
--
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] mxm commented on pull request #466: [FLINK-30280] Default operator logging configuration is broken
Posted by GitBox <gi...@apache.org>.
mxm commented on PR #466:
URL: https://github.com/apache/flink-kubernetes-operator/pull/466#issuecomment-1335454419
@morhidi @tweise What do you think about getting rid of the `append` flag and simply putting the default config in the ConfigMap volume. Users can modify the default config as they wish.
IMHO it is not acceptable that there can be a configuration where logging is completely disabled which is the case if you set `append: false` and don't add your own.
--
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] tweise commented on pull request #466: [FLINK-30280] Default operator logging configuration is broken
Posted by GitBox <gi...@apache.org>.
tweise commented on PR #466:
URL: https://github.com/apache/flink-kubernetes-operator/pull/466#issuecomment-1335342783
https://github.com/apache/flink-kubernetes-operator/blob/8c0de99fd25d2bbf99ea0742fb6e2607b8799d80/helm/flink-kubernetes-operator/values.yaml#L93
--
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] mxm commented on pull request #466: [FLINK-30280] Default operator logging configuration is broken
Posted by GitBox <gi...@apache.org>.
mxm commented on PR #466:
URL: https://github.com/apache/flink-kubernetes-operator/pull/466#issuecomment-1335422102
The ConfigMap volumes are mounted by default. This is the relevant content of the `flink-operator-config` ConfigMap:
```
log4j-operator.properties:
----
# Flink Operator Logging Overrides
# rootLogger.level = DEBUG
# logger.operator.name= org.apache.flink.kubernetes.operator
# logger.operator.level = DEBUG
```
The ConfigMap gets mounted at the correct path. Based on the following it should actually include the default configuration. Not sure why that isn't the case. `append` default to true. https://github.com/apache/flink-kubernetes-operator/blob/72ad3639e60fbf27dd408dabbe69f46d69ff52f9/helm/flink-kubernetes-operator/templates/flink-operator.yaml#L217
--
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 #466: [FLINK-30280] Default operator logging configuration is broken
Posted by GitBox <gi...@apache.org>.
morhidi commented on PR #466:
URL: https://github.com/apache/flink-kubernetes-operator/pull/466#issuecomment-1335563158
> @morhidi @tweise What do you think about getting rid of the `append` flag and simply putting the default config in the ConfigMap volume. Users can modify the default config as they wish.
>
> IMHO it is not acceptable that there can be a configuration where logging is completely disabled which is the case if you set `append: false` and don't add your own.
I don't know with `append=false` you override the default configs with whatever you define in the values file. Even with empty values. No one has complained about it, it's been like this since forever :)
--
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 #466: [FLINK-30280] Default operator logging configuration is broken
Posted by GitBox <gi...@apache.org>.
gyfora commented on PR #466:
URL: https://github.com/apache/flink-kubernetes-operator/pull/466#issuecomment-1339389481
> > @mxm can we set the log4j location conditionally? When append is false, set it to empty as shown in your original commit. Otherwise `value: -Dlog4j.configurationFile=/opt/flink/conf/log4j-operator.properties`
>
> Good point, @tweise.
Wouldn’t that mean that whatever the user sets in the config part won’t take any effect?
I think the property always have to be set .
--
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] tweise commented on pull request #466: [FLINK-30280] Default operator logging configuration is broken
Posted by GitBox <gi...@apache.org>.
tweise commented on PR #466:
URL: https://github.com/apache/flink-kubernetes-operator/pull/466#issuecomment-1335361447
The setting that guards the config map creation: `.Values.defaultConfiguration.append`
--
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] mxm commented on pull request #466: [FLINK-30280] Default operator logging configuration is broken
Posted by GitBox <gi...@apache.org>.
mxm commented on PR #466:
URL: https://github.com/apache/flink-kubernetes-operator/pull/466#issuecomment-1337175396
The issue is that `append: false` will break logging because there are no sane defaults. I think this is a bug and not robust at all. At the least you would expect some kind of default logger to be loaded with an empty logging configuration.
>No one has complained about it, it's been like this since forever :)
Just because it has been like that forever doesn't necessarily mean it's the best way. The amount of people using the operator seems to be increasing. Clearly, we ran into this and it has bit us. That is already one user too many.
--
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] tweise commented on pull request #466: [FLINK-30280] Default operator logging configuration is broken
Posted by GitBox <gi...@apache.org>.
tweise commented on PR #466:
URL: https://github.com/apache/flink-kubernetes-operator/pull/466#issuecomment-1335339491
Are the volume mounts created by default? If not, then we cannot assume the file to be present.
At the very least we can add a few comments so the setup is easier to understand.
--
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 #466: [FLINK-30280] Default operator logging configuration is broken
Posted by GitBox <gi...@apache.org>.
morhidi commented on PR #466:
URL: https://github.com/apache/flink-kubernetes-operator/pull/466#issuecomment-1335279354
> Have you checked the contents of this file? It is empty, hence the logging breaks. Unless we duplicate the default logging file which is already included in the operator jar, I'd suggest to not set the LOG_CONFIG environment variable.
If you install the operator with Helm the file is there:
```flink@flink-kubernetes-operator-6f9bbfd557-vdzvr:/flink-kubernetes-operator$ cat /opt/flink/conf/log4j-operator.properties
################################################################################
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
################################################################################
rootLogger.level = INFO
rootLogger.appenderRef.console.ref = ConsoleAppender
# Log all infos to the console
appender.console.name = ConsoleAppender
appender.console.type = CONSOLE
appender.console.layout.type = PatternLayout
appender.console.layout.pattern = %style{%d}{yellow} %style{%-30c{1.}}{cyan} %highlight{[%-5level]%notEmpty{[%X{resource.namespace}/}%notEmpty{%X{resource.name}]} %msg%n%throwable}
# Do not log config loading
logger.conf.name = org.apache.flink.configuration.GlobalConfiguration
logger.conf.level = WARN
# Flink Operator Logging Overrides
# rootLogger.level = DEBUG
# logger.operator.name= org.apache.flink.kubernetes.operator
# logger.operator.level = DEBUG
```
--
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] tweise commented on pull request #466: [FLINK-30280] Default operator logging configuration is broken
Posted by GitBox <gi...@apache.org>.
tweise commented on PR #466:
URL: https://github.com/apache/flink-kubernetes-operator/pull/466#issuecomment-1339332382
@mxm can we set the log4j location conditionally? When append is false, set it to empty as shown in your original commit. Otherwise `value: -Dlog4j.configurationFile=/opt/flink/conf/log4j-operator.properties`
--
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] mxm commented on pull request #466: [FLINK-30280] Default operator logging configuration is broken
Posted by GitBox <gi...@apache.org>.
mxm commented on PR #466:
URL: https://github.com/apache/flink-kubernetes-operator/pull/466#issuecomment-1339258151
I've documented the flags for now. PTAL. I think that should fix users accidentally overriding the defaults.
The only other solution I see is to copy the default files into the `values.yml` or removing the append feature entirely, i.e. permanently set it to true. I think it would be way easier for users if they had one source of truth for their config instead of multiple config sources.
--
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] mxm commented on pull request #466: [FLINK-30280] Default operator logging configuration is broken
Posted by GitBox <gi...@apache.org>.
mxm commented on PR #466:
URL: https://github.com/apache/flink-kubernetes-operator/pull/466#issuecomment-1335436354
Looks like we set `defaultConfiguration.append` to `false` in our overlay.
--
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 #466: [FLINK-30280] Default operator logging configuration is broken
Posted by GitBox <gi...@apache.org>.
morhidi commented on PR #466:
URL: https://github.com/apache/flink-kubernetes-operator/pull/466#issuecomment-1335260981
@max the file in question is mounted by the Helm chart, this PR makes no sense to me.
--
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] mbalassi commented on pull request #466: [FLINK-30280] Default operator logging configuration is broken
Posted by GitBox <gi...@apache.org>.
mbalassi commented on PR #466:
URL: https://github.com/apache/flink-kubernetes-operator/pull/466#issuecomment-1339375907
> @mxm can we set the log4j location conditionally? When append is false, set it to empty as shown in your original commit. Otherwise `value: -Dlog4j.configurationFile=/opt/flink/conf/log4j-operator.properties`
Good point, @tweise.
--
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] mxm commented on pull request #466: [FLINK-30280] Default operator logging configuration is broken
Posted by GitBox <gi...@apache.org>.
mxm commented on PR #466:
URL: https://github.com/apache/flink-kubernetes-operator/pull/466#issuecomment-1339533525
> > > @mxm can we set the log4j location conditionally? When append is false, set it to empty as shown in your original commit. Otherwise `value: -Dlog4j.configurationFile=/opt/flink/conf/log4j-operator.properties`
> >
> >
> > Good point, @tweise.
>
> Wouldn’t that mean that whatever the user sets in the config part won’t take any effect?
>
> I think the property always have to be set .
We can only skip the `LOG_FILE` environment variable if `defaultConfiguration.create` is set to `false`. Otherwise we would ignore the configuration changes under `defaultConfiguration.*`.
--
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] mbalassi commented on pull request #466: [FLINK-30280] Default operator logging configuration is broken
Posted by GitBox <gi...@apache.org>.
mbalassi commented on PR #466:
URL: https://github.com/apache/flink-kubernetes-operator/pull/466#issuecomment-1339680730
Given that we are looking to create a release candidate tomorrow I feel that the discussion has sufficiently converged, so I am merging the current state.
--
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