Skip to content

Commit

Permalink
Self-Test: Run nginx (#699)
Browse files Browse the repository at this point in the history
Adds a self-test that runs nginx and doesn't basic sanity checks on the
files it serves. This is nice to make sure that `--open` works but
*also* it should serve as another line of defence against making invalid
nginx config files. This is important as those nginx config files will
be used on the web site and we'd really prefer not to bring that down.

To get this to work consistently I have to drop the "don't run twice"
behavior of `build_docs.pl`. That behavior is implemented by checking
for a pidfile on startup. If the file is there it checks if `ps` for
that pid comes back with a process that looks like the running one. When
the perl process terminates naturally it destroys the file. This is
known to cause trouble when a server loses power and can't delete the
file and the process is started on startup. Startup is fairly
predictable and it is common that the process would end up with the same
pid so the `ps` check will *think* that the new process collides with
itself.

Pulling a server's power from the wall is *fairly* similar to what
happens when the docker process that runs the `--self-test` comes to the
end and destroys that docker image. This hasn't come up before because
`build_docs.pl` always terminates fairly quickly. It doesn't when you do
`--open`. I tried to get something that consistently removed the pidfile
but such a thing is complex in make, I think. So I just removed it.

"Don't run twice" wasn't really buying us much anyway. At this point we
run `build_docs` on immutable infrastructure in CI and Jenkins manages
whether or not to run the build concurrently. We have much more precise
control there which is what we want anyway. It might have some value
during `--self-test` to make sure that we don't leave `build_docs.pl`
running between tests. At this point we shoot it if the test is
successful and let it die if the test fails. Which isn't *great* but it
should work.
  • Loading branch information
nik9000 authored Mar 15, 2019
1 parent dae2c93 commit 8139b96
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 5 deletions.
3 changes: 0 additions & 3 deletions build_docs.pl
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@ BEGIN
}

use lib 'lib';
use Proc::PID::File;
die "$0 already running\n"
if Proc::PID::File->running( dir => '.run' );

use ES::Util qw(
run $Opts
Expand Down
22 changes: 20 additions & 2 deletions integtest/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ check: \
relative_conf_file \
keep_hash \
sub_dir \
keep_hash_and_sub_dir
keep_hash_and_sub_dir \
open_all

.PHONY: style
style: html_diff
Expand Down Expand Up @@ -234,6 +235,23 @@ keep_hash_and_sub_dir:
git clone $(TMP)/dest.git $(TMP)/dest
$(call GREP,'extra extra extra',$(TMP)/dest/html/test/current/_chapter.html)

.PHONY: open_all
open_all:
# Test that `--all --open` starts nginx in a usable way.
rm -rf $(TMP)
$(SETUP_MINIMAL_ALL)
/docs_build/build_docs.pl --in_standard_docker --all \
--target_repo $(TMP)/dest.git \
--conf $(TMP)/conf.yaml --open | tee $(TMP)/out &
curl -I --retry 10 --retry-connrefused http://127.0.0.1:8000/guide/
mkdir $(TMP)/guide
curl -sS http://127.0.0.1:8000/guide/ > $(TMP)/guide/index.html
$(call GREP,'Test book',$(TMP)/guide/index.html)
curl -sSI http://127.0.0.1:8000/guide/reference/setup > $(TMP)/guide/rdir
$(call GREP,'301 Moved Permanently',$(TMP)/guide/rdir)
$(call GREP,'Location: http://localhost:8000/guide/en/elasticsearch/reference/current/setup.html',$(TMP)/guide/rdir)
kill -QUIT $$(cat /run/nginx/nginx.pid)

define GREP=
# grep for a string in a file, outputting the whole file if there isn't
# a match.
Expand All @@ -258,7 +276,7 @@ define SETUP_MINIMAL_ALL=
# the filesystem just fine.
git init --bare $(TMP)/dest.git

# Actually build the docs
# Setup the config
sed 's|--tmp--|$(TMP)|' small_conf.yaml > $(TMP)/conf.yaml
endef

Expand Down

0 comments on commit 8139b96

Please sign in to comment.