Conversation
|
/azp run runtime-libraries stress-http |
|
/azp run runtime-libraries stress-ssl |
|
Tagging subscribers to this area: @karelz, @dotnet/ncl |
|
Azure Pipelines successfully started running 1 pipeline(s). |
1 similar comment
|
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
Updates System.Net stress-test Docker Compose runner scripts to address infra changes affecting compose invocation.
Changes:
- Switch the bash runner to use
docker compose(Compose v2 plugin) forbuildandup. - Update the PowerShell runner to use
docker-compose(Compose v1) forbuildandup(including log/error text).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/libraries/Common/tests/System/Net/StressTests/run-docker-compose.sh | Uses docker compose for compose build/run. |
| src/libraries/Common/tests/System/Net/StressTests/run-docker-compose.ps1 | Uses docker-compose for compose build/run and updates related messaging. |
| if ! docker compose --file "$compose_file" build $build_args; then | ||
| exit $? | ||
| fi |
There was a problem hiding this comment.
In if ! ...; then exit $?; fi, $? inside the then block is the status of the negated ! pipeline (typically 0 when the docker compose command failed), so build failures can be reported as success. Consider removing the if ! wrapper and relying on set -e, or capture the command exit code before negation / use || exit $? to preserve the failing exit code.
| write-output "docker-compose --file $COMPOSE_FILE build $buildArgs" | ||
| docker-compose --file $COMPOSE_FILE build @buildArgs 2>&1 | ||
| if ($LASTEXITCODE -ne 0) { | ||
| throw "docker compose exited with error code $LASTEXITCODE" | ||
| throw "docker-compose exited with error code $LASTEXITCODE" | ||
| } |
There was a problem hiding this comment.
This script now invokes docker-compose (v1) while the bash version and the stress pipelines use docker compose (v2 plugin). If the infra change motivating this PR is removal of the docker-compose binary, this will still fail on Windows. Suggest using docker compose consistently here (and update the log/throw text accordingly), or implement a small fallback that prefers docker compose but uses docker-compose if the plugin isn't available.
| $env:STRESS_CLIENT_ARGS = $clientStressArgs | ||
| $env:STRESS_SERVER_ARGS = $serverStressArgs | ||
| docker compose --file "$COMPOSE_FILE" up --abort-on-container-exit | ||
| docker-compose --file "$COMPOSE_FILE" up --abort-on-container-exit |
There was a problem hiding this comment.
Same concern as the build step: docker-compose is used here, but CI stress pipelines already run docker compose ... on Windows (e.g., eng/pipelines/libraries/stress/ssl.yml). If docker-compose isn't installed on the updated images, this up invocation will fail. Align to docker compose (or add a fallback) so both scripts behave the same across platforms.
Fixes broken stress by recent changes in used images.
Test infra only change, no product code affected.