Complete instructions to deploy Tessera GCP #86
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #86 +/- ##
==========================================
- Coverage 35.80% 34.11% -1.69%
==========================================
Files 16 16
Lines 1363 1407 +44
==========================================
- Hits 488 480 -8
- Misses 801 855 +54
+ Partials 74 72 -2 ☔ View full report in Codecov by Sentry. |
deployment/README.md
Outdated
| @@ -20,6 +20,18 @@ the project: | |||
| gcloud auth application-default login | |||
| ``` | |||
|
|
|||
| Then, specify your Google Cloud project ID: | |||
There was a problem hiding this comment.
This GCP specific stuff should probably go into a README in the live/example-gcp directory (my fault - I accidentally put the gcloud line in above)
deployment/README.md
Outdated
| @@ -20,6 +20,18 @@ the project: | |||
| gcloud auth application-default login | |||
| ``` | |||
|
|
|||
| Then, specify your Google Cloud project ID: | |||
| ```bash | |||
| export GOOGLE_PROJECT={VALUE} | |||
There was a problem hiding this comment.
Could use this:
export GOOGLE_PROJECT=$(gcloud config get-value project 2> /dev/null)There was a problem hiding this comment.
Maybe that works if you run this command after you've ran gcloud auth from a VM inside a project, but that might not always be the case. Otherwise, how can gcloud know which project it should select? When you authenticate via a web link, it doesn't give you the option to select the project.
deployment/README.md
Outdated
| Eventually, customize the region (defaults to "us-east1"), and bucket name prefix | ||
| (defaults to "tessera"): | ||
| ```bash | ||
| export GOOGLE_REGION={VALUE} |
There was a problem hiding this comment.
It feels a bit weird to me to have the behaviour of terragrunt be affected by environment vars (I think it's a niggle from "hermetic config as code & checked in" being violated), but:
a) I'm far from a TF/TG expert, and
b) I suppose this is meant to be an easily runnable example and having folks edit config files in a repo is a bit of an anti pattern.
phbnf
force-pushed
the
gcloudinstructions
branch
from
922d679 to
739081d
Compare
|
|
||
| Then, specify your Google Cloud project ID: | ||
| ```bash | ||
| export GOOGLE_PROJECT={VALUE} |
There was a problem hiding this comment.
What's the advantage of doing this with environment variables? It moves away from the the deployment files being fully declarative, which means that two people can check out the same repo and then create different deployments because of inconsistencies in the env naming. It's also more work each time you set up a shell to get back to the same state. The obvious workaround is to create a shell script that sets these values, but then you've just put them in a file, which this change moves away from.
There was a problem hiding this comment.
Huh, I just saw Al's comments which only appear at the overview screen and not on the "files changed" view. Looks like these comments are somewhat redundant!
There was a problem hiding this comment.
Moving the conversation here since I looked at this with Al yesterday, and the goal was specifically to get your opinion on it given that you've spent a lot of time looking at Terraform already.
which means that two people can check out the same repo and then create different deployments because of inconsistencies in the env naming.: I think this is a feature, I have my own dev GCP project and I'd like to deploy tessera there. Alternatively, I might want to play around with a deployment in the trillian-tessera project with a CT log, without impacting other folks working on the current existing deployment.
The current setup is already affected by environment variables, the deployment files as they are do not work without setting GOOGLE_PROJECT:
╷
│ Error: Failed to retrieve project, pid: , err: project: required field is not set
│
│ with google_spanner_instance.log_spanner,
│ on main.tf line 52, in resource "google_spanner_instance" "log_spanner":
│ 52: resource "google_spanner_instance" "log_spanner" {
This PR doesn't change this behavior, but it's a improvement because now you only have to set your project ID once, and not both in GOOGLE_PROJECT and in terragrunt.hcl. I think it's the smallest change we can make to allow to:
- fix the current deployment setup
- have customizations in a single place
- allow easy customizations for folks no using the default values in this file
I tried two alternatives before sending this PR:
- not using environment variables, in which case
modules/gcs/main.tfneeds to be edited to specify the provider and project (which can be passed from terragrunt.hcl). That would allow to have all the config declarative, but folks would have to make sure not to add this file edits to their commits. Al wasn't supper keen on modifyingmodules/gcs/main.tfnor on making folks edit a config file for running quick examples. - using non "reserved" environment variable instead of GOOGLE_PROJECT and GOOGLE_REGION. I thought I'd stick to the defaults ones before customizing things.
I think that another alternative is to use terragrunt to generate the provider config, either with or without making use of environment variables.
What do you recommend?
| project_id = "trillian-tessera" | ||
| location = "us-central1" | ||
| base_name = "example-gcp" | ||
| project_id = get_env("GOOGLE_PROJECT", "trillian-example") |
There was a problem hiding this comment.
Is this supposed to be trillian-tessera to match the old version? I think this name is particularly confusing given we have a repo with (almost) this name which is for the old trillian!
phbnf
force-pushed
the
gcloudinstructions
branch
3 times, most recently
from
6003ce4 to
11a3f9c
Compare
phbnf
force-pushed
the
gcloudinstructions
branch
from
11a3f9c to
57bfc12
Compare
This PR also changes the base name from "example-gcp" to "trillian-tessera" since I think it makes more sense as a default value. I'd rather fix this sooner rather than later, but happy to revert this change if you think it's too disruptive.