microsoft / go

The Microsoft build of the Go toolset
BSD 3-Clause "New" or "Revised" License
274 stars 27 forks source link

Generate build pipeline YML to avoid unclear template usage #1073

Open dagood opened 12 months ago

dagood commented 12 months ago

@gdams mentioned yml generation in a meeting (another team uses this general approach), and I really like it. So, I wrote it up as a pseudo-ADR, picking MADR in particular, to try out that structure. (Although there are some "I" and clear opinions that I think I'd swap out if I were writing this to check in.)

Context and Problem Statement

We use AzDO Pipeline templates a lot. To overcome reusability limitations in the pipeline yml language, we have templates that accept a "inner" template's filename as a parameter and calls the "inner" template with some additional parameters. This can be quite hard to unravel when reading straight through without already knowing the end goal.

This works ok once you're used to it, and sticking to template-time logic avoids a lot of other odd AzDO Pipelines evaluation behavior. However, adding additional layers of parameters-based-on-parameters is a significant mental tax for both implementers and readers, encouraging copy-paste and quick hacks over reuse, and resulting in unclear template name distinctions like pool.yml and pool-core.yml that only exist to create two template contexts.

This applies to microsoft/go for build/test/sign/publish, but also https://github.com/microsoft/go-infra for release automation pipelines. Those also use complicated pipeline yml patterns that would benefit from generation, perhaps even more than microsoft/go pipelines.

Decision Drivers

Considered Options

Decision Outcome

Generate build pipeline YML.

Consequences

However:

Confirmation

Some templates should still be used to make the resulting yml readable, and keep sharing functionality with .NET Arcade. However, a reasonable complexity metric would be: no template accepts a template as a parameter.

More Information

Example template accepting a template, used to make parameters that depend on other parameters. Templates are from eng/pipeline/stages. Our pipeline entrypoint calls a "matrix" template to set up the list of jobs:

stages:
  - template: stages/go-builder-matrix-stages.yml
    parameters:
      innerloop: true

The go-builder-matrix-stages template calls a "wrapper" template that will expand the objects in the shorthandBuilders array, telling the wrapper template what template to run internally once each object has additional properties added to it:

stages:
  - template: shorthand-builders-to-builders.yml
    parameters:
      jobsTemplate: builders-to-stages.yml
      jobsParameters:
        sign: ${{ parameters.sign }}
        createSourceArchive: ${{ parameters.createSourceArchive }}
        releaseVersion: ${{ parameters.releaseVersion }}
      shorthandBuilders:
        - ${{ if eq(parameters.innerloop, true) }}:
          - { os: linux, arch: amd64, config: buildandpack }
          - { os: linux, arch: amd64, config: devscript }
          - { os: linux, arch: amd64, config: test }
[...]

shorthand-builders-to-builders exists to create values that can be reused by the inner jobsTemplate. Without adding another layer of templates, you can't refer to one yml element's value from another one to build upon an existing value.

stages:
  - template: ${{ parameters.jobsTemplate }}
    parameters:
      ${{ insert }}: ${{ parameters.jobsParameters }}
      builders:
        - ${{ each builder in parameters.shorthandBuilders }}:
          - ${{ insert }}: ${{ builder }}
            # Use 'default' in place of null to define ID. This value just needs to be unique and
            # only contain "[A-z_]+".
            id: ${{ builder.os }}_${{ coalesce(builder.distro, 'default') }}_${{ coalesce(builder.hostArch, 'default') }}_${{ builder.arch }}_${{ builder.config }}_${{ coalesce(builder.experiment, 'default') }}_${{ coalesce(builder.fips, false) }}
            ${{ if not(builder.hostArch) }}:
              hostArch: ${{ builder.arch }}
[...]

In this case, jobsTemplate is builders-to-stages, which takes the expanded builder objects and passes them into the actual job template, adding more pipeline-specific parameters on top.

stages:
  - ${{ each builder in parameters.builders }}:
    - template: pool.yml
      parameters:
        inner:
          template: run-stage.yml
          parameters:
            builder: ${{ builder }}
            createSourceArchive: ${{ parameters.createSourceArchive }}
            releaseVersion: ${{ parameters.releaseVersion }}
[...]
gdams commented 11 months ago

@dagood agree with all the suggestions here. This is a great ADR writeup! Do you want to split this up into sub tasks and we can look at implementing this?

dagood commented 11 months ago

Sure! Here are some parts that we went over in the Go sync, with some additional thoughts:

Related opportunity:

dagood commented 1 week ago

Changing the go-infra release pipelines to use a release agent (https://github.com/microsoft/go-lab/issues/122) removes a significant amount of the complex templates in go-infra. This has made me think about this issue again from a more critical perspective.

I still agree with this goal:

People new to the team should be able to follow how each pipeline works with minimal AzDO pipeline experience.

However, I think we can improve our pipelines to be understandable without writing an abstraction in Go. My concerns are generally that the pipelines would then be brittle, harder to debug, and tedious to work on for new reasons. You'd need to know a lot about AzDO Pipelines and also our abstraction. Security changes, for example, will be shown to us as AzDO pipeline yml. The dev work is then not only to update our pipeline, but also edit the abstraction to emit the right yml.

Something that's coming to mind that I think I like is actually just improving/adding some relatively small AzDO yml preprocessing features. So, we would still edit AzDO yml (an augmented version of it), and our tool would preprocess the extra features into ordinary AzDO yml.

Feature ideas that could address current pain points:

Better value and expression reuse

A major reason for the deeply nested templates is simply to share some simple values at template-evaluation-time.

We can subtly improve the - template: syntax to support more types of templates that copy values into places we can't with AzDO's limited types of templates. This could fill in small values, or the template could e.g. write an inlined loop of our set of platforms in a more intuitive way.

Perhaps each file could have a top-level expressionliterals: key that contains common expressions that can then be reused later in the same file like ${{ expressionliterals.IsWindows }}. This would be useful to clarify the purpose of an expression in a centralized location while keeping the runtime logic (e.g. steps) concise.

To share more logic in microsoft/go pipelines with a source of truth in microsoft/go-infra, we can embed some templates into the tool and share via the eng/_util dependency.

Better computed parameter reuse

Sometimes deeply nested templates are there to simply reuse a calculation, like ${{ startsWith(variables['Build.SourceBranch'], 'refs/heads/microsoft/release-branch') }}.

I think we could inline the expression into any place it's used without calculating the result ourselves (which we can't always do).

We could generate nested AzDO yml templates to create actually-shared values. (This may not be feasible because AzDO pipelines have nesting limits.)

Better looping

This nested hierarchy used to be worse, but still screams for a feature that simplifies it and makes it not fragile if the number of steps needs to change:

https://github.com/microsoft/go-infra/blob/a1e18487bf2bb90a38eee940a095decb0e60cac6/eng/pipelines/release-build-pipeline.yml#L108-L136

The release agent work will remove the need for this pattern, so I think the justification here for loop features is slim. But, worth mentioning because the release agent is why I'm posting this comment. 🙂

Another place we use loops is the retry logic in microsoft/go, and it could potentially be improved, but it's honestly not all that fragile or hard to read IMO.