Build HttpStress and SslStress with live-built runtime using current TFM #61689

antonfirsov · 2021-11-16T20:00:38Z

As noted in #60769 (comment), the assumption that the live-built runtime of version (X+1).0 can be a drop-in replacement in an SDK of version X.0 is sometimes wrong, and it currently breaks our stress builds.

This PR changes both local (non-containerized) and containerized stress builds to build against the live-built runtime with the help of targetingpacks.targets.

Some tricky parts:

The docker builds no longer replace the shared dotnet runtime. Instead, we copy the live built runtime dll-s and ref dlls under /live-runtime-artifacts and reference that location from properties required by targetingpacks.targets
We also copy testhost to /live-runtime-artifacts. Tests are being executed by:
TESTHOSTDIR/dotnet exec STRESSBINDIR/HttpStress.dll <args> .
Since I wanted to avoid using the entire repository as the container context, targetingpacks.targets also goes to /live-runtime-artifacts
HttpStress needs compatible (7.0) Microsoft.AspNetCore.App bits. These are acquired by getting the latest daily SDK build with dotnet-install.sh/ps1
- This mechanism replaces the outdated privateAspNetCore stuff implemented in Run stress tests on private Asp.Net Core package #33860. Scripts args and dockerfiles dealing with that have been removed.
To support local, non-containerized test execution helper scripts build-local.ps1/sh have been implemented. In case of HttpStress these scripts need to grab the daily SDK just like container scripts.
Visual Studio experience is now broken for HttpStress, but it can be "fixed" by temporary local hacks if necessary. We may open a tracking issue to fix this.

Fixes #60769.

/cc @ManickaP @safern

ghost · 2021-11-16T20:00:45Z

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

As noted in #60769 (comment), the assumption that the live-built runtime of version (X+1).0 can be a drop-in replacement in an SDK of version X.0 is sometimes wrong, and it currently breaks our stress tests.

This PR changes both local (non-containerized) and containerized stress builds to build against the live-built runtime with the help of targetingpacks.targets.

Some tricky parts:

The docker builds no longer replace the shared dotnet runtime. Instead, we copy the live built runtime dll-s and ref dlls under /live-runtime-artifacts and reference that location from properties required by targetingpacks.targets
We also copy testhost to /live-runtime-artifacts. Tests being executed by TESTHOSTDIR/dotnet exec STRESSBINDIR/HttpStress.dll <args> then.
Since I wanted to avoid using the entire repository as the container context, targetingpacks.targets also goes to /live-runtime-artifacts
HttpStress needs compatible (7.0) Microsoft.AspNetCore.App bits. These are acquired by getting the latest daily SDK build with dotnet-install.sh/ps1
This mechanism replaces the outdated privateAspNetCore stuff implemented in Run stress tests on private Asp.Net Core package #33860
To support local, non-containerized test execution helper scripts build-local.ps1/sh have been implemented. In case of HttpStress these scripts need to grab the daily SDK just like container scripts.
Visual Studio experience is now broken for HttpStress, but it can be "fixed" by temporary local hacks if necessary

Fixes #60769.

Author:	antonfirsov
Assignees:	-
Labels:	`area-System.Net`
Milestone:	-

antonfirsov · 2021-11-16T20:01:19Z

src/libraries/System.Net.Http/tests/StressTests/HttpStress/StressServer.cs

+        {
+            // For some reason, System.Security.Cryptography.Encoding.dll fails to resolve when being loaded on-demand by AspNetCore.
+            // Enforce early-loading to workaround this issue.
+            _ = new Oid();


Here is the error I get by removing this line:
https://dev.azure.com/dnceng/public/_build/results?buildId=1459792&view=logs&j=0da5d1d9-276d-5173-c4c4-9d4d4ed14fdb&t=6c56d998-158d-5232-283c-5140104799fc

I'm open to ideas or recommendations that can help getting rid of this hack, but I would like to avoid spending time root-causing the issue.

@wfurt any ideas? Should we file this as a separate issue? Or is this related to all the targeting pack magic around?

eng/docker/libraries-sdk.linux.Dockerfile

src/libraries/System.Net.Http/tests/StressTests/HttpStress/Directory.Build.props

also delete OSX <PackageRid>, since OSX stress runs are practically unsupported

antonfirsov · 2021-11-22T12:29:02Z

/azp run runtime-libraries stress-http

azure-pipelines · 2021-11-22T12:29:12Z

Azure Pipelines successfully started running 1 pipeline(s).

antonfirsov · 2021-11-22T15:12:14Z

I followed PS convention (Copy-Item, Write-Host etc), but you are right that all dotnet/runtime scripts are named in lowercase do-something.ps1 style, so I will rename.

antonfirsov · 2021-11-22T19:38:15Z

/azp run runtime-libraries stress-http

azure-pipelines · 2021-11-22T19:38:25Z

Azure Pipelines successfully started running 1 pipeline(s).

antonfirsov · 2021-11-23T11:20:05Z

/azp run runtime-libraries stress-ssl

azure-pipelines · 2021-11-23T11:20:15Z

Azure Pipelines successfully started running 1 pipeline(s).

antonfirsov · 2021-11-23T11:20:44Z

eng/pipelines/libraries/stress/http.yml

@@ -57,6 +57,7 @@ jobs:
      export HTTPSTRESS_CLIENT_ARGS="$HTTPSTRESS_CLIENT_ARGS -http 3.0"
      export HTTPSTRESS_SERVER_ARGS="$HTTPSTRESS_SERVER_ARGS -http 3.0"
      docker-compose up --abort-on-container-exit --no-color
+    timeoutInMinutes: 35 # In case the HTTP/3.0 run hangs, we timeout shortly after the expected 30 minute run


I'm adding this because I experienced mysterious HTTP/3.0 hangs on some runs:

https://dev.azure.com/dnceng/public/_build/results?buildId=1478084&view=logs&j=0da5d1d9-276d-5173-c4c4-9d4d4ed14fdb&t=1451f5f3-0108-5a08-5b92-e984b2a85bbd

Do we know what happened? I see only blank screen (no output from the run)? Either way, we shouldn't hold this PR on it and investigate it separately.

It stopped logging after this line, then that log disappeared for some reason after the whole job becoming cancelled. Unfortunately I wasn't able to reproduce this behavior locally.

I would be surprised if this is caused by my changes, we haven't seen working stress runs on the CI for a while.

ManickaP · 2021-11-23T16:37:20Z

I can build the HttpStress project, but I cannot run it:

It was not possible to find any compatible framework version
The framework 'Microsoft.NETCore.App', version '7.0.0' (x64) was not found.
...

I have 7.0.0-alpha.1.21567.1 installed globally. I vaguely remember being able to do that with 6.0...

antonfirsov · 2021-11-23T16:53:41Z

@ManickaP docker or local? In case of local, did you use build-local.sh and the generated runscript which uses testhost extended with daily Microsoft.NETCore.App to dotnet exec the tests?

https://github.com/dotnet/runtime/blob/48a960215db417874803660e0667dce13df42dd5/src/libraries/System.Net.Http/tests/StressTests/HttpStress/Readme.md#running-the-suite-locally

https://github.com/dotnet/runtime/blob/48a960215db417874803660e0667dce13df42dd5/src/libraries/System.Net.Http/tests/StressTests/HttpStress/build-local.sh

ManickaP

Proper first pass through the change (it's big 😄)
I haven't gotten to fully test this yet though.

eng/docker/build-docker-sdk.ps1

eng/docker/libraries-sdk.linux.Dockerfile

ManickaP · 2021-11-23T16:39:31Z

src/libraries/System.Net.Http/tests/StressTests/HttpStress/StressServer.cs

+        {
+            // For some reason, System.Security.Cryptography.Encoding.dll fails to resolve when being loaded on-demand by AspNetCore.
+            // Enforce early-loading to workaround this issue.
+            _ = new Oid();


@wfurt any ideas? Should we file this as a separate issue? Or is this related to all the targeting pack magic around?

src/libraries/System.Net.Http/tests/StressTests/HttpStress/build-local.ps1

src/libraries/System.Net.Http/tests/StressTests/HttpStress/build-local.sh

ManickaP · 2021-11-23T16:50:20Z

src/libraries/System.Net.Security/tests/StressTests/SslStress/build-local.sh

@@ -0,0 +1,44 @@
+#!/usr/bin/env bash


Is this script same (or very similar) as the one in HttpStress? Should we somehow share it (in eng dir potentially)?

HttpStress doesn't need Microsoft.AspNetCore.App so in that case it's much simpler. There is a lot of duplication including msbuild stuff and scripts, but moving these bits to a shared location would make things only harder to maintain. It's already pretty hard to track what lives in /eng/docker vs individual stress projects.

If we had a 3rd stress project that would make these bits worth to dedupe.

ManickaP · 2021-11-23T16:54:04Z

src/libraries/System.Net.Http/tests/StressTests/HttpStress/build-local.sh

+    stress_configuration=${1,,}                   # Lowercase all characters in $1
+    stress_configuration=${stress_configuration^} # Uppercase first character


I don't think this really matter, I've been using lower-case interchangeably.

As a result, the whole if-else could be reduced to stress_configuration="${1:-Release}"

Sometimes the scripts refer to filesystem locations based on the configuration variable values:

runtime/src/libraries/System.Net.Http/tests/StressTests/HttpStress/build-local.sh

Line 38 in 48a9602

testhost_root=$repo_root/artifacts/bin/testhost/net$version-Linux-$libraries_configuration-x64

I learned the hard way it does matter in certain scenarios 🤣.
However, I was thinking how to make this more convenient to use, e.g.: testhost_root=$(ls -t -d $repo_root/artifacts/bin/testhost/net$version-Linux-*-x64 | head -n 1) which picks the most recent testhost dir if you do not specify which one you want to use.

So the script would work nicely without providing any argument with either release or debug testhost builds.

BTW, this is just a suggestion for possible future improvement. I certainly won't hold the PR on this.

For now I just made it fail with a verbose message if you build it with the wrong config.

ManickaP · 2021-11-23T16:57:48Z

In case of local, did you use build-local.sh

No, I used to be able to build and run against globally installed SDK/runtime, which I've used extensively. Note that my connection is not so great to pull SDKs all the time, I usually install them globally once for all clones I have.

antonfirsov · 2021-11-23T17:15:19Z

No, I used to be able to build and run against globally installed SDK/runtime, which I've used extensively. Note that my connection is not so great to pull SDKs all the time, I usually install them globally once for all clones I have.

After this PR, you really need the testhost to run the stress project. I don't know if there is a reliable way to hack the global dotnet host & ~~runtime~~ SDK to make dotnet run work with the live-built bits.

Note that my script does dotnet-install only the first time after cloning the repo just like build.sh in the root. If you really prefer to grab your globally installed Microsoft.AspNetCore.App, you need to copy it to testhost manually. If your main concern against the buildscript is the bandwidth usage, maybe we should expose a parameter aspnetlocation, so you can do:

./build-local.sh -aspnetlocation "/usr/share/dotnet/shared/Microsoft.AspNetCore.App/7.0.0-alpha.1.21567.1"

src/libraries/System.Net.Http/tests/StressTests/HttpStress/build-local.sh

ManickaP

Thanks!
The script for running stress with your local bits is very convenient, I like it a lot!

antonfirsov · 2021-11-26T18:10:53Z

/azp run runtime-libraries stress-http

azure-pipelines · 2021-11-26T18:11:03Z

Azure Pipelines successfully started running 1 pipeline(s).

antonfirsov · 2021-11-26T18:14:20Z

/azp run runtime-libraries stress-ssl

azure-pipelines · 2021-11-26T18:14:35Z

Azure Pipelines successfully started running 1 pipeline(s).

…current TFM (#61689)" This reverts commit 85642f8.

antonfirsov added 6 commits November 16, 2021 14:34

refactor stress test builds

bfd6dcd

cleanup container scripts

72c0b81

delete LiveBuildTest

3b70584

cleanup stress projects

459611d

remove unnecessarry newlines

49338ed

cleanup dockerfiles

cd243e6

antonfirsov added the area-System.Net label Nov 16, 2021

antonfirsov commented Nov 16, 2021

View reviewed changes

This comment has been minimized.

Sign in to view

safern reviewed Nov 16, 2021

View reviewed changes

eng/docker/libraries-sdk.linux.Dockerfile Outdated Show resolved Hide resolved

safern reviewed Nov 16, 2021

View reviewed changes

src/libraries/System.Net.Http/tests/StressTests/HttpStress/Directory.Build.props Outdated Show resolved Hide resolved

antonfirsov added 6 commits November 18, 2021 13:45

better way to find reference locations in stress .props

f27e4f0

also delete OSX <PackageRid>, since OSX stress runs are practically unsupported

Dockerfiles: use $VERSION, delete $ARCH

5e90f14

fix windows.Dockerfile

673b5f6

fix Dockerfiles

0af9f7b

fix linux.Dockerfile

5a04963

fix linux linux.Dockerfile

361f1e5

dotnet deleted a comment from azure-pipelines bot Nov 19, 2021

disable HTTP/3 to see if the rest succeeds

f87f45b

antonfirsov added 3 commits November 22, 2021 17:55

rename Build-Local.ps1 and extend README.md

e0a3ef8

re-enable HTTP3 with timeout

83b0835

remove trailing whitespace

48a9602

dotnet deleted a comment from azure-pipelines bot Nov 23, 2021

antonfirsov commented Nov 23, 2021

View reviewed changes

ManickaP reviewed Nov 23, 2021

View reviewed changes

src/libraries/System.Net.Http/tests/StressTests/HttpStress/build-local.sh Show resolved Hide resolved

ManickaP approved these changes Nov 25, 2021

View reviewed changes

antonfirsov added 3 commits November 26, 2021 16:01

nits

280a3df

improve build-local.sh

ab4072f

improve the rest of the scripts

6557474

antonfirsov merged commit 85642f8 into dotnet:main Nov 30, 2021

joshpeterson mentioned this pull request Nov 30, 2021

bot upstream main merge 2021 11 30 Unity-Technologies/runtime#11

Closed

agocke added a commit that referenced this pull request Dec 1, 2021

Revert "Build HttpStress and SslStress with live-built runtime using …

9ef3659

…current TFM (#61689)" This reverts commit 85642f8.

joshpeterson mentioned this pull request Dec 1, 2021

bot upstream main merge 2021 12 01 Unity-Technologies/runtime#12

Closed

ghost locked as resolved and limited conversation to collaborators Dec 30, 2021

karelz added this to the 7.0.0 milestone Jan 11, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Build HttpStress and SslStress with live-built runtime using current TFM #61689

Build HttpStress and SslStress with live-built runtime using current TFM #61689

antonfirsov commented Nov 16, 2021 •

edited

Loading

ghost commented Nov 16, 2021

antonfirsov Nov 16, 2021 •

edited

Loading

ManickaP Nov 23, 2021

This comment has been minimized.

This comment has been minimized.

antonfirsov commented Nov 22, 2021

azure-pipelines bot commented Nov 22, 2021

antonfirsov commented Nov 22, 2021

antonfirsov commented Nov 22, 2021

azure-pipelines bot commented Nov 22, 2021

antonfirsov commented Nov 23, 2021

azure-pipelines bot commented Nov 23, 2021

antonfirsov Nov 23, 2021

ManickaP Nov 23, 2021

antonfirsov Nov 23, 2021

ManickaP commented Nov 23, 2021

antonfirsov commented Nov 23, 2021 •

edited

Loading

ManickaP left a comment

ManickaP Nov 23, 2021

ManickaP Nov 23, 2021

antonfirsov Nov 23, 2021

ManickaP Nov 23, 2021

antonfirsov Nov 23, 2021

ManickaP Nov 25, 2021

ManickaP Nov 25, 2021

antonfirsov Nov 26, 2021

ManickaP commented Nov 23, 2021 •

edited

Loading

antonfirsov commented Nov 23, 2021 •

edited

Loading

ManickaP left a comment

antonfirsov commented Nov 26, 2021

azure-pipelines bot commented Nov 26, 2021

antonfirsov commented Nov 26, 2021

azure-pipelines bot commented Nov 26, 2021

		stress_configuration=${1,,} # Lowercase all characters in $1
		stress_configuration=${stress_configuration^} # Uppercase first character

Build HttpStress and SslStress with live-built runtime using current TFM #61689

Build HttpStress and SslStress with live-built runtime using current TFM #61689

Conversation

antonfirsov commented Nov 16, 2021 • edited Loading

ghost commented Nov 16, 2021

antonfirsov Nov 16, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

antonfirsov commented Nov 22, 2021

azure-pipelines bot commented Nov 22, 2021

antonfirsov commented Nov 22, 2021

antonfirsov commented Nov 22, 2021

azure-pipelines bot commented Nov 22, 2021

antonfirsov commented Nov 23, 2021

azure-pipelines bot commented Nov 23, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ManickaP commented Nov 23, 2021

antonfirsov commented Nov 23, 2021 • edited Loading

ManickaP left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ManickaP commented Nov 23, 2021 • edited Loading

antonfirsov commented Nov 23, 2021 • edited Loading

ManickaP left a comment

Choose a reason for hiding this comment

antonfirsov commented Nov 26, 2021

azure-pipelines bot commented Nov 26, 2021

antonfirsov commented Nov 26, 2021

azure-pipelines bot commented Nov 26, 2021

antonfirsov commented Nov 16, 2021 •

edited

Loading

antonfirsov Nov 16, 2021 •

edited

Loading

antonfirsov commented Nov 23, 2021 •

edited

Loading

ManickaP commented Nov 23, 2021 •

edited

Loading

antonfirsov commented Nov 23, 2021 •

edited

Loading