Issue 4136: Fix logging configurations are broken in docker image

[Shawyeok]

Motivation

Fix #4136

Changes

Modify apply-config-from-env.py to accept individual files as parameters instead of a configuration folder. This change will enable the script to selectively apply the configuration override mechanism only to the necessary files.

[hangc0276]

@Shawyeok Please rebase the master, thanks.

[Shawyeok]

@Shawyeok Please rebase the master, thanks.

Done, thanks.

[hangc0276]

@zymap Please help take a look at this PR, thanks.

[zymap]

Actually, the apply-config-from-env will go through the configuration file and only update the existing key with the format k=v. So I don't think it will update the log4j configuration file. Could you please paste your log4j configuration file as well?

[Shawyeok]

Actually, the apply-config-from-env will go through the configuration file and only update the existing key with the format k=v. So I don't think it will update the log4j configuration file. Could you please paste your log4j configuration file as well?

@zymap
There is an edge case here. For example, with the log4j2.xml configuration below, <Console name="CONSOLE" and <Console name="CONSOLEMDC" are treated as having the same key due to their k=v pattern (here, it’s <Consolename). As a result, any configuration lines that appear later are discarded, thus breaking the validity of the XML format.

bookkeeper/conf/log4j2.xml

Lines 30 to 35 in 5be03cf

	<Console name="CONSOLE" target="SYSTEM_OUT">
	<PatternLayout pattern="%d{ISO8601} - %-5p - [%t:%C{1}@%L] - %m%n"/>
	</Console>
	<Console name="CONSOLEMDC" target="SYSTEM_OUT">
	<PatternLayout pattern="%d{ISO8601} - %-5p - [%t:%C{1}@%L] %X - %m%n"/>
	</Console>

Here is a docker-compose file that can quickly reproduce the issue. You will see the error information mentioned in #4136.

version: '3'
services:
  zk:
    image: zookeeper:3.8.1
  bookie0:
    image: apache/bookkeeper:4.16.3
    environment:
      - BK_zkServers=zk:2181
    depends_on:
      - zk

[Shawyeok]

@zymap
The current capability of apply-config-from-env.py is limited to handling configuration files in the k=v format, like properties files. Applying it to XML format configurations is meaningless and may lead to unexpected configuration replacements.

[zymap]

okay. How about letting the apply-conf-from-env support use the specified files to update? Most of the time, we only need to update the bookkeeper configuration file and zookeeper configuration file. I think we always manually update the rocksdb and log4j configuration files.

[Shawyeok]

okay. How about letting the apply-conf-from-env support use the specified files to update? Most of the time, we only need to update the bookkeeper configuration file and zookeeper configuration file. I think we always manually update the rocksdb and log4j configuration files.

Yes, this is indeed what this PR does: it modifies apply-config-from-env.py to accept individual files as parameters. The usage would be like this:

$ ./apply-config-from-env file ...

[zymap]

Ah. Sorry. I misunderstood this pr. LGTM

[raymondBourges]

Thanks @Shawyeok for this PR :-)

I use the three files it contains via docker volumes to get around the log configuration problem on my 4.16.3 image.

What do you think @zymap about also pushing this PR on the 4.16 branch so that it will be present in future 4.16.x images?

THANKS

[zymap]

@raymondBourges Sure, will include this in the next release.

[raymondBourges]

Thanks but 4.16.4 was already released on January 29 (but image on dockerhub still in 4.16.3).
This is why I talk about an 4.16 branch so that the PR will be included in a possible 4.16.5.
Thanks

[lhotari]

I'm including this fix in #4207 and #4206 while fixing CI for branch-4.15 and branch-4.16 .