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

Add CD with github actions #704

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

brunoparma88
Copy link
Contributor

Board:


Description:

Why:

To streamline app deployment, enhancing team efficiency. Utilizing GitHub Actions centralizes processes, aiding devs familiar with the repo.

Documentation:

GitHub CD Documentation

.github/workflows/cd.yml Outdated Show resolved Hide resolved
Comment on lines 3 to 6
on:
push:
branches:
- main
Copy link
Member

Choose a reason for hiding this comment

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

maybe if we comment this part the job doesn't run by default (what we want) and in the docs we can clarify that they need to uncomment it if they are willing to deploy to AWS?

Copy link
Member

Choose a reason for hiding this comment

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

Similar to what we did here:

on:
workflow_dispatch:
# This workflow should be scheduled at certain intervals
# schedule:
# - cron: '0 0 1 */3 *'
# The above cron would schedule it every 3 months. Replace with the wanted periodicity.

And we can even add worfklow_dispatch so that it can be manually triggered

task-definition: ${{ steps.task-def.outputs.task-definition }}
service: ${{ vars.ECS_SERVICE }}
cluster: ${{ vars.ECS_CLUSTER }}
wait-for-service-stability: true
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing EOF

3. **AWS Credentials**:
- AWS Access Key ID
- AWS Secret Access Key
- These credentials should have permissions to interact with ECR and ECS.
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT

Suggested change
- These credentials should have permissions to interact with ECR and ECS.
- These credentials should have permission to interact with ECR and ECS.

@brunoparma88 brunoparma88 had a problem deploying to Add_CD_with_github_actions May 28, 2024 12:03 — with GitHub Actions Failure
Copy link
Member

@santib santib left a comment

Choose a reason for hiding this comment

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

Great! LGTM 🤩

Comment on lines +3 to +7
# on:
# push:
# Uncomment and add the necessary branches to enable automatic deployment on AWS
# branches:
# - main
Copy link
Member

Choose a reason for hiding this comment

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

The only doubt I have is if having the on section completely commented might make the GHA invalid
image

Other option would be doing the same as here

on:
  workflow_dispatch:
  # Uncomment and add the necessary branches to enable automatic deployment on AWS
  # push:
    # branches:
      # - main

Comment on lines +13 to +14
environment:
name: ${{ github.ref_name }}
Copy link
Contributor

Choose a reason for hiding this comment

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

If I understand correctly, this requires us to create a Github environment and a branch with the exact same name. If we are forcing any naming convention, we should add it to the docs

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm also wondering if we should keep track of all the releases using the "Github releases" feature, and I think that relies on tagging instead of branch pushes

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.

None yet

4 participants