Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] Avoid boostrap check by not using elasticsearch in a cluster mode #153

Merged
merged 1 commit into from
Jan 11, 2017

Conversation

joelwurtz
Copy link
Contributor

@joelwurtz joelwurtz commented Jan 11, 2017

Disclaimer : Even if this is a PR, it's more a RFC to understand the purpose of the elasticsearch image.

Hey,

As discussed in #98 and many others issue the current elasticsearch 5.0 version depends on setting a correct vm.max_map_count on the host having docker engine.

This is due to the fact that when inside a container it's obvious that elasticsearch must listen on correct ip and not on the localhost (otherwise it would not be accessible).

This proposal intend to clearly define what is the purpose of the official elasticsearch image :

  • Is this image intended for production environment ?
  • Or is it intended for new developers wanting to discover elasticsearch ?

In the first case this PR can then be closed.

In the second case this PR make sense and should be discussed. The following configuration allows to use an elasticsearch server on a docker container without having to set the vm.max_map_count on the host of docker and other configurations needed to have a production environment for elasticsearch (see this comment : elastic/elasticsearch#4978 (comment), i have also test this configuration which works perfectly)

But it removes the possibility to have a cluster of elasticsearch, which is IMO not need when discovering elasticsearch and just want to develop around it.

The third choice would be that this image want to support both production and "easy" development environment then it could make sense to add an additional tag for this image and choose a default setting (prod or dev).

I know this has been discussed a lot, but all discussion never clarify the purpose of this image nor this possibility.

@@ -1,5 +1,5 @@
network.host: 0.0.0.0
http.host: 0.0.0.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is exactly what we need! ❤️ ❤️ ❤️

(@yosifkit pointed this out over in docker-library/docs#778 (comment), but hadn't gotten around to making a PR yet)

@tianon
Copy link
Member

tianon commented Jan 11, 2017

I'm definitely super +1 on this -- do you want to copy that change over to the Alpine variant's config as well, or would you rather I take care of that? 👍

@joelwurtz
Copy link
Contributor Author

I will add it later (must go right now), and also want to add a line in the documentation explaining that this image can not be used for a cluster.

@joelwurtz
Copy link
Contributor Author

(But if i'm too slow for you feel free to do it i don't mind :) )

@tianon
Copy link
Member

tianon commented Jan 11, 2017

It's waited this long, a bit longer shouldn't hurt -- happy to give you the credit. 👍

This will give us a good opportunity to point out that production deployments really ought to be supplying custom configuration anyhow. 😄

@joelwurtz
Copy link
Contributor Author

Should be good

joelwurtz added a commit to joelwurtz/docs that referenced this pull request Jan 11, 2017
joelwurtz added a commit to joelwurtz/docs that referenced this pull request Jan 11, 2017
@yosifkit
Copy link
Member

I really like the change and want this image to work everywhere by default.

Unfortunately, this will break systems that are using this image in production (and not already providing their own config) so we should figure out the best way to break less of them while still providing a useful image to everyone else.

It is actually fairly simple to go from the new config of http.host to a "production" deployment with no config files to worry about:

$ docker run -d --name elas elasticsearch -Etransport.host=0.0.0.0 -Ediscovery.zen.minimum_master_nodes=1

The opposite is also true with our current config to run it in dev mode

$ docker run -d --name elas elasticsearch -Etransport.host=127.0.0.1
$ # this will work on a system with a "low" vm.max_map_count

On the other hand, users should not be running this in production without providing their own config. It is only currently in "production" mode because we did not know about http.host and transport.host being separate parts of network.host.

@tianon
Copy link
Member

tianon commented Jan 11, 2017

Indeed, a good point, but our image being in "production" mode was a side effect of working around the "listen on localhost-only" change upstream made in the 5.x series, and folks running this image in production really probably ought to be tweaking jvm.options in addition to tweaking discovery.* to be sane values, so I'm not as worried about that (especially since they ought to test image updates before deploying them to production).

@joelwurtz
Copy link
Contributor Author

joelwurtz commented Jan 11, 2017

Like you said people using this image in production certainly have their own config already overriding this. Futhermore Elasticsearch 5.x is a very recent release (2 months old) so the combination of having this version with no overriding config for production should be very low.

Nevertheless is still a BC Break, on this problem is there any discussion about having 2 version for an image ? :

  • a version for the service inside an image
  • a version for how its packaged

I know that many package manager use somehow a package version (like in arch linux by adding -r{number} after the software version number)

@yosifkit
Copy link
Member

I looked up the current image via the handy docker-library/repo-info git repo and found this contains the sha256 for the most recent image of elasticsearch and here for the alpine version. Users in production systems should be pinning to a specific sha256 of the image so that they can properly test upgrades before deploying or at least have a rollback strategy based upon a known good sha256 of the image.

Copied docker pull for reference:

$ # Debian
$ docker pull elasticsearch@sha256:6b111f95c72f9b9f560c9495f6906e73f0baef0d3e842528da2d236f9a19423a
$ # Alpine:
$ docker pull elasticsearch@sha256:ae81a5a6e7db097ce65482c3dc1c22723b21abc977163a41d7607d4a656220c0

@yosifkit yosifkit merged commit 9c9d2d6 into docker-library:master Jan 11, 2017
tianon added a commit to infosiftr/stackbrew that referenced this pull request Jan 11, 2017
- `cassandra`: use `/etc/apt/trusted.gpg.d` instead of `apt-key adv` (docker-library/cassandra#91)
- `elasticsearch`: avoid bootstrap checks by using `http.host` instead of `network.host` (docker-library/elasticsearch#153), use `/etc/apt/trusted.gpg.d` instead of `apt-key adv` (docker-library/elasticsearch#152)
- `golang`: 1.8rc1, explicit Alpine 3.5 variant of Go 1.7 (docker-library/golang#134)
- `kibana`: use `/etc/apt/trusted.gpg.d` instead of `apt-key adv` (docker-library/kibana#69)
- `logstash`: use `/etc/apt/trusted.gpg.d` instead of `apt-key adv` (docker-library/logstash#78)
- `mariadb`: use `/etc/apt/trusted.gpg.d` instead of `apt-key adv` (MariaDB/mariadb-docker#93)
- `memcached`: 1.4.34
- `mongo`: use `/etc/apt/trusted.gpg.d` instead of `apt-key adv` (docker-library/mongo#132)
- `openjdk`: debian 9~b151-2
- `percona`: use `/etc/apt/trusted.gpg.d` instead of `apt-key adv` (docker-library/percona#39)
- `piwik`: 3.0.1
- `postgres`: use `/etc/apt/trusted.gpg.d` instead of `apt-key adv` (docker-library/postgres#246)
- `rabbitmq`: allow custom definitions file (docker-library/rabbitmq#112), use `/etc/apt/trusted.gpg.d` instead of `apt-key adv` (docker-library/rabbitmq#133)
- `redmine`: 3.2.5, 3.3.2
- `tomcat`: improve OpenSSL comments (docker-library/tomcat#59)
- `wordpress`: 4.7.1, fix Windows-newlines `sed` (docker-library/wordpress#197)
@joelwurtz joelwurtz deleted the patch-1 branch January 12, 2017 16:10
@uschtwill
Copy link

uschtwill commented Jan 15, 2017

Wanted to comment in #98, but as it is locked: Works like a charm, thanks guys!

uschtwill added a commit to uschtwill/docker_monitoring_logging_alerting that referenced this pull request Jan 15, 2017
1gtm pushed a commit to appscode-images/elasticsearch that referenced this pull request Feb 14, 2024
This commit was created by the elastic-dockerfiles-publisher.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants