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

Update Docker docs for 6.0.0-rc2 #27166

Merged
merged 6 commits into from Nov 1, 2017
Merged

Update Docker docs for 6.0.0-rc2 #27166

merged 6 commits into from Nov 1, 2017

Conversation

jarpy
Copy link
Contributor

@jarpy jarpy commented Oct 30, 2017

Update the docs to match the new Docker "image flavours" of "basic",
"platinum", and "oss".

@jarpy jarpy requested a review from dliappis October 30, 2017 05:53
@jarpy jarpy added v6.0.0 >docs General docs changes labels Oct 30, 2017
@@ -49,7 +69,7 @@ Elasticsearch can be quickly started for development or testing use with the fol

["source","sh",subs="attributes"]
--------------------------------------------
docker run -p 9200:9200 -p 9300:9300 -e "discovery.type=single-node" {docker-image}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't really need to bind transport to the host for dev mode, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

This was discussed in this thread

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Restored. I think it's a bit noisy, however.

@@ -97,7 +117,7 @@ sysctl -w vm.max_map_count=262144
+
The `vm.max_map_count` setting must be set via docker-machine:
+
["source","sh"]
["source","txt"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Trying to control crazy syntax highlighting.

@@ -109,7 +129,8 @@ To bring up the cluster, use the <<docker-prod-cluster-composefile,`docker-compo

ifeval::["{release-state}"=="unreleased"]

WARNING: Version {version} of the Elasticsearch Docker image has not yet been released, so a `docker-compose.yml` is not available for this version.
WARNING: Version {version} of the Elasticsearch has not yet been released, so a
`docker-compose.yml` is not available for this version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just using less words here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is the redundant here? i.e.

instead of

Version {version} of the Elasticsearch has not yet been released

to read

Version {version} of Elasticsearch has not yet been released

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Trying to use fewer words... but not few enough! Thanks.

image: docker.elastic.co/elasticsearch/elasticsearch:{version}
container_name: elasticsearch1
elasticsearch:
image: {docker-image}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renaming this container to the default name makes it easier to add other Stack components without having to explicitly configure them to hit elasticsearch1. This also applies to Kibana and Logstash monitoring, which work out-of-the-box with this change.

additionally need the `memlock: true` ulimit, either defined in the
https://docs.docker.com/engine/reference/commandline/dockerd/#default-ulimits[Docker
Daemon] or specifically set for the container. This is demonstrated above in the
<<docker-prod-cluster-composefile,docker-compose.yml>>. If using `docker run`:
+
Copy link
Contributor Author

@jarpy jarpy Oct 30, 2017

Choose a reason for hiding this comment

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

Made the "demonstrated earlier" part a bit friendlier sounding.

@@ -154,25 +178,24 @@ services:
memlock:
soft: -1
hard: -1
mem_limit: 1g
volumes:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think we should suggest this anymore because I believe it constrains the disk cache.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 this will help us close elastic/elasticsearch-docker#122

Copy link
Contributor

@dliappis dliappis left a comment

Choose a reason for hiding this comment

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

Great and solid work, thanks!

I left a few comments.

I will also shortly issue a PR on top of this branch with some additional bug fixes and clarifications.

@@ -49,7 +69,7 @@ Elasticsearch can be quickly started for development or testing use with the fol

["source","sh",subs="attributes"]
--------------------------------------------
docker run -p 9200:9200 -p 9300:9300 -e "discovery.type=single-node" {docker-image}
Copy link
Contributor

Choose a reason for hiding this comment

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

This was discussed in this thread

@@ -109,7 +129,8 @@ To bring up the cluster, use the <<docker-prod-cluster-composefile,`docker-compo

ifeval::["{release-state}"=="unreleased"]

WARNING: Version {version} of the Elasticsearch Docker image has not yet been released, so a `docker-compose.yml` is not available for this version.
WARNING: Version {version} of the Elasticsearch has not yet been released, so a
`docker-compose.yml` is not available for this version.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is the redundant here? i.e.

instead of

Version {version} of the Elasticsearch has not yet been released

to read

Version {version} of Elasticsearch has not yet been released

@@ -154,25 +178,24 @@ services:
memlock:
soft: -1
hard: -1
mem_limit: 1g
volumes:
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 this will help us close elastic/elasticsearch-docker#122

USER root
RUN chown elasticsearch:elasticsearch config/elasticsearch.yml
USER elasticsearch
COPY --chown=elasticsearch:elasticsearch elasticsearch.yml /usr/share/elasticsearch/config/
Copy link
Contributor

Choose a reason for hiding this comment

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

This is great, however we need to be mindful that will will not work for people running anything older than docker-ce 17.09

Copy link
Contributor Author

@jarpy jarpy Oct 31, 2017

Choose a reason for hiding this comment

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

I know, but I think we should write for the present. Everyone should build with the latest Docker. Upgrading your build workstation is not a big deal like upgrading your orchestrator.

Copy link
Contributor

Choose a reason for hiding this comment

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

That makes sense.


NOTE: {xpack-ref}/index.html[X-Pack] is preinstalled in this image.
Please take a few minutes to familiarize yourself with {xpack-ref}/security-getting-started.html[X-Pack Security] and how to change default passwords. The default password for the `elastic` user is `changeme`.
The images are available in three different configurations or "flavours". The
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe the style guide mandates American English for conformity, so probably needs to be flavors. I get caught by this frequently.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't find that in the guide. Do we have an official dialect, @hstephenson?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Switched to US spelling since I want to ship this and it doesn't really hurt anything except my cultural pride (which is not particularly relevant, let's face it).

@dliappis
Copy link
Contributor

@jarpy You may want to review https://github.com/jarpy/elasticsearch/pull/1/files as before tweaking this further.

jarpy and others added 4 commits October 31, 2017 11:51
Update the docs to match the new Docker "image flavours" of "basic",
"platinum", and "oss".
* Clarifications for Openshift and bind-mounts

* Bump docker-compose 2.x format to 2.2

* Combine Docker Toolbox instructions for setting vm.max_map_count for
  both macOS + Windows

* devicemapper is not the default storage driver any more on RHEL
chmod g+rwx esdatadir
chgrp 1000 esdatadir
+
As a last resort, you can also force the container to mutate the ownership of any bind-mounts used for the <<path-settings,data and log dirs>> through the environment variable `TAKE_FILE_OWNERSHIP`; in this case they will be owned by uid:gid `1000:0` providing read/write access to the elasticsearch process as required.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

"As a last resort" 👍

@jarpy jarpy merged commit b71f7d3 into elastic:master Nov 1, 2017
jarpy added a commit that referenced this pull request Nov 1, 2017
* Update Docker docs for 6.0.0-rc2

* Update the docs to match the new Docker "image flavours" of "basic",
"platinum", and "oss".

* Clarifications for Openshift and bind-mounts

* Bump docker-compose 2.x format to 2.2

* Combine Docker Toolbox instructions for setting vm.max_map_count for
  both macOS + Windows

* devicemapper is not the default storage driver any more on RHEL
jarpy added a commit that referenced this pull request Nov 1, 2017
* Update Docker docs for 6.0.0-rc2

* Update the docs to match the new Docker "image flavours" of "basic",
"platinum", and "oss".

* Clarifications for Openshift and bind-mounts

* Bump docker-compose 2.x format to 2.2

* Combine Docker Toolbox instructions for setting vm.max_map_count for
  both macOS + Windows

* devicemapper is not the default storage driver any more on RHEL
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Nov 1, 2017
* master:
  Enhances exists queries to reduce need for `_field_names` (elastic#26930)
  Added new terms_set query
  Set request body to required to reflect the code base (elastic#27188)
  Update Docker docs for 6.0.0-rc2 (elastic#27166)
  Add version 6.0.0
  Docs: restore now fails if it encounters incompatible settings (elastic#26933)
  Convert index blocks to cluster block exceptions (elastic#27050)
  [DOCS] Link remote info API in Cross Cluster Search docs page
  Fix Laplace scorer to multiply by alpha (and not add) (elastic#27125)
  [DOCS] Clarify migrate guide and search request validation
  Raise IllegalArgumentException if query validation failed (elastic#26811)
  prevent duplicate fields when mixing parent and root nested includes (elastic#27072)
  TopHitsAggregator must propagate calls to `setScorer`. (elastic#27138)
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Nov 2, 2017
* master:
  Remove checkpoint tracker bit sets setting
  Fix stable BWC branch detection logic
  Fix logic detecting unreleased versions
  Enhances exists queries to reduce need for `_field_names` (elastic#26930)
  Added new terms_set query
  Set request body to required to reflect the code base (elastic#27188)
  Update Docker docs for 6.0.0-rc2 (elastic#27166)
  Add version 6.0.0
  Docs: restore now fails if it encounters incompatible settings (elastic#26933)
  Convert index blocks to cluster block exceptions (elastic#27050)
  [DOCS] Link remote info API in Cross Cluster Search docs page
  Fix Laplace scorer to multiply by alpha (and not add) (elastic#27125)
  [DOCS] Clarify migrate guide and search request validation
  Raise IllegalArgumentException if query validation failed (elastic#26811)
  prevent duplicate fields when mixing parent and root nested includes (elastic#27072)
  TopHitsAggregator must propagate calls to `setScorer`. (elastic#27138)
jasontedor added a commit that referenced this pull request Nov 2, 2017
* master:
  Lazy initialize checkpoint tracker bit sets
  Remove checkpoint tracker bit sets setting
  Fix stable BWC branch detection logic
  Fix logic detecting unreleased versions
  Enhances exists queries to reduce need for `_field_names` (#26930)
  Added new terms_set query
  Set request body to required to reflect the code base (#27188)
  Update Docker docs for 6.0.0-rc2 (#27166)
  Add version 6.0.0
  Docs: restore now fails if it encounters incompatible settings (#26933)
  Convert index blocks to cluster block exceptions (#27050)
  [DOCS] Link remote info API in Cross Cluster Search docs page
  Fix Laplace scorer to multiply by alpha (and not add) (#27125)
  [DOCS] Clarify migrate guide and search request validation
  Raise IllegalArgumentException if query validation failed (#26811)
  prevent duplicate fields when mixing parent and root nested includes (#27072)
  TopHitsAggregator must propagate calls to `setScorer`. (#27138)
jasontedor added a commit that referenced this pull request Nov 2, 2017
* 6.x:
  Lazy initialize checkpoint tracker bit sets
  Remove checkpoint tracker bit sets setting
  Tests: Fix FullClusterRestartIT.testSnapshotRestore test failing in 6.x (#27218)
  Fix FullClusterRestartIT using lenient booleans with 6.0
  Fix stable BWC branch detection logic
  Add version 6.0.0
  Fix logic detecting unreleased versions
  Skips exists query tests on unsupported versions
  Enhances exists queries to reduce need for `_field_names` (#26930)
  Added new terms_set query
  Set request body to required to reflect the code base (#27188)
  Update Docker docs for 6.0.0-rc2 (#27166)
  Docs: restore now fails if it encounters incompatible settings (#26933)
  Convert index blocks to cluster block exceptions (#27050)
  [DOCS] Link remote info API in Cross Cluster Search docs page
  prevent duplicate fields when mixing parent and root nested includes (#27072)
  TopHitsAggregator must propagate calls to `setScorer`. (#27138)
@lcawl lcawl removed the v6.1.0 label Dec 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants