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

[WIP] Generic provider template #1140

Draft
wants to merge 17 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 15 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
2 changes: 1 addition & 1 deletion provider-ci/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ format:
# files for other bridged provider repositories should be ephemeral.
.PHONY: test-providers test-provider/%

test-providers: test-provider/aws test-provider/docker test-provider/cloudflare test-provider/acme
test-providers: test-provider/aws test-provider/docker test-provider/cloudflare test-provider/acme test-provider/eks

# 1. Delete all files except the .ci-mgmt.yaml file and run the provider-ci generate command.
# 2. Copy the generated provider repository to a temporary git repo and run actionlint on it.
Expand Down
12 changes: 12 additions & 0 deletions provider-ci/internal/pkg/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,14 @@ type Config struct {
// ExtraTests run as part of `run-acceptance-tests.yml`, `master.yml`,
// `main.yml`, `prerelease.yml` and `release.yml`. Only used for aws:
// https://github.com/search?q=org%3Apulumi+path%3A.ci-mgmt.yaml+%22extraTests%3A%22&type=code
//
// Not available in generic providers -- override make targets instead.
ExtraTests map[string]any `yaml:"extraTests"` // Only used by AWS...

// IntegrationTestProvider will run e2e tests in the provider as well as in
// the examples directory when set to true. Defaults to false.
//
// Not available in generic providers -- override make targets instead.
IntegrationTestProvider bool `yaml:"integrationTestProvider"`

// TestPulumiExamples runs e2e tests using the examples and test suite in
Expand Down Expand Up @@ -204,6 +208,9 @@ type Config struct {
// tests in CI job. This should be provided from a secret. Used by the
// docker provider only:
// https://github.com/search?q=org%3Apulumi+path%3A.ci-mgmt.yaml+%22sshPrivateKey%3A%22&type=code
//
// Not available in generic providers -- see docker-build for an example of
// how to programatically generate a key.
SSHPrivateKey string `yaml:"sshPrivateKey"`

// GCP authenticates with GCP before running tests in CI job. Used in gcp
Expand All @@ -219,6 +226,8 @@ type Config struct {
// SetupScript executes a script before running tests in CI job. Used in 3
// providers:
// https://github.com/search?q=org%3Apulumi+path%3A.ci-mgmt.yaml+%22setup-script%3A%22&type=code
//
// Not available in generic providers -- override make targets instead.
SetupScript string `yaml:"setup-script"`

// GenerateNightlyTestWorkflow will include the nightly-test workflow. Used
Expand Down Expand Up @@ -255,6 +264,9 @@ type Config struct {
// https://github.com/search?q=org%3Apulumi+path%3A.ci-mgmt.yaml+%22parallel%3A%22&type=code
Parallel int `yaml:"parallel"`

// Shards controls how many jobs integration tests are distributed across.
Shards int `yaml:"shards"`

// Hybrid has no effect but is set by the docker provider.
// https://github.com/search?q=org%3Apulumi+path%3A.ci-mgmt.yaml+%22hybrid%3A%22&type=code
Hybrid bool `yaml:"hybrid"`
Expand Down
2 changes: 2 additions & 0 deletions provider-ci/internal/pkg/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ func getTemplateDirs(templateName string) ([]string, error) {
case "external-bridged-provider":
// Render more specific templates last to allow them to override more general templates.
return []string{"dev-container", "provider", "bridged-provider"}, nil
case "generic":
return []string{"provider", "pulumi-provider", "generic"}, nil
Comment on lines +105 to +106
Copy link
Member

Choose a reason for hiding this comment

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

When we use this template for the native boilerplate, we'll probably add external-generic which will exclude the pulumi-provider sub-template.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or maybe "third-party"?

Copy link
Member

Choose a reason for hiding this comment

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

Just above is the external-bridged-provider template which is like bridged-provider but with bits removed for third parties. Whatever name we use should aim to keep these consistent.

default:
return nil, fmt.Errorf("unknown template: %s", templateName)
}
Expand Down
137 changes: 137 additions & 0 deletions provider-ci/internal/pkg/templates/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
# Provider templates

This directory contains all of the templates we use for generating GitHub
workflows (among other things) for Pulumi providers.

These templates are composable and additive, for example many templates mix in
the "pulumi-provider" to include our code of conduct.

The full list of supported templates is available in
[`generate.go`](../generate.go), but this documentation focuses on the
"generic" template and the general direction and design principles we should
apply when modifying these templates.

## Generic template

The [`generic`](./generic) template was forked from our battle-tested
[`bridged`](./bridged-provider) template with an eye towards generalizing
things such that we could enable _all_ providers to be managed by `ci-mgmt` --
with an eventual goal of allowing third-party parties to benefit from this
tooling as well.

(This is still a work in progress and the current state of the template may not
yet fully reflect these goals.)

After running the bridged template for a number of years several problems
emerged:

1. Accumulation of special-casing and one-off configuration options adds
complexity to workflows and makes it harder to maintain and reason about all
possible workflow behaviors.

2. Over-reliance on GitHub actions for setting up CI environments makes it
difficult to reproduce failures locally. For example it's very easy for CI
to use a different version of `golangci-lint` than what you have locally.

3. A tight coupling of tooling and workflows means that workflow updates can
require manual intervention when tooling changes are included. For example
workflows can fail until someone manually resolves errors due to a
`golantci-lint` update.

With those problems in mind we have a couple principles for these templates
going forward:

1. Inversion of control: The provider should be the source of truth for as much
as possible, and `ci-mgmt` should be as "dumb" as possible. The provider/CI
interaction should be driven entirely by `make` targets, and `ci-mgmt`
should know nothing about the provider's implementation details -- not even
the language of the provider.

2. Local first: CI should leverage the same setup steps that a developer would
run locally.

3. Separation of concerns: Workflows and tooling can and should be managed
separately. It is OK for a long-tail provider to use an older version of
`golangci-lint` if we haven't yet had an opportunity to update its code, but
that should not prevent it from being released if we need to ship an urgent
fix.

Concretely, this means:
* We should avoid adding new configuration that leaks implementation details of
the provider to `ci-mgmt`.
* We should provide sane default `make` targets but allow the provider to
override them if necessary.
* We should prefer to perform setup as part of a `make` target or as part of
tests instead of adding additional GitHub actions.

## Contract

The generic template drives all workflows via `make` targets.
(If an action _doesn't_ invoke a `make` target that's a bug!)

A `./bin` and `./sdk` must exist at the root of the provider's repo.

Targets should be parallelizable (`-j`).

### Required targets

#### Prerequisites

This workflow is the first step run during releases, pre-releases, PR tests,
and merges to main.

* `make install_plugins`: (TODO: Use a more generic `make prepare` or just drop
this.)
* `make schema`: Ensures generated schema is in place.
Copy link
Member

Choose a reason for hiding this comment

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

This could be a file target of bin/schema.json which can then be expected to be in place and included as a publishable artifact along with the provider binaries for reference (and perhaps integration with the registry in the future). This could still also output the schema to any other required locations as needed.

* `make provider`: Builds the provider binary.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* `make provider`: Builds the provider binary.
* `make provider`: Builds the provider binary for the purpose of local testing. The final distributable binary is built using `make provider_dist-*`.

Copy link
Member

Choose a reason for hiding this comment

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

This could also be a file target, though slightly more tricky because the name of the binary changes per provider, so perhaps just documenting it's in the bin directory is enough.

* `make test_provider`: Runs "fast" tests, typically unit tests for the
provider. These tests should run fast enough to not need sharding across
multiple workers. The provider is responsible for deciding how to run these,
but default behavior will be to execute `go test ./...` under the
`./provider` path.

#### Build provider

This workflow is run during releases, pre-releases, PR tests, and merges to
main after the prerequisites step has succeeded.

* `make provider_dist-${OS}-${ARCH}`: (TODO: use a file path) Responsible for
building a provider binary under `./bin` for the given architecture and OS.
These binaries will be uploaded and re-used in later steps.

#### Resync build

* `make build`: A single target to re-build everything (schema, SDKs, binaries,
etc.).

#### Build SDK

This workflow is run during releases, pre-releases, PR tests, and merges to
main after the prerequisites step has succeeded.

* `make build_${language}`: Generates the SDK for the given language.

#### Test

This workflow is run during releases, pre-releases, PR tests, and merges to
main after the provider binaries and SDKs have been generated.

This differs from the `bridged` template in that sharding is arbitrary and left
to the discretion of the provider. Typically we have use fixed shards based on
languages, but this is restrictive and a poor developer experience in general
(https://github.com/pulumi/ci-mgmt/issues/676).

* `make install_sdks`: Install SDKs for all available languages.
* `make shard`: This target takes two environment variables -- `$SHARD_TOTAL`
and `$SHARD_INDEX` -- and is responsible for determining tests to run for
this shard. It will probably mutate the environment in some way, for example
by appending to `$GITHUB_ENV`, in order to inform the `test_shard` target.
* `make test_shard`: This target is responsible for executing tests identified
in the `shard` target.
Comment on lines +125 to +130
Copy link
Member

Choose a reason for hiding this comment

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

I'm a little confused why we have two targets here. It feels like we shouldn't need make shard at all .. and we don't want to have to tell the provider author to know how to interact with github env.

Perhaps we should be just using make test or make integration_test with variables for SHARD_TOTAL and SHARD_INDEX e.g. make integration_test SHARD_TOTAL=5 SHARD_INDEX=3.

It would also be nice to link from here to an example of how to do sharding with go test.


## Configuring a template

[`config.go`](../config.go) contains all of the allowable options for `.ci-mgmt.yaml` files.

While it's possible to add new options here, in general we would like to reduce
the amount of configuration options available.
2 changes: 2 additions & 0 deletions provider-ci/internal/pkg/templates/defaults.config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,8 @@ checkUpstreamUpgrade: true
# Used in 5 providers: https://github.com/search?q=org%3Apulumi+path%3A.ci-mgmt.yaml+%22goBuildParallelism%22&type=code
#goBuildParallelism: 1

shards: 10

# Sets PULUMI_CONVERT to 1 if truthy
# Is set to "1" in 74 providers: https://github.com/search?q=org%3Apulumi+path%3A.ci-mgmt.yaml+%22pulumiConvert%22&type=code
#pulumiConvert: false
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
name: Download binary assets
description: Downloads provider binaries to `bin/`.

runs:
using: "composite"
steps:
- name: Download provider binaries
uses: #{{ .Config.ActionVersions.DownloadArtifact }}#
with:
name: #{{ .Config.Provider }}#-provider.tar.gz
path: ${{ github.workspace }}/bin
- name: Untar provider binaries
shell: bash
run: |
tar -zxf ${{ github.workspace }}/bin/provider.tar.gz -C ${{ github.workspace}}/bin
find ${{ github.workspace }} -name "pulumi-*-#{{ .Config.Provider }}#" -print -exec chmod +x {} \;
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
name: Upload bin assets
description: Uploads the provider binaries to `bin/`.

runs:
using: "composite"
steps:
- name: Tar provider binaries
shell: bash
run: tar -zcf ${{ github.workspace }}/bin/provider.tar.gz -C ${{ github.workspace }}/bin/ pulumi-resource-#{{ .Config.Provider }}#
- name: Upload artifacts
uses: #{{ .Config.ActionVersions.UploadArtifact }}#
with:
name: #{{ .Config.Provider }}#-provider.tar.gz
path: ${{ github.workspace }}/bin/provider.tar.gz
retention-days: 30
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
name: "Build Provider"

on:
workflow_call:
inputs:
version:
required: true
type: string
description: Version of the provider to build

jobs:
build_provider:
name: Build ${{ matrix.platform.os }}-${{ matrix.platform.arch }}
runs-on: #{{ if .Config.Runner.BuildSDK }}##{{- .Config.Runner.BuildSDK }}##{{ else }}##{{- .Config.Runner.Default }}##{{ end }}#
env:
PROVIDER_VERSION: ${{ inputs.version }}
strategy:
fail-fast: true
matrix:
platform:
- os: linux
arch: amd64
- os: linux
arch: arm64
- os: darwin
arch: amd64
- os: darwin
arch: arm64
- os: windows
arch: amd64
steps:
#{{- if .Config.FreeDiskSpaceBeforeBuild }}#
# Run as first step so we don't delete things that have just been installed
- name: Free Disk Space (Ubuntu)
uses: #{{ .Config.ActionVersions.FreeDiskSpace }}#
with:
tool-cache: false
swap-storage: false
dotnet: false
#{{- end }}#
- name: Checkout Repo
uses: #{{ .Config.ActionVersions.Checkout }}#
with:
#{{- if .Config.CheckoutSubmodules }}#
submodules: #{{ .Config.CheckoutSubmodules }}#
#{{- end }}#
persist-credentials: false
- name: Setup tools
uses: ./.github/actions/setup-tools
with:
tools: pulumictl, go
- name: Download schema-embed.json
uses: #{{ .Config.ActionVersions.DownloadArtifact }}#
with:
# Use a pattern to avoid failing if the artifact doesn't exist
pattern: schema-embed.*
# Avoid creating directories for each artifact
merge-multiple: true
path: provider/cmd/pulumi-resource-#{{ .Config.Provider }}#/schema-embed.json
Comment on lines +52 to +59
Copy link
Member

Choose a reason for hiding this comment

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

Not all providers use schema-embed right now. It's only realy a performance optimisation which could be implemented internally within the provider rather than needing CI to have insight of.

We mostly still have the original schema commited (though this has been taken out Azure Native due to the file being too big for GitHub, and LFS causing a load of other problems) but I think it would be more correct to treat the schema like a build artifact as you're doing here, though it would be nicer to not assume the Go provider directory structure if possible within the CI job. I think this could be abstracted by the makefile if needed.

  1. Prerequisites writes all artifacts to the bin directory including the embeddable schema if it needs it.
  2. Restore the bin directory in this subsequent job.
  3. Within the provider_dist target, copy the embeddable schema into the provider directory before building.

- name: Build & package provider
run: make provider_dist-${{ matrix.platform.os }}-${{ matrix.platform.arch }}
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps we should use the file target here to match the path we're going to upload rather than having the assumption of the path this phony target will write to:

make dist/pulumi-resource-#{{ .Config.Provider }}#-v${{ inputs.version }}-${{ matrix.platform.os }}-${{ matrix.platform.arch }}.tar.gz

- name: Upload artifacts
uses: #{{ .Config.ActionVersions.UploadArtifact }}#
with:
name: pulumi-resource-#{{ .Config.Provider }}#-v${{ inputs.version }}-${{ matrix.platform.os }}-${{ matrix.platform.arch }}.tar.gz
path: dist/pulumi-resource-#{{ .Config.Provider }}#-v${{ inputs.version }}-${{ matrix.platform.os }}-${{ matrix.platform.arch }}.tar.gz
retention-days: 30
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
name: "Build SDK"

on:
workflow_call:
inputs:
version:
required: true
type: string

env:
#{{ .Config.Env | toYaml | indent 2 }}#
PROVIDER_VERSION: ${{ inputs.version }}

jobs:
build_sdk:
name: build_sdk
runs-on: #{{ if .Config.Runner.BuildSDK }}##{{- .Config.Runner.BuildSDK }}##{{ else }}##{{- .Config.Runner.Default }}##{{ end }}#
strategy:
fail-fast: true
matrix:
language:
#{{ .Config.Languages | toYaml | indent 8 }}#
steps:
#{{- if .Config.FreeDiskSpaceBeforeSdkBuild }}#
# Run as first step so we don't delete things that have just been installed
- name: Free Disk Space (Ubuntu)
uses: #{{ .Config.ActionVersions.FreeDiskSpace }}#
with:
tool-cache: false
swap-storage: false
dotnet: false
#{{- end }}#
- name: Checkout Repo
uses: #{{ .Config.ActionVersions.Checkout }}#
with:
#{{- if .Config.CheckoutSubmodules }}#
submodules: #{{ .Config.CheckoutSubmodules }}#
#{{- end }}#
persist-credentials: false
- name: Cache examples generation
uses: actions/cache@v4
with:
path: |
.pulumi/examples-cache
key: ${{ runner.os }}-${{ hashFiles('provider/go.sum') }}
- name: Setup tools
uses: ./.github/actions/setup-tools
with:
tools: pulumictl, pulumicli, ${{ matrix.language }}
- name: Download bin
uses: ./.github/actions/download-bin
- name: Install plugins
run: make install_plugins
Comment on lines +52 to +53
Copy link
Member

Choose a reason for hiding this comment

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

This is a bit of an odd step. I know for TF providers it's related to installing other bridged TF providers which the bridge will use for examples generation, but for a generic template we might want something a little more generic such as make prepare or something that allways runs at the start of the job before restoring any targets or doing make --touch ... etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I was also wondering about this because the CLI should automatically download these plugins anyway. Related - these plugins eat a ton of space! pulumi/pulumi#17718

Copy link
Member

Choose a reason for hiding this comment

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

I think for bridged providers, if they're opted into using the converter plugin, then it explicitly disables auto-download because we need to have the provider version pinned, so failing fast is good. For generic providers, perhaps we can just let the provider figure out what it needs and how it will install them internally within the makefile.

- name: Update path
run: echo "${{ github.workspace }}/bin" >> "$GITHUB_PATH"
- name: Build SDK
run: make build_${{ matrix.language }}
- name: Check worktree clean
uses: pulumi/git-status-check-action@v1
with:
allowed-changes: |
sdk/**/pulumi-plugin.json
sdk/dotnet/*.csproj
sdk/go/**/pulumiUtilities.go
sdk/nodejs/package.json
sdk/python/pyproject.toml
- name: Upload SDK
uses: ./.github/actions/upload-sdk
with:
language: ${{ matrix.language }}
Loading
Loading