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.

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

CI: Fix conditions for running mono, and coreclr jobs #75645

Merged
merged 6 commits into from
Sep 15, 2022

Conversation

radical
Copy link
Member

@radical radical commented Sep 14, 2022

  • this will fix the case where src/mono/ changes didn't trigger some
    mono jobs that depend on corresponding coreclr jobs.
  1. Fix conditions for runtime tests
For all the non-wasm mono runtime tests:
- run if runtime tests themselves have changes
- or run if there are any mono changes
  1. run coreclr and library build jobs when mono_except_wasm, coreclr, or installer have changes.
  2. Disable AOT offsets, and crossaot jobs for wasm as they are not currently used.
  3. runtime-dev-innerloop: condition jobs for coreclr, or mono accordingly
  4. runtime-linker-tests: adjust conditions for wasm, and non wasm jobs
  5. Run the installer jobs in runtime.yml only if the dependencies succeed
... otherwise they will fail when trying to get the results of
those dependencies. For example, `Installer Build and Test coreclr
OSX_arm64 Release` fails with:

[error]Artifact CoreCLRProduct___OSX_arm64_release not found for build 17835. Please ensure you have published artifacts in any previous phases of the current build.

.. when `CoreCLR Product Build OSX arm64 release` timed out.

@radical
Copy link
Member Author

radical commented Sep 14, 2022

cc @fanyang-mono

@ghost ghost assigned radical Sep 14, 2022
Copy link
Member

@trylek trylek left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for fixing this!

@radical
Copy link
Member Author

radical commented Sep 14, 2022

@trylek One of the issues that I run into is that if you have a JobA depending on a JobB, and JobB is skipped for some reason then even JobA will get skipped, because the condition on the jobs has and(succeeded(), ... I'm wondering how we can catch such cases. I know we can get the result for a dependency and check it for =="Skipped", which maybe we can use in a "first step" to fail if any of the dependencies got skipped? Or is there a better way to do this?

@trylek
Copy link
Member

trylek commented Sep 14, 2022

Hmm, what is the situation where depending on a skipped job should result in a failure? Normally I would expect that if jobA gets skipped and jobB depends on it it's natural that jobB gets skipped too as it sits on a pruned branch of the build tree. I agree that if jobB requires some jobA to succeed and jobA gets skipped whereas jobB does not, that indeed seems conflicting and should trigger a failure. Can you please share a more detailed example where you're hitting this situation in the yml files?

@radical
Copy link
Member Author

radical commented Sep 14, 2022

Hmm, what is the situation where depending on a skipped job should result in a failure? Normally I would expect that if jobA gets skipped and jobB depends on it it's natural that jobB gets skipped too as it sits on a pruned branch of the build tree. I agree that if jobB requires some jobA to succeed and jobA gets skipped whereas jobB does not, that indeed seems conflicting and should trigger a failure.

I'm thinking that the "leaf" jobs that will run, should have their full dependency also run. At least that makes sense to me in the cases where JobA wants to use results of JobB .

Can you please share a more detailed example where you're hitting this situation in the yml files?

Looking at https://dev.azure.com/dnceng-public/public/_build/results?buildId=7053&view=logs&j=8687d66b-8ae3-5bcb-b2ee-49e1823a16f0 , the job shows:

Expanded: and(False, or(eq(dependencies['evaluate_paths']['outputs']['SetPathVars_mono_excluding_wasm.containsChange'], True), eq(dependencies['evaluate_paths']['outputs']['SetPathVars_runtimetests.containsChange'], True), eq(variables['isR...

From the expanded yaml:

  - job: run_test_p0_monollvmfullaot__Linux_x64_release
    condition: >-
      and(succeeded(), or(
        eq(dependencies.evaluate_paths.outputs['SetPathVars_mono_excluding_wasm.containsChange'], true),
        eq(dependencies.evaluate_paths.outputs['SetPathVars_runtimetests.containsChange'], true),
        eq(variables['isRollingBuild'], true)))
    ...
    dependsOn:
    - evaluate_paths
    - mono_common_test_build_p0_AnyOS_AnyCPU_release
    - coreclr__product_build_Linux_x64_release
    - mono_llvmaot_product_build_Linux_x64_release
    - libraries_build_Linux_x64_Debug
    displayName: mono llvmfullaot Pri0 Runtime Tests Run  Linux x64 release
..
  • IIUC, the job is getting skipped because succeeded() is False because coreclr__product_build_Linux_x64_release was skipped
  • Because of this if we don't have the correct conditions on coreclr__product_build_Linux_x64_release, and it is skipped, then even though we have the correct conditions on the leaf job run_test_p0_monollvmfullaot__Linux_x64_release, it will also be skipped.
  • And this will get noticed only if you are explicitly looking for it

And this became an issue because I have been changing the paths that trigger various builds. And I'm not sure how to catch cases where a build is unexpectedly getting skipped, because I messed up the condition on a dependency.

@radical
Copy link
Member Author

radical commented Sep 14, 2022

In this PR for instances, I broadened the condition so all the mono, and coreclr jobs will get triggered on mono changes. But not all of the coreclr jobs are needed for mono runs - only the desktop ones.
And trying to map from the expanded yaml back to our various templates seems to be non-trivial.

@radical radical changed the title Run all mono, and coreclr jobs when we have mono_excluding_wasm changes CI: Fix conditions for running mono, and coreclr jobs Sep 15, 2022
@radical radical requested a review from trylek September 15, 2022 02:02
@radical
Copy link
Member Author

radical commented Sep 15, 2022

Updated to have better conditions. Still waiting for the evaluate jobs to run on all the pipelines, to test this.

@radical radical marked this pull request as ready for review September 15, 2022 03:24
For all the non-wasm mono runtime tests:
- run if runtime tests themselves have changes
- or run if there are any mono changes
.. when mono_except_wasm, coreclr, or installer have changes.
.. as they are not currently used.
.. succeed, otherwise they will fail when trying to get the results of
those dependencies. For example, `Installer Build and Test coreclr
OSX_arm64 Release` fails with:

```
[error]Artifact CoreCLRProduct___OSX_arm64_release not found for build 17835. Please ensure you have published artifacts in any previous phases of the current build.
```
.. when `CoreCLR Product Build OSX arm64 release` timed out.
Copy link
Member

@fanyang-mono fanyang-mono left a comment

Choose a reason for hiding this comment

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

The previous skipped runtime CI lanes are running now. Thanks for the prompt fix!

@fanyang-mono
Copy link
Member

fanyang-mono commented Sep 15, 2022

The failures on mono llvmaot Pri0 Runtime Tests Run Linux x64 release has been captured by #75606. Feel free to merge if the change is complete.

@radical radical deleted the ci-fix-mono-jobs branch September 16, 2022 19:48
@ghost ghost locked as resolved and limited conversation to collaborators Oct 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants