Allow running the build with docker #564

nik9000 · 2019-01-29T20:36:23Z

This adds support for building the docs using docker with the expectation
without breaking support for building the docs without docker. My goal
is to make it simpler for to add dependencies. In the long term this'll
allow us to add support for automatically building diagrams and fancy
mathematical notation. In the short term this'll make it simpler to run
asciidoctor in CI.

On macOS docker is a little slower than running outside of docker. At
this point the overhead is about 20%. This isn't great, but Asciidoctor
will more than make up the difference in the long run. And, in the short
run at least, docker isn't reaquired. I'll make sure it is never
required to build Asciidoc.

One fairly questionable choice that this makes is to diable seccomp by
default. The story goes like this: on linux, building the docs in docker
is somewhere between faster than building them outside of docker and
macOS's overhead. Unless you turn on seccomp. With seccomp enabled the
docs take about twice as long to build in docker. Which is terrible! As
much as I want security, this is terrible! I'd really love to turn it on
but I can't defend that kind of hit.

2x performance hit on mac

Much simpler

nik9000 · 2019-01-29T20:37:32Z

Dockerfile

@@ -0,0 +1,35 @@
+# Debian builds the docs about 20% faster than alpine. The image is larger
+# and takes longer to build but that is worth it.
+FROM bitnami/minideb:stretch


Alpine is much nicer to work with but 20% slower is a big deal given how often we run this locally.

nik9000 · 2019-01-29T20:38:17Z

README.asciidoc

@@ -17,23 +17,7 @@ on any other website without explicit permission.
 [float]
 == Requirements

-First, you need to make sure that you have the following installed:


I'm just pointing folks right at docker by default because it ought to be simpler to set up.

nik9000 · 2019-01-29T20:39:54Z

README.asciidoc

@@ -1027,13 +1005,13 @@ footnote to a particular line of code:
 ==================================

 [[sense-snippets]]
-=== View in Sense
+=== View in Console


These aren't related but they jumped out at me while I was here. I can break this out if folks are more comfortable with that.

Grep tells me there are two more SENSEs lurking in here if you wanna smack 'em.

$ grep -iFn sense README.asciidoc 1066: // SENSE: path/to/snippet.json 1082:// SENSE:one/two/three/example_1.json <1>

nik9000 · 2019-01-29T20:40:50Z

build_docs.pl

+            if ( $running_in_docker ) {
+                # We use nginx to serve files instead of the python built in web server
+                # when we're running inside docker because the python web server performs
+                # badly there. nginx is fine.


The python web server is fine some of the time but hangs for many seconds at a time. I suspect it'll do that on some Linux systems as well, but nginx runs just fine here.

nik9000 · 2019-01-29T20:41:14Z

build_docs.pl

+    listen 8000;
+    location / {
+      root $dir;
+      add_header 'Access-Control-Allow-Origin' '*';


These are copied from the python runner.

nik9000 · 2019-01-29T20:44:54Z

build_docs.sh

+# Build the docker image from stdin so we don't try to pack up everything in
+# this directory. It is huge and we don't need any of it in the image because
+# we'll mount it into the image on startup.
+docker image build -t elastic/docs_build - < "$DIR/Dockerfile"


Building the docker image right here feels good to me from a development background. It feels "like gradle but for native dependencies". I know the analogy isn't quite right, but it is pretty close in this case.

I like that we could do things like git bisect and get the docker image specified by the Dockerfile in the repo right now. I'd be quite happy with a caching strategy that makes it so we don't have to build the image locally all the time, but this feels like a simple solution for now.

nik9000 · 2019-01-29T20:45:38Z

build_docs.sh

+
+# Seccomp adds a *devestating* performance overhead if you happen
+# to have it installed.
+DOCKER_RUN_ARGS+=('--security-opt' 'seccomp=unconfined')


See the commit message for this one. I don't like it, but without it my laptop hates this change. I've spent a few days looking into it and I'm cutting my losses for now.

nik9000 · 2019-01-29T20:46:20Z

build_docs.sh

+# Running read-only with a proper tmp directory gives us a little
+# performance boost and it is simple enough to do.
+DOCKER_RUN_ARGS+=('--read-only')
+DOCKER_RUN_ARGS+=('--tmpfs' '/tmp')


It turns out it isn't that simple to do for nginx, but it isn't too bad and it feels good to know all the places where we write.

nik9000 · 2019-01-29T20:49:16Z

build_docs.sh

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


Any advice for writing a bash script that isn't terrible. I'm quite attached to the "iterate all the args and rewrite" method, but beyond that I have no opinions.

Um... write a Python script instead? Where does the imperative to use Bash come from? If it's deps, then Mac and Linux users have Bash and Python already. Windows users don't have either, so they are going to have to install something either way. Or, you could embrace the chaos and run the script in a container.

nik9000 · 2019-01-29T20:52:39Z

build_docs.sh

+      DOCKER_RUN_ARGS+=('-e' "SSH_AUTH_SOCK=$SSH_AUTH_SOCK")
+    fi
+    # Mount our known_hosts file into the VM so it won't ask about github
+    DOCKER_RUN_ARGS+=('-v' "$(to_absolute_path ~/.ssh/known_hosts):/tmp/.ssh/known_hosts:ro,cached")


I know we could use something like ssh-keyscan -t rsa github.com >> ~/.ssh/known_hosts to add github's key but this feels, like, slightly better. I feel like this trusts that the user cloned from github and accepted the host. I could copy it in I suppose rather than mount. I've kind of gone mount crazy here but lots of mounts is kind of required for a project like this.

nik9000 · 2019-01-29T21:17:04Z

One thing I didn't do in this PR is switch to using the asciidoctor gem in the docker image. We're just using ruby from the image. I'd like to save that for a followup PR because it is fairly mechanical and not actually interesting from a "docker" perspective.

Dockerfile

ghost · 2019-01-30T22:35:30Z

README.asciidoc

@@ -1080,7 +1058,7 @@ The local web browser can be stopped with `Ctrl-C`.
 ==== Custom Sense snippets



Here's another "Sense".

ghost · 2019-01-30T22:38:51Z

build_docs.pl

+    return 0 unless ( -e $root_cgroup );
+    open(my $root_cgroup_file, $root_cgroup);
+    return grep {/docker/} <$root_cgroup_file>;
+}


Yeah. I consider myself a "linguist programmer", as opposed to a "mathematical programmer" (yay, false dichotomies!), so postfix conditionals make me so happy. Ruby has them, but it's passing into the mists of time, too. 👋

build_docs.pl

ghost · 2019-01-30T22:47:06Z

build_docs.sh

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


Um... write a Python script instead? Where does the imperative to use Bash come from? If it's deps, then Mac and Linux users have Bash and Python already. Windows users don't have either, so they are going to have to install something either way. Or, you could embrace the chaos and run the script in a container.

build_docs.sh

Now if they python process we clean up!

nik9000 · 2019-02-02T02:24:55Z

@Jarpy I think this is ready for another round!

I say, and then 10 hours later I push 3 more patches.

ghost · 2019-02-04T23:12:14Z

README.asciidoc

@@ -54,7 +38,7 @@ this README as follows:
 [source,bash]
 ----------------------------
 cd docs/
-./build_docs.pl --doc README.asciidoc --open
+./build_docs.sh --doc README.asciidoc --open


There's no .sh any more, right?

nik9000 · 2019-02-04T23:34:28Z

The other two senses I'm not sure what they are! I'm worried that if I zap them it'll be a lie! I have to investigate them further.

…

On Mon, Feb 4, 2019, 18:18 Toby McLaughlin ***@***.*** wrote: ***@***.**** commented on this pull request. ------------------------------ In README.asciidoc <#564 (comment)>: > @@ -1027,13 +1005,13 @@ footnote to a particular line of code: ================================== [[sense-snippets]] -=== View in Sense +=== View in Console Grep tells me there are two more SENSEs lurking in here if you wanna smack 'em. $ grep -iFn sense README.asciidoc 1066: // SENSE: path/to/snippet.json 1082:// SENSE:one/two/three/example_1.json <1> — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#564 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AANLookOjqx0SfcXUX3wVElwA_m0JskDks5vKL_NgaJpZM4aY4ov> .

nik9000 · 2019-02-04T23:34:49Z

Good call!

…

On Mon, Feb 4, 2019, 18:12 Toby McLaughlin ***@***.*** wrote: ***@***.**** commented on this pull request. ------------------------------ In README.asciidoc <#564 (comment)>: > @@ -54,7 +38,7 @@ this README as follows: [source,bash] ---------------------------- cd docs/ -./build_docs.pl --doc README.asciidoc --open +./build_docs.sh --doc README.asciidoc --open ---------------------------- There's no .sh any more, right? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#564 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AANLouv1dazdshBwoVw8e10WynSlZXaIks5vKL4_gaJpZM4aY4ov> .

These might not be right but the others were probably wrong too.

nik9000 · 2019-02-05T01:05:12Z

@Jarpy could you have another look when you get a chance?

nik9000 · 2019-02-05T01:55:15Z

build_docs

@@ -0,0 +1,258 @@
+#!/usr/bin/env python2


oh no! mac doesn't like this shebang!

nik9000 · 2019-02-05T14:49:17Z

I talked to @Jarpy last night. The shebang issue is that last one that we could find. Sadly, it looks like the best way to solve this is to make the script compatible with any version of python which is a little funky, but I'll work on it today. It is the one thing in this repo that'll have to deal with "funny" versions. Everything else had docker to control it.

nik9000 · 2019-02-05T19:38:59Z

build_docs

+    # it needs to die.
+    docker_args.append('-i')
+
+    # NOCOMMIT handle "tell me who you are"!


Hmmm - this is a blocker! I didn't realize I hadn't got to it yet.

It is already fixed

nik9000 · 2019-02-05T21:47:24Z

I just tested the NOCOMMIT - it was a leftover. I'd already handled it. I was able to build docs locally with docker and push the changes. This might just work!

nik9000 · 2019-02-05T22:04:58Z

@Jarpy, could you give this another round today?

ghost

I say ship this bad baby.

ghost · 2019-02-06T05:03:07Z

build_docs

+
+
+def decode(bytes_or_str):
+    """Decode the result of reading from popen's stdout. In python 2 the


Sorry it came to this :(

nik9000 · 2019-02-06T14:14:01Z

I say ship this bad baby.

❤️! Thanks for all of your reviewing! It was a long road, but we're getting there!

nik9000 · 2019-02-06T15:42:31Z

Things are pretty busy in this repo right now so I'll hold on to the PR until things calm down.

nik9000 · 2019-02-06T17:58:00Z

OK! Things are looking calmer so I've merged this. Now you can use docker! I'll see about fixing ssh auth on mac now.

This adds support for building the docs using docker with the expectation without breaking support for building the docs *without* docker. My goal is to make it simpler for to add dependencies. In the long term this'll allow us to add support for automatically building diagrams and fancy mathematical notation. In the short term this'll make it simpler to run asciidoctor in CI. On macOS docker is a little slower than running outside of docker. At this point the overhead is about 20%. This isn't great, but Asciidoctor will more than make up the difference in the long run. And, in the short run at least, docker isn't reaquired. I'll make sure it is *never* required to build Asciidoc. One fairly questionable choice that this makes is to diable seccomp by default. The story goes like this: on linux, building the docs in docker is somewhere between *faster* than building them outside of docker and macOS's overhead. Unless you turn on seccomp. With seccomp enabled the docs take about twice as long to build in docker. Which is terrible! As much as I want security, this is terrible! I'd really love to turn it on but I can't defend that kind of hit.

nik9000 added 21 commits January 25, 2019 12:04

WIP

1a0b3b7

2x performance hit on mac

Bump osx performance

0541ba7

Switch http server in docker

656ae08

Mostly readme

f816179

smaller

89dec4f

WIP

0697ac4

Bump alpine version

e582ce7

Run as myself

0b7cba1

WIP

99e9bb8

Deb is faster....

c9f0bd4

WIP

39da578

Cleanup build

53847a0

Comments

a17f366

Minideb

4be5fb3

Much simpler

Fix --open

14d73f3

--all works now

89e4d33

Commit to debian

4fbf9cc

Support not specifying --out

b9c711a

Fix nss wrapper

1449fbe

Mount volumes readonly if possible

80bf9d3

Disable seccomp by default....

13b55e6

nik9000 requested review from a user and ddillinger January 29, 2019 20:36

Drop git requirement

67d1469

nik9000 commented Jan 29, 2019

View reviewed changes

ghost reviewed Jan 30, 2019

View reviewed changes

nik9000 added 3 commits January 31, 2019 07:20

Merge branch 'master' into dockerize_2

a4154fa

Indent dockerfile

6300453

Merge branch 'master' into dockerize_2

820d870

nik9000 added 3 commits February 1, 2019 16:35

Web browser and paranoia around boss dying

d81f738

Now if they python process we clean up!

fixup

9efb7bb

Fix git config

aea45cc

ghost reviewed Feb 4, 2019

View reviewed changes

nik9000 added 2 commits February 4, 2019 20:03

Not .sh

8c6c137

Fixup

f4ff9dc

These might not be right but the others were probably wrong too.

nik9000 commented Feb 5, 2019

View reviewed changes

build_docs Outdated

@@ -0,0 +1,258 @@

#!/usr/bin/env python2

Copy link

Member Author

nik9000 Feb 5, 2019

Choose a reason for hiding this comment

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

oh no! mac doesn't like this shebang!

nik9000 added 2 commits February 4, 2019 21:24

Fix error handling if docker is down

99a3cfb

Magic

169c22c

nik9000 added 4 commits February 5, 2019 09:53

WIP

c419bb5

Merge branch 'master' into dockerize_2

dfc3963

pep8

7c63c6b

Explain shebang

a7560c2

nik9000 commented Feb 5, 2019

View reviewed changes

nik9000 added 2 commits February 5, 2019 15:40

Merge branch 'master' into dockerize_2

024c56b

Drop nocommit

b204930

It is already fixed

Pitch ctrl-c

063065a

ghost approved these changes Feb 6, 2019

View reviewed changes

build_docs

def decode(bytes_or_str):

"""Decode the result of reading from popen's stdout. In python 2 the

Copy link

ghost Feb 6, 2019

Choose a reason for hiding this comment

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

Sorry it came to this :(

nik9000 merged commit 3b73603 into elastic:master Feb 6, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow running the build with docker #564

Allow running the build with docker #564

nik9000 commented Jan 29, 2019

nik9000 Jan 29, 2019

nik9000 Jan 29, 2019

nik9000 Jan 29, 2019

ghost Feb 4, 2019

nik9000 Jan 29, 2019

nik9000 Jan 29, 2019

nik9000 Jan 29, 2019

nik9000 Jan 30, 2019

nik9000 Jan 29, 2019

nik9000 Jan 29, 2019

nik9000 Jan 29, 2019

ghost Jan 30, 2019

nik9000 Jan 29, 2019

nik9000 commented Jan 29, 2019

ghost Jan 30, 2019

nik9000 Jan 31, 2019

ghost Jan 30, 2019

ghost Jan 30, 2019

nik9000 commented Feb 2, 2019

ghost Feb 4, 2019

nik9000 commented Feb 4, 2019 via email

nik9000 commented Feb 4, 2019 via email

nik9000 commented Feb 5, 2019

nik9000 Feb 5, 2019

nik9000 commented Feb 5, 2019

nik9000 Feb 5, 2019

nik9000 commented Feb 5, 2019

nik9000 commented Feb 5, 2019

ghost left a comment

ghost Feb 6, 2019

nik9000 commented Feb 6, 2019

nik9000 commented Feb 6, 2019

nik9000 commented Feb 6, 2019

		@@ -1080,7 +1058,7 @@ The local web browser can be stopped with `Ctrl-C`.
		==== Custom Sense snippets



		def decode(bytes_or_str):
		"""Decode the result of reading from popen's stdout. In python 2 the

Allow running the build with docker #564

Allow running the build with docker #564

Conversation

nik9000 commented Jan 29, 2019

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nik9000 commented Jan 29, 2019

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

nik9000 commented Feb 2, 2019

Choose a reason for hiding this comment

nik9000 commented Feb 4, 2019 via email

nik9000 commented Feb 4, 2019 via email

nik9000 commented Feb 5, 2019

Choose a reason for hiding this comment

nik9000 commented Feb 5, 2019

Choose a reason for hiding this comment

nik9000 commented Feb 5, 2019

nik9000 commented Feb 5, 2019

ghost left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nik9000 commented Feb 6, 2019

nik9000 commented Feb 6, 2019

nik9000 commented Feb 6, 2019