pblittle / docker-logstash

Docker image for Logstash 1.4
https://hub.docker.com/r/pblittle/docker-logstash
MIT License
236 stars 90 forks source link

The container should not overwrite my custom configuration file #69

Closed mikehaertl closed 9 years ago

mikehaertl commented 9 years ago

As mentioned in #65 I think the approach to rewrite my custom config file is flawed. It requires me to add the config vars ES_HOST and ES_PORT back in, each time I restart my setup.

First I thought, you could use the logstash contrib plugins and then have support for environment vars in the config file. But this seems not to work for the elasticsearch host:

https://github.com/elasticsearch/logstash/issues/1910

So I suggest a different approach: Maybe you copy my configuration files into a directory inside the container before you modify them and then include them in the logstash config from there. This way they could get safely regenerated each time I rebuild my container.

pblittle commented 9 years ago

@mikehaertl thanks for the suggestion. When the current functionality was implemented in #16, I didn't think about the use case you're describing. I definitely see how it can be a pain though.

Would simply not applying the string interpolation when a config file(s) exist work for you?

mikehaertl commented 9 years ago

Can you explain what you mean by string interpolation?

pblittle commented 9 years ago

@mikehaertl, sure. I meant the process of replacing the ES_HOST and ES_PORT placeholders in logstash.conf with the values returned by the es_service_host and es_service_port functions.

This is the current flow for downloading the Elasticsearch config when you run the build:

  1. If /opt/logstash/conf.d doesn't exist, create it.
  2. If /opt/logstash/conf.d is empty, download the config file using the value of $ES_CONFIG_URL.
  3. Next, replace all occurrences of ES_HOST and ES_PORT in the config file with the es_service_host and es_service_port function return values.

As it stands now, if you run your image with a volume mounted at /opt/logstash/conf.d, and the mount contains a config file, steps 1 and 2 will be skipped. However, step 3 will still perform the interpolation described above.

My idea is to skip steps 1, 2, and 3 if your config file is in a mounted volume.

One quick note, if you think either step 1, 2, or 3 isn't working as expected, let me know.

I hope that helps. I apologize if I'm misunderstanding your issue. Correct me if I am.

mikehaertl commented 9 years ago

But if you skip step 3, then the ES_HOST and ES_PORT vars will never be replaced, which doesn't work either. Or maybe I don't understand. I think in principle everything is fine the way it is - you only should add one more step and copy my file somewhere else, before you do string interpolation on that copied file.

pblittle commented 9 years ago

@mikehaertl I'm not sure if the change I just made helps or hurts your issue, I hope it helps...

Now, rather than always applying the interpolation, the config file is only changed if the config directory was previously empty. A config file that already existed from either a mounted source or previous download should not be updated.

https://github.com/pblittle/docker-logstash/blob/develop/1.4/base/logstash.sh#L79-L104

The change is on the develop branch. If you have a minute, please take a look and let me know what you think.

pblittle commented 9 years ago

Closing due to inactivity. @mikehaertl please reopen this issue if want to work through my comment above.