Skip to content
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

Deploy release branches to staging ECS environment #4758

Merged
merged 9 commits into from
Nov 5, 2018

Conversation

griggheo
Copy link
Contributor

Type: feature

Issue

We want to be able to test code in a release branch in a staging environment.

Solution

This PR uses the latest version of the propel CLI tool and a single propel.yaml manifest file to deploy the Docker image built in CircleCI to a staging environment in ECS. The data set used for the Reaction service is the 'dev' dataset hosted on a MongoDB server running on an EC2 instance in the staging environment.

Testing

@griggheo griggheo requested review from spencern and aldeed October 18, 2018 23:09
mkdir -p ~/.aws
echo "[default]" > ~/.aws/credentials
echo "aws_access_key_id = ${AWS_ACCESS_KEY_ID}" >> ~/.aws/credentials
echo "aws_secret_access_key = ${AWS_SECRET_ACCESS_KEY}" >> ~/.aws/credentials
Copy link
Contributor

Choose a reason for hiding this comment

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

@griggheo Just curious, why write these to a file when AWS lib typically already looks for env variables with these names? Seems more secure to avoid any credentials in files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It didn't work for me when I specified the AWS keys without actually writing them to the credentials file. I'll try again. Agreed it's better not to save them to disk.

- container_port: 3000
host_port: 3000
image: reactioncommerce/reaction
image_tag: release-2.0.0-rc.5
Copy link
Contributor

Choose a reason for hiding this comment

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

So this will need to be updated every time we make a new release branch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The image_tag is used only if you deploy using propel from the command line. CircleCI deployments will automatically use the SHA1 of the newly built Docker image.

Copy link
Contributor

@aldeed aldeed left a comment

Choose a reason for hiding this comment

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

See some comments and questions

@@ -2,7 +2,7 @@
.fileStorage/
.vscode
.idea
.env
.env*
Copy link
Contributor

Choose a reason for hiding this comment

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

There's an open issue to add a .env file for this repo for local development, and that will involve creating a file named .env.example which will need to be committed (same pattern as in other repos). Can you change this ignore rule so that .env.example will be committed when we add that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do.

# only: /^release-2\.0.\d+$/
filters:
branches:
only: /^release*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Were we going to do /^release-2*/ instead? There is currently a release-1.17.0 branch and could at some point be release-1.17.1. Should branches like that overwrite staging deployment?

cc @spencern

Copy link
Contributor

Choose a reason for hiding this comment

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

@aldeed @griggheo Good catch, I don't think we should overwrite the staging branch with 1.x releases. I think we can probably get by without having a staging branch for 1.x as we're putting minimal development effort towards it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK I'll change it to /^release-2.*/ for now.

- name: ROOT_URL
- name: HYDRA_ADMIN_URL
- name: HYDRA_OAUTH2_INTROSPECT_URL
- name: SKIP_FIXTURES
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this list need to change when we add env variables? Where do the values come from? Maybe we can make this more straightforward when we do #4706

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Those are there for informational purposes only. I can remove them once we have the .env.example file.

@spencern
Copy link
Contributor

Seems to work - I'm seeing an issue where prices on the grid are displaying without a currency symbol, but I doubt that is unique to this deployment.

image

@griggheo
Copy link
Contributor Author

@aldeed I think I've addressed all your requests. Please take a look when you have a chance. Thanks!

@aldeed
Copy link
Contributor

aldeed commented Oct 29, 2018

This LGTM if you approve, @spencern. The app tests failed with a strange error, so I reran them. If they fail again, may need to investigate whether there is something in this PR that is causing it.

@spencern spencern merged commit 09d6420 into release-2.0.0-rc.6 Nov 5, 2018
@spencern spencern deleted the propel-ecs-multiple-containers branch November 5, 2018 16:56
@spencern spencern mentioned this pull request Nov 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants