-
Notifications
You must be signed in to change notification settings - Fork 174
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
Use stress test environment defaults for group and subscription #2363
Conversation
benbp
commented
Dec 1, 2021
•
edited
Loading
edited
- There's no need to have to specify the cluster resource group and subscription when we already know these values ahead of time. We still need to keep them as parameters to enable people to deploy to dev environments they've provisioned themselves.
- Fix secret logging in CI mode from New-TestResources
- Fix the stress deployment script overwriting pre-existing env variables
- Simplify stress chart image handling
- Bump addons chart to 0.1.13
The following pipelines have been queued for testing: |
The following pipelines have been queued for testing: |
@heaths @weshaggard I'd appreciate your review on the |
4cd203e
to
0b8e225
Compare
The following pipelines have been queued for testing: |
d9966b9
to
9c56886
Compare
The following pipelines have been queued for testing: |
eng/common/scripts/stress-testing/stress-test-deployment-lib.ps1
Outdated
Show resolved
Hide resolved
The following pipelines have been queued for testing: |
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.
After going through the replies and discussing a few things offline, this LGTM but I really dislike the $DevopsLogging
name. It's misleading given the intent. Perhaps the original $RedactLogs
or something like $SuppressSecretsLogging
would be more obvious to future maintainers. The name as it is now is already logging-specific, so I see no harm in hitching a new name to logging. The original $CI
was meant to be obvious that it's set only when in a CI, and several behaviors are keyed off that. In this case, it all seems to come down to logging.
eng/common/scripts/stress-testing/stress-test-deployment-lib.ps1
Outdated
Show resolved
Hide resolved
The following pipelines have been queued for testing: |
291d4a6
to
96bec15
Compare
The following pipelines have been queued for testing: |
The following pipelines have been queued for testing: |
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.
Look reasonable but I would use the new DevOpsLogging to condition all vso specific logging not just the secret parts.
c65252f
to
b3afd25
Compare
The following pipelines have been queued for testing: |
b3afd25
to
6c1b6b2
Compare
The following pipelines have been queued for testing: |
...kubernetes/stress-test-addons/images/test-resource-deployer/deploy-stress-test-resources.ps1
Outdated
Show resolved
Hide resolved
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 (
|
/check-enforcer override |