Add 1es changes for job/matrix generation and publish #7758
Conversation
benbp
added
the
Central-EngSys
benbp
force-pushed
the
benbp/common-template-tasks
branch
from
cf61332 to
75d988d
Compare
| jobs: | ||
| - job: ${{ parameters.GenerateJobName }} | ||
| variables: | ||
| - template: /eng/pipelines/templates/variables/image.yml |
There was a problem hiding this comment.
image.yml should be part of this PR I think.
There was a problem hiding this comment.
Originally the idea was to have image.yml be repo specific, but it is a hard requirement since this has a reference.
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
The following pipelines have been queued for testing: |
| - name: Pool | ||
| type: string | ||
| default: azsdk-pool-mms-ubuntu-2204-general | ||
| - name: OsVmImage |
There was a problem hiding this comment.
Is this still interesting after the migration?
There was a problem hiding this comment.
Similar question for the Pool parameter.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
vmImagefor 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.
| steps: | ||
| - task: 1ES.PublishPipelineArtifact@1 | ||
| condition: and(succeeded(), ${{ parameters.CustomCondition }}) | ||
| ${{ if parameters.DisplayName }}: |
There was a problem hiding this comment.
As mentioned in the other PR I'd suggest just moving to DisplayName everywhere and for the existing places just map them to call DisplayName: Publish ArtifactName Artifacts if we really care about the string being the same. We could also potentially use a coalesce but that might not work as well for the failed condition.
edit: Another option is just to force everyone to use the standard Publish ${{ parameters.ArtifactName }} Artifacts as I'm not sure if we need this flexibility.
There was a problem hiding this comment.
Updated a bit to remove the conditionals. I think keeping the naming for now is helpful but we can go in and remove custom ones after.
There was a problem hiding this comment.
Looks like you updated the publish-artifacts.yml file but not this one. Do you plan to keep both for the transition or did you edit the wrong one.
There was a problem hiding this comment.
Thanks for catching that, fixed.
|
The following pipelines have been queued for testing: |
|
The following pipelines have been queued for testing: |
Sync eng/common directory with azure-sdk-tools for PR Azure/azure-sdk-tools#7758 See [eng/common workflow](https://github.com/Azure/azure-sdk-tools/blob/main/eng/common/README.md#workflow) --------- Co-authored-by: Ben Broderick Phillips <[email protected]>
| displayName: Set Failed Artifact Name | ||
|
|
||
| - task: 1ES.PublishPipelineArtifact@1 | ||
| condition: and(succeeded(), ${{ parameters.CustomCondition }}) |
There was a problem hiding this comment.
Shouldn't we remove or at least update this condition to be failedOrSucceeded()?
There was a problem hiding this comment.
Ah, yes, good catch. I'll do another PR. FYI @scbedd but shouldn't be necessary for your testing.
There was a problem hiding this comment.
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
|
|
||
| steps: | ||
| - pwsh: | | ||
| Write-Host "##vso[task.setvariable variable=PublishArtifactName;]${{ parameters.ArtifactName }}" |
There was a problem hiding this comment.
To help reduce the number of injected steps can we use $env:AGENT_JOBSTATUS -eq "Failed" see https://learn.microsoft.com/en-us/azure/devops/pipelines/build/variables?view=azure-devops&tabs=yaml for definition of that variable. https://learn.microsoft.com/en-us/azure/devops/pipelines/process/expressions?view=azure-devops#failed
Adding these files as separate copies of their originals so we can do a rollout more easily.
archetype-sdk-tests-generate.ymltogenerate-job-matrix.yml. Changes to this file include generating a matrix with corresponding env vars for OS pools/agents, and passing in the now requiredOSNameparameter which cannot be set as a variable for the 1es templates.publish-artifact.ymltopublish-1es-artifact.ymlto use the 1es publishing wrapper task. I don't think this task is available in a backwards compatible way without the repo reference and/or onboarding we're having to do, see https://dev.azure.com/azure-sdk/internal/_build/results?buildId=3503459&view=logs&j=96791242-dbf3-587e-3a06-ae5af5c1a705&t=8e5c4a57-b21e-59c3-59e8-95279bb1be65&l=16 for example.