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

Add cross product matrix tooling #1254

Closed
wants to merge 1 commit into from
Closed

Add cross product matrix tooling #1254

wants to merge 1 commit into from

Conversation

benbp
Copy link
Member

@benbp benbp commented Dec 4, 2020

This adds scripts, docs and samples supporting dynamic, cross-product matrix generation for azure pipeline jobs.
It aims to replicate the cross-product matrix functionality in github actions,
but also adds some additional features like sparse matrix generation, cross-product includes and excludes, and (TODO) programmable matrix filters.

This functionality is made possible by the ability for the azure pipelines yaml to take a dynamic variable as an input
for a job matrix definition
(see the code sample at the bottom of the linked section).

See the README.md file for more details on the config file syntax and usage, as well as implementation details.

The tests (functions.tests.ps1) contain a lot of detail on expected data structures at various processing stages. The functions.ps1 file could perhaps be split up or use some more organization, so let me know if it's hard to navigate.

Example:

{
  "displayNames": {
    "/p:UseProjectReferenceToAzureClients=true": "UseProjectRef"
  },
  "matrix": {
    "operatingSystem": [
      "windows-2019",
      "ubuntu-18.04",
      "macOS-10.15"
    ],
    "framework": [
      "net461",
      "netcoreapp2.1",
      "net50"
    ]
  },
  "include": [
    {
      "operatingSystem": "windows-2019",
      "framework": "net461",
      "additionalTestArguments": "/p:UseProjectReferenceToAzureClients=true"
    }
  ],
  "exclude": [
    {
      "operatingSystem": "windows-2019",
      "framework": "net461"
    }
  ]
}

Sparse matrix job generation in a pipeline: https://dev.azure.com/azure-sdk/playground/_build/results?buildId=643038&view=results

image

Related discussion: microsoft/azure-pipelines-yaml#20

@benbp benbp requested a review from a team as a code owner December 4, 2020 21:37
@azure-sdk
Copy link
Collaborator

The following pipelines have been queued for testing:
java - template
js - template
net - template
python - template
You can sign off on the approval gate to test the release stage of each pipeline.
See eng/common workflow

@@ -0,0 +1,345 @@
Set-StrictMode -Version "4.0"
Copy link
Member

Choose a reason for hiding this comment

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

I know this is scoped to a folder but I'd still call this something more specific then functions.

$serialized = SerializePipelineMatrix $matrix

Write-Output $serialized.pretty
Write-Output "##vso[task.setVariable variable=matrix;isOutput=true]$($serialized.compressed)"
Copy link
Member

Choose a reason for hiding this comment

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

We are likely going to want to have a way to set the output variable name.

jobs:
- job: generate_matrix
steps:
- pwsh: |
Copy link
Member

Choose a reason for hiding this comment

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

default: 'matrix.json'

jobs:
- job: generate_matrix
Copy link
Member

Choose a reason for hiding this comment

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

I suspect this should actually be a shared yml template for folks to include as well.


#### include

The `include` field defines any number of matrices to be appended to the base matrix after processing exclusions.
Copy link
Member

Choose a reason for hiding this comment

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

Would it be after processing exclusions? I would expect the higher level filter to potentially exclude this include as well.

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 based the behavior off the github actions behavior. I think the reasoning is that you could do a broad exclude, but then need to add something back. Once we set the behavior we'll want to stick to it, so maybe we should brainstorm some scenarios. We could also support exclude/include priority but that feels like an overcomplication (which is ironic to say given what this PR is...).

Copy link
Member

Choose a reason for hiding this comment

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

I was thinking about the scenario of I want to run only one entry from the matrix for manual testing. So I was expecting a constraint that runs after the full matrix processing is done.

@@ -0,0 +1,345 @@
Set-StrictMode -Version "4.0"

function CreateDisplayName {
Copy link
Member

Choose a reason for hiding this comment

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

I tend to put write functions like function CreateDisplayName([string]$parameter, [hashtable]$displayNames) and put the { on a new line for larger scopes. While what you have is correct I've never liked using the extra verbose param for functions.

@@ -0,0 +1,23 @@
<#
Copy link
Member

Choose a reason for hiding this comment

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

instead of having 2 entry point scripts should we just make it a parameter?

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'll do that. This whole entry point is going to go away as well once I add support (which all/sparse will be implemented through).

@benbp
Copy link
Member Author

benbp commented Dec 9, 2020

Closing in favor of landing in a non-shared repo first: Azure/azure-sdk-for-net#17417

@benbp benbp closed this Dec 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants