Upgrade to ES 5 #8

sberryman · 2017-04-06T23:25:49Z

Please don’t merge this yet, this is a work in progress. Doing this to get feedback from you guys.

sberryman · 2017-04-06T23:34:29Z

I'm not really a big fan of elasticsearch-master, elasticsearch-data and elasticsearch (with a service name of elasticsearch-master). I'm guessing this has to do with elasticsearch_master being a "gateway" where you would expose the ports? Wouldn't it be better to have HA running and reverse proxy to the elastic masters?
It is ugly how it just keeps hitting consul every second to see if there are any other services registered as 'elasticsearch-master'. Maybe creating a lock is a good idea?
Seeing warnings for consol 0.8.0 now, any blockers to upgrading that you are aware of?
I'm not cleaning up unzip or tar yet, that will be uncommented in the future.
You had ES_BOOTSTRAP_HOST as an env var for discovery.zen.ping.unicast.hosts: ["${ES_BOOTSTRAP_HOST}"] but it isn't defined in the project.
When it comes to security I lack a lot of common sense :) How does the following code block in Dockerfile look?

# Create and take ownership over required directories
RUN mkdir -p /opt/consul/config \
    && mkdir -p /opt/consul/data \
    && chmod 770 /opt/consul/data \
    && chown -R elasticsearch:elasticsearch /opt/consul \
    && mkdir -p /etc/containerpilot \
    && chmod -R g+w /etc/containerpilot \
    && chmod +x /usr/local/bin/elastic-server.sh \
    && chown -R elasticsearch:elasticsearch /etc/containerpilot

tgross · 2017-04-07T12:40:00Z

Dockerfile

+COPY /bin/* /usr/local/bin/
+
+# Should we remove unzip?
+# RUN apk del unzip tar


Removing anything in a different layer doesn't reduce total image size.

Interesting, does it if you squash it?

tgross · 2017-04-07T12:41:15Z

bin/elastic-server.sh

@@ -0,0 +1,6 @@
+#!/bin/sh -xe
+manage.sh onStart #|| exit $?


Anything like this should be in a preStart, not wrapping the main app. That would also eliminate the need for this shell script.

I copied this from the redis autopilot example.
https://github.com/autopilotpattern/redis/blob/master/bin/redis-server-sentinel.sh Where it obviously does a bit more.

Does preStart fire BEFORE you start the co-processes? I'm asking because we need the co-process consul agent to start before we start the onStart function.

Did some research and unfortunately this has to be a separate wrapper as the coprocess doesn't start until immediately after preStart. We need the coprocess (consul agent) to start prior to onStart. onStart's job is there to check whether it has joined the consul cluster at which point we can verify if other services exist. If we did this in preStart, the consul agent won't be running and will assume every container should become the master.

Coprocesses are started immediately following the exit of the preStart hook, before polling for health or backend hooks, and before the main application starts. - Containerpilot Docs

I think this could be confusing for developers like me who are just getting started with containerpilot and consul agent running as a coprocess. Is there a reason you guys use CONSUL_AGENT=1 in most of the examples. What is the purpose of making that optional? Meaning, why wouldn't you always use consul agent on each container?

Bah, no you're right. This will be fixed in CPv3 because we're forcing people to admit they need a real init system. But in the meantime we're stuck with it.

How would a "real" init system change this? I'm not sure what you mean by real init though.

In CPv3 we're supporting multiple processes as first-class, rather than having a "main" process and coprocesses. The end user will create the chain of dependencies explicitly, so in this case we'll be able to have everything wait on the consul agent. This is pretty much how every modern init system works, but was something we were avoiding in ContainerPilot originally because of community... obstinance(?) around the notion of there being more than one process in a container. Turns out that's exactly what you want a lot of the time. 😀

tgross · 2017-04-07T12:47:42Z

Dockerfile

@@ -1,54 +1,65 @@
-FROM debian:jessie
+FROM docker.elastic.co/elasticsearch/elasticsearch:5.3.0


Why swap out for the official image from Elastic? We were using the debian image because it's lean and gives us a good starting point.

There are a couple problems with the official image for us here:

according to the Elastic docs this image includes X-Pack, which has only a trial license. I don't even want to start down the road of reconciling this repo's MPL license with that. 😀 So I don't think this is appropriate for us to distribute.

I can't find the Dockerfile for this image anywhere, or at least not easily, so even if we did use this image we should include a link on where to find the Dockerfile.

Yeah, I had a feeling this would be an issue. This is due to the "official" elasticsearch image being depreciated.

This image will receive no further updates after 2017-06-20 (June 20, 2017). Please adjust your usage accordingly. - Docker Hub

X-Pack is definitely part of the official image and has been turned off via env vars. I can remove it if that will help? Or obviously drop back to debian and build it.

Funny you mentioned not being able to find the Dockerfiles, I remember coming across a github issue mentioning the same thing. I can't seem to find that issue right now though.

Dockerfile
Base Dockerfile

X-Pack is definitely part of the official image and has been turned off via env vars. I can remove it if that will help?

Removing it doesn't remove it from the shipped binary blobs because of the layered file system. If we going to try to use their current build I'd say we should just fork from what they have in their Dockerfiles.

Okay, would you like to fork based on their dockerfile? I would think copying the "official" repository from Elastic would be the best option for long term support.

We could use FROM docker.elastic.co/elasticsearch/elasticsearch-alpine-base:latest but after that I imagine we could just copy in the contents of https://github.com/elastic/elasticsearch-docker/blob/master/build/elasticsearch/Dockerfile into this directory and remove the bits we don't want. We'd need to put the LICENSE notice for their repo at the top of that file.

Sounds good, I'll work on that tomorrow morning

tgross · 2017-04-07T12:49:14Z

Dockerfile

-# leave this here then we get the ports as well-known environment variables
-# for purposes of linking.
-EXPOSE 9200
-EXPOSE 9300


Were these removed just because the official image has them already? (Again, not sure because I can't find the Dockerfile.)

Yes, the Dockerfile includes them.
https://github.com/elastic/elasticsearch-docker/blob/master/build/elasticsearch/Dockerfile#L44

tgross · 2017-04-07T12:52:04Z

bin/manage.sh

+consulCommand() {
+    consul-cli --quiet --consul="${CONSUL_HOST}:8500" $*
+}
+
 preStart() {


Unless I'm missing something this preStart doesn't do anything, right? Almost everything you have in the onStart should really be in the preStart I think.

I'm guessing you mean preStart not the highlighted consulCommand... But correct, I just left it in there as a placeholder, will remove it now.

tgross · 2017-04-07T12:52:25Z

bin/manage.sh


-if [[ -z ${CONSUL} ]]; then
+readonly lockPath=service/elasticsearch-master/locks/master


What is this for? It's unused anywhere else I think.

I'm going to pull it, it was left in there when I was testing using a lock

tgross · 2017-04-07T12:55:37Z

local-compose.yml

+          - ES_NODE_DATA=true
+
+    consul:
+        image: autopilotpattern/consul:0.7.2-r0.8


Seeing warnings for consol 0.8.0 now, any blockers to upgrading that you are aware of?

ContainerPilot has only 0.7 bindings, so that shouldn't be a problem but it's a possible source of problems. Also we don't have our Consul image updated yet, so we'd want to verify that the updated 0.8 Consul agent works with 0.7 first too. I'd suggest not doing this in this PR to keep the changeset manageable.

tgross · 2017-04-07T13:00:52Z

local-compose.yml

-      - ES_NODE_DATA=true
-    links:
-      - consul:consul
+    elasticsearch_master:


I'm not really a big fan of elasticsearch-master, elasticsearch-data and elasticsearch (with a service name of elasticsearch-master). I'm guessing this has to do with elasticsearch_master being a "gateway" where you would expose the ports? Wouldn't it be better to have HA running and reverse proxy to the elastic masters?

The reason we have 3 variants is for flexibility in deploying the 3 roles. In a small-to-middling-size environment one would just deploy all the ES instances as master+data. In a large production environment typically you'll want to send all the writes thru masters and do all the reads on data-only instances. We didn't want to be overly opinionated about the topology in this blueprint.

Good to know, I've never run a big ES environment. My experience is limited to < 1 TB.

tgross · 2017-04-07T13:03:03Z

bin/manage.sh

-            n=$((n+1))
+    else
+
+        # wait for a healthy master


It is ugly how it just keeps hitting consul every second to see if there are any other services registered as 'elasticsearch-master'. Maybe creating a lock is a good idea?

You mean here? That's only during startup, so while I suppose it's ugly it's also rather simple and it goes away after the initial startup. How does creating a lock help us out here?

It doesn't now that I've had a chance to sleep on it.

tgross · 2017-04-07T13:05:55Z

etc/elasticsearch.yml

 #
-# gateway.recover_after_nodes: 3
+discovery.zen.ping.unicast.hosts: ["${ES_BOOTSTRAP_HOST}"]


You had ES_BOOTSTRAP_HOST as an env var for discovery.zen.ping.unicast.hosts: ["${ES_BOOTSTRAP_HOST}"] but it isn't defined in the project.

True, and this should be better documented. It's not used by default but it's an option for a user who has an existing cluster they want to bootstrap from. We didn't end up using it in https://github.com/autopilotpattern/elk and it seems to be of dubious use. I'd be fine with eliminating that.

So I'll comment out that line and the function replace (which has been renamed replaceZenHosts to be a little more verbose) will place the host and uncomment it.

Something I just realized is that we only populate it with a single host. If that container restarts and the referenced node is no longer part of the cluster it will fail to join the cluster.

sberryman · 2017-04-07T16:54:11Z

@tgross I've implemented several changes based on your feedback. Thanks for being so detailed and giving it a thorough review.

sberryman · 2017-04-08T21:15:00Z

Joyent issue:

+ exec /usr/share/elasticsearch/bin/es-docker
time="2017-04-08T20:57:01Z" level=info msg="    2017/04/08 20:57:01 [INFO] agent: Synced node info" 
[2017-04-08T20:57:06,064][WARN ][o.e.b.JNANatives         ] unable to install syscall filter: 
java.lang.UnsupportedOperationException: seccomp unavailable: requires kernel 3.5+ with CONFIG_SECCOMP and CONFIG_SECCOMP_FILTER compiled in
	at org.elasticsearch.bootstrap.SystemCallFilter.linuxImpl(SystemCallFilter.java:350) ~[elasticsearch-5.3.0.jar:5.3.0]
	at org.elasticsearch.bootstrap.SystemCallFilter.init(SystemCallFilter.java:638) ~[elasticsearch-5.3.0.jar:5.3.0]
	at org.elasticsearch.bootstrap.JNANatives.tryInstallSystemCallFilter(JNANatives.java:215) [elasticsearch-5.3.0.jar:5.3.0]
	at org.elasticsearch.bootstrap.Natives.tryInstallSystemCallFilter(Natives.java:99) [elasticsearch-5.3.0.jar:5.3.0]
	at org.elasticsearch.bootstrap.Bootstrap.initializeNatives(Bootstrap.java:111) [elasticsearch-5.3.0.jar:5.3.0]
	at org.elasticsearch.bootstrap.Bootstrap.setup(Bootstrap.java:204) [elasticsearch-5.3.0.jar:5.3.0]
	at org.elasticsearch.bootstrap.Bootstrap.init(Bootstrap.java:360) [elasticsearch-5.3.0.jar:5.3.0]
	at org.elasticsearch.bootstrap.Elasticsearch.init(Elasticsearch.java:123) [elasticsearch-5.3.0.jar:5.3.0]
	at org.elasticsearch.bootstrap.Elasticsearch.execute(Elasticsearch.java:114) [elasticsearch-5.3.0.jar:5.3.0]
	at org.elasticsearch.cli.EnvironmentAwareCommand.execute(EnvironmentAwareCommand.java:58) [elasticsearch-5.3.0.jar:5.3.0]
	at org.elasticsearch.cli.Command.mainWithoutErrorHandling(Command.java:122) [elasticsearch-5.3.0.jar:5.3.0]
	at org.elasticsearch.cli.Command.main(Command.java:88) [elasticsearch-5.3.0.jar:5.3.0]
	at org.elasticsearch.bootstrap.Elasticsearch.main(Elasticsearch.java:91) [elasticsearch-5.3.0.jar:5.3.0]
	at org.elasticsearch.bootstrap.Elasticsearch.main(Elasticsearch.java:84) [elasticsearch-5.3.0.jar:5.3.0]

This error does NOT show up when running on Docker for Mac (Version 17.03.1-ce-mac5 (16048)).

Update: https://www.elastic.co/guide/en/elasticsearch/reference/current/system-call-filter-check.html
Update 2: This does appear to be a blocking issue.

tgross · 2017-04-10T12:48:02Z

Joyent issue:

Thanks. Let me open a ticket with the appropriate team here at Joyent and see what they say.

tgross · 2017-04-10T13:02:20Z

Taking a deeper look I would not expect their bootstrap check to work. Seccomp isn't currently supported by LX/sdc-docker. It's not even supported in all Linux distros, so more enterprisey Linux has a similar problem. (See also elasticsearch GH #23159, puppet GH #798, and elk-docker GH #107)

Update 2: This does appear to be a blocking issue.

Yeah looks like this was a breaking change for 5.2, where they changed it from a warning to an error. For now it looks like we can disable this check. Elastic says:

To pass the system call filter check you must either fix any configuration errors on your system that prevented system call filters from installing (check your logs), or at your own risk disable system call filters by setting bootstrap.system_call_filter to false.

sberryman · 2017-04-10T16:31:18Z

@tgross any suggestion on how to check in preStart or onStart to see if Seccomp is available? I would prefer not to turn it off completely. Ideally it should be left on for hosts that support it and disabled for hosts such as Joyent?

But to confirm I was able to disable system call filter checks via config and have it running on Triton this weekend.

tgross · 2017-04-10T21:02:22Z

@tgross any suggestion on how to check in preStart or onStart to see if Seccomp is available? I would prefer not to turn it off completely. Ideally it should be left on for hosts that support it and disabled for hosts such as Joyent?

Not sure. Maybe we could hijack whatever mechanism Elastic is using? Do we know if they're running a shell script to do that check or is it all in Java?

sberryman · 2017-04-11T21:29:18Z

This is what I found on seccomp:

Running on triton:

bash-4.3$ head /proc/config.gz
head: /proc/config.gz: No such file or directory
bash-4.3$ cat /boot/config-`uname -r`
cat: can't open '/boot/config-3.13.0': No such file or directory

Using /proc/config.gz worked when testing on Docker for Mac. It did NOT disable seccomp when running on docker locally. It properly disabled it on Joyent but I have not tested anywhere else.

Requires us to duplicate a few files from dockers official image. Also add in a seccomp check to disable it on systems that don’t have it

sberryman · 2017-04-11T21:50:12Z

@tgross: I've changed the base docker image and added checks for seccomp. Unfortunately we have to duplicate a few files from their official repo.

I wasn't sure what license you were talking about included on the top of the dockerfile so I took a guess.

I have it running on Joyent and scaling up and down just fine

tgross · 2017-04-13T13:04:51Z

bin/es-docker

@@ -0,0 +1,39 @@
+#!/bin/bash
+
+# Run Elasticsearch and allow setting default settings via env vars


Given that we control the command we're using to call ES along with its environment, we don't really need this shell script. We can include its effects entirely in the docker-compose.yml's command section.

I think we can move it to elastic-server.sh to reduce the number of files but it doesn't really simplify it that much does it? I don't think it is smart to remove the code, anyone who has used the official docker repo for Elasticsearch will expect that functionality to continue. I would expect switching to containerpilot from the official image to work without any changes. I would want all ENV variables I used to configure elastic to continue working as is with containerpilot.

I would want all ENV variables I used to configure elastic to continue working as is with containerpilot.

Sure but there's literally only one line that doesn't work as-is: https://github.com/autopilotpattern/elasticsearch/pull/8/files/2f807599ea62bf41d3fc811586ffd0648ac40496#diff-35c179adb2b0c25802542935645540a7R37

tgross · 2017-04-13T13:05:32Z

bin/manage.sh

-        exit 0
-    fi
+consulCommand() {
+    consul-cli --quiet --consul="${CONSUL_HOST}:8500" $*


There's already a consul binary in the container. We don't need this wrapper.

So you want me to pull this out and make the following changes?

Service addresses

Use curl and jq to get service addresses
https://github.com/sberryman/elasticsearch/blob/wip-v5/bin/manage.sh#L28

waitForLeader

waitForLeader using consul members -status alive | grep server

Replace: https://github.com/sberryman/elasticsearch/blob/wip-v5/bin/manage.sh#L48-L64
With: https://github.com/sberryman/autopilotpattern-nats/blob/master/bin/manage.sh#L28-L44

Yeah, that should do it.

tgross · 2017-04-13T13:07:31Z

bin/manage.sh

-replace() {
-    REPLACEMENT=$(printf 's/^discovery\.zen\.ping\.unicast\.hosts.*$/discovery.zen.ping.unicast.hosts: ["%s"]/' ${MASTER})
-    sed -i "${REPLACEMENT}" /etc/elasticsearch/elasticsearch.yml
+    # seccomp (not supported on joyent)


I'm going to nitpick this comment slightly: # disable seccomp (only supported on newer Linux kernels)

…for other services

sberryman added 2 commits April 6, 2017 16:24

Fairly major changes, they work locally... haven't tested on triton yet

5e378c3

Cleanup

a3b5b14

tgross reviewed Apr 7, 2017

View reviewed changes

sberryman added 6 commits April 7, 2017 08:44

preStart doesn't do anything, need to research when preStart triggers

62bbc55

No point in the if/else as they do the same thing...

f061dc0

Wouldn't save space as this isn't squashed prior to release

cf4613a

Pull out ES_BOOTSTRAP_HOST

6a5ecc9

Remove x-pack

30059ba

Update readme to include links to the dockerfile

ed21abe

sberryman added 2 commits April 7, 2017 10:08

Remove unused lockPath

c41705e

Removed preStart from help

cfae2f6

Change the base docker image

2f80759

Requires us to duplicate a few files from dockers official image. Also add in a seccomp check to disable it on systems that don’t have it

tgross reviewed Apr 13, 2017

View reviewed changes

sberryman added 4 commits April 13, 2017 11:25

🐳

b31c5e6

Use consul members instead of consul-client

10546c6

No need to wait for a leader if using consul-agent

54eb733

Not sure why I was using 9200 here? We want to discover on port 9200 …

80d9c2b

…for other services

		@@ -1,54 +1,65 @@
		FROM debian:jessie
		FROM docker.elastic.co/elasticsearch/elasticsearch:5.3.0


		if [[ -z ${CONSUL} ]]; then
		readonly lockPath=service/elasticsearch-master/locks/master

		@@ -0,0 +1,39 @@
		#!/bin/bash

		# Run Elasticsearch and allow setting default settings via env vars

Upgrade to ES 5 #8

Are you sure you want to change the base?

Upgrade to ES 5 #8

Conversation

sberryman commented Apr 6, 2017

sberryman commented Apr 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sberryman Apr 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sberryman Apr 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sberryman commented Apr 7, 2017

sberryman commented Apr 8, 2017 • edited Loading

tgross commented Apr 10, 2017

tgross commented Apr 10, 2017

sberryman commented Apr 10, 2017

tgross commented Apr 10, 2017

sberryman commented Apr 11, 2017

sberryman commented Apr 11, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Service addresses

waitForLeader

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sberryman Apr 7, 2017 •

edited

Loading

sberryman Apr 7, 2017 •

edited

Loading

sberryman commented Apr 8, 2017 •

edited

Loading