-
Notifications
You must be signed in to change notification settings - Fork 1
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
Automate deployment #163
base: main
Are you sure you want to change the base?
Automate deployment #163
Conversation
…increase the target_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.
Great start, but I'd recommend doing some cleanup and tailoring before this is ready to go.
tags: | | ||
ghcr.io/${{ env.REPO }}/query-connector:main, ghcr.io/${{ env.REPO }}/query-connector:latest | ||
|
||
# - name: Output image digest |
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.
Is this commented code needed? If not, consider deleting.
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.
Done.
context: ./query-connector | ||
push: true | ||
tags: | | ||
ghcr.io/${{ env.REPO }}/query-connector:main, ghcr.io/${{ env.REPO }}/query-connector:latest |
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.
We're going to need some sort of hash or semantic versioning to be added here. main
and latest
will not reliably trigger cloud platforms to pull and deploy a new image if one already exists with these tags.
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.
In addition, does env.REPO
contain the CDCgov
part of your tag? Again, I recommend simplifying this.
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.
For triggering the new image to be pulled and deployed, the up_helm_chart job triggers the workflow in phdi-charts to create a PR that increases the chart version. I linked the PR that adds this workflow in phdi-charts. Then the workflow waits until this step is completed before attempting to pull the new image and deploy it in the build and push step.
I'm not oppose to adding the semantic versioning or a hash at the end. Am I right in assuming we would not have to increase the chart version if we went this route?
For the env.REPO, it returns cdcgov/dibbs-query-connector
which evaluates to the full path ghcr.io/cdcgov/dibbs-query-connector/query-connector:latest`, this links to the path configured in phdi-charts
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.
name: Poll for status of workflow | ||
description: This action will wait and poll for the status of workflow | ||
|
||
# on: |
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.
More code comments. Clean these up, please.
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 code is no longer being used due to the built in github keyword needs
being used in its placed. Removed action from code.
if: github.event_name == 'push' | ||
id: version | ||
run: | | ||
# Check if there's an existing version file; if not, start from 1.0.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.
I wouldn't recommend doing things this way. If the version file doesn't exist, I would recommend failing the deployment outright.
Reason: Assume someone has deleted the version file in their commit on accident. You run this, and create a new 1.0.0 image. You're actually on 5.0.6. You've just overwritten your archived 1.0.0 image that someone in production may have been depending on.
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.
That makes sense, I modified the bash script to add an error message and fail the step if the version file does not exist.
env: | ||
REPO: ${{ github.repository }} | ||
run: | | ||
echo "REPO=${GITHUB_REPOSITORY,,}" >> $GITHUB_ENV |
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! Any particular reason why we opted to make this into a variable? Theoretically, this should never change. This seems like it's a little overkill.
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 assuming whomever initially created the CD workflow added it so it would align with the value configured in phdi-charts.
I can modify it and create a PR in phdi-charts to change this as well
@@ -0,0 +1,58 @@ | |||
name: Poll for status of workflow | |||
description: This action will wait and poll for the status of workflow |
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.
Can you fill me in on what the purpose of this is, or why it's needed? Is this for checking the status of a remote workflow in another repo? Also, are there potential issues with this approach if the scope of the PAT changes?
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 code is no longer being used due to the built in github keyword needs
being used in its placed. Removed action from code.
if: github.event_name == 'push' | ||
id: version | ||
run: | | ||
# Check if there's an existing version file; if not, start from 1.0.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.
See comments below regarding version files.
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.
Modified the bash script to add an error message and fail the step if the version file does not exist.
.github/workflows/full_cd.yaml
Outdated
push: | ||
branches: | ||
- main | ||
- shanice/automated-workflow-deployment |
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.
Is your personal branch intended to be here?
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.
Used for testing purposes, removed.
PULL REQUEST
Summary
The full_cd.yaml file creates a workflow the ties together all of the actions and workflows to automate the deployment of the new image created by this repository to EKS. See the workflow details below:
Related Issue
Fixes # Automates most of the manual steps that were required in order to deploy a new image to the demo site.
Additional Information
Original Deployment
Note: Manual means the developer has to go to Github actions to trigger the workflow
Current Deployment with this PR
Note: This deployment would involve changing the individual workflows in the current deployment into actions that will be compiled together into one workflow and wait until the previous action is complete before moving to the next action.
Interdependent on the following PR:
- phdi-charts
Checklist