[DRAFT] VMR Verification in SBRP PRs

[NikolaMilosavljevic]

Verification of changes needed for source-only runs of the VMR build pipeline.

[NikolaMilosavljevic]

Hmm, default value for isSourceOnlyBuild is false. Not sure if it's possible to specify a different value of the source-build-reference-package-unified-build pipeline.

The alternative would be to rename this pipeline to be source-build-reference-packages-source-build, and update the template to set isSourceOnlyBuild based on the pipeline name, similar to how we had it in old sdk pipelines/templates: https://github.com/dotnet/sdk/blob/9dee3cf508254bc3859d9cd2fe96aeb2ad1b9751/eng/pipelines/vmr-build-pr.yml#L56-L58

Another option would be to have a different template for source-only builds. Pipelines would reference the template that is most appropriate for the repo, i.e. all legs, source-only, Windows-only, etc.

@MichaelSimons

[MichaelSimons]

Hmm, default value for isSourceOnlyBuild is false. Not sure if it's possible to specify a different value of the source-build-reference-package-unified-build pipeline.

The alternative would be to rename this pipeline to be source-build-reference-packages-source-build, and update the template to set isSourceOnlyBuild based on the pipeline name, similar to how we had it in old sdk pipelines/templates: https://github.com/dotnet/sdk/blob/9dee3cf508254bc3859d9cd2fe96aeb2ad1b9751/eng/pipelines/vmr-build-pr.yml#L56-L58

Another option would be to have a different template for source-only builds. Pipelines would reference the template that is most appropriate for the repo, i.e. all legs, source-only, Windows-only, etc.

@MichaelSimons

I'm not fond of a name convention based approach. It is not intuitive and doesn't provide a very good UX for changing the configuration down the road. It would be nice to have a scalable solution that would solve all future scenarios e.g. Windows only legs, single linux leg, etc. plus a combination of configurations

Could the parameters be changed to variables which can then be saved in the AzDO pipeline configuration?

[NikolaMilosavljevic]

Hmm, default value for isSourceOnlyBuild is false. Not sure if it's possible to specify a different value of the source-build-reference-package-unified-build pipeline.
The alternative would be to rename this pipeline to be source-build-reference-packages-source-build, and update the template to set isSourceOnlyBuild based on the pipeline name, similar to how we had it in old sdk pipelines/templates: https://github.com/dotnet/sdk/blob/9dee3cf508254bc3859d9cd2fe96aeb2ad1b9751/eng/pipelines/vmr-build-pr.yml#L56-L58
Another option would be to have a different template for source-only builds. Pipelines would reference the template that is most appropriate for the repo, i.e. all legs, source-only, Windows-only, etc.
@MichaelSimons

I'm not fond of a name convention based approach. It is not intuitive and doesn't provide a very good UX for changing the configuration down the road. It would be nice to have a scalable solution that would solve all future scenarios e.g. Windows only legs, single linux leg, etc. plus a combination of configurations

Could the parameters be changed to variables which can then be saved in the AzDO pipeline configuration?

Thanks - yes we could use variables instead - will test it out, and expand for other scenarios.

[NikolaMilosavljevic]

/azp run source-build-reference-packages-unified-build

[azure-pipelines]

Azure Pipelines failed to run 1 pipeline(s).

[NikolaMilosavljevic]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[NikolaMilosavljevic]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s), but failed to run 1 pipeline(s).

[NikolaMilosavljevic]

So, this works, but it is a draft PR and will not be merged.

[NikolaMilosavljevic]

Based on the comment above (#1262 (comment)) I've investigated what is possible and implemented a solution using one of the available options. There is a different way to solve this, and I wanted to to call out those two options and close on this, as it will become a plan for other types of build job conditioning.

We want to allow repos to run VMR Verification of the smaller set of jobs, than what is a default set for PRs. For instance, in this repo, source-build-reference-packages, we want only source-build (source-only) jobs to run. To allow this, we need to set some variable in pipeline definition.

This variable is a runtime variable, and can only be used in a limited number of places and scenarios, due to the following:

• Runtime variables are always of a string type
• It cannot be used in compile-time conditions, i.e. if statements that condition template inclusions
• It can be passed as a parameter, but only if it stays as string, i.e. cannot be converted to boolean when passing to a template - this would be a compile-time conversion
• It can be used directly in stage.condition or job.condition parameters, to condition specific stages and jobs

Based on the goal and constraints, there are 2 options:

1. Pass this variable as a parameter, from the main template - this allows the use, of the value, in all compile-time conditions. This is how it's currently implemented in this PR and the dependent, pending, change in VMR: https://github.com/dotnet/dotnet/compare/ac2aaf2..137a824
2. Use the runtime variable directly in stage.condition parameter, to condition stages independently. This would require conditions in many more places, i.e. here, here and here

Option 1 seems cleaner as all unified-build jobs get conditioned with a single if condition, of a template. The downside is the use of a string type parameter, that represents a variable that we treat as boolean (false vs true, or non-false). In option 2, in stage.condition we would have a condition that uses the runtime variable, directly, without a need for a template parameter. It would still be a string based comparison.

In this case, where we want to skip all unified-build stages/jobs, I prefer option 1, as it seems a better way to condition larger groups of stages/jobs.

In a different case, where we, for instance, want to only run Windows jobs, we could also use a parameter, to pass it around templates, or we could use the runtime variable directly in job.condition parameter.

Runtime variable name could be more descriptive - instead of isSourceOnlyBuild we could use something like runOnlySourceBuildJobs, runOnlyWindowsJobs

@ViktorHofer @MichaelSimons @mmitche what do you think of this model and proposal for conditioning VMR build jobs?

We do have an issue tracking a request for allowing a scoped-down builds of unified-build pipeline: dotnet/dotnet#1068 This PR, and dependent VMR changes, will contribute to that work.

[MichaelSimons]

I think Option 1 is the more attractive option. With this option, is it possible to define the parameter as a bool and do a conversion? This could be extended to support casing variants.

${{ eq(variables['isSourceOnlyBuild'], 'true') }}

[NikolaMilosavljevic]

I think Option 1 is the more attractive option. With this option, is it possible to define the parameter as a bool and do a conversion? This could be extended to support casing variants.

${{ eq(variables['isSourceOnlyBuild'], 'true') }}

It's not possible unfortunately, as conversion to bool would happen at compile-time. I also tried the option that was suggested by Copilot - $[eq(variables['isSourceOnlyBuild'], 'true')] which also didn't work as this evaluates to a string, not bool.

Here's what I got in a chat:

Common pitfalls
Don't use $(aBC) for boolean parameters—it returns a string, not a boolean.
Don't use ${{ variables.aBC }} unless the variable is defined in the YAML itself (compile-time).
Don't quote the result of $[eq(...)]—that would turn it into a string.

Runtime variable can be used to conditionally include a stage or template - option 2 above. But, cannot be converted to boolean in a compile-time conversion, to be passed in to a template. This is a really unfortunate constraint.

[mmitche]

Another option (already discussed) would be to, rather than having eng/common/vmr-build-pr.yml be the root YAML for the job, have a repo implement a stub. The stub would have the triggers and call vmr-build-pr.yml, specifying that this is a repo that only needs source-build legs.

Runtime variable can be used to conditionally include a stage or template - option 2 above. But, cannot be converted to boolean in a compile-time conversion, to be passed in to a template. This is a really unfortunate constraint.

Is it possible to use the conditional include of a template to include a variables template which defines boolean typed variables, which then get used? Therefore you're not actually converting the variable directly to a boolean.

[NikolaMilosavljevic]

Another option (already discussed) would be to, rather than having eng/common/vmr-build-pr.yml be the root YAML for the job, have a repo implement a stub. The stub would have the triggers and call vmr-build-pr.yml, specifying that this is a repo that only needs source-build legs.

This is interesting and likely a cleaner option, as we'd be able to use bool parameter in vmr templates. It also aids maintainability as it's easier to update the YML than to request an update pipeline configuration - something that is not owned by the repo owners.

@MichaelSimons @ViktorHofer any opinion on this approach? It makes pipeline yml and intent more clear and descriptive, and any modification becomes straightforward.

I will modify the pending VMR PR accordingly, based on feedback.

Runtime variable can be used to conditionally include a stage or template - option 2 above. But, cannot be converted to boolean in a compile-time conversion, to be passed in to a template. This is a really unfortunate constraint.

Is it possible to use the conditional include of a template to include a variables template which defines boolean typed variables, which then get used? Therefore you're not actually converting the variable directly to a boolean.

I believe those variables are evaluated at compile-time. It's worth a check.

[MichaelSimons]

@MichaelSimons @ViktorHofer any opinion on this approach? It makes pipeline yml and intent more clear and descriptive, and any modification becomes straightforward.

I think this is worthwhile. I would like to see what this would look like.

[NikolaMilosavljevic]

@MichaelSimons @ViktorHofer any opinion on this approach? It makes pipeline yml and intent more clear and descriptive, and any modification becomes straightforward.

I think this is worthwhile. I would like to see what this would look like.

Will prepare a draft PR to test this out.

[NikolaMilosavljevic]

@MichaelSimons @ViktorHofer any opinion on this approach? It makes pipeline yml and intent more clear and descriptive, and any modification becomes straightforward.

I think this is worthwhile. I would like to see what this would look like.

Here's how the changes would look: c51f6bf

With this approach, we would be adding a requirement for another repo-specific YML. Various elements need to be moved to this new YML, including trigger:, pr:, variables: and resources:

This would diminish the value of the current shared pipeline YML. The benefit is that we can use YML boolean parameters to condition VMR verification. The sample isn't complete, but I have added a, currently unused, parameter for conditioning VMR for Windows builds.

@MichaelSimons @mmitche

[mmitche]

@MichaelSimons @ViktorHofer any opinion on this approach? It makes pipeline yml and intent more clear and descriptive, and any modification becomes straightforward.

I think this is worthwhile. I would like to see what this would look like.

Here's how the changes would look: c51f6bf

With this approach, we would be adding a requirement for another repo-specific YML. Various elements need to be moved to this new YML, including trigger:, pr:, variables: and resources:

This would diminish the value of the current shared pipeline YML. The benefit is that we can use YML boolean parameters to condition VMR verification. The sample isn't complete, but I have added a, currently unused, parameter for conditioning VMR for Windows builds.

@MichaelSimons @mmitche

The root pipeline is still a template that is easy to copy between repos, so no concerns there.

One suggestion is that I would avoid going with booleans to conditionalize the VMR PR testing that is performed. Instead, long term it may be cleaner to pass this info as a list. e.g.:

parameters:
  - name: verifications
    type: object
    default: []

jobs:

# Job for A
- ${{ if containsValue(parameters.verifications, 'source-build') }}:
  - job: TestA
    steps:
    - script: echo "Do source-build check"

[NikolaMilosavljevic]

@MichaelSimons @ViktorHofer any opinion on this approach? It makes pipeline yml and intent more clear and descriptive, and any modification becomes straightforward.

I think this is worthwhile. I would like to see what this would look like.

Here's how the changes would look: c51f6bf
With this approach, we would be adding a requirement for another repo-specific YML. Various elements need to be moved to this new YML, including trigger:, pr:, variables: and resources:
This would diminish the value of the current shared pipeline YML. The benefit is that we can use YML boolean parameters to condition VMR verification. The sample isn't complete, but I have added a, currently unused, parameter for conditioning VMR for Windows builds.
@MichaelSimons @mmitche

The root pipeline is still a template that is easy to copy between repos, so no concerns there.

One suggestion is that I would avoid going with booleans to conditionalize the VMR PR testing that is performed. Instead, long term it may be cleaner to pass this info as a list. e.g.:

parameters:
  - name: verifications
    type: object
    default: []

jobs:

# Job for A
- ${{ if containsValue(parameters.verifications, 'source-build') }}:
  - job: TestA
    steps:
    - script: echo "Do source-build check"

Yeah, I agree with a list of verifications, and the intent to use these types of checks for PR verification jobs only. The alternative would be to use these conditions for every job we run, which would make YMLs much harder to read.

However, verifications parameter needs to have a default value that enables running all of the current jobs, to not regress the VMR build.

This would imply that we need the following default list of items:: unified-build, source-build, windows, linux, ios, browser, android. The alternative would be: unified-build-linux, unified-build-windows, etc.

verifications parameter, with correct set of default values, needs to exist in both VMR template (vmr-build.yml stage) and in arcade's vmr-build-pr.yml`. This adds maintenance cost, to keep these in sync.

We'd end up with something list this:

jobs:

# Job for android - unified-build
- ${{ if and(containsValue(parameters.verifications, 'unified-build'), containsValue(parameters.verifications, 'android')) }}:
  - template: ../jobs/vmr-build.yml
    ...

# Job for Windows - unified-build
- ${{ if and(containsValue(parameters.verifications, 'unified-build'), containsValue(parameters.verifications, 'windows')) }}:
  - template: ../jobs/vmr-build.yml
    ...

[mmitche]

I don't think you'd need the default list to exist in both templates. You'd set the default in the outer template to something like default, then pass that to the inner template. If the inner template receives default then the default list is used.

[NikolaMilosavljevic]

I don't think you'd need the default list to exist in both templates. You'd set the default in the outer template to something like default, then pass that to the inner template. If the inner template receives default then the default list is used.

Hmm, never used that before. Is default a magic value telling Azdo preprocessor to use the default values in the inner template? Or would I need to check for this value and initialize some inner value (perhaps variable) based on the supplied value of this parameter?

What we will have are 3 levels of YMLs:

repo-specific.yml

- template arcade-shared.yml
  parameters:
    verifications: source-build

arcade-shared.yml

parameters:
- name: verifications
  type: object
  default:
    source-build
    unified-build
    ...etc...

- template vmr-template.yml@vmr
  parameters:
    verifications: {{ parameters.verifications }}

vmr-template.yml

parameters:
- name: verifications
  type: object
  default:
    source-build
    unified-build
    ...etc...

jobs:
# Job for android - unified-build
- ${{ if and(containsValue(parameters.verifications, 'unified-build'), containsValue(parameters.verifications, 'android')) }}:
  - template: ../jobs/vmr-build.yml

vmr-template.yml definitely needs to have a default list of values, to allow VMR to build all default verification jobs.

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[NikolaMilosavljevic]

@NikolaMilosavljevic I think you can keep your nicely prefixed items, which I think are more readable. I think you can do the following when conditionalizing the stages:

${{ if contains(join(';',parameters.verifications), 'unified-build-') }}

That joins all elements of the verifications array to one string separated by ;, then checks a contains for your prefix.

Here are the final changes that work: dotnet/dotnet@main...vmrverification.list

Full verification build was successful: https://dev.azure.com/dnceng/internal/_build/results?buildId=2729174&view=results

I will test few more repo-side options in this PR and switch to a repo-specific pipeline yml - we will get rid of shared arcade yml.

[MichaelSimons]

Here are the final changes that work: dotnet/dotnet@main...vmrverification.list

This looks good. One thing that I just realized is that we should split the source-build verification into stage1 and stage2. SBRP should only need stage1 which is what I would expect other repos would want as well.

[NikolaMilosavljevic]

Here are the final changes that work: dotnet/[email protected]

This looks good. One thing that I just realized is that we should split the source-build verification into stage1 and stage2. SBRP should only need stage1 which is what I would expect other repos would want as well.

Yeah, makes sense, I was thinking of that. How about source-build-stage1 and source-build-stage2? Also, if just stage2 job is specified, we would still build stage1 job, as it is a prerequisite.

[NikolaMilosavljevic]

/azp run source-build-reference-packages-unified-build

[azure-pipelines]

Azure Pipelines failed to run 1 pipeline(s).

[MichaelSimons]

Yeah, makes sense, I was thinking of that. How about source-build-stage1 and source-build-stage2? Also, if just stage2 job is specified, we would still build stage1 job, as it is a prerequisite.

Makes sense.

[NikolaMilosavljevic]

/azp run source-build-reference-packages-unified-build

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[NikolaMilosavljevic]

/azp run source-build-reference-packages-unified-build

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[NikolaMilosavljevic]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[NikolaMilosavljevic]

/azp run source-build-reference-packages-unified-build

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[mmitche]

WHat's the difference between scope and verifications?

[NikolaMilosavljevic]

scope is used to condition jobs in CI and PRs, in an all-or-nothing fashion. lite jobs are considered a subset of full.

verifications contains the list of jobs that are considered to be verifications and currently ran in both lite and full scopes. This parameter allow us to only run specific verifications. This might only make sense for lite scope, therefore I added the scope parameter to be explicit.

Here's where scope is currently used: https://github.com/dotnet/dotnet/blob/1ec8c1e9aebde14c1813d1c878dda7fd38ab298f/eng/pipelines/templates/stages/source-build-stages.yml#L65 and https://github.com/dotnet/dotnet/blob/1ec8c1e9aebde14c1813d1c878dda7fd38ab298f/eng/pipelines/templates/stages/vmr-verticals.yml#L171

lite is the first value listed, so it is the implicit default value - https://github.com/dotnet/dotnet/blob/1ec8c1e9aebde14c1813d1c878dda7fd38ab298f/eng/pipelines/templates/stages/vmr-build.yml#L13-L20 We might not need to specify scope: lite in repo pipeline.

[mmitche]

Got it.

[NikolaMilosavljevic]

/azp run source-build-reference-packages-unified-build

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[NikolaMilosavljevic]

/azp run source-build-reference-packages-unified-build

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[NikolaMilosavljevic]

/azp run source-build-reference-packages-unified-build

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[NikolaMilosavljevic]

/azp run source-build-reference-packages-unified-build

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).