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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new build pipeline #2735

Merged
merged 11 commits into from Feb 20, 2020
Merged

Add new build pipeline #2735

merged 11 commits into from Feb 20, 2020

Conversation

amrocha
Copy link
Contributor

@amrocha amrocha commented Feb 12, 2020

WHY are these changes introduced?

The consumer build test has long been an unstable part of polaris-react's CI. We meant to introduce the current test as a temporary measure to improve shipping confidence, while also working on an improved version of the test that was more reliable. This is that version.

In the first implementation of the test, we duplicated web's Buildkite pipeline in polaris-react, and ran a build there. The problems with this are:

  • Duplicated code. If web updates their pipeline, we don't get those updates by default;
  • Incompatible dependencies. For several months, web used a different version of node than polaris-react. This meant that the test was impossible to run until web updated their node version.
  • Flakiness. Web's pipelines are under constant stress, and people work to make sure they're always working. This wasn't the case in polaris-react, leading to our pipeline breaking and no one being available to fix it

This PR addresses this by building web in web's pipeline, and building polaris-react in polaris-react's pipeline. By separating the environments, we solve the problems outlined above.

WHAT is this pull request doing?

The new test works as follows:

  1. Build polaris-react in our own pipeline
  2. Package the build using yarn pack, to create a package similar to what is uploaded to npm
  3. Upload this build to Google Cloud Platform. I had prod-eng create a bucket and an authorization key solely for this purpose.
  4. Trigger a pipeline run in web that uses the uploaded asset
  5. Wait for results

I also created a web pipeline over at https://github.com/Shopify/web/pull/23433. This pipeline runs the same as any other web pipeline, but it downloads and installs the package we uploaded to the bucket earlier.

Improvements to the test:

  • Rename "Consumer Build Test" -> "System Integration Test"
  • Rename pipeline "polaris-react" -> "polaris-system-integration-test"
  • Pipeline now builds and uploads assets to GCP using a custom bucket and IAM auth key created solely for this purpose
  • After uploading asset, triggers a build in web that downloads said asset from GCP and uses it
  • Reports results in polaris-react's pipeline

How to 馃帺

Trigger a new build here:
https://buildkite.com/shopify/polaris-system-integration-test

@amrocha
Copy link
Contributor Author

amrocha commented Feb 14, 2020

This PR merely lays out the foundation for the test, but doesn't enable it. I plan to do this in a follow-up PR. The order of operations I intend is:

  • Merge https://github.com/Shopify/web/pull/23433
  • Merge this PR
  • Configure it to run on new commits to master
  • Have a meeting with the team to determine how we should run this test: on all branches, just in master, make it required, make it optional, etc.
  • Ship a follow-up PR with the agreed upon changes

@amrocha
Copy link
Contributor Author

amrocha commented Feb 14, 2020

Note to self: update branch part of pipeline config to point to master before shipping

@amrocha amrocha marked this pull request as ready for review February 14, 2020 19:31
@github-actions
Copy link
Contributor

github-actions bot commented Feb 14, 2020

馃煝 No significant changes to src/**/*.tsx were detected.

@amrocha
Copy link
Contributor Author

amrocha commented Feb 14, 2020

@BPScott @AndrewMusgrave @tmlayton @danrosenthal @daniedleroux @kyledurand @alex-page Hey team, this is the PR updating the consumer build test

You don't need to comment on this if you don't feel comfortable giving feedback on it, but I thought it would be good for you to be aware of these changes. If you have any questions feel free to ask!

It seems like there's some interest around this, so I'll also be creating a meeting to go over what it took to ship this, and hopefully giving some insight into how Shopify's infrastructure is managed and orchestrated.

Copy link
Member

@AndrewMusgrave AndrewMusgrave left a comment

Choose a reason for hiding this comment

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

馃槏

UNRELEASED.md Outdated Show resolved Hide resolved
.shopify-build/polaris-system-integration-test.yml Outdated Show resolved Hide resolved
Copy link
Member

@BPScott BPScott left a comment

Choose a reason for hiding this comment

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

A few things inline.

Also, naming is annoying - we've been grumbling about 'polaris' being an overloaded word - would naming the pipeline polaris-react-system-integration-test or something that's not just polaris-... help with that?

.shopify-build/polaris-system-integration-test.yml Outdated Show resolved Hide resolved
.shopify-build/polaris-system-integration-test.yml Outdated Show resolved Hide resolved
.shopify-build/polaris-system-integration-test.yml Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
amrocha added 11 commits February 19, 2020 17:51
- Rename "Consumer Build Test" -> "System Integration Test"
- Rename pipeline "polaris-react" -> "polaris-system-integration-test"
- Pipeline now builds and uploads assets to GCP using a custom bucket and IAM auth key created solely for this purpose
- After uploading asset, triggers a build in web that downloads said asset from GCP and uses it
- Reports results in polaris-react's pipeline
@amrocha
Copy link
Contributor Author

amrocha commented Feb 20, 2020

Just waiting for CI to be green before merging

@amrocha amrocha merged commit cc82a42 into master Feb 20, 2020
@amrocha amrocha deleted the add-new-build-pipeline branch February 20, 2020 02:09
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