Skip to content
This repository was archived by the owner on Oct 11, 2024. It is now read-only.

Simplify, cleanup and update docker files - #763

Merged
gdbelvin merged 5 commits into
google:masterfrom
liamsi:docker_update
Aug 18, 2017
Merged

gdbelvin merged 5 commits into
google:masterfrom
liamsi:docker_update

Conversation

@liamsi

@liamsi liamsi commented Aug 17, 2017 •

Copy link
Copy Markdown
Contributor

Changes:

  • simplify docker files to be "unopinionated"/containing only the bare minimum flags to run them locally
  • remove obsolete libtool dependency (faster builds) 🎉
  • update some defaults

It could make sense to wait for #762 to be merged first (removes --log-key flag).

Tested locally. Travis fails because of some dependency in vendored code in trillian:
https://travis-ci.org/google/keytransparency/builds/265725916#L639-L640

Fixed with google/trillian@a1b0c2d and google/trillian@b144701

closes #712

@liamsi liamsi added the WIP label Aug 17, 2017
@codecov-io

codecov-io commented Aug 17, 2017 •

Copy link
Copy Markdown

Codecov Report

Merging #763 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #763   +/-   ##
=======================================
  Coverage   48.42%   48.42%           
=======================================
  Files          28       28           
  Lines        2480     2480           
=======================================
  Hits         1201     1201           
  Misses       1093     1093           
  Partials      186      186

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bef78fc...26a5a8b. Read the comment docs.

@liamsi liamsi removed the WIP label Aug 17, 2017
@liamsi liamsi changed the title WIP: simplify and update docker files Simplify, cleanup and update docker files Aug 17, 2017
@liamsi
liamsi requested a review from gdbelvin August 17, 2017 22:58
RUN apt-get update && apt-get install -y libtool libltdl-dev
RUN go get -tags="mysql" ./cmd/keytransparency-server

ENTRYPOINT /go/bin/keytransparency-server \

@gdbelvin gdbelvin Aug 18, 2017 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We need to set an entry point. Otherwise, it's not possible to add additional arguments via the docker command line.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

docker-compose and kubernetes work though. I'll add an ENTRYPOINT in case someone wants to use the images with any these. Thanks

Comment thread docker-compose.yml Outdated
- --map-id=$MAP_ID
- --map-url=trillian-map:8090
- --vrf=/kt/vrf-key.pem
- --key=/kt/server.key

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would you mind renaming this flag and the following to tls-key and tls-cert ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

@gdbelvin gdbelvin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do the Kube scripts need to be updated as well?

gdbelvin added a commit to gdbelvin/keytransparency that referenced this pull request Aug 18, 2017
Cleans up a few off by on errors and poor stopping condition logic in
the list history command.

Fixes google#763
@gdbelvin gdbelvin closed this in bef78fc Aug 18, 2017
@gdbelvin gdbelvin reopened this Aug 18, 2017
@liamsi

liamsi commented Aug 18, 2017

Copy link
Copy Markdown
Contributor Author

I've updated the kube scripts as well. Will rebase. Then this is ready for another review.

liamsi added 2 commits August 18, 2017 12:33
# Conflicts:
#	cmd/keytransparency-signer/Dockerfile
#	cmd/keytransparency-signer/main.go
#	docker-compose.yml

@gdbelvin gdbelvin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks!

@gdbelvin
gdbelvin merged commit eb7ec67 into google:master Aug 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update Dockerfiles

3 participants