Fix permissions of logs directory

[tristan0x]

When logs directory /usr/share/elasticsearch/logs is mount as data volume,
then it is owned by root and elasticsearch was not allowed to write logs.

This PR modifies the Dockerfile entrypoint to properly chown the logs directory if available.

[daghack]

Moghedrin punts explanation to @yosifkit

[yosifkit]

Following the docker way, we log to stdout and stderr (logging.yml). Docker 1.6 now has --log-driver in docker run which can facilitate things like syslog.

Also, @Moghedrin was grumpy about the whitespace changes. #holywars (we use tabs)

[daghack]

[tristan0x]

Sorry guys, I can understand the religion war between spaces and tabs which has occured for decades. But I don't understand why logging to files is not a good pattern.

Why stdout and stderr are not enough?

My Elasticsearch logging.yml settings create several files per index, among them files dedicated to slow queries, shard replication, and so on. Logs are dispatched in the first place, and it helps a log their processing. This dispatched is application specific, and should be specified in the container, not in the Docker host syslog config.

Why using files for logs?

Once a log is emitted, the application does not have to worry wether the log will be processed or no. The log is safe, its processing asynchronous, meaning that there is no performance degradation of the application.

Why using a volume for logs?

Logs processing can be handled by other containers. For instance I can use a rsyslog container reading logs of application containers hosted on the same registry. Rsyslog then pushes logs to a logstash container, so that I can use a Kibana container to monitor my applications logs.

Please explain me why this is not a good pattern. If you can share some resources that can enlight me, please do.

[md5]

The reasons you list for using a mounted log directory are reasonable, but using stdout and stderr is how all the official images work. You're free to create your own entrypoint in a derived image that chowns whatever directories it wants and doesn't log in a way that works with the “docker logs " command, but it doesn't make sense for this image to do so by default.

[tianon]

The differentiator here for me is that we don't configure any logging to files by default, so if elasticsearch is logging to files it's already due to custom configuration, at which point "which files to chown" becomes hard to guess.

[bobrik]

Elasticsearch has default logging location set to /usr/share/elasticsearch/logs. There is no need to guess this location.

Changing permissions won't break existing containers and would enable logs on disk without any extending required.

[Garito]

For those who said no as full response:

How it is supossed to consume those logs?

I understand that stdout is the docker's way to send logs to the user because is the basic way to comunicate with the user not because it has to be religion or because is better or, at least, no if you don't point out how to deal with this

Logs aren't something for weird people is THE NORM everyone uses them in a way or another so seems to me that an image considered official has to provide some solution for that

[yosifkit]

Given that currently most official images only log to stdout/stderr it is unlikely to change, but not impossible, where we default the logs. That said, we should be able to at least document how a user can provide their own config for elasticsearch and output those logs to a mounted volume or data container.

[tristan0x]

@yosifkit 👍 Couldn't agree more.

But this PR is still required to do this the most standard way though. It properly chown the default elasticsearch log directory if present (more likely because it has been mount).
I offer my help to create the according PR on docker-library/docs...

[Garito]

Seems good to me but, sorry, don't would like to offend no one, ok, but let me do a joke:

seems to me a little Sheldon Cooper to put a for loop for 2 folders: what you safe with the for, giving the fact that there are only 2 folders and is so difficult that need more, in terms of lines of code you expend in make the AST bigger

In more serious terms: I see the point but I see this as an over optimization since there is and will be only two folders

But the pattern seems to me good enought
+1

Thanks

[tristan0x]

@Garito very funny :)

I don't really mind, but I think the for loop is the exact implementation of the comment above.

[tianon]

I'm still confused because the only way I see to actually use this is by supplying custom configuration that tells elasticsearch to log here (since we overwrite the default configuration with one that states logs should go to stdout). Can you provide an example of how this would be intended to be used?

[bobrik]

"Custom" configuration is the default one, it comes with elasticsearch itself:

https://github.com/elastic/elasticsearch/blob/master/config/logging.yml

Paths there are relative to path.logs, it defaults to /usr/share/elasticsearch/logs.

I bind-mount logging.yml that comes with elasticsearch to the container and expect it to work.

[j0hnsmith]

This is a tiny change (chmoding the 'default' elasticsearch logs dir, not necessarily this pull request) that won't change the default behaviour or have any detrimental effect to the way containers based on this image work (they'll still log to stdout, the docker way).

People want to use official images rather than maintain derived ones, companies moving to docker may be comfortable with using official images but not comfortable with maintaining their own. This refusal seems overzealous, to say the least.

[mi-hol]

@yosifkit would it make sense to close this?

[tianon]

@j0hnsmith makes a good point that this logs directory does exist by default, so supporting it is sane enough I suppose; I'll work up a replacement PR to carry this change 👍