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

lts/* installs old LTS version intermittently #940

Closed
2 of 5 tasks
karlhorky opened this issue Jan 12, 2024 · 32 comments
Closed
2 of 5 tasks

lts/* installs old LTS version intermittently #940

karlhorky opened this issue Jan 12, 2024 · 32 comments
Assignees
Labels
bug Something isn't working

Comments

@karlhorky
Copy link

karlhorky commented Jan 12, 2024

Description:

Using the following configuration installs Node.js 20.10.0, an old version. Node.js 20.11.0 has been out since 10 Jan 2024.

      - uses: actions/setup-node@v4
        with:
          node-version: 'lts/*'
          check-latest: true

The output looks like this:

Run actions/setup-node@v4
  with:
    node-version: lts/*
    always-auth: false
    check-latest: true
    token: ***
Attempt to resolve LTS alias from manifest...
Found in cache @ /opt/hostedtoolcache/node/20.10.0/x64
Environment details
  node: v20.10.0
  npm: 10.2.3
  yarn: 1.22.21

Action version:

actions/setup-node@v4

Platform:

  • Ubuntu
  • macOS
  • Windows

Runner type:

  • Hosted
  • Self-hosted

Tools version:

Node.js lts/*

  node: v20.10.0
  npm: 10.2.3
  yarn: 1.22.21

Repro steps:

Use the configuration above and observe the output above

Expected behavior:

Node.js latest LTS (20.11.0) is installed

Actual behavior:

Node.js older LTS (20.10.0) is installed

cc @MaksimZhukov @dmitry-shibanov @dusan-trickovic

@karlhorky karlhorky added bug Something isn't working needs triage labels Jan 12, 2024
@karlhorky karlhorky changed the title Node.js 20.11.0 not installing with lts/* lts/* installs old LTS version Jan 12, 2024
@karlhorky
Copy link
Author

karlhorky commented Jan 12, 2024

Looks like the manifest might be here (maybe this should be linked in the output of actions/setup-node for easier debugging, if this will continue to be potentially problematic in future?):

https://www.github.com/actions/node-versions/blob/main/versions-manifest.json

I opened a PR there as well, in case that's a better place to discuss:

@HarithaVattikuti
Copy link
Contributor

Hello @karlhorky
Thank you for creating this issue. We will investigate it and get back to you as soon as we have some feedback.

@HarithaVattikuti
Copy link
Contributor

Hello @karlhorky
The latest version 20.11.0 is merged in node-versions repository. Could you please confirm if the latest version is reflecting at your end.
If you have any concerns please feel free to ping us.

@karlhorky
Copy link
Author

@HarithaVattikuti I just tried re-running my job and it's still on Node.js 20.10.0

Attempt to resolve LTS alias from manifest...
Found in cache @ /opt/hostedtoolcache/node/20.10.0/x64

@karlhorky
Copy link
Author

Is there any way I can manually clear the manifest cache on GitHub Actions?

I know that you can clear other named caches, but these are unrelated to the manifest cache, from what I can tell.

@HarithaVattikuti
Copy link
Contributor

Hello karlhorky, We have tried to reproduce the issue and unable to do so. Below is the screenshot of the same.

Screenshot 2024-01-25 at 4 28 52 PM

Please share a sample repo with minimum code to reproduce the issue.
Feel free to reach out to us in case of any issues

@karlhorky
Copy link
Author

karlhorky commented Jan 26, 2024

@HarithaVattikuti now it's working, thanks!

Attempt to resolve LTS alias from manifest...
Found in cache @ /opt/hostedtoolcache/node/20.11.0/x64

Maybe I tried it before the change was fully rolled out.

Closing.

@karlhorky
Copy link
Author

karlhorky commented Jan 26, 2024

Interesting, this is still intermittently failing on some of our repositories (with a macos-12 or macos-13 runner):

Attempt to resolve LTS alias from manifest...
Found in cache @ /Users/runner/hostedtoolcache/node/20.10.0/x64

https://github.com/upleveled/hotline-bling-codealong/actions/runs/7667389682/job/20897108024?pr=164


@HarithaVattikuti maybe there's a problem with the caches?

Reopening for visibility.

@karlhorky karlhorky reopened this Jan 26, 2024
@karlhorky karlhorky changed the title lts/* installs old LTS version lts/* installs old LTS version intermittently Jan 26, 2024
@HarithaVattikuti
Copy link
Contributor

Hello @karlhorky,
According to your repository set up-node is run on different OS. The hostedtoolcache will reflect the latest node version from the OS image. For Ex: In MacOS-12 Image Cached Tool of node is 20.11.0 but in MacOS-13 Image cached Tool of node is still 20.10.0.
https://github.com/actions/runner-images/blob/macOS-12/20240119.1/images/macos/macos-13-Readme.md#cached-tools
https://github.com/actions/runner-images/blob/macOS-12/20240119.1/images/macos/macos-12-Readme.md#cached-tools
Please use the related OS image for latest node version 20.11.0.
Closing this issue as it is not part of our scope.
Feel Free to reach out to us in case of any issues.

@karlhorky
Copy link
Author

Oh wow, that's a surprising detail 🤔

To me, it seems like Node.js lts/* should be the latest Node.js LTS, not an older cached version.

If it would be possible to fix that and return the latest LTS, I'm sure that would be widely appreciated and reduce confusion.

@karlhorky
Copy link
Author

Update: this continues to be a problem with Node.js v20.12.0 - the lts/* specifier does not install 20.12.0, which has been out for over 5 days now:

      - name: Use Node.js
        uses: actions/setup-node@v4
        with:
          node-version: 'lts/*'
          cache: 'pnpm'

Logs for the workflow run:

Run actions/setup-node@v4
  
Attempt to resolve LTS alias from manifest...
Found in cache @ /opt/hostedtoolcache/node/20.11.1/x64
Environment details
/home/runner/setup-pnpm/node_modules/.bin/pnpm store path --silent
/home/runner/setup-pnpm/node_modules/.bin/store/v3
Received 0 of 177801572 (0.0%), 0.0 MBs/sec
Received 125829120 of 177801572 (70.8%), 60.0 MBs/sec
Cache Size: ~170 MB (177801572 B)
/usr/bin/tar -xf /home/runner/work/_temp/36665e6f-3c08-4aa9-a645-44132e6[14](https://github.com/upleveled/preflight-test-project-next-js-passing/actions/runs/8507621988/job/23299779905?pr=152#step:7:14)367/cache.tzst -P -C /home/runner/work/preflight-test-project-next-js-passing/preflight-test-project-next-js-passing --use-compress-program unzstd
Received 17780[15](https://github.com/upleveled/preflight-test-project-next-js-passing/actions/runs/8507621988/job/23299779905?pr=152#step:7:16)72 of 177801572 (100.0%), 56.5 MBs/sec
Cache restored successfully
Cache restored from key: node-cache-Linux-pnpm-4ef79db9b37b4913d3034ac221942fa51b304d30abec481a5894d6356c04e3e6

@karlhorky
Copy link
Author

@HarithaVattikuti can you reopen this issue please?

Or if not, I'll create a new issue.

@aparnajyothi-y aparnajyothi-y self-assigned this Apr 2, 2024
@karlhorky
Copy link
Author

@aparnajyothi-y if you are taking this over, could you reopen this issue?

@aparnajyothi-y
Copy link
Contributor

Hello @karlhorky, I have tried to run the same job to use lts* in my local and it is downloading the latest version 20.12.0.
If a previous LTS version was cached in the runner, actions/setup-node might reuse the cached version instead of fetching the latest one. As a workaround, we need to explicitly setting the cache input to 'none' in the actions/setup-node step of your job. This will prevent actions/setup-node from using a cached version of Node.js.
- name: Setup Node.js
uses: actions/setup-node@v4
with:
node-version: 'lts/*'
cache: 'none'

If we want to keep the caching but ensure that always using the latest LTS version then add a step to your workflow that updates Node.js to the latest LTS version after setting it up

    - name: Setup Node.js
              uses: actions/setup-node@v4
              with:
                  node-version: 'lts/*'

   - name: Update to latest LTS version
      run: |
        npm install -g n
        sudo n lts
        echo ""NODE_PATH=$(npm root --quiet -g)"" >> $GITHUB_ENV.

@aparnajyothi-y aparnajyothi-y reopened this Apr 3, 2024
@karlhorky
Copy link
Author

karlhorky commented Apr 3, 2024

@aparnajyothi-y what I think should be supported (and what I believe would be the behavior that users would expect and want):

  1. lts/* retrieves the latest Node.js LTS version
  2. The cache is used when it is available
  3. The cache is evicted *without user intervention* when any new LTS version is released (from my understanding, this seems like the area where setup-node or its dependencies/infrastructure could be fixed)
  4. No additional configuration such as cache: 'none' is required (also, cache: 'none' is invalid, unrelated configuration)
  5. No additional obscure steps (such as installing a Node.js version manager) are required

@aparnajyothi-y
Copy link
Contributor

Hello @karlhorky, We have furthermore investigated and found that the action itself doesn't control the cache, it's managed by the runner environment. Automatic cache eviction may need changes on the runner's side as setup-node uses the caching facilities provided by the runner environment, you can refer here. Automatic cache eviction is a potential feature where old or unused cached dependencies would be automatically removed. This ensures that the cache does not keep growing indefinitely and consume all available storage.
Using cache when available is actually a part of the Setup-action's functionality. The action will check if the required Node.js version is in the cache and use it if it's available. By caching these dependencies, subsequent runs of the workflow can be faster as they can reuse the cached dependencies instead of downloading them again.
Please let us know in case of any further clarification.

@karlhorky
Copy link
Author

I'm not sure I'm understanding what you're saying, but maybe you're saying this bug should be also reported somewhere else?

Where exactly can this be reported also for the runners so that the behavior is automatic cache evictions (or cache upgrades) on new LTS versions?

I and others think that the current behavior is broken, so it should be fixed.

I assume it may not be the runner's responsibility to know about new LTS versions of Node.js (that seems like responsibility of actions/setup-node) - it may actually be a split responsibility: both actions/setup-node and also in the runner environments:

  1. To me, it seems that logically, the place that would be related to "performing some action when a new LTS version is released" would be actions/setup-node (this current repo)
  2. But in this case, the action needs to evict the cache, where you mentioned the management is the responsibility of the runner

So, if this mental model is correct, there may need to be a "cache eviction API" exposed by the runner, in order for actions/setup-node to be able to evict the cache on new LTS versions.

@aparnajyothi-y
Copy link
Contributor

Hello @karlhorky, As we have mentioned in the above comment, this feature might be shared between the actions/setup-node and the GitHub Actions runner. The actions/setup-node should ideally handle actions related to the release of new LTS versions, while cache management is typically handled by the runner.
If we got ""cache eviction API"" support in the runner, that would allow actions like actions/setup-node to evict the cache when new LTS versions are released, ensuring that the cache is always up-to-date.
Please let us know in case of any clarification is needed from setup-node. Thank you.

@karlhorky
Copy link
Author

karlhorky commented Apr 18, 2024

Ok great, are you a part of the GitHub Actions team?

Can you reach out to someone internally on the runner side to convey the need for a Cache Eviction API to allow for the actions/setup-node LTS version identifier lts/* to use the latest LTS release without user interaction?

I see that the following people are maintainers / contributors on the public GitHub repos, but I have no visibility / connections beyond that:

@aparnajyothi-y
Copy link
Contributor

Hello @karlhorky, We are discussing with the runner-images about the possibility of the cache eviction feature from the runner-images . Please raise an issue in the runner-images repository to request support for this feature as discussed in the above comment and we can add the facility for cache eviction for LTS versions from setup-actions once we receive that feature from runner-images.
Please feel free to reach us in case of any further clarifications.

@karlhorky
Copy link
Author

@mikhailkoliada
Copy link

@karlhorky as I was mentioned here, I am no longer a part of GH but still an avid user for my personal belongings, cache eviction should never be done on the runner side, the purpose of the images containing cache is to speed up the basic customer needs but all the management still should be done via the task. Well, surely I can now speak my own opinion based on multiple years of maintenance :)

@karlhorky
Copy link
Author

karlhorky commented Jul 19, 2024

Ok, I'm not sure of the implementation details or inner workings. But most users won't know / care about that anyway.

Maybe we can agree that node-version: 'lts/*' retrieving an old version is not only a bad UX, but also sometimes a security risk.

The cache being automatically evicted without user interaction when a new Node.js LTS version is released (regardless of where that automatic cache eviction happens) does not seem problematic:

  1. LTS releases do not happen very often (still would result in many more cache hits than misses)
  2. More subjective: being fast for a few users who would get cache misses seems like less of a concern than being correct for all users

@aparnajyothi-y
Copy link
Contributor

Hello @karlhorky, As @mikhailkoliada mentioned, cache eviction should not be handled on the runner side; instead, cache management should be performed through the task itself. The latest Node.js version, 20.15.1, is available with runner images on both Ubuntu 20 and 22 runners. We are looking for version 20.11.0, which is outdated and Installing the required version at runtime through the workflow is the best workaround.

We are proceeding to close this issue as it is confirmed that cache eviction is not possible through runner-images. Please feel free to reach out if you have any concerns or need further information.

@karlhorky
Copy link
Author

karlhorky commented Jul 24, 2024

We are proceeding to close this issue as it is confirmed that cache eviction is not possible through runner-images

Where is this confirmed? I opened an issue in actions/runner-images and it's now been assigned:

This issue #940 in actions/setup-node should stay open, because there is still an active bug in actions/setup-node - the following config will intermittently lead to old, stale LTS versions being used:

      - name: Use Node.js
        uses: actions/setup-node@v4
        with:
          node-version: 'lts/*'

This is an unexpected and broken behavior for users, and can lead to security problems.

@aparnajyothi-y Please reconsider leaving this issue open, as it represents an active problem for users, and it should be visible in the issues.

@karlhorky
Copy link
Author

karlhorky commented Jul 24, 2024

Maybe it should just be publicly recommended to avoid using lts/* and similar version specifiers with actions/setup-node because of this problem.

Probably anything without a fixed version specifier is broken

(at least the way that actions/setup-node is currently built)

Eg. if it's impossible for the teams at GitHub Actions to write code to retrieve the latest Node.js LTS version when a new one is released, maybe users should be suggested to switch to third party alternatives from the community like https://github.com/dcodeIO/setup-node-nvm (GitHub Marketplace)

uses: dcodeIO/setup-node-nvm@master
with:
  node-version: lts/*

Which would work reliably, unlike actions/setup-node.

@karlhorky
Copy link
Author

I opened a PR to add a warning to the readme about NVM syntax here, and steer users away from actions/setup-node if they use this syntax:

@karlhorky
Copy link
Author

karlhorky commented Aug 13, 2024

@aparnajyothi-y I'm not sure why this issue is closed now.

Can you reopen and leave open until it's resolved?

If you close the issue, users will be unable to find this problem, which is still an open, unresolved problem in actions/setup-node.

As an alternative, a new issue could be opened, but that will lose the context / conversation contained in this issue.

@karlhorky
Copy link
Author

karlhorky commented Aug 13, 2024

Workaround

Full replacement for broken actions/setup-node version resolution, with nvm:

# Use nvm because actions/setup-node does not install latest versions
# https://github.com/actions/setup-node/issues/940
- name: Install latest LTS with nvm
  run: |
    nvm install 'lts/*'
    echo "$(dirname $(nvm which node))" >> $GITHUB_PATH
  shell: bash -l {0}

If you also need pnpm caching (replacement for the cache setting of actions/setup-node), config for actions/cache:

- name: Get pnpm store directory
  shell: bash
  run: |
    echo "STORE_PATH=$(pnpm store path --silent)" >> $GITHUB_ENV
- uses: actions/cache@v4
  name: Setup pnpm cache
  with:
    path: ${{ env.STORE_PATH }}
    key: ${{ runner.os }}-pnpm-store-${{ hashFiles('**/pnpm-lock.yaml') }}
    restore-keys: |
      ${{ runner.os }}-pnpm-store-

Also posted on github-tricks: https://github.com/karlhorky/github-tricks/blob/main/readme.md#github-actions-correct-broken-actionssetup-node-version-resolution

@aparnajyothi-y
Copy link
Contributor

Hello @karlhorky , Thank for your proposals and apologies for the delay in response.
When setup-node action was implemented initially, we designed it to optimize workflow duration to speed up customers' workflows and reduce VM cost for customers. It is why check-latest is disabled by default and action fallbacks to cached version of Node.js on VM. It allows to speed up workflows by 10-15 seconds. Usually, VM cache is up-to-date and it can be 5-6 days old in worst case which is acceptable for the most of customers.

If some customers need to always use the freshest version of Node.js, ""check-latest"" flag is intended for it. If ""check-latest: true"" is passed, action will ignore VM cache and always download latest version.
So, using version: lts/* with check-latest: true will give you exact behaviour which you expect. We have pretty good documentation in repo to explain this behavior: https://github.com/actions/setup-node/blob/main/docs/advanced-usage.md#check-latest-version
We don't have plans to set check-latest: true by default because it will cause increasing action execution duration for all customers because Node.js will always download on-flight.

@karlhorky
Copy link
Author

karlhorky commented Aug 22, 2024

When setup-node action was implemented initially, we designed it to optimize workflow duration to speed up customers' workflows and reduce VM cost for customers

This is good. Speed and reduction of cost / resource usage is great.

Usually, VM cache is up-to-date and it can be 5-6 days old in worst case which is acceptable for the most of customers.

For the "which is acceptable for most of customers" assertion, where do you get the data from? Have you spoken with a high number of customers who use a version specifier like lts/* and asked whether they think it's ok that it downloads old versions? Or is it just an assumption that you and the team have? Have you considered security releases of Node.js too, where it is often important to upgrade faster?

My assumption would be the opposite - lts/* and other version specifiers downloading old versions because of caching for multiple days is confusing, buggy, broken behavior that will lead users to waste time debugging.

I could imagine a short delay would be acceptable (between 1-3 hours after release), but even that is able to be mitigated with a proper cache design.

If some customers need to always use the freshest version of Node.js, ""check-latest"" flag is intended for it. If ""check-latest: true"" is passed, action will ignore VM cache and always download latest version.
So, using version: lts/* with check-latest: true will give you exact behaviour which you expect.

First of all, if you read the original post again, I used check-latest: true and this was failing to retrieve the latest version. So this feature is also broken (or at least it was, when I reported the issue).

Also, this is *not* the behavior I'm looking for, nor should it be the solution proposed to customers, let me demonstrate:

1. actions/setup-node@v4 with node-version: 'lts/*'

  • 👍 Uses the cache every time - performant
  • 👎 The cache can be multiple days stale, leading to old versions

2. actions/setup-node@v4 with node-version: 'lts/*' and check-latest: true

  • 👎 Uses no cache every time - slow
  • 👍 The cache is always fresh

Neither of those options are good. This makes actions/setup-node@v4 an inferior product to other options. I would suggest that the official GitHub Actions setup-node action should be the better product.

I'm proposing a 3rd option, which is in my opinion how the caching should have been designed from the start.

3. actions/setup-node@v4 with node-version: 'lts/*' (new default behavior)

  • 👍 Uses the cache every time - performant
  • 👍 The cache is invalidated remotely, when a new Node.js version is released, leading to fresh versions

There should be no check-latest option.

@karlhorky
Copy link
Author

Actually, maybe there's even more to be improved here, if actions/setup-node changed its design from the manifest + hosted tool cache to the built-in nvm:

It is why check-latest is disabled by default and action fallbacks to cached version of Node.js on VM. It allows to speed up workflows by 10-15 seconds

It only takes 3 additional seconds to download a fresh copy of Node.js via nvm:

actions/setup-node@v4 with manifest + hosted tool cache

8 seconds total

Screenshot 2024-08-22 at 07 52 07

nvm with no Node.js cache

11 seconds total

Screenshot 2024-08-22 at 07 59 20

Don't be confused by actions/cache - in both cases this is used for pnpm store cache - unrelated to the Node.js cache.

So an alternative would be that actions/setup-node could probably be faster by using nvm in some way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants
@karlhorky @HarithaVattikuti @mikhailkoliada @aparnajyothi-y and others