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

Jump to bottom

Install tools package publishing #1004

Merged
merged 3 commits into from
Jun 27, 2024
Merged

Install tools package publishing #1004

merged 3 commits into from
Jun 27, 2024

Conversation

danielrbradley
Copy link
Member

@danielrbradley danielrbradley commented Jun 26, 2024 •
edited
Loading

Re-use our own tool setups which can include additional options for caching rather than duplicating the setup logic into the publishing action.

This depends on first merging:

This PR assumes that pulumi/pulumi-package-publisher will then be published as v0.0.19 (the next version).

@danielrbradley danielrbradley self-assigned this Jun 26, 2024
@danielrbradley danielrbradley mentioned this pull request Jun 26, 2024
Merged
blampe
blampe approved these changes Jun 26, 2024
View reviewed changes
Copy link
Contributor

@blampe blampe left a comment

Choose a reason for hiding this comment

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

This PR assumes that pulumi/pulumi-package-publisher will then be published as v0.0.19 (the next version).

Recommend testing this using a specific commit before cutting v0.0.19, so you don't get into a situation where you need to keep cutting releases to fix problems if anything comes up.

Base automatically changed from reuse-setup to master June 26, 2024 18:11
@danielrbradley danielrbradley force-pushed the install-tools-package-publishing branch from d4dc478 to 0fc1da0 Compare June 26, 2024 18:18
@danielrbradley
Copy link
Member Author

danielrbradley commented Jun 26, 2024 •
edited
Loading

Recommend testing this using a specific commit before cutting v0.0.19, so you don't get into a situation where you need to keep cutting releases to fix problems if anything comes up.

Good thought! Here's the downstream test commit pinning to pulumi/pulumi-package-publisher@4118cd6: pulumi/pulumi-xyz@fe9e392

Here's the test release run (v1.0.2-alpha.10) https://github.com/pulumi/pulumi-xyz/actions/runs/9692869239

@danielrbradley
Upgrade to v0.0.19 of pulumi-package-publisher
df06d6f 
- Upgrade action version in config.
- Install tools before calling pulumi-package-publisher.
- Remove version arguments.
@danielrbradley
Generate test-providers
8f0f0a7
@danielrbradley danielrbradley force-pushed the install-tools-package-publishing branch from 0fc1da0 to 8f0f0a7 Compare June 26, 2024 19:35
@danielrbradley
Copy link
Member Author

Rebased and removed the now-obsolete version arguments when calling the action.

@danielrbradley danielrbradley marked this pull request as ready for review June 26, 2024 19:36
@danielrbradley
Checkout repo before publish SDK
e61ebb0 
See: pulumi/pulumi-package-publisher@28c1190

Also install pulumictl via standard tool setup as prerequisite of pulumi-package-publisher.
@danielrbradley
Copy link
Member Author

danielrbradley commented Jun 27, 2024 •
edited
Loading

Test showed issue that we can't call local reusable workflow without a checkout. So I've also removed the checkout phase from pulumi-package-publisher to be done before calling the pulumi-package-publisher action and before calling the setup-tools action.

Next test run: https://github.com/pulumi/pulumi-xyz/actions/runs/9696437354 ✅

danielrbradley added a commit that referenced this pull request Jun 27, 2024
@danielrbradley
Replicate fix from #1004
d7154ef 
e61ebb0
- Check out repo to fix setup-tools.
- pulumi/pulumi-package-publisher won't install its own tools.
@danielrbradley
Copy link
Member Author

A further thought on this ... we seem to have a lot of breaking changes already in the pulumi-package-publisher action. Perhaps using a standalone action here is the wrong abstraction right now.

In addition to removing the tool setup and the repository checkout, we probably also want to remove the artifact restore because this needs to have knowledge of the inner workings of the workflow that's calling the action:

  • What version of the asset upload/download action are we using (these must be in sync)
  • What's the name of the asset
  • What the folder structure inside the asset
  • Where's the SDK located within the calling repo.

Given needing to keep all these things in sync, it would be easier for now to move the implementation for the pulumi-package-publisher action back inside ci-mgmt for now to keep them versioned in lock-step with the calling workflows rather than having to coordinate breaking changes.

danielrbradley added a commit to pulumi/pulumi-package-publisher that referenced this pull request Jun 27, 2024
@danielrbradley
BREAKING: Remove installing tools (#32)
ecb7cac 
Leave it to the caller to install the prerequisite tools as additional
options might want to be used for caching etc.

- Remove tool install steps.
- Remove step to check out repository.
- Remove all version inputs.
- Add tool pre-requisites to the documentation.

The other half of the work to integrate this breaking change into
ci-mgmt is in:
- pulumi/ci-mgmt#1004
@danielrbradley
Copy link
Member Author

Merging as-is for now ... can re-visit combining pulumi-package-publisher into ci-mgmt at a later date.

@danielrbradley danielrbradley merged commit f8daeb9 into master Jun 27, 2024
5 checks passed
@danielrbradley danielrbradley deleted the install-tools-package-publishing branch June 27, 2024 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@blampe blampe blampe approved these changes

Assignees

@danielrbradley danielrbradley

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@danielrbradley @blampe