Add scripts to generate new Virtual Machine Scale Sets and images for build machines by BillyONeal · Pull Request #633 · microsoft/STL

BillyONeal · 2020-03-24T05:12:54Z

This change reduces the likelihood of differing behavior between builds due to updates of Visual Studio, since all workers will be updated atomically. It also enables automatic scaling of the number of builders to meet pull request demand, since starting a VM takes a few minutes rather than 40+ minutes.

barcharcraz · 2020-03-24T05:44:48Z

azure-devops/provision-image.ps1

@@ -201,8 +179,4 @@ $environmentKey = Get-ItemProperty -Path 'HKLM:\SYSTEM\CurrentControlSet\Control
 Set-ItemProperty -Path 'HKLM:\SYSTEM\CurrentControlSet\Control\Session Manager\Environment' `
    -Name Path `
    -Value "$($environmentKey.Path);C:\Program Files\CMake\bin;C:\Program Files\LLVM\bin"
-InstallZip 'Azure DevOps Agent' $VstsAgentUrl 'C:\agent'
-Add-MpPreference -ExclusionPath C:\agent


does this still happen at some point? or do we just never enable defender on the generated images

We don't turn Defender off any more, that's something we'll need to talk to Azure Pipelines team about since we no longer know where the work tree is.

And actually this was broken before since it should have excluded D:\ after we moved the build there.

StephanTLavavej

I think I found an issue that will prevent this from actually working (the branch issue). Other comments aren't critical.

StephanTLavavej · 2020-03-24T06:56:17Z

azure-devops/provision-image-bootstrap.ps1

+$temp = $env:TEMP
+
+curl.exe -L -o "$temp\psexec.exe" https://live.sysinternals.com/PsExec64.exe
+curl.exe -L -o "$temp\provision-image.ps1" https://raw.githubusercontent.com/microsoft/STL/autoscale/azure-devops/provision-image.ps1


This line appears to expect microsoft/STL to have a branch named autoscale. I believe it should be master.

StephanTLavavej · 2020-03-24T06:59:07Z

azure-devops/provision-image-bootstrap.ps1

+$temp = $env:TEMP
+
+curl.exe -L -o "$temp\psexec.exe" https://live.sysinternals.com/PsExec64.exe
+curl.exe -L -o "$temp\provision-image.ps1" https://raw.githubusercontent.com/microsoft/STL/autoscale/azure-devops/provision-image.ps1


How does the machine have this file, yet needs to download provision-image.ps1?

The cmdlet Invoke-AzVMRunCommand on 204 of create-new-agent-image.ps takes care of getting this script onto the box. I suppose I could eliminate the need for this with a script that invokes itself....

StephanTLavavej · 2020-03-24T06:59:38Z

azure-devops/provision-image-bootstrap.ps1

+
+curl.exe -L -o "$temp\psexec.exe" https://live.sysinternals.com/PsExec64.exe
+curl.exe -L -o "$temp\provision-image.ps1" https://raw.githubusercontent.com/microsoft/STL/autoscale/azure-devops/provision-image.ps1
+& "$temp\psexec.exe" -u AdminUser -p $AdminUserPassword -accepteula -h C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe -ExecutionPolicy Unrestricted -File "$temp\provision-image.ps1"


Is it possible to wrap to 120?

Maybe, we'll see

azure-devops/provision-image-bootstrap.ps1

StephanTLavavej · 2020-03-24T07:01:06Z

azure-devops/create-new-agent-image.ps1

+#
+# This script assumes you have installed Azure tools into Powershell by following the instructions
+# at https://docs.microsoft.com/en-us/powershell/azure/install-az-ps?view=azps-3.6.1
+# or are running from Azure Cloud Shell


Period? Sounds like a complete sentence to me.

StephanTLavavej · 2020-03-24T07:13:18Z

azure-devops/create-new-agent-image.ps1

+
+  $result = ''
+  for ($idx = 0; $idx -lt $length; $idx++) {
+    $result += $Chars[(Get-Random -Minimum 0 -Maximum ($Chars.Length - 1))]


I don't think this can ever generate '9'. According to https://docs.microsoft.com/en-us/powershell/module/microsoft.powershell.utility/get-random?view=powershell-7 , "Get-Random returns a value that is less than the maximum (not equal)." Can you verify?

PS D:\STL> for ($idx = 0; $idx -lt 10000; $idx++) { if ((Get-Random -Minimum 0 -Maximum 1) -eq 1) { >> Write-Output Pass >> return >> }} PS D:\STL>

Talk about terrible parameter naming.

StephanTLavavej · 2020-03-24T07:15:33Z

azure-devops/create-new-agent-image.ps1

+  Write-Output "Location: $Location"
+  Write-Output "Resource group name: $ResourceGroupName"
+  Write-Output "User name: AdminUser"
+  Write-Output "Using Generated Password: $AdminPW"


Why Is This Title Case?

StephanTLavavej · 2020-03-24T07:16:52Z

azure-devops/create-new-agent-image.ps1

+
+$allowHttp = New-AzNetworkSecurityRuleConfig `
+  -Name AllowHTTP `
+  -Description 'Allow HTTP(s)' `


Capitalize S?

azure-devops/create-new-agent-image.ps1

StephanTLavavej · 2020-03-24T07:20:07Z

azure-devops/create-new-agent-image.ps1

+####################################################################################################
+Write-Progress -Activity $ProgressActivity -Status -Completed
+Write-Reminders $AdminPW
+Write-Output 'Finished! Terminate.'


Terminate sounds a little ominous, do we need that? END OF LINE.

I always like little helper tools like this to emit that message but no skin off my nose to remove it. Done.

“Finished” is fine 😸

…rnal script access.

…ve Terminate,

cbezault

I'm happy with what I see here and the output in ADO, but take my approval with a grain of salt, I don't have psychic debugging powers.

cbezault

Only one comment, no need to resolve if you don't feel like it.

cbezault · 2020-03-24T15:10:42Z

azure-devops/create-new-agent-image.ps1

+$VmssIpConfigName = $ResourceGroupName + 'VmssIpConfig'
+$VmssIpConfig = New-AzVmssIpConfig -SubnetId $Nic.IpConfigurations[0].Subnet.Id -Primary -Name $VmssIpConfigName
+$VmssName = $ResourceGroupName + 'Vmss'
+$Vmss = New-AzVmssConfig `


It might not matter that much but I think for our purposes setting -ScaleInPolicy to OldestVM could make sense.

CaseyCarter

There are no errors here so glaringly and obviously incorrect that I noticed them.

BillyONeal · 2020-03-24T19:17:01Z

After this merges I will update the instructions at https://github.com/microsoft/STL/wiki/Checklist-for-Updating-Toolset-and-or-Dependencies to:

Create a new branch in your own fork.
Edit $/azure-devops/create-new-agent-image.ps1 and $/azure-devops/provision-image.ps1 to make the changes/updates you want to make. Most changes will be merely updating version numbers which happens in provision-image.ps1.
If you have not before, follow the instructions at https://docs.microsoft.com/en-us/powershell/azure/install-az-ps?view=azps-3.6.1 to install the Azure PowerShell tools. (This only needs to be done once per machine.)
Run $/azure-devops/create-new-agent-image.ps1.
On https://dev.azure.com/vclibs/STL/_settings/agentqueues, add a new Azure virtual machine scale set pool pointing to the scale set generated in the previous step. Name the pool the same name as the generated resource group to make them easy to find. Choose 1 standby machine with a maximum of 6 so as to not exceed our 100 core quota.
Update $/azure-devops/run-build.yml to point to the new pool.
Create/update a PR from your branch to Microsoft/STL master. If there are substantial code review changes, delete the created pool from https://dev.azure.com/vclibs/_settings/agentpools, delete the resource group from the Azure portal, make the code review requested changes, and restart from step 4.
After your PR merges, delete the old unused pool from https://dev.azure.com/vclibs/_settings/agentpools, and the previous resource group from the Azure portal.

StephanTLavavej · 2020-03-24T20:11:13Z

I believe that $/meow is TFS jargon and shouldn't be in the wiki.
"This only need be done once per machine." would sound better as "This only needs to be done once per machine."
run-build.yaml is a typo - the correct filename is run-build.yml.
"Create/update a PR from your branch to master." Should you clarify that you mean microsoft/STL master?
"delete the old no longer used pool" would sound much better as "old unused pool".

StephanTLavavej · 2020-03-24T20:17:15Z

azure-devops/provision-image.ps1

 }

+
+Write-Output "AdminUser password not supplied; assuming already running as AdminUser"


Can/should this verify something like %USERNAME%?

No, it must not be that because here we're running as NT AUTHORITY\SYSTEM (that's the whole reason we need to do this dance in the first place)

Oh, I see, this is in the "not elevating" branch; I think saying AdminUser is good still because it helps explain the explosion that will happen if you try to run the script unelevated :)

BillyONeal · 2020-03-24T20:58:42Z

/azp run

azure-pipelines · 2020-03-24T20:58:50Z

Azure Pipelines successfully started running 1 pipeline(s).

BillyONeal · 2020-03-24T21:01:59Z

I believe that $/meow is TFS jargon and shouldn't be in the wiki.

Do you have other suggestions? It is admittedly jargon I took from TFS world but I've used it in Git and svn worlds before and everyone always has understood what that meant. (I believe I applied your other suggestions)

StephanTLavavej · 2020-03-24T22:15:49Z

I wouldn't use any prefix at all; I might consider linking to the files in master like azure-devops/create-new-agent-image.ps1 but I suppose $/ is fine.

BillyONeal added 6 commits March 23, 2020 17:20

Initial commit of script to generate Virtual Machine Scale Sets.

ec40894

Add progress and date.

af0ec0e

Remove explicit "pool" from run-build.yml.

70f233a

No overprovision.

bd91e4a

Shinier output.

bc75662

Added delays due to race between this script and sysprep.

36e5d0e

BillyONeal requested a review from a team as a code owner March 24, 2020 05:12

BillyONeal self-assigned this Mar 24, 2020

BillyONeal added 2 commits March 23, 2020 22:15

Set pool name explicitly.

8b8877d

Update script location to Microsoft.

48b1977

barcharcraz reviewed Mar 24, 2020

View reviewed changes

StephanTLavavej added the infrastructure Related to repository automation label Mar 24, 2020

StephanTLavavej requested changes Mar 24, 2020

View reviewed changes

BillyONeal added 4 commits March 24, 2020 01:49

Fix branch to mention master rather than 'autoscale'.

fc7041a

Change provision script to invoke itself to avoid needing stable exte…

2da977e

…rnal script access.

PowerShell, extract collision detect, New-Password gen 9, HTTPS, remo…

22ada1d

…ve Terminate,

bugs bugs bugs

44515e0

BillyONeal requested a review from StephanTLavavej March 24, 2020 10:13

cbezault approved these changes Mar 24, 2020

View reviewed changes

cbezault reviewed Mar 24, 2020

View reviewed changes

CaseyCarter approved these changes Mar 24, 2020

View reviewed changes

Scale in by oldest.

205b8d7

BillyONeal added 2 commits March 24, 2020 12:22

Update pool with changes.

d27126f

Drop bogus status.

59b8a59

StephanTLavavej approved these changes Mar 24, 2020

View reviewed changes

Merge remote-tracking branch 'origin/master' into autoscale

33b2489

BillyONeal merged commit b75dd43 into microsoft:master Mar 24, 2020

BillyONeal deleted the autoscale branch April 16, 2020 04:11

StephanTLavavej mentioned this pull request Apr 10, 2024

Toolset update: VS 2022 17.10 Preview 3 #4576

Merged

StephanTLavavej mentioned this pull request May 22, 2024

Toolset update: VS 2022 17.11 Preview 1 #4687

Merged

		}


		Write-Output "AdminUser password not supplied; assuming already running as AdminUser"

Conversation

BillyONeal commented Mar 24, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

StephanTLavavej left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cbezault left a comment

Choose a reason for hiding this comment

Uh oh!

cbezault left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CaseyCarter left a comment

Choose a reason for hiding this comment

Uh oh!

BillyONeal commented Mar 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

StephanTLavavej commented Mar 24, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BillyONeal commented Mar 24, 2020

Uh oh!

azure-pipelines bot commented Mar 24, 2020

Uh oh!

BillyONeal commented Mar 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

StephanTLavavej commented Mar 24, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

BillyONeal commented Mar 24, 2020 •

edited

Loading

BillyONeal commented Mar 24, 2020 •

edited

Loading