Skip to content
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.

Sign up for GitHub

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

Infrastructure: correct dependencies and clean helix agent from stray corerun procs #54094

Merged
merged 3 commits into from
Jun 13, 2021
Merged

Infrastructure: correct dependencies and clean helix agent from stray corerun procs #54094

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
232aabe
Add dependency on coreclr in monojit/monointerpreter jobs explicitly …
hoyosjs Jun 12, 2021
d6bc1e8
Use Helix Pre/Post hooks to kill linkering corerun instances to work …
hoyosjs Jun 12, 2021
01c10aa
Update src/coreclr/scripts/superpmi.proj
hoyosjs Jun 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions eng/pipelines/common/templates/runtimes/run-test-job.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ jobs:
- '${{ parameters.runtimeFlavor }}_common_test_build_p1_AnyOS_AnyCPU_${{parameters.buildConfig }}'
- ${{ if ne(parameters.stagedBuild, true) }}:
- ${{ if or( eq(parameters.runtimeVariant, 'minijit'), eq(parameters.runtimeVariant, 'monointerpreter')) }}:
# This is needed for creating a CORE_ROOT in the current design.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

courious, shouldn't this be when runtimeFlavor is mono? that way we cover all mono flavors? Or are minjit/monointerpreter always a value for when we are running tests on mono?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the ones where I saw the issue, I thought about it, but I am not sure how the other mono runs work.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@naricc do you know?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@safern Its just that with minijit/monointerpreter, there is no seperate product build (we only have one build that has both of them). Other runtime variants do have their own package, thus it needs to be parameterized by the runtimeVariant.

- ${{ format('coreclr_{0}_product_build_{1}{2}_{3}_{4}', '', parameters.osGroup, parameters.osSubgroup, parameters.archType, parameters.buildConfig) }}
# minijit and mono interpreter runtimevariants do not require any special build of the runtime
- ${{ format('{0}_{1}_product_build_{2}{3}_{4}_{5}', parameters.runtimeFlavor, '', parameters.osGroup, parameters.osSubgroup, parameters.archType, parameters.buildConfig) }}
- ${{ if not(or(eq(parameters.runtimeVariant, 'minijit'), eq(parameters.runtimeVariant, 'monointerpreter'))) }}:
Expand Down
7 changes: 6 additions & 1 deletion src/coreclr/scripts/superpmi.proj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,10 @@
<WorkItemTimeout>5:00</WorkItemTimeout>
</PropertyGroup>

<ItemGroup Condition=" '$(AGENT_OS)' == 'Windows_NT' ">
<HelixPreCommand Include="taskkill.exe /f /im corerun.exe"/>
<HelixPostCommand Include="taskkill.exe /f /im corerun.exe"/>
</ItemGroup>
<!-- We're currently not using .dotnet on the Helix agents -->
<!--
<ItemGroup Condition=" '$(AGENT_OS)' == 'Windows_NT' ">
Expand All @@ -94,11 +98,12 @@
<ItemGroup Condition=" '$(AGENT_OS)' != 'Windows_NT' ">
<HelixPreCommand Include="export PATH=$HELIX_CORRELATION_PAYLOAD/.dotnet:$PATH" Condition=" '$(CollectionType)' == 'crossgen2' "/>
</ItemGroup>
-->

<PropertyGroup>
<HelixPreCommands>@(HelixPreCommand)</HelixPreCommands>
<HelixPostCommands>@(HelixPostCommand)</HelixPostCommands>
</PropertyGroup>
-->

<ItemGroup>
<HelixCorrelationPayload Include="$(CorrelationPayloadDirectory)">
Expand Down
7 changes: 6 additions & 1 deletion src/libraries/sendtohelixhelp.proj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,11 @@
<HelixPreCommand Condition="'$(TargetsWindows)' != 'true' and '$(BrowserHost)' != 'windows'" Include="export MONO_ENV_OPTIONS='$(MonoEnvOptions)'" />
</ItemGroup>

<ItemGroup Condition="'$(TargetsWindows)' == 'true' or '$(BrowserHost)' == 'windows'">
<HelixPreCommand Include="taskkill.exe /f /im corerun.exe"/>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we also add similar command for unix?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather get this in sooner rather than later given the stability issues. I am not sure if all unix systems we have can use pkill, otherwise something like ps aux | grep -i corerun | grep -v grep | awk '{ print $2 }' | xargs kill -9 would work.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. Lets get this in first.

<HelixPostCommand Include="taskkill.exe /f /im corerun.exe"/>
</ItemGroup>

<ItemGroup Condition="'$(TargetOS)' == 'Browser' and '$(BrowserHost)' != 'windows'">
<HelixPreCommand Condition="'$(Scenario)' != 'WasmTestOnBrowser'" Include="export XHARNESS_COMMAND=test" />
<HelixPreCommand Condition="'$(Scenario)' == 'WasmTestOnBrowser'" Include="export XHARNESS_COMMAND=test-browser" />
Expand All @@ -82,7 +87,7 @@
</ItemGroup>

<PropertyGroup Condition="'$(TargetOS)' == 'Browser'">
<!--
<!--
We are hosting the payloads for the WASM/browser on kestrel in the xharness process.
We also run some network tests to this server and so, we are running it on both HTTP and HTTPS.
For the HTTPS endpoint we need development SSL certificate.
Expand Down
18 changes: 12 additions & 6 deletions src/tests/Common/helixpublishwitharcade.proj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,15 @@
PALTestsDir=$(_PALTestsDir)
</_PropertiesToPass>


<_PropertiesToPass Condition="'$(TargetOS)' == 'Browser' Or '$(TargetOS)' == 'Android'">
$(_PropertiesToPass);
IncludeDotNetCli=$(IncludeDotNetCli);
DotNetCliRuntime=$(DotNetCliRuntime);
DotNetCliPackageType=$(DotNetCliPackageType);
DotNetCliVersion=$(DotNetCliVersion)
</_PropertiesToPass>
</PropertyGroup>
</PropertyGroup>
<Message Text="DotNetCliVersion: $(DotNetCliVersion)" Importance="High" />
<Message Text="DotNetCliPackageType: $(DotNetCliPackageType)" Importance="High" />
<Message Text="HelixRuntimeRid: $(HelixRuntimeRid)" Importance="High" />
Expand Down Expand Up @@ -95,7 +95,7 @@
<IncludeDotNetCli>true</IncludeDotNetCli>
<DotNetCliPackageType>runtime</DotNetCliPackageType>
<DotNetCliVersion>$(BundledNETCoreAppPackageVersion)</DotNetCliVersion>
<DotNetCliRuntime>$(HelixRuntimeRid)</DotNetCliRuntime>
<DotNetCliRuntime>$(HelixRuntimeRid)</DotNetCliRuntime>
</PropertyGroup>

<PropertyGroup Condition="'$(TargetOS)' == 'Browser' Or '$(TargetOS)' == 'Android'">
Expand Down Expand Up @@ -168,7 +168,7 @@
<!-- Remove the managed pdbs from our payloads.
This is for performance reasons to reduce our helix payload size -->
<ReducedPayloadFiles Include="@(_PayloadFiles)" Condition=" '%(Extension)' != '.pdb' " />

<ReducedPayloadFilesFinal Include="@(ReducedPayloadFiles)" Condition=" '%(Extension)' != '.sh' and '$(TestWrapperTargetsWindows)' == 'true' "/>
<ReducedPayloadFilesFinal Include="@(ReducedPayloadFiles)" Condition=" '%(Extension)' != '.cmd' and '$(TestWrapperTargetsWindows)' != 'true' "/>
</ItemGroup>
Expand Down Expand Up @@ -202,7 +202,7 @@
</Target>

<Target Name="PreparePALTestArchive">
<Exec Condition="'$(PALTestsDir)' != '' and '$(TestWrapperTargetsWindows)' != 'true'"
<Exec Condition="'$(PALTestsDir)' != '' and '$(TestWrapperTargetsWindows)' != 'true'"
WorkingDirectory="$(PALTestsDir)"
Command="tar -czvf $(PayloadsRootDirectory)paltests.tar.gz `ls -A`"/>
</Target>
Expand Down Expand Up @@ -234,7 +234,7 @@
<RunCrossGen2 Condition=" '$(RunCrossGen2)' != 'true' ">false</RunCrossGen2>
<LongRunningGCTests Condition=" '$(LongRunningGCTests)' != 'true' ">false</LongRunningGCTests>
<GcSimulatorTests Condition=" '$(GcSimulatorTests)' != 'true' ">false</GcSimulatorTests>
<TestRunNamePrefix Condition="'$(RuntimeFlavorDisplayName)' != ''">$(RuntimeFlavorDisplayName) </TestRunNamePrefix>
<TestRunNamePrefix Condition="'$(RuntimeFlavorDisplayName)' != ''">$(RuntimeFlavorDisplayName) </TestRunNamePrefix>
<TestRunNamePrefix Condition=" '$(RunCrossGen)' == 'true' ">R2R </TestRunNamePrefix>
<TestRunNamePrefix Condition=" '$(RunCrossGen2)' == 'true' ">R2R-CG2 </TestRunNamePrefix>
<TestRunNamePrefix Condition=" '$(Scenario)' == 'normal' ">$(TestRunNamePrefix)$(TargetOS) $(TargetArchitecture) $(Configuration) @ </TestRunNamePrefix>
Expand All @@ -251,6 +251,7 @@
<!-- WARNING: HelixPreCommand ItemGroup is intentionally minimal and should be kept that way. -->

<ItemGroup Condition=" '$(TestWrapperTargetsWindows)' == 'true' ">
<HelixPreCommand Include="taskkill.exe /f /im corerun.exe"/>
<HelixPreCommand Include="set CORE_ROOT=%HELIX_CORRELATION_PAYLOAD%" />
<!-- Set _NT_SYMBOL_PATH so VM _ASSERTE() asserts can find the symbol files when doing stack walks -->
<HelixPreCommand Include="set _NT_SYMBOL_PATH=%HELIX_CORRELATION_PAYLOAD%\PDB" />
Expand All @@ -268,6 +269,10 @@
<HelixPreCommand Include="type %__TestEnv%" />
</ItemGroup>

<ItemGroup Condition=" '$(TestWrapperTargetsWindows)' == 'true' ">
<HelixPostCommand Include="taskkill.exe /f /im corerun.exe"/>
</ItemGroup>

<ItemGroup Condition=" '$(TestWrapperTargetsWindows)' != 'true' ">
<HelixPreCommand Include="export CORE_ROOT=$HELIX_CORRELATION_PAYLOAD" />
<HelixPreCommand Include="export RunCrossGen2=1" Condition=" '$(RunCrossGen)' == 'true' " />
Expand All @@ -286,6 +291,7 @@

<PropertyGroup>
<HelixPreCommands>@(HelixPreCommand)</HelixPreCommands>
<HelixPostCommands>@(HelixPostCommand)</HelixPostCommands>
</PropertyGroup>

<PropertyGroup Condition=" '$(TestWrapperTargetsWindows)' == 'true' ">
Expand Down