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

Optimization: Skip Download Pulumi CLI when already present #789

Closed
RobbieMcKinstry opened this issue Nov 30, 2022 · 5 comments · Fixed by #860
Closed

Optimization: Skip Download Pulumi CLI when already present #789

RobbieMcKinstry opened this issue Nov 30, 2022 · 5 comments · Fixed by #860
Assignees
Labels
good-first-issue Start here if you'd like to start contributing to Pulumi help-wanted We'd love your contributions on this issue impact/performance Something is slower than expected kind/engineering Work that is not visible to an external user resolution/fixed This issue was fixed size/S Estimated effort to complete (1-2 days).

Comments

@RobbieMcKinstry
Copy link
Contributor

Hello!

  • Vote on this issue by adding a 👍 reaction
  • If you want to implement this feature, comment to let us know (we'll work with you on design, scheduling, etc.)

Issue details

With the exception of MacOS and self-hosted runners, Pulumi comes installed on GitHub Actions runners by default. Right now, when a user does not specify a specific version of Pulumi to use, this action defaults to latest. Instead, we should use whatever version of Pulumi is installed on the runner, if one exists. This will remove the need to re-download Pulumi for the vast majority of users (anyone who is using Windows or Ubuntu runners).

As described in #787, this would eliminate ~100MBs of network traffic. And, detailed in #674, it would allow the action to be used in the context of where outbound traffic is blocked.

Affected area/feature

The downloadCli function.

@RobbieMcKinstry RobbieMcKinstry added help-wanted We'd love your contributions on this issue good-first-issue Start here if you'd like to start contributing to Pulumi impact/performance Something is slower than expected kind/engineering Work that is not visible to an external user size/S Estimated effort to complete (1-2 days). labels Nov 30, 2022
@ehafenmaier
Copy link
Contributor

I'm interested in helping with this issue.

@RobbieMcKinstry
Copy link
Contributor Author

Awesome! Thanks @ehafenmaier! Let me know if you need any input :)

@ehafenmaier
Copy link
Contributor

@RobbieMcKinstry I do have few questions based on this issue being mentioned in #674:

  • Can we assume a specific Pulumi CLI version was not requested if the range parameter is ^3?
  • Can the isAvailable() function be used to check if the Pulumi CLI exists on the runner? (It seems like that function was created for this scenario)
  • If both of the above are true then should we just exit the downloadCli() function right before getVersionObject() is called so the outbound network call is skipped as well? (Logging to the console that the download was skipped and the runner CLI will be used)

@RobbieMcKinstry
Copy link
Contributor Author

RobbieMcKinstry commented Jan 11, 2023

@ehafenmaier I will answer your second and third questions first because they're easier! 😅

Can the isAvailable() function be used to check if the Pulumi CLI exists on the runner? (It seems like that function was created for this scenario)

Yes! Very convenient that already exists! ;D It's a little gnarly, so it might be wise to verify it works as expected.

f both of the above are true then should we just exit the downloadCli() function right before getVersionObject() is called so the outbound network call is skipped as well? (Logging to the console that the download was skipped and the runner CLI will be used)

Exactly! Bingo!


Can we assume a specific Pulumi CLI version was not requested if the range parameter is ^3?

The behavior we need is slightly more complex. I think the workflow looks like this:

  1. Check if pulumi is installed using isAvailable()
  2. If it's NOT installed, then we have to proceed as normal.
  3. If it is installed, we need to check whether version is compatible with the version specified in the pulumi-version parameter.

Step 3 will require parsing the stdout of pulumi version and checking the compatibility. I believe the semver.satisfies() function will work for this.

If it's easier to rewrite isAvailable(): Promise<bool> to a function like getVersion(): Promise<string?>, usually essentially the same shell command, but returning stdout if stderr === ""

Does that make sense? I'm sorry if my explanation is a confusing.

Thanks very much for your assistance! <3

@ehafenmaier
Copy link
Contributor

Clear as mud @RobbieMcKinstry 😜

In all seriousness, your explanation makes perfect sense. I'm happy to see my approach wasn't too far off the mark. I'm pretty sure I have enough to proceed.

Thanks again for the input!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good-first-issue Start here if you'd like to start contributing to Pulumi help-wanted We'd love your contributions on this issue impact/performance Something is slower than expected kind/engineering Work that is not visible to an external user resolution/fixed This issue was fixed size/S Estimated effort to complete (1-2 days).
Projects
None yet
3 participants