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.

Sign up for GitHub

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

Allow bridged providers to opt-in to sharding #1217

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Allow bridged providers to opt-in to sharding #1217

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
89a0422
Update aws .ci-mgmt
blampe Dec 11, 2024
4f9db6c
Allow bridged providers to opt-in to sharding
blampe Dec 11, 2024
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
8 changes: 8 additions & 0 deletions provider-ci/internal/pkg/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,14 @@ 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"`

// E2ETestDirectory controls the working directory when executing long-running/sharded integration tests.
//
// Defaults to "examples".
E2ETestDirectory string `yaml:"e2eTestDirectory"`

// 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
42 changes: 40 additions & 2 deletions provider-ci/internal/pkg/templates/bridged-provider/.github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,29 +39,50 @@ jobs:
#{{- end }}#
persist-credentials: false
- name: Checkout p/examples
#{{- if not .Config.Shards }}#
if: matrix.testTarget == 'pulumiExamples'
#{{- end }}#
uses: #{{ .Config.ActionVersions.Checkout }}#
with:
repository: pulumi/examples
path: p-examples
- name: Setup tools
uses: ./.github/actions/setup-tools
#{{- if not .Config.Shards }}#
with:
tools: pulumictl, pulumicli, ${{ matrix.language }}
#{{- end }}#
- name: Prepare local workspace
run: make prepare_local_workspace
- name: Download bin
uses: ./.github/actions/download-bin
#{{- if not .Config.Shards }}#
- name: Download SDK
uses: ./.github/actions/download-sdk
with:
language: ${{ matrix.language }}
- name: Restore makefile progress
run: make --touch provider schema build_${{ matrix.language }}
- name: Install dependencies
run: make install_${{ matrix.language}}_sdk
#{{- else }}#
#{{- range $_, $language := .Config.Languages }}#
- name: Download #{{ $language }}# SDK
uses: ./.github/actions/download-sdk
with:
language: #{{ $language }}#
- name: Restore makefile progress
run: make --touch provider schema build_#{{ $language }}#
- name: Install dependencies
run: make install_#{{ $language }}#_sdk
#{{- end }}#
Comment on lines +69 to +78
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 we can make this less repetitive in the generated code by doing the touch and install outside the loop and leveraging the phony grouping targets.

Suggested change
#{{- range $_, $language := .Config.Languages }}#
- name: Download #{{ $language }}# SDK
uses: ./.github/actions/download-sdk
with:
language: #{{ $language }}#
- name: Restore makefile progress
run: make --touch provider schema build_#{{ $language }}#
- name: Install dependencies
run: make install_#{{ $language }}#_sdk
#{{- end }}#
#{{- range $_, $language := .Config.Languages }}#
- name: Download #{{ $language }}# SDK
uses: ./.github/actions/download-sdk
with:
language: #{{ $language }}#
#{{- end }}#
- name: Restore makefile progress
run: make --touch provider schema build_sdks
- name: Install dependencies
run: make install_sdks

Copy link
Member

Choose a reason for hiding this comment

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

Additionally, we could allow the "Download SDK" action to accept multiple languages (e.g. comma separated or something) to make the call sight cleaner and more understandable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Totally forgot about install_sdks, good call!

#{{- end }}#
- name: Update path
run: echo "${{ github.workspace }}/bin" >> "$GITHUB_PATH"
- name: Install Python deps
#{{- if not .Config.Shards }}#
if: matrix.language == 'python'
#{{- end }}#
run: |-
pip3 install virtualenv==20.0.23
pip3 install pipenv
Expand Down Expand Up @@ -102,8 +123,6 @@ jobs:
- name: Run setup script
run: #{{ index .Config.SetupScript }}#
#{{- end }}#
- name: Install dependencies
run: make install_${{ matrix.language}}_sdk
- name: Install gotestfmt
uses: GoTestTools/gotestfmt-action@v2
with:
Expand All @@ -112,6 +131,7 @@ jobs:
#{{- if .Config.Actions.PreTest }}#
#{{ .Config.Actions.PreTest | toYaml | indent 4 }}#
#{{- end }}#
#{{- if not .Config.Shards }}#
#{{- if .Config.IntegrationTestProvider }}#
- name: Run provider tests
if: matrix.testTarget == 'local'
Expand All @@ -134,6 +154,24 @@ jobs:
#{{- else }}#
testTarget: [local]
#{{- end }}#
#{{- else }}#
- name: Generate shard
run: make shard
env:
SHARD_TOTAL: ${{ matrix.total }}
SHARD_INDEX: ${{ matrix.index }}
- name: Run integration tests
run: make test_shard
strategy:
fail-fast: false
matrix:
total:
- #{{ .Config.Shards }}#
index:
#{{- range $i, $_ := until .Config.Shards }}#
- #{{ $i }}#
#{{- end }}#
#{{- end }}#
#{{- if .Config.ExtraTests }}#
#{{ .Config.ExtraTests | toYaml | indent 2 }}#
#{{ end }}#
13 changes: 11 additions & 2 deletions provider-ci/internal/pkg/templates/bridged-provider/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ bin/$(PROVIDER): .make/schema

test: export PATH := $(WORKING_DIR)/bin:$(PATH)
test:
cd examples && go test -v -tags=all -parallel $(TESTPARALLELISM) -timeout 2h
cd #{{ .Config.E2ETestDirectory }}# && go test -v -tags=all -parallel $(TESTPARALLELISM) -timeout 2h
Copy link
Member

Choose a reason for hiding this comment

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

Alternative design - proxy all test runs through your sharded test runner which could then apply the sharding in a single step, if needed:

Suggested change
cd #{{ .Config.E2ETestDirectory }}# && go test -v -tags=all -parallel $(TESTPARALLELISM) -timeout 2h
cd #{{ .Config.E2ETestDirectory }}# && go run github.com/pulumi/go-test-shard@v1 --total "$(SHARD_TOTAL)" --index "$(SHARD_INDEX)" -- -v -tags=all -parallel $(TESTPARALLELISM) -timeout 2h

The exact same change could be applied to the provider_test target too and could be moved from the prerequisites to the test matrix too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The exact same change could be applied to the provider_test target too and could be moved from the prerequisites to the test matrix too.

I've been thinking of provider_test as essentially unit tests which make sense to keep in prerequisites as a lightweight sanity check before the heavy tests start.

See my comment here about integration tests in provider. Basically our poor module structure is pushing us towards some awkward conventions, and there are a ton of benefits to fixing the underlying issue instead of catering to those conventions.

By default we can expect "slow" tests to live in one directory, but this doesn't have to be a requirement as long as the provider can eventually customize targets. With a root-level go.mod it becomes trivial to run integration tests across any number of directories -- in other words it's possible to specify SHARD_PATHS="./examples ./provider" when they belong to the same module.

.PHONY: test

test_provider:
Expand Down Expand Up @@ -379,6 +379,15 @@ provider_dist-darwin-arm64: bin/$(PROVIDER)-v$(VERSION_GENERIC)-darwin-arm64.tar
provider_dist-windows-amd64: bin/$(PROVIDER)-v$(VERSION_GENERIC)-windows-amd64.tar.gz
provider_dist: provider_dist-linux-amd64 provider_dist-linux-arm64 provider_dist-darwin-amd64 provider_dist-darwin-arm64 provider_dist-windows-amd64
.PHONY: provider_dist-linux-amd64 provider_dist-linux-arm64 provider_dist-darwin-amd64 provider_dist-darwin-arm64 provider_dist-windows-amd64 provider_dist

#{{ if .Config.Shards -}}#
# shard computes tests to run and modifies the CI runner's environment.
shard:
Comment on lines +383 to +384
Copy link
Member

Choose a reason for hiding this comment

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

We can't use $(GITHUB_ENV) to modify the environment as that won't be runnable locally. I really think we want to leverage the single target that's integrated via the existing test target which might be a little more fiddly to write but makes it easier for the caller to repro exactly what CI did.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Totally agree with everything you're saying but would prefer to make those optimizations in a followup. It's nice that we manage the Makefile for these bridged providers because we're not locked in to anything here.

It is possible to repro things locally by looking up the shard information

Run make test_shard
env:
    SHARD_TESTS: ^(?:TestACMUpgrade|TestAccLogGroup|TestAccRoute53|TestAccServerlessRaw|TestAccWebserverGo|TestEcrLifecyclePolicyUpgrade|TestIMDSAuth|TestLogGroupUpgrade|TestRdsInstanceUpgrade|TestRegress3421Update|TestRegress4128|TestSecretManagerPy)\$
    SHARD_PATHS: ./.

and you can generate that information yourself if you really want to (GITHUB_ENV=env SHARD_TOTAL=10 SHARD_INDEX=2 make shard && cat env).

So you can in theory run SHARD_TESTS=... SHARD_PATHS=... make test_shard locally. But in practice there are so many other environment variables I don't think this will actually be feasible until we've done something like #1207.

@(cd #{{ .Config.E2ETestDirectory }}# && go run github.com/blampe/shard@latest --total $(SHARD_TOTAL) --index $(SHARD_INDEX) --output env) >> "$(GITHUB_ENV)"
Copy link
Member

Choose a reason for hiding this comment

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

We need to move this out of your personal repo before we ship this so we can fix any issues while you're out.


# test_shard runs the tests specified by a regex contained in $SHARD_TESTS for paths $SHARD_PATHS.
test_shard:
cd #{{ .Config.E2ETestDirectory }}# && \
go test -tags=all -v -count=1 -coverprofile="coverage.txt" -coverpkg=./... -timeout 3h -parallel ${TESTPARALLELISM} -run "$(SHARD_TESTS)" $(SHARD_PATHS)
#{{- end }}#
# Permit providers to extend the Makefile with provider-specific Make includes.
include $(wildcard .mk/*.mk)
12 changes: 12 additions & 0 deletions provider-ci/internal/pkg/templates/defaults.config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,8 @@ env:
# Bridge-related - could be set in makefile instead?
TF_APPEND_USER_AGENT: pulumi

# DEPRECATED
#
# actions can contain preBuild and preTest additional steps to be spliced into workflows.
# The use of these hooks vary - quite a few just build upstream and run provider tests.
# Usage: https://github.com/search?q=org%3Apulumi+path%3A.ci-mgmt.yaml+%22actions%3A%22&type=code
Expand All @@ -134,6 +136,8 @@ actions:
# preTest:
# - Your action here

# DEPRECATED
#
# Additional tests 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
Expand All @@ -145,10 +149,18 @@ extraTests:
# Run e2e tests in the provider as well as in the examples directory
integrationTestProvider: false

# DEPRECATED
#
# Run e2e tests using the examples and test suite in the pulumi/examples repo.
# This is unused: https://github.com/search?q=org%3Apulumi+path%3A.ci-mgmt.yaml+%22testPulumiExamples%3A%22&type=code
testPulumiExamples: false

# How many shared to execute integration tests with. If omitted, shard behavior defaults to language-based sharding.
shards: 0

# The working directory for running integration tests.
e2eTestDirectory: examples

# runner defines the runs-on property for various stages of the build
# These are not overridden by any providers: https://github.com/search?q=org%3Apulumi+path%3A.ci-mgmt.yaml+%22runner%3A%22&type=code
runner:
Expand Down
4 changes: 2 additions & 2 deletions provider-ci/test-providers/acme/.github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,15 @@ jobs:
language: ${{ matrix.language }}
- name: Restore makefile progress
run: make --touch provider schema build_${{ matrix.language }}
- name: Install dependencies
run: make install_${{ matrix.language}}_sdk
- name: Update path
run: echo "${{ github.workspace }}/bin" >> "$GITHUB_PATH"
- name: Install Python deps
if: matrix.language == 'python'
run: |-
pip3 install virtualenv==20.0.23
pip3 install pipenv
- name: Install dependencies
run: make install_${{ matrix.language}}_sdk
- name: Install gotestfmt
uses: GoTestTools/gotestfmt-action@v2
with:
Expand Down
95 changes: 36 additions & 59 deletions provider-ci/test-providers/aws/.ci-mgmt.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,23 @@ provider: aws
lint: false
major-version: 6
parallel: 1
shards: 10
timeout: 150
generate-nightly-test-workflow: true
providerVersion: github.com/hashicorp/terraform-provider-aws/version.ProviderVersion
toolVersions:
dotnet: "6.0.x"
go: "1.23.x"
java: "11"
gradle: "7.6"
nodejs: "20.x"
pulumi: "dev"
python: "3.11.8"
env:
PULUMI_MISSING_DOCS_ERROR: true
AWS_REGION: "us-west-2"
OIDC_ROLE_ARN: ${{ secrets.OIDC_ROLE_ARN }}
template: bridged-provider
makeTemplate: bridged
checkoutSubmodules: true
freeDiskSpaceBeforeBuild: true
freeDiskSpaceBeforeSdkBuild: true
Expand Down Expand Up @@ -62,6 +71,11 @@ extraTests:
runs-on: ubuntu-latest
timeout-minutes: 60
steps:
- name: Free Disk Space (Ubuntu)
uses: jlumbroso/free-disk-space@main
with:
swap-storage: false
tool-cache: false
- name: Checkout Repo
uses: actions/checkout@v4
with:
Expand All @@ -74,8 +88,8 @@ extraTests:
uses: ./.github/actions/setup-tools
with:
tools: pulumictl, pulumi, go
- name: Make upstream
run: make upstream
- name: Prepare local workspace
run: make prepare_local_workspace
- name: go test
run: |
cd upstream
Expand Down Expand Up @@ -105,10 +119,10 @@ extraTests:
- name: Install Go
uses: actions/setup-go@v5
with:
go-version: "1.22.x"
go-version: "1.23.x"
cache: false
- name: Make upstream
run: make upstream
- name: Prepare local workspace
run: make prepare_local_workspace
- name: upstream lint
run: |
cd upstream
Expand Down Expand Up @@ -139,25 +153,16 @@ extraTests:
uses: ./.github/actions/setup-tools
with:
tools: pulumictl, pulumi, go, node
- name: Download provider + tfgen binaries
uses: actions/download-artifact@v4
with:
name: aws-provider.tar.gz
path: ${{ github.workspace }}/bin
- name: Untar provider binaries
run: >-
tar -zxf ${{ github.workspace }}/bin/provider.tar.gz -C ${{
github.workspace}}/bin

find ${{ github.workspace }} -name "pulumi-*-aws" -print -exec chmod +x {} \;
- name: Prepare local workspace
run: make prepare_local_workspace
- name: Download bin
uses: ./.github/actions/download-bin
- name: Download SDK
uses: actions/download-artifact@v4
uses: ./.github/actions/download-sdk
with:
name: ${{ matrix.language }}-sdk.tar.gz
path: ${{ github.workspace}}/sdk/
- name: Uncompress SDK folder
run: tar -zxf ${{ github.workspace }}/sdk/${{ matrix.language }}.tar.gz -C ${{
github.workspace }}/sdk/${{ matrix.language }}
language: ${{ matrix.language }}
- name: Restore makefile progress
run: make --touch provider schema build_${{ matrix.language }}
- name: Update path
run: echo "${{ github.workspace }}/bin" >> "$GITHUB_PATH"
- name: Install dependencies
Expand All @@ -167,8 +172,6 @@ extraTests:
with:
token: ${{ secrets.GITHUB_TOKEN }}
version: v2.4.0
- name: Make upstream
run: make upstream
- name: Run selected tests with manual web identity/OIDC auth
run: cd examples && go test -v -json -count=1 -run TestAccCloudWatchOidcManual -tags=${{ matrix.language }} -parallel 4 . 2>&1 | tee /tmp/gotest.log | gotestfmt
- name: Configure AWS Credentials for OIDC
Expand All @@ -181,13 +184,6 @@ extraTests:
role-to-assume: ${{ secrets.OIDC_ROLE_ARN }}
- name: Run selected tests with configure-aws-credentials web identity/OIDC auth
run: cd examples && go test -v -json -count=1 -run TestAccCloudWatch -tags=${{ matrix.language }} -parallel 4 . 2>&1 | tee /tmp/gotest.log | gotestfmt
- if: failure() && github.event_name == 'push'
name: Notify Slack
uses: 8398a7/action-slack@v3
with:
author_name: Failure in running ${{ matrix.language }} tests
fields: repo,commit,author,action
status: ${{ job.status }}
strategy:
fail-fast: false
matrix:
Expand Down Expand Up @@ -218,28 +214,16 @@ extraTests:
uses: ./.github/actions/setup-tools
with:
tools: pulumictl, pulumi, go, node, dotnet, python, java
- name: Make upstream
run: make upstream
- name: Download provider + tfgen binaries
uses: actions/download-artifact@v4
with:
name: aws-provider.tar.gz
path: ${{ github.workspace }}/bin
- name: Untar provider binaries
run: >-
tar -zxf ${{ github.workspace }}/bin/provider.tar.gz -C ${{
github.workspace}}/bin

find ${{ github.workspace }} -name "pulumi-*-aws" -print -exec chmod +x {} \;
- run: dotnet nuget add source ${{ github.workspace }}/nuget
- name: Prepare local workspace
run: make prepare_local_workspace
- name: Download bin
uses: ./.github/actions/download-bin
- name: Download SDK
uses: actions/download-artifact@v4
uses: ./.github/actions/download-sdk
with:
name: ${{ matrix.language }}-sdk.tar.gz
path: ${{ github.workspace}}/sdk/
- name: Uncompress SDK folder
run: tar -zxf ${{ github.workspace }}/sdk/${{ matrix.language }}.tar.gz -C ${{
github.workspace }}/sdk/${{ matrix.language }}
language: ${{ matrix.language }}
- name: Restore makefile progress
run: make --touch provider schema build_${{ matrix.language }}
- name: Update path
run: echo "${{ github.workspace }}/bin" >> "$GITHUB_PATH"
- name: Install Python deps
Expand All @@ -265,13 +249,6 @@ extraTests:
- name: Run provider tests
run: |
cd provider && go test -v -json -count=1 -cover -timeout 2h -tags=${{ matrix.language }} -parallel 4 . 2>&1 | tee /tmp/gotest.log | gotestfmt
- if: failure() && github.event_name == 'push'
name: Notify Slack
uses: 8398a7/action-slack@v3
with:
author_name: Failure in running ${{ matrix.language }} provider tests
fields: repo,commit,author,action
status: ${{ job.status }}
strategy:
fail-fast: false
matrix:
Expand Down
2 changes: 1 addition & 1 deletion provider-ci/test-providers/aws/.github/actions/setup-tools/action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ runs:
if: inputs.tools == 'all' || contains(inputs.tools, 'go')
uses: actions/setup-go@3041bf56c941b39c61721a86cd11f3bb1338122a # v5
with:
go-version: "1.21.x"
go-version: "1.23.x"
cache-dependency-path: |
provider/*.sum
upstream/*.sum
Expand Down
Loading
Loading