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

Fix Publish Service Artifacts workflow call #302

Merged
merged 1 commit into from
May 14, 2023
Merged

Conversation

ulucinar
Copy link
Collaborator

@ulucinar ulucinar commented May 12, 2023

Description of your changes

Relevant PR: upbound/official-providers-ci#110

This PR removes the optional: true specification in the inputs of the Publish Service Artifacts reusable workflow call and converts types of size and concurrency input parameters from number to string.

I have:

  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

Please see: https://github.com/upbound/provider-gcp/actions/runs/4963402570


jobs:
publish-service-artifacts:
uses: upbound/uptest/.github/workflows/provider-publish-service-artifacts.yml@main
uses: ulucinar/uptest/.github/workflows/provider-publish-service-artifacts.yml@fix-ssop
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To be reverted back once the uptest PR is merged.

@ulucinar
Copy link
Collaborator Author

ulucinar commented May 14, 2023

Looks like with the larger runners, we are able to achive a concurrency of ~4 (# of batches x batch concurrency) while pushing to the registry (when there's only one family being built & pushed): https://github.com/upbound/provider-gcp/actions/runs/4973387477

Here'a another run of concurrency ~6: https://github.com/upbound/provider-gcp/actions/runs/4973546523/jobs/8899466534

Timings from these runs:

Concurrency Time
~2 (~2x1)) 12m 13s
~4 (~2x2) 9m 59s
~6 (~2x3) 8m 29s
8 (2x4) 9m 1s

As we increase concurrency, the error rate at the registry and the total number of retries increase.

@ulucinar ulucinar merged commit c4417c2 into main May 14, 2023
@ulucinar ulucinar deleted the test-uptest-pr-110 branch May 14, 2023 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants