-
Notifications
You must be signed in to change notification settings - Fork 40
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
ci: Add workflow that builds DefraDB AMI upon tag push #1304
Conversation
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.
question: Should this file not be excluded from the repo? (I have no idea what best practice is with these, but is common enough to do with similar)
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.
No, it should stay.
More context: https://developer.hashicorp.com/terraform/language/files/dependency-lock#lock-file-location
"
Lock File Location
The dependency lock file is a file that belongs to the configuration as a whole, rather than to each separate module in the configuration. For that reason Terraform creates it and expects to find it in your current working directory when you run Terraform, which is also the directory containing the .tf files for the root module of your configuration.
...
Terraform automatically creates or updates the dependency lock file each time you run the terraform init command. You should include this file in your version control repository so that you can discuss potential changes to your external dependencies via code review, just as you would discuss potential changes to your configuration itself.
"
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.
cheers for the explanation! :)
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'm approving this with the caveat that I'm not an expert and if something was wrongly configured I'm not sure I would spot it. I do have a few questions however.
Maybe give others the day to chime in in case they know more than I do.
# DEV BACKEND CONFIG SETTINGS | ||
#################################################################################### | ||
# S3 backend | ||
bucket = "source-tfstate-dev" # REPLACE WITH YOUR BUCKET NAME |
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.
question: should the comment still be there?
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.
Yes, due to (5) from PR description. Don't want to lose any help / hint until then.
# DEV BACKEND CONFIG SETTINGS | ||
#################################################################################### | ||
# S3 backend | ||
bucket = "source-tfstate-prod" # REPLACE WITH YOUR BUCKET NAME |
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.
question: should the comment still be there?
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.
Yes, due to (5) from PR description. Don't want to lose any help / hint until then.
# TEST BACKEND CONFIG SETTINGS | ||
#################################################################################### | ||
# S3 backend | ||
bucket = "source-tfstate-test" # REPLACE WITH YOUR BUCKET NAME |
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.
question: should the comment still be there?
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.
Yes, due to (5) from PR description. Don't want to lose any help / hint until then.
|
||
env: | ||
PACKER_LOG: 1 | ||
# RELEASE_VERSION: v0.4.0 |
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.
question: Should this line be uncommented or removed?
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.
Would rather leave it in, is there for testing manually.
|
||
- name: Environment version target | ||
run: echo "RELEASE_VERSION=${GITHUB_REF#refs/*/}" >> $GITHUB_ENV | ||
# run: echo ${{ env.RELEASE_VERSION }} |
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.
question: Should this line be uncommented or removed?
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.
Would rather leave it in, is there for testing manually.
I've skimmed over it, but I have zero experience with terraform, limited and out of date aws experience, and only a very vague notion of what this is for :) |
No worries @AndrewSisley was in a similar boat a couple of weeks back haha. Thanks for taking a look at it nonetheless =D |
8c2bc54
to
3b5dfac
Compare
Ensured the action passes and makes a new AMI, that also shows up on AWS console.
Codecov Report
@@ Coverage Diff @@
## develop #1304 +/- ##
========================================
Coverage 70.47% 70.47%
========================================
Files 184 184
Lines 17823 17823
========================================
Hits 12561 12561
Misses 4310 4310
Partials 952 952 |
- Resolves #1241 - Description: Enables an automated creation of a DefraDB AMI whenever a tag is pushed.
…#1304) - Resolves sourcenetwork#1241 - Description: Enables an automated creation of a DefraDB AMI whenever a tag is pushed.
Relevant issue(s)
Resolves #1241
Description
Move the action from: https://github.com/sourcenetwork/defradb-cloud inside this repo as it was intended.
Enables an automated creation of a defradb AMI whenever a tag is pushed.
The action won't be visible in the PR as it is to only be triggered on tag push.
However I did test it in this PR and the triggered CI action run can be found here: https://github.com/sourcenetwork/defradb/actions/runs/4631156601/jobs/8193666739?pr=1304
Note:
(1) I didn't write everything included in the pr, this was an outcome of our partnership with AWS 3rd team that was helping us.
(2) The packer workflow (i.e. ami creation) uses and might seem familiar to: https://github.com/sourcenetwork/defradb/blob/develop/tools/scripts/deploy_defradb.sh
(3) The creation of ami was tested and works properly.
(4) I spotted some bugs in the terraform workflow (mostly how the secret was being handled, which is now fixed).
(5) Due to (4) not as confident that the deployment of an image to spin up an ec2 instance will go smoothly as the workflow currently stands (I would not worry about that yet, as we can fix it when we get approved on marketplace and can actually test it end-to-end).
NEED FEEDBACK:
v0.5.1
, but won't run onv0.5
orv0.4.1-alpha
Tasks
How has this been tested?
Specify the platform(s) on which this was tested: