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

refactor: Make all pipelines use new agent pools in eastus2 #23072

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

Conversation

alexvy86
Copy link
Contributor

@alexvy86 alexvy86 commented Nov 12, 2024

Description

We're moving our agent pools to the eastus2 region as part of a capacity management initiative. This PR updates all pipelines to use the new pools.

Reviewer Guidance

The review process is outlined on this wiki page.

I did manual runs of all pipelines with these changes applied, and they all run fine. Failures in test pipelines are unrelated to this change, just the usual flaky tests & similar. And build-docs is not designed to run from test/ branches so its failure is also expected.

AB#23299

@github-actions github-actions bot added base: main PRs targeted against main branch area: build Build related issues labels Nov 12, 2024
Copy link
Collaborator

@msfluid-bot msfluid-bot left a comment

Choose a reason for hiding this comment

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

Code Coverage Summary

No packages impacted by the change.


Baseline commit: aa2718b
Baseline build: 306963
Happy Coding!!

Code coverage comparison check passed!!

@msfluid-bot
Copy link
Collaborator

msfluid-bot commented Nov 12, 2024

@fluid-example/bundle-size-tests: +245 Bytes
Metric NameBaseline SizeCompare SizeSize Diff
aqueduct.js 465.72 KB 465.75 KB +35 Bytes
azureClient.js 563.04 KB 563.09 KB +49 Bytes
connectionState.js 724 Bytes 724 Bytes No change
containerRuntime.js 262.34 KB 262.35 KB +14 Bytes
fluidFramework.js 426.99 KB 427.01 KB +14 Bytes
loader.js 134.18 KB 134.19 KB +14 Bytes
map.js 42.71 KB 42.71 KB +7 Bytes
matrix.js 149.84 KB 149.85 KB +7 Bytes
odspClient.js 528.83 KB 528.88 KB +49 Bytes
odspDriver.js 97.88 KB 97.9 KB +21 Bytes
odspPrefetchSnapshot.js 42.81 KB 42.83 KB +14 Bytes
sharedString.js 165.77 KB 165.78 KB +7 Bytes
sharedTree.js 417.45 KB 417.46 KB +7 Bytes
Total Size 3.37 MB 3.37 MB +245 Bytes

Baseline commit: aa2718b

Generated by 🚫 dangerJS against fe80cfd

Choose a reason for hiding this comment

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

Copilot reviewed 6 out of 21 changed files in this pull request and generated no suggestions.

Files not reviewed (15)
  • tools/pipelines/templates/include-conditionally-run-stress-tests.yml: Evaluated as low risk
  • tools/pipelines/build-client.yml: Evaluated as low risk
  • tools/pipelines/test-real-service.yml: Evaluated as low risk
  • tools/pipelines/repo-policy-check.yml: Evaluated as low risk
  • tools/pipelines/templates/build-npm-package.yml: Evaluated as low risk
  • tools/pipelines/templates/include-upload-stage-telemetry.yml: Evaluated as low risk
  • tools/pipelines/test-service-clients.yml: Evaluated as low risk
  • tools/pipelines/templates/include-test-real-service.yml: Evaluated as low risk
  • tools/pipelines/templates/include-policy-check.yml: Evaluated as low risk
  • tools/pipelines/test-real-service-stress.yml: Evaluated as low risk
  • tools/pipelines/templates/build-docker-service.yml: Evaluated as low risk
  • tools/pipelines/templates/include-publish-npm-package-deployment.yml: Evaluated as low risk
  • tools/pipelines/templates/include-test-stability.yml: Evaluated as low risk
  • tools/pipelines/test-perf-benchmarks.yml: Evaluated as low risk
  • tools/pipelines/templates/include-publish-docker-service-steps.yml: Evaluated as low risk

Tip: Copilot only keeps its highest confidence comments to reduce noise and keep you focused. Learn more

@alexvy86 alexvy86 marked this pull request as ready for review November 12, 2024 22:48
@alexvy86 alexvy86 requested review from a team, rohandubal and scarlettjlee November 12, 2024 22:48
@tylerbutler
Copy link
Member

Do we expect any perf or latency impact?

@alexvy86
Copy link
Contributor Author

alexvy86 commented Nov 13, 2024

Do we expect any perf or latency impact?

No, with the exception of the e2e tests against routerlicious. Since our internal routerlicious deployment stays in westus2, there's now a longer network path from the agents where the tests run, and the service they're talking to. ~10min runtime increase for that particular stage of the e2e tests. However, I have some design/proposal in the works to get rid of that environment entirely in the short(ish) term, and the increase is not that huge, so I think it's acceptable. I'll also note it's a bit funny that in the 2 runs I've done in the new pools, the r11s tests seems to have fewer failures/timeouts 🤷 .

There is a ~12% increase in cost though. @scarlettjlee @rohandubal I pinged the thread to see if we can get info of which other regions we can move to, to see if there's one with the same costs as westus2. I'll hold a bit on merging this.
EDIT: that applied when we planned to move to centralus. eastus2 has the same prices as westus2 so they won't change in the end.

Today:
image

After this change:
image

@@ -114,7 +114,7 @@ extends:
- test:copyresults
taskLint: true
taskLintName: ci:eslint
poolBuild: NewLarge-linux-1ES
poolBuild: Large-centralus
Copy link
Contributor

Choose a reason for hiding this comment

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

What was the reason we had to use the 1ES node pool to begin with? Just double checking we're good to move off of them

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We're actually not moving off them, the new ones are 1ES pools, I just dropped that bit from the name since we don't really need to differentiate between 1ES and non-1ES ones. We can run all pipelines on 1ES pools, whether they leverage 1ES templates or not.

@alexvy86 alexvy86 changed the title refactor: Make all pipelines use the centralus agent pools refactor: Make all pipelines use new agent pools in eastus2 Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: build Build related issues base: main PRs targeted against main branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants