Add Flexible Flexible ResetDB and Docker Support by witoff · Pull Request #631 · google/trillian

witoff · 2017-05-22T03:15:14Z

Problem
Trillian has a number of hardcoded assumptions that DB and Trillian are running on the same host. This is problematic running most cloud environments and in most configurations of docker.

Updates

This PR adds Docker support, cherrypicked from @gdbelvin's work in 607. I'd prefer that his PR is merged first and I rebase on his work. Even though that PR is recent (10 days), it's out of date enough that this PR was easier to cherrypick in directly.

ResetDB is generalized to run on any host

./resetdb.sh --verbose --force -h localhost

Usage

# Build
docker build -t trillian-server -f server/trillian_log_server/Dockerfile .
docker build -t trillian-signer -f server/trillian_log_signer/Dockerfile .
docker build -t trillian-db-client -f server/trillian_db_client/Dockerfile .

# Run
docker run \
  --name mysql \
  -p 3306:3306 \
  -e MYSQL_ROOT_PASSWORD=password123 \
  mysql

# Seed the DB
docker run --rm -it \
  --link mysql \
  -e DB_USER=root \
  -e DB_PASSWORD=password123 \
  trillian-db-client ./scripts/resetdb.sh --verbose --force -h mysql

# Startup the Server
docker run --rm -it \
  --link mysql \
  -p 8091:8091 \
  -p 8090:8090 \
  -e DB_USER=root \
  -e DB_PASSWORD=password123 \
  -e DB_HOST=mysql:3306 \
  trillian-server

AlCutter

Just nits :)

AlCutter · 2017-05-22T10:41:23Z

scripts/resetdb.sh

+  # set unset envars to defaults
+  [ -z ${DB_USER+x} ] && DB_USER="root"
+  [ -z ${DB_NAME+x} ] && DB_NAME="test"
+  # format reused supplied envas


environment variables. i'm so used to calling them envars in other places i never considered writing the whole word :)

(just fixed)

I think "envars" would have been ok; Al was highlighting the typo that was here I think ("envas").

AlCutter · 2017-05-22T10:44:55Z

server/trillian_log_server/Dockerfile

+ENTRYPOINT /go/bin/trillian_log_server \
+	--mysql_uri="${DB_USER}:${DB_PASSWORD}@tcp(${DB_HOST})/${DB_DATABASE}" \
+	--rpc_endpoint="$RPC_HOST:$RPC_PORT" \
+  --http_endpoint="$HTTP_HOST:$HTTP_PORT" \


funny indent here

eek, tabs! still configuring IDE on new machine. fixed.

AlCutter · 2017-05-22T10:47:42Z

server/trillian_log_signer/Dockerfile

+    DUMP_METRICS=0s \
+    FORCE_MASTER=true
+
+


can remove this extra blank line

RJPercival · 2017-05-22T16:23:29Z

scripts/resetdb.sh

+  then
+      # A command line supplied -u will override the first argument.
+      echo "Resetting DB..."
+      mysql -u $DB_USER $FLAGS -e "DROP DATABASE IF EXISTS ${DB_NAME};"


Some mysql flags (e.g. --defaults-extra-file) need to come first. Putting -u $DB_USER first will prevent them being used.

changed order to: mysql $FLAGS -u $DB_USER -e "DROP DATABASE IF EXISTS ${DB_NAME};"

RJPercival · 2017-05-22T16:25:50Z

scripts/resetdb.sh

+      echo "Resetting DB..."
+      mysql -u $DB_USER $FLAGS -e "DROP DATABASE IF EXISTS ${DB_NAME};"
+      mysql -u $DB_USER $FLAGS -e "CREATE DATABASE ${DB_NAME};"
+      mysql -u $DB_USER $FLAGS -e "GRANT ALL ON ${DB_NAME}.* TO '${DB_NAME}'@'localhost' IDENTIFIED BY 'zaphod';"


You're going to want to allow access to the Trillian database from hosts other than localhost, I expect. I think this only works at the moment because MySQL servers grant unrestricted access by default to any table named "test" or starting with "test_".

yeah, i'm not yet sure if/how these permissions are being used. the function of this line is unchanged from what's currently in maser so i'd prefer to merge this PR as a refactor of hosts, and fix the grants in a future PR if a problem is identified.

for the purposes of this PR, the seeding works against external DBs (e.g. aurora) when used with the root user that's currently recommended.

If the database isn't called "test", can you then connect to that database as a user named ${DB_NAME} from a machine other than localhost? I'm ok with fixing this as a separate PR, but adding a TODO here and/or creating an issue to track it would be helpful.

RJPercival · 2017-05-22T16:28:47Z

server/trillian_db_client/Dockerfile

+FROM golang:1.8
+
+RUN apt-get update
+RUN apt-get install -y mysql-client


It is important to do apt-get update and apt-get install as a single RUN command, otherwise you can get screwed over by Docker layer caching. See https://docs.docker.com/engine/userguide/eng-image/dockerfile_best-practices/#run.

RJPercival · 2017-05-23T09:48:38Z

scripts/resetdb.sh

+
+  if [ -z ${REPLY+x} ] || [[ $REPLY =~ ^[Yy]$ ]]
+  then
+      # A command line supplied -u will override the first argument.


This comment is no longer true.

fixed: comment is now removed.

witoff · 2017-06-16T05:44:23Z

incorporated into #667, but please comment if you'd rather merge separately.

witoff added 2 commits May 21, 2017 20:00

Should have a dockerized DB Client with sourcecode and mysql DB ops

cd59237

DB operations should be flexibile to run against many environments

47d3b81

googlebot added cla: yes labels May 22, 2017

witoff force-pushed the witoff/deploy branch 2 times, most recently from 80f0149 to f09ef41 Compare May 22, 2017 03:44

AlCutter approved these changes May 22, 2017

View reviewed changes

RJPercival reviewed May 22, 2017

View reviewed changes

Dockerize core components

ac45391

witoff force-pushed the witoff/deploy branch from f09ef41 to ac45391 Compare May 22, 2017 17:57

RJPercival reviewed May 23, 2017

View reviewed changes

witoff and others added 2 commits June 7, 2017 21:44

Merge branch 'master' into witoff/deploy

e90d162

remove deprecated comment

893a1a7

witoff mentioned this pull request Jun 8, 2017

Automate Local and AWS Deployments #667

Merged

Merge branch 'master' into witoff/deploy

0a143d1

witoff closed this Jun 16, 2017

Comments

Conversation

witoff commented May 22, 2017

Uh oh!

AlCutter left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

witoff commented Jun 16, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants