-
Notifications
You must be signed in to change notification settings - Fork 26
Configure the Production environment #218
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@@ -145,6 +145,10 @@ CAT_OAUTH_RESTRICT_ACCESS=org:my-organization | |||
CAT_OAUTH_RESTRICT_REQUESTER_ACCESS=team:123456 | |||
``` | |||
|
|||
## Deployment | |||
|
|||
This site is deployed in `production` and in `staging` following our [web application deployment workflow](https://github.com/src-d/guide/blob/master/engineering/continuous-delivery.md) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is something important - we should think (and document it as well) that it's not a web site or a service, it's an application. This has consequences on many levels, including the way structure the release, package&distribute artifacts, monitor it, etc.
Ofcourse as a web application, it can be deployed by different people in different scenarios on different environments (and that's what we aiming for), but I think it's worth highlighting it here and making a clear distinction between general expectations and sourced-specific things.
How about
- changing the title from
Deployment
->source{d} internal deployment
- changing
site
->application
production and staging sourced{d} environments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just copied the code from Blog and Landing but yes: your proposals are even better and they could be also applied there.
I'll proceed with your suggestions before merging, Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 LGTM, sans minor doc issue, highlighted above.
@@ -50,7 +50,7 @@ spec: | |||
name: {{ $secretName }} | |||
key: jwt_signing_key | |||
- name: CAT_GA_TRACKING_ID | |||
value: "{{ .Values.deployment.gaTrackingID }}" | |||
value: "{{ required "gaTrackingID is required" .Values.deployment.gaTrackingID }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found two different ways to write it:
wrapping the value with quotes:
image: "{{ .Values.image.repository }}:{{ required "Image tag is required" .Values.image.tag }}"
leaving the value without wrapper quotes:
- host: {{ required "Hostname is missing" .Values.ingress.hostname }}
@rporres should I use one style for this CAT_GA_TRACKING_ID
env value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All env variables must be strings, so they must be quoted to avoid YAML interpreting as numbers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
couple of minor questions, looks good
sorry for the delay, I've been busy these days
@@ -2,7 +2,7 @@ workspace: | |||
base: /go | |||
path: src/github.com/src-d/code-annotation | |||
|
|||
branches: [master, staging, release/*] | |||
branches: [master, staging] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why have you removed release/*
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was copied from Landing .drone.yml
, that run tests, and triggers the deploy;
In Landing (and blog and talks), tests are also run on release/*
branches.
If we want to reproduce ☝️ this behavior, it should be done in .travis.yml
file
(because in CAT tests are run from Travis instead of from Drone) but it is out of the scope of this PR (and we didn't consider it yet for this app)
Does it answer your question @rporres ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure! :)
required by #215
This PR prepares the Prod env.
Changes done:
gaTrackingID
must be passed by drone.io when deployingGA_TRACKING_ID
for testing purposesOther changes that should be done by @src-d/infrastructure just after merging this PR: