phases to jobs schema change #1344

chcosta · 2018-11-15T17:31:25Z

Fixes https://github.com/dotnet/core-eng/issues/4560

I'm open to feedback on this, but here's my current thinking for handling the phases -> jobs breaking schema change and enabling other breaking changes as well.

eng\common\templates\phases\base.yml roughly translates to eng\common\templates\job\base.yml

For teams that want to maintain the pattern we previously established which is something like...

.vsts-ci.yml (defines configuration)
|- [template] eng\build.yml (defines steps to run per configuration)
    |- [template] eng\common\templates\phases\base.yml (modifies the job to do Arcade-y things)
|- [template] eng\common\templates\phases\publish-build-assets.yml (explicitly depends on above phases)

… they can continue that pattern with slight modification to some parameters but essentially looks like...

azure-pipelines.yml
|- eng\build.yml
   |- eng\common\templates\job\base.yml
|- eng\common\templates\job\publish-build-assets.yml

Another pattern, the pattern I'm moving dotnet/Arcade to, is to try to simplify your entry yml file such that you don't need to have the eng\build.yml layer or explicitly include publish-build-assets.yml

azure-pipelines.yml
|- eng\common\templates\jobs\base.yml

Notice that jobs\base.yml as opposed to job\base.yml is referenced. jobs\base.yml wraps job\base.yml but uses "each" to modify the passed jobs and to automatically handle the dependency stuff for "publish-build-assets.yml"

For variables, notice that I'm moving from treating them as an object to the slightly uglier array syntax but that is so that we can support variable groups via yaml.

@vtbassmatt for any additional feedback

I'll be adding others for additional feedback but I wanted to get your thoughts first @mmitche @vtbassmatt

jcagme · 2018-11-15T18:14:40Z

azure-pipelines.yml

@@ -0,0 +1,112 @@
+variables:
+  - name: _HelixType
+    value: build/product


build/product [](start = 11, length = 13)

Could you please make this build/product/ so the classifier tool finds failed builds? Also, most of the other repos also do build/product/ #Resolved

jcagme · 2018-11-15T21:20:53Z

eng/common/templates/jobs/base.yml

+  # Required: A collection of jobs to run - https://docs.microsoft.com/en-us/azure/devops/pipelines/yaml-schema?view=vsts&tabs=schema#job
+  jobs: []
+
+  # Optional: If specified, 


, [](start = 26, length = 1)

'.' ?

jcagme · 2018-11-15T21:38:03Z

In general looks good. What's the plan on removing eng/common/templates/phases/*?

chcosta · 2018-11-15T23:34:00Z

No current plan. I think that we could set a date some time early next year to delete that folder if it's a problem. I imagine that BYOC is going to force everyone to move out of phases and into job sooner than later.

jcagme · 2018-11-16T00:23:21Z

I don't think there is a problem only not to forget that those would be orphaned files. BYOC should serve as a reminder then

chcosta · 2018-11-16T16:25:37Z

/cc @riarenas

riarenas · 2018-11-16T16:57:57Z

eng/common/templates/jobs/base.yml

+  # Optional: If specified, 
+  helixRepo: ''
+
+  publishBuildAssetsDependsOn: ''


publishBuildAssetsDependsOn: '' [](start = 1, length = 32)

This one is missing a description

riarenas · 2018-11-16T17:19:38Z

eng/build.yml

@@ -1,98 +0,0 @@
-parameters:


We reference this file in https://github.com/dotnet/arcade/blob/master/Documentation/AzureDevOps/AzureDevOpsOnboarding.md#notes-about-yaml

riarenas · 2018-11-16T17:21:04Z

LGTM. Do you plan to make the documentation changes as part of this PR, or create a followup issue/pr for that?

chcosta · 2018-11-16T17:23:16Z

I'm going to include some documentation with this PR so that I don't have to hand-hold everyone through consuming this change. I wanted to get feedback on the direction before writing anything.

riarenas · 2018-11-16T17:25:06Z

Ooh. I think it'll help people if once this is in, we make a PR to the minimalci-sample repo and reference the commit from our docs. That will give them an idea of what the transition entails.

chcosta · 2018-11-16T17:25:11Z

Also, I'm trying to think of better names than base.yml cuz if you get some yaml problem in your build for whatever reason it just tells you the filename and the line number so you wouldn't know if your error was in job\base.yml or jobs\base.yml

mmitche · 2018-11-19T16:05:04Z

@chcosta taking a look now

mmitche · 2018-11-19T16:52:20Z

azure-pipelines.yml

+      - name: _HelixBuildConfig
+        value: $(_BuildConfig)
+      # Only enable publishing in non-public, non PR scenarios.
+      - ${{ if and(ne(variables['System.TeamProject'], 'public'), notin(variables['Build.Reason'], 'PullRequest')) }}:


Given that these are likely going to be identical between a lot of repos, would it make more sense to pull these variables into a template?

_HelixBuildConfig makes sense and could easily be moved into job\base.yml.

I can't decide if _PublishBlobFeedUrl, _SignArgs, _PublishArgs, and _OfficialBuildIdArgs makes sense. I guess it's optional whether a repo actually uses those values (though they couldn't redefine them). Alternatively, they could be provided behind some other parameter (enableDefaultArgs or something), but that feels a bit overly magical. I guess, because of dependency flow it's all in your local repo and that takes some of the magic out.

I'm conflicted, because it would clean up azure-pipelines.yml, but then things are a little weird in the sense that this code block

- script: eng\common\cibuild.cmd -configuration $(_BuildConfig) -prepareMachine $(_PublishArgs) $(_SignArgs) $(_OfficialBuildIdArgs)

makes no sense at all unless you go look at another file, plus we're forced into a tighter contract where we will define those args in the base templates until the end of time or risk breaking some repo.

My preference, for now, is to move _HelixBuildConfig but leave the rest up to the repo to define.

Yeah that's a good point.

We will eventually be removing all these publishing steps, since they will go into a release pipebuild after BAR publish. It will probably eventually just say something like "publishPackages=true, publishSymbols=true"

I like that direction

chcosta · 2018-11-20T00:14:52Z

I'll merge this in the morning and then kick off builds / look for issues. Tuesday (11/27), I'll write up some documentation on this change cuz I'm sure it won't be obvious to people when to use jobs\base.yml vs directly referencing job\base.yml

markwilkie · 2018-11-26T17:52:43Z

cc/ @garath

chcosta added 2 commits November 15, 2018 09:13

phases to jobs schema change

6676a17

remove v2 from publish-build-assets

575f156

jcagme reviewed Nov 15, 2018

View reviewed changes

pr feedback, add slash

920e5ae

jcagme reviewed Nov 15, 2018

View reviewed changes

riarenas reviewed Nov 16, 2018

View reviewed changes

chcosta added 2 commits November 17, 2018 21:14

move variable inclusion

d222b57

fix parameter

1e01e88

mmitche reviewed Nov 19, 2018

View reviewed changes

mmitche approved these changes Nov 19, 2018

View reviewed changes

Move some helix variables into the templates

7663c73

chcosta added 4 commits November 20, 2018 08:27

rename base.yml

d03d038

change default type for publishbuildassetsdependson

7e0c0b1

remove doc reference to build.yml

44d283a

Remove reference to build.yml

4c19da2

chcosta merged commit af5dae4 into dotnet:master Nov 20, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

phases to jobs schema change #1344

phases to jobs schema change #1344

chcosta commented Nov 15, 2018

jcagme Nov 15, 2018 •

edited

Loading

jcagme Nov 15, 2018

jcagme commented Nov 15, 2018

chcosta commented Nov 15, 2018

jcagme commented Nov 16, 2018

chcosta commented Nov 16, 2018

riarenas Nov 16, 2018

riarenas Nov 16, 2018

riarenas commented Nov 16, 2018

chcosta commented Nov 16, 2018

riarenas commented Nov 16, 2018

chcosta commented Nov 16, 2018

mmitche commented Nov 19, 2018

mmitche Nov 19, 2018

chcosta Nov 19, 2018 •

edited

Loading

mmitche Nov 19, 2018

mmitche Nov 19, 2018

chcosta Nov 19, 2018

chcosta commented Nov 20, 2018

markwilkie commented Nov 26, 2018

phases to jobs schema change #1344

phases to jobs schema change #1344

Conversation

chcosta commented Nov 15, 2018

jcagme Nov 15, 2018 • edited Loading

Choose a reason for hiding this comment

jcagme Nov 15, 2018

Choose a reason for hiding this comment

jcagme commented Nov 15, 2018

chcosta commented Nov 15, 2018

jcagme commented Nov 16, 2018

chcosta commented Nov 16, 2018

riarenas Nov 16, 2018

Choose a reason for hiding this comment

riarenas Nov 16, 2018

Choose a reason for hiding this comment

riarenas commented Nov 16, 2018

chcosta commented Nov 16, 2018

riarenas commented Nov 16, 2018

chcosta commented Nov 16, 2018

mmitche commented Nov 19, 2018

mmitche Nov 19, 2018

Choose a reason for hiding this comment

chcosta Nov 19, 2018 • edited Loading

Choose a reason for hiding this comment

mmitche Nov 19, 2018

Choose a reason for hiding this comment

mmitche Nov 19, 2018

Choose a reason for hiding this comment

chcosta Nov 19, 2018

Choose a reason for hiding this comment

chcosta commented Nov 20, 2018

markwilkie commented Nov 26, 2018

jcagme Nov 15, 2018 •

edited

Loading

chcosta Nov 19, 2018 •

edited

Loading