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 1es changes for job/matrix generation and publish #7758

Merged
merged 6 commits into from
Mar 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
140 changes: 140 additions & 0 deletions eng/common/pipelines/templates/jobs/generate-job-matrix.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
parameters:
- name: AdditionalParameters
type: object
- name: DependsOn
type: object
default: null
- name: CloudConfig
type: object
default: {}
- name: MatrixConfigs
type: object
default: []
- name: MatrixFilters
type: object
default: []
- name: MatrixReplace
type: object
default: {}
- name: JobTemplatePath
type: string
# Set this to false to do a full checkout for private repositories with the azure pipelines service connection
- name: SparseCheckout
type: boolean
default: true
- name: SparseCheckoutPaths
type: object
default: []
- name: Pool
type: string
default: $(LINUXPOOL)
- name: OsVmImage
Copy link
Member

Choose a reason for hiding this comment

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

Is this still interesting after the migration?

Copy link
Member

Choose a reason for hiding this comment

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

Similar question for the Pool 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.

Correct me if I'm wrong @scbedd, but we found demands does not work for the Azure Pipelines mac pool, and we still need to pass vmImage for that. Additionally for the migration we'll want to use the main pools and specify the alternate image.

Copy link
Member

@scbedd scbedd Feb 27, 2024

Choose a reason for hiding this comment

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

Correct me if I'm wrong @scbedd, but we found demands does not work for the Azure Pipelines mac pool, and we still need to pass vmImage for that. Additionally for the migration we'll want to use the main pools and specify the alternate image.

Correct. The specific combo of

pool:
  name: 
  vmImage: 
  os: linux|windows|macOS

works for both devops + the additional agent checks. I swear I tried everything else and could not get them to work otherwise.

type: string
default: $(LINUXVMIMAGE)
- name: Os
type: string
default: linux
# This parameter is only necessary if there are multiple invocations of this template within the SAME STAGE.
# When that occurs, provide a name other than the default value.
- name: GenerateJobName
type: string
default: 'generate_job_matrix'
- name: PreGenerationSteps
type: stepList
default: []
# Mappings to OS name required at template compile time by 1es pipeline templates
- name: Pools
type: object
default:
- name: Linux
filter: .*Linux.*Pool$
os: linux
- name: Windows
filter: .*Windows.*Pool$
os: windows
- name: Mac
filter: .*MacPool$
os: macOS

jobs:
- job: ${{ parameters.GenerateJobName }}
variables:
- template: /eng/pipelines/templates/variables/image.yml
Copy link
Member

Choose a reason for hiding this comment

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

image.yml should be part of this PR I think.

Copy link
Member Author

Choose a reason for hiding this comment

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

Originally the idea was to have image.yml be repo specific, but it is a hard requirement since this has a reference.

Copy link
Member Author

Choose a reason for hiding this comment

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

@weshaggard maybe this is something we do want in eng/common just to avoid hidden file dependencies in our templates. However if we wanted to roll out image changes per-repo it would not work.

Copy link
Member

Choose a reason for hiding this comment

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

I don't mind having a default one in eng\common and perhaps we can come up with a way to override the image in the language repo if needed. We could even make this a default template parameter that could be set to something as needed.

Copy link
Member Author

Choose a reason for hiding this comment

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

One way would be to have the underlying values get set via coalesce e.g. $[ coalesce(variables.LinuxPoolOverride, variables.LinuxPool) ] but I'm not too excited about the config variable bloat of doubling all the values with *Override.

- name: skipComponentGovernanceDetection
value: true
- name: displayNameFilter
value: $[ coalesce(variables.jobMatrixFilter, '.*') ]
pool:
name: ${{ parameters.Pool }}
vmImage: ${{ parameters.OsVmImage }}
os: ${{ parameters.Os }}
${{ if parameters.DependsOn }}:
dependsOn: ${{ parameters.DependsOn }}
steps:
# Skip sparse checkout for the `azure-sdk-for-<lang>-pr` private mirrored repositories
# as we require the github service connection to be loaded.
- ${{ if and(parameters.SparseCheckout, not(contains(variables['Build.DefinitionName'], '-pr - '))) }}:
- template: /eng/common/pipelines/templates/steps/sparse-checkout.yml
parameters:
${{ if ne(length(parameters.SparseCheckoutPaths), 0) }}:
Paths: ${{ parameters.SparseCheckoutPaths }}
${{ if and(eq(length(parameters.SparseCheckoutPaths), 0), ne(parameters.AdditionalParameters.ServiceDirectory, '')) }}:
Paths:
- "sdk/${{ parameters.AdditionalParameters.ServiceDirectory }}"

- ${{ parameters.PreGenerationSteps }}

- ${{ each config in parameters.MatrixConfigs }}:
- ${{ each pool in parameters.Pools }}:
- ${{ if eq(config.GenerateVMJobs, 'true') }}:
- task: Powershell@2
inputs:
pwsh: true
filePath: eng/common/scripts/job-matrix/Create-JobMatrix.ps1
arguments: >
-ConfigPath ${{ config.Path }}
-Selection ${{ config.Selection }}
-DisplayNameFilter '$(displayNameFilter)'
-Filters '${{ join(''',''', parameters.MatrixFilters) }}', 'container=^$', 'SupportedClouds=^$|${{ parameters.CloudConfig.Cloud }}', 'Pool=${{ pool.filter }}'
-Replace '${{ join(''',''', parameters.MatrixReplace) }}'
-NonSparseParameters '${{ join(''',''', config.NonSparseParameters) }}'
displayName: Create ${{ pool.name }} Matrix ${{ config.Name }}
name: vm_job_matrix_${{ config.Name }}_${{ pool.name }}

- ${{ if eq(config.GenerateContainerJobs, 'true') }}:
- task: Powershell@2
inputs:
pwsh: true
filePath: eng/common/scripts/job-matrix/Create-JobMatrix.ps1
arguments: >
-ConfigPath ${{ config.Path }}
-Selection ${{ config.Selection }}
-DisplayNameFilter '$(displayNameFilter)'
-Filters '${{ join(''',''', parameters.MatrixFilters) }}', 'container=^$', 'SupportedClouds=^$|${{ parameters.CloudConfig.Cloud }}', 'Pool=${{ pool.filter }}'
-NonSparseParameters '${{ join(''',''', config.NonSparseParameters) }}'
displayName: Create ${{ pool.name }} Container Matrix ${{ config.Name }}
name: container_job_matrix_${{ config.Name }}_${{ pool.name }}

- ${{ each config in parameters.MatrixConfigs }}:
- ${{ each pool in parameters.Pools }}:
- ${{ if eq(config.GenerateVMJobs, 'true') }}:
- template: ${{ parameters.JobTemplatePath }}
parameters:
UsePlatformContainer: false
OSName: ${{ pool.os }}
Matrix: dependencies.${{ parameters.GenerateJobName }}.outputs['vm_job_matrix_${{ config.Name }}_${{ pool.name }}.matrix']
DependsOn: ${{ parameters.GenerateJobName }}
CloudConfig: ${{ parameters.CloudConfig }}
${{ each param in parameters.AdditionalParameters }}:
${{ param.key }}: ${{ param.value }}

- ${{ if eq(config.GenerateContainerJobs, 'true') }}:
- template: ${{ parameters.JobTemplatePath }}
parameters:
UsePlatformContainer: true
OSName: ${{ pool.os }}
Matrix: dependencies.${{ parameters.GenerateJobName }}.outputs['vm_job_matrix_${{ config.Name }}_${{ pool.name }}.matrix']
DependsOn: ${{ parameters.GenerateJobName }}
CloudConfig: ${{ parameters.CloudConfig }}
${{ each param in parameters.AdditionalParameters }}:
${{ param.key }}: ${{ param.value }}
30 changes: 30 additions & 0 deletions eng/common/pipelines/templates/steps/publish-1es-artifact.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# This step is used to prevent duplication of artifact publishes when there is an issue that would prevent the overall success of the job.
# Ensuring that we only publish when successful (and two a differently named artifact otherwise) will allow easy retry on a build pipeline
# without running into the "cannot override artifact" failure when we finally do get a passing run.

# ArtifactName - The name of the artifact in the "successful" case.
# ArtifactPath - The path we will be publishing.
# CustomCondition - Used if there is additional logic necessary to prevent attempt of publish.

parameters:
ArtifactName: ''
ArtifactPath: ''
CustomCondition: true

steps:
- pwsh: |
Write-Host "##vso[task.setvariable variable=PublishArtifactName;]${{ parameters.ArtifactName }}"
Copy link
Member

Choose a reason for hiding this comment

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

condition: and(succeeded(), ${{ parameters.CustomCondition }})
displayName: Set Artifact Name

- pwsh: |
Write-Host "##vso[task.setvariable variable=PublishArtifactName;]${{ parameters.ArtifactName }}-FailedAttempt$(System.JobAttempt)"
condition: and(failed(), ${{ parameters.CustomCondition }})
displayName: Set Failed Artifact Name

- task: 1ES.PublishPipelineArtifact@1
condition: and(succeeded(), ${{ parameters.CustomCondition }})
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we remove or at least update this condition to be failedOrSucceeded()?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, yes, good catch. I'll do another PR. FYI @scbedd but shouldn't be necessary for your testing.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

I would suggest succededOrFailed here as well in your pwsh step to set the artifact name. https://learn.microsoft.com/en-us/azure/devops/pipelines/process/expressions?view=azure-devops#succeededorfailed

displayName: 'Publish ${{ parameters.ArtifactName }} Artifacts'
inputs:
artifact: '$(PublishArtifactName)'
path: '${{ parameters.ArtifactPath }}'