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

Updated CI/CD Tests #405

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

Updated CI/CD Tests #405

wants to merge 15 commits into from

Conversation

JordanChen123
Copy link
Contributor

Description

Verification

  • [ ]

Resources

@JordanChen123 JordanChen123 requested a review from SPDonaghy as a code owner June 13, 2024 03:43
@JordanChen123 JordanChen123 requested review from Aimxni and removed request for Aimxni June 13, 2024 03:43
#push:
# branches:
# - main
pull_request:
Copy link
Contributor

Choose a reason for hiding this comment

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

What would be the motivation for running this on every pull request rather than on pushes to main?
Wouldn't this just be testing the configuration of the main branch and not the changes being introduced?
If we want to keep it as being triggered by pull requests can we only trigger it when files in src/website and .devcontainer are modified?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From my understanding, running this on every PR would ensure that changes in the PR would not break the deployment before they are merged into the main branch, which would also provide another layer of testing to catch issues early. What do you think?

Also, good point. If we do want to keep it as being triggered by PR's, then it should only trigger when files in src/website and .devcontainer are modified. I will fix that

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah sounds like a great idea to test out how changes deploy before actually deploying to the real site.

The main branch will be checked out on the VM, and not the development branch
I'm sorry that's totally wrong ^

Would the VM that this workflow deploys to be like a pre-production environment and not the actual web host?

If so, then I think keeping it as triggered by PRs makes sense, and maybe we can keep the push trigger as well and deploy to either the pre-production VM or the actual website host VM depending on the trigger event.

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Your idea sounds great, since the VM that the workflow deploys will not be the actual web host.

git pull
touch src/website/.env.local # Adds a local environment file to VM to override env vars in .env.production
echo NEXT_PUBLIC_SERVER_HOST=http://${{ secrets.SSH_HOST }} >> src/website/.env.local
docker compose -f .devcontainer/docker-compose.yml -f .devcontainer/website/docker-compose.website.prod.yml up --force-recreate -d --build --remove-orphans
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see a verification that docker is installed on the VM prior to deploying to it. Which is fine, just wanted to clarify as you mentioned it resolved that issue

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.

[CD] Setup docker on remote VM
2 participants