Have shared-node-cache install yarn

[lillialexis]

Summary:

Some of the new checks on OLC were failing, and seemed to be failing because the check Khan/actions@shared-node-cache-v0 was failing because yarn couldn't be found.

Now this check installs yarn.

Issue: XXX-XXXX

Test plan:

I tested this on the branch yarn-bug and changed OLC to point to that branch:
Khan/actions@408dc12489a0a87a7a1f5368afa0a251c147dc43 and now it's working. (PR here)

[changeset-bot]

🦋 Changeset detected

Latest commit: 19dc3da

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
shared-node-cache	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[khan-actions-bot]

Gerald

Required Reviewers

• @Khan/github-actions for changes to .changeset/serious-suns-camp.md, actions/shared-node-cache/action.yml

---

Don't want to be involved in this pull request? Comment #removeme and we won't notify you of further changes.

[jeresig]

@lillialexis Before making this change, I noticed that you were running v0 of the action, instead of v2 (which it currently is) - can you try upgrading first and confirming that it works? I haven't seen this bug elsewhere, and we use Yarn in all our repos, so I want to make sure we're doing to right thing here.

[lillialexis]

@jeresig, yeah, that was pretty much the first thing I tried. You can see that the checks are still failing:
https://github.com/Khan/our-lovely-cli/actions/runs/9277113217/job/25525630409
https://github.com/Khan/our-lovely-cli/actions/runs/9277113224/job/25525630405

I also thought it was weird that this would be working for so many other PRs, but not mine, but I think that probably that has to do with there being an available cache that never expires or something. The issue happens because, for this PR, there's a cache-miss. When a cache-miss happens, the check tries to yarn install. Since this is a brand-new workflow for OLC (that Jared just added), then that certainly explains the cache-miss happening here and not elsewhere.

The only thing it doesn't explain is how all the other repo's checks got it working the very first time, when they cache-missed. Was there a yarn-install step that got removed? Is the cache shared by other checks that have yarn install and are run first for those repos?

[somewhatabstract]

The only thing it doesn't explain is how all the other repo's checks got it working the very first time, when they cache-missed. Was there a yarn-install step that got removed? Is the cache shared by other checks that have yarn install and are run first for those repos?

I am wary of this change given just how many uses we have that don't need this at all. Looking at the workflows, the things I see (based on the screenshot in the PR description) are:

1. Using an up-to-date version of checkout and setup-node. (the workflow depicted here uses v2 and v3, respectively, but v4 is the latest)
2. They appear to be using ubuntu-latest as the image

I suspect it's that second one that's the reason we aren't needing it - what platform is the failing workflow running against?

I certainly wouldn't want this change to be mandatory for all the cases when it appears to only be needed in some corner cases. Perhaps just add this to the workflow that needs it rather than in the action? Or provide an input to control this so that we turn it on only when needed?

[somewhatabstract]

Is the failing workflow running on a self-hosted runner or using GitHub's own runners? It seems there's a need to install yarn for self-hosted runners.

actions/setup-node#182

But it also requires that it is uninstalled afterwards to ensure the worker is left clean.

[somewhatabstract]

Yarn is pre-installed on GitHub images.

Examples:
https://github.com/actions/runner-images/blob/main/images/ubuntu/Ubuntu2004-Readme.md
https://github.com/actions/runner-images/blob/main/images/macos/macos-14-Readme.md

Given that, I really want to understand the circumstances here. It seems like this has to be an odd corner case and that this isn't the right fix.

[somewhatabstract]

Aha! Looking at the workflow: https://github.com/Khan/our-lovely-cli/blob/main/.github/workflows/pr-autofix.yml

I see this...

 autofix:
    runs-on:
      - self-hosted
      - linux
      - x64

So it's the self-hosted issue. Which means this definitely isn't the right fix. The fix should be local to the workflow or separate to this action. See the comment thread to actions/setup-node#182 for solutions.

[lillialexis]

Thanks for digging in deeper to this @somewhatabstract!

I'm not really sure what the solution is. I had considered installing yarn in my local check, but, as one of the folks in your linked thread points out, I can't do that without node, and shared-node-cache-v2.1.0 calls actions/setup-node@v4. So do I run shared-node-cache then install yarn and then shared-node-cache again? That definitely feels wrong.

I'm not sure if we need self-hosted-runner. How do I even know if that's being used or needed? Actually, I guess it must be, because that's probably why yarn isn't found...

Should I change shared-node-cache to only install yarn if it isn't installed already? Do I then uninstall it at the end of this action?

Should I pass a flag from my repo to shared-node-cache to tell it to install yarn if need?

What do you suggest?

[somewhatabstract]

Thanks for digging in deeper to this @somewhatabstract!

I'm not really sure what the solution is. I had considered installing yarn in my local check, but, as one of the folks in your linked thread points out, I can't do that without node, and shared-node-cache-v2.1.0 calls actions/setup-node@v4. So do I run shared-node-cache then install yarn and then shared-node-cache again? That definitely feels wrong.

I think it's OK to treat the action as a blackbox. If you need node in your workflow, set it up. We do that elsewhere and it seems fine.

I'm not sure if we need self-hosted-runner. How do I even know if that's being used or needed? Actually, I guess it must be, because that's probably why yarn isn't found...

I don't know why we're using a self-hosted runner in this case. @jeresig may have some context? Could be a hangover from when we dabbled in using those more (before we discovered they were too slow for most of our uses). An easy fix would be to not do that, but understanding why we are comes before making that decision.

Should I change shared-node-cache to only install yarn if it isn't installed already? Do I then uninstall it at the end of this action?

I don't think the fix is to modify this shared action. I think it should stay as-is and this issue should be tackled separately since it only affects the self-hosted scenario.

Should I pass a flag from my repo to shared-node-cache to tell it to install yarn if need?

I don't think so.

What do you suggest?

My suggestions:

1. Find out why we use a self-hosted runner. If we don't need to (and presuming @jeresig concurs), just don't. Problem solved.
2. If we need to use a self-hosted runner, then add setup-node to the workflow. Then install yarn via npm, and probably make sure to uninstall on workflow end/cancellation

[jeresig]

We definitely shouldn't be using a self hosted runner, it should be Ubuntu-latest. This must be a holdover from our past experiments!