Skip to content

Conversation

@witoff
Copy link
Contributor

@witoff witoff commented May 22, 2017

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

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

@AlCutter AlCutter left a comment

Choose a reason for hiding this comment

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

Just nits :)

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

Choose a reason for hiding this comment

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

envars?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

(just fixed)

Copy link
Contributor

Choose a reason for hiding this comment

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

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

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" \
Copy link
Member

Choose a reason for hiding this comment

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

funny indent here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

DUMP_METRICS=0s \
FORCE_MASTER=true


Copy link
Member

Choose a reason for hiding this comment

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

can remove this extra blank line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

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};"
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

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';"
Copy link
Contributor

Choose a reason for hiding this comment

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

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_".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

FROM golang:1.8

RUN apt-get update
RUN apt-get install -y mysql-client
Copy link
Contributor

Choose a reason for hiding this comment

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

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.


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

Choose a reason for hiding this comment

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

This comment is no longer true.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed: comment is now removed.

@witoff
Copy link
Contributor Author

witoff commented Jun 16, 2017

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

@witoff witoff closed this Jun 16, 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.

4 participants