-
Notifications
You must be signed in to change notification settings - Fork 227
Stress development maturity updates #3481
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
Conversation
|
The following pipelines have been queued for testing: |
|
The following pipelines have been queued for testing: |
c4070ed to
50c61d7
Compare
|
The following pipelines have been queued for testing: |
50c61d7 to
9106a65
Compare
|
The following pipelines have been queued for testing: |
9106a65 to
8cb8610
Compare
|
The pipeline is currently failing due to permissions on the service principal that fail validation. I am following up on this. |
|
The following pipelines have been queued for testing: |
|
Pipelines will be failing until I can rebase with @ckairen's changes to deploy to the new playground cluster which has been migrated to the latest versions of our cluster dependencies. Everything is looking as I expect it to otherwise. |
|
The following pipelines have been queued for testing: |
| default: test | ||
| values: | ||
| - prod | ||
| - test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it be pg?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I was planning on rebasing/updating after your changes went in.
| $WhatIfPreference = $false | ||
| . (Join-Path $PSScriptRoot "../../../eng/common/scripts/Helpers" PSModule-Helpers.ps1) | ||
| Install-ModuleIfNotInstalled "powershell-yaml" "0.4.1" | Import-Module | ||
| $WhatIfPreference = $LastWhatIfPreference |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't quite understand why we are changing WhatIfPreference to false and then to original value again
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's somewhat annoying. There are sub-components called within Install-ModuleIfNotInstalled that don't run when -WhatIf is set that we absolutely need to run, but because the top level cmdlet doesn't have the SupportsShouldProcess binding, I don't think I can just pass -WhatIf:$false. We need this dependency installed whether we are in whatif mode or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same for Import-Module. It doesn't support -WhatIf but if Install-ModuleIfNotInstalled doesn't return a module, then we throw.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explicitly pass -WhatIf:$false to those cmdlets instead of mucking around with the global variable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated the underlying function to add SupportsShouldProcess, so it should work now.
e2a75ed to
1b8ec23
Compare
|
The following pipelines have been queued for testing: |
| - prod | ||
| - test | ||
| - name: WhatIf | ||
| type: boolean |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will be an interesting pattern. If you find that it works out OK can you please write up some guideline docs about this pattern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep can do. Plumbing WhatIf support through all our scripts will be pretty useful for general testing and development, I think. Similarly we could run a lot more validation pipelines/scripts on PR changes if we had WhatIf enabled to catch surface level issues.
weshaggard
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Took a high-level look at add a couple comments but otherwise seems reasonable.
|
The following pipelines have been queued for testing: |
|
Hello @azure-sdk! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
-LocalAddonsPathto the stress deploy script to use a local stress-test-addons instead of the remote one. Currently this has to be updated manually inChart.yamland is easy to forget to do.-WhatIfflag to the provision script to do bicep and helm template validation.deployIdis the same for all local users, so anyone deploying stress tests locally may see other users deployed tests listed in the output.Fixes #3135 #2635 #2764 #2635