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

Parallelise our CI/CD deployment of hubs #1159

Merged
merged 26 commits into from
Apr 12, 2022

Conversation

sgibson91
Copy link
Member

@sgibson91 sgibson91 commented Apr 1, 2022

fixes #818

Summary

This PR implements a first pass of parallelised deployment of hubs in our CI/CD as discussed in #1131 There is still work to be done to make this match the vision in that issue, but I think this is a good start and we can iterate in future PRs.

What has changed

  • I have renamed .github/actions/deploy to .github/actions/setup-deploy. It now contains all the steps required to be ready to deploy a hub to a cluster, many of which we had originally defined .github/workflows/deploy-hubs.yaml, but no longer performs the deploy itself.
  • Parallelised jobs have been added to .github/workflows/deploy-hubs.yaml.
    • The generate-jobs job generates all the matrix jobs required as defined by the deployer logic implemented in (Attempt 2) Add logic to the deployer to decide which clusters/hubs require a helm upgrade #1112. In PRs, this job provides a rich output table of jobs.
    • The upgrade-support-and-staging job upgrades the support chart and/or the staging hub on each cluster that require it in parallel. This job requires generate-jobs to complete before running.
    • The upgrade-prod-hubs job then upgrades every production hub that requires it in parallel. This job requires both generate-jobs and upgrade-support-and-staging to complete before running.

Failing test

The message appearing in the failed test is being tracked here: jitterbit/get-changed-files#42

But I actually think this will resolve itself once this PR is merged. I opened a test PR to my test repo via a fork and all worked fine: sgibson91/helm-upgrade-decision-logic#6

@sgibson91 sgibson91 changed the title Parallelise-ci Parallelise our CI/CD deployment of hubs Apr 1, 2022
@sgibson91 sgibson91 marked this pull request as ready for review April 1, 2022 13:03
@sgibson91 sgibson91 requested a review from a team April 1, 2022 13:03
Co-authored-by: Erik Sundell <[email protected]>
@sgibson91
Copy link
Member Author

sgibson91 commented Apr 1, 2022

I don't think the prettier pre-commit failure is related to what I've changed here, but it's really hard to tell from the logs. There's no spaces between the filenames and so it's just one giant line - very difficult to parse 😕

image

Copy link
Member

@GeorgianaElena GeorgianaElena left a comment

Choose a reason for hiding this comment

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

This work that has been done around parallelizing hub deployment is just awesome 🚀 ❤️
Thanks @sgibson91!
I left a few questions, but I believe we should merge this and see how it goes. However, I believe that others on the @2i2c-org/tech-team should should also have a look before since it's a pretty significant change.

.github/workflows/deploy-hubs.yaml Show resolved Hide resolved
.github/workflows/deploy-hubs.yaml Outdated Show resolved Hide resolved
.github/workflows/deploy-hubs.yaml Outdated Show resolved Hide resolved
Copy link
Member

@consideRatio consideRatio left a comment

Choose a reason for hiding this comment

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

Nice work @sgibson91 this LGTM!!!

I've added a few comments focused on documentation that I think are quite important for maintenance sustainability but not technically critical - from the technical standpoint is approved as it is!

.github/actions/setup-deploy/action.yaml Outdated Show resolved Hide resolved
.github/actions/setup-deploy/action.yaml Outdated Show resolved Hide resolved
.github/actions/setup-deploy/action.yaml Outdated Show resolved Hide resolved
.github/actions/setup-deploy/action.yaml Outdated Show resolved Hide resolved
.github/workflows/deploy-hubs.yaml Show resolved Hide resolved
.github/workflows/deploy-hubs.yaml Outdated Show resolved Hide resolved
.github/workflows/deploy-hubs.yaml Show resolved Hide resolved
.github/workflows/deploy-hubs.yaml Outdated Show resolved Hide resolved
@sgibson91
Copy link
Member Author

sgibson91 commented Apr 6, 2022

Note: we may need to address #1171 first

@sgibson91
Copy link
Member Author

@consideRatio I propose we start our co-working tomorrow by hitting merge on this, fix any bugs that happen to arise, and then decide where to go with the rest of our time from there

@consideRatio consideRatio merged commit 8e7c7b8 into 2i2c-org:master Apr 12, 2022
@consideRatio
Copy link
Member

Wieee! London collaboration merge!

@sgibson91 sgibson91 deleted the parallelise-ci branch April 27, 2022 15:37
This pull request was closed.
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.

Parallelize the deployment of hubs on a cluster
3 participants