feat: migrate morgan stanley test to canadacentral#1145
feat: migrate morgan stanley test to canadacentral#1145vittoriasalim wants to merge 7 commits intomainfrom
Conversation
vittoriasalim
requested review from
alyssa1303,
anson627,
liyu-ma,
sumanthreddy29,
wonderyl and
xinWeiWei24
as code owners
There was a problem hiding this comment.
Pull request overview
Migrates the nap-complex perf-eval (“Morgan Stanley”) benchmark setup to support running in Azure canadacentral, updating scenario inputs and the benchmark pipeline to include a new canadacentral stage.
Changes:
- Added a new
azure-canadacentral.tfvarsterraform input for the nap-complex scenario. - Updated the nap-complex Karpenter NodePool allowed VM SKUs.
- Updated the autoscale benchmark pipeline schedules and added a canadacentral stage wired to the new tfvars.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
scenarios/perf-eval/nap-complex/terraform-inputs/azure-canadacentral.tfvars |
New canadacentral-specific Terraform inputs (networking, firewall, ACR, AKS, jumpbox). |
scenarios/perf-eval/nap-complex/kubernetes/karpenter_nodepool.azure.yml |
Adjusted allowed Azure SKU names for the default NodePool. |
pipelines/perf-eval/Autoscale Benchmark/node-auto-provisioning-benchmark-complex.yml |
Added an additional schedule and a new canadacentral benchmark stage using the new tfvars. |
| "mcr.microsoft.com", | ||
| "*.mcr.microsoft.com", | ||
| "*.data.mcr.microsoft.com", | ||
| "*.azurecr.io", |
There was a problem hiding this comment.
The target_fqdns list contains duplicate entries (e.g., mcr.microsoft.com and *.azurecr.io appear more than once). Duplicates make the rule harder to audit and can contribute to Azure Firewall rule-size limits; please deduplicate the list.
| "mcr.microsoft.com", | |
| "*.mcr.microsoft.com", | |
| "*.data.mcr.microsoft.com", | |
| "*.azurecr.io", | |
| "*.mcr.microsoft.com", | |
| "*.data.mcr.microsoft.com", |
| # eastus2 - weekly | ||
| - cron: "0 21 * * 0" | ||
| displayName: "Every week at 9:00 PM" | ||
| branches: | ||
| include: | ||
| - main | ||
| always: true | ||
| # canadacentral - daily | ||
| - cron: "0 21 * * *" | ||
| displayName: "Every day at 9:00 PM" |
Copilot
AI
Apr 22, 2026
•
There was a problem hiding this comment.
The added weekly and daily schedules both trigger the entire pipeline; as written, the eastus2 stage will still run on the daily schedule too. If the intent is “eastus2 weekly, canadacentral daily”, add stage conditions based on the schedule display name (e.g., Build.CronSchedule.DisplayName) or split into separate pipelines.
| name = "AzureBastionSubnet" | ||
| # Dedicated subnet required by Azure Bastion (/27 or larger) | ||
| address_prefix = "10.224.0.32/27" | ||
| }, |
There was a problem hiding this comment.
The header comment says there is “No Bastion host”, but the VNet still defines an AzureBastionSubnet with a comment stating it’s required for Bastion. Either remove the Bastion subnet (if truly unused) or update the comments to reflect why it’s still present.
| regions: | ||
| - canadacentral | ||
| terraform_input_file_mapping: | ||
| - canadacentral: "scenarios/perf-eval/nap-complex/terraform-inputs/azure-canadacentral.tfvars" |
There was a problem hiding this comment.
Why do we need a new input file here?
There was a problem hiding this comment.
because there is different rule for different regions
There was a problem hiding this comment.
maybe we can add canadacentral and eastus2 to the rule
There was a problem hiding this comment.
yeah, agree this. if we only have 2 regions the only difference is the FQDN, we can add both in the rules instead of a new file, wdyt
| trigger: none | ||
| schedules: | ||
| # On-Demand Schedule with Morgan Stanley cluster config | ||
| # eastus2 - weekly |
There was a problem hiding this comment.
Why we still keep eastus2?
| name = "AllowSSH" | ||
| priority = 100 | ||
| destination_port_range = "22" | ||
| # No Bastion in canadacentral; allow SSH from any source (pipeline agent). |
There was a problem hiding this comment.
migrate benchmark test to canadacentral