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 Tool Cache for Self Hosted Runners #461

Closed
wants to merge 2 commits into from

Conversation

techman83
Copy link
Contributor

@techman83 techman83 commented Jul 15, 2022

Description:
As of actions/python-versions@f75c0e6 the default location for Linux/Mac/Windows was aligned to default to RUNNER_TOOL_CACHE if AGENT_TOOLCACHE wasn't specified. The change introduced and released in v4.1.0 didn't account for this default being different for self hosted runners and hard coded it to /opt/hostedtoolcache.

This change aligns the behaviour of actions/setup-python with action/python-versions, and makes the experience of using the action the same on both hosted + self-hosted runners.

I have adjusted the documentation, and made the changes in line with how the project appears to operate, but as shouldn't be much of a surprise, TypeScript isn't my day-to-day language 🙂 and welcome any feedback.

If there are test cases that can be added, happy for some guidance, but I didn't see any with the commit that changed this behaviour. From my manual testing, both hosted + self-hosted work as I would expect.

Hosted Linux

Requirement already satisfied: setuptools in /opt/hostedtoolcache/Python/3.11.0-beta.4/x64/lib/python3.11/site-packages (58.1.0)
Requirement already satisfied: pip in /opt/hostedtoolcache/Python/3.11.0-beta.4/x64/lib/python3.11/site-packages (22.0.4)`

Hosted Windows

Requirement already satisfied: setuptools in c:\hostedtoolcache\windows\python\3.11.0-beta.4\x64\lib\site-packages (58.1.0)
Requirement already satisfied: pip in c:\hostedtoolcache\windows\python\3.11.0-beta.4\x64\lib\site-packages (22.0.4)
Requirement already satisfied: pip in c:\hostedtoolcache\windows\python\3.11.0-beta.4\x64\lib\site-packages (22.0.4)

Hosted Mac

Requirement already satisfied: setuptools in /Users/runner/hostedtoolcache/Python/3.11.0-beta.4/x64/lib/python3.11/site-packages (58.1.0)
Requirement already satisfied: pip in /Users/runner/hostedtoolcache/Python/3.11.0-beta.4/x64/lib/python3.11/site-packages (22.0.4)

Self-Hosted Linux

Requirement already satisfied: setuptools in /home/actions/actions-runner/_work/_tool/Python/3.11.0-beta.4/x64/lib/python3.11/site-packages (58.1.0)
Requirement already satisfied: pip in /home/actions/actions-runner/_work/_tool/Python/3.11.0-beta.4/x64/lib/python3.11/site-packages (22.0.4)

Related issue:
Fixes #459

Check list:

  • Mark if documentation changes are required.
  • Mark if tests were added or updated to cover the changes.

This aligns the tool cache path logic with the process used by
actions/python-versions, maintaining the ability to locate the
version binaries, and fixing self-hosted runners.

    Fixes actions#459
This updates and simplies the tool cache documentation to match the implementation in
both  and

  Relates actions#459
@techman83 techman83 requested a review from a team July 15, 2022 07:21
@techman83 techman83 changed the title Fix/self hosted Fix Tool Cache for Self Hosted Runners Jul 15, 2022
@IvanZosimov
Copy link
Contributor

Hi, @techman83 👋 ! Thank you for your contribution! Please, read this part of the setup-python README.md file again. The main sense here is:
The Python packages that are downloaded from actions/python-versions are originally compiled from source in /opt/hostedtoolcache/ directory with the --enable-shared flag, which makes them non-relocatable. It means that on the runner, Python should be placed on the same directory /opt/hostedtoolcache/ in order to work properly. It's just a matter of chance that in your case with location /home/actions/actions-runner/_work/_tool it works as expected. In fact, we had a lot of issues when users forgot to set ACTION_TOOLSDIRECTORY, installed Python into the path that was different from /opt/hostedtoolcache/ and got issues with it. But now functionality for setting this variable automatically is added, all you need just to configure /opt/hostedtoolcache/ folder and check required permissions.

We understand that sometimes there could be a problem to use /opt/hostedtoolcache/ because of the lack of permissions. In that case, you can configure AGENTS_TOOLDIRECTORY variable with any path that suits you, but you have to understand that you're taking a risk of getting unpredictable issues.

By now I'm going to close this PR, if you have any questions feel free to ping us here or in the mentioned above issue.

@techman83
Copy link
Contributor Author

techman83 commented Jul 15, 2022

@IvanZosimov I have read that section of the readme a number of times, whilst I hear and appreciate what you are saying, that interpretation as far as I can gather, is incorrect.

This change was to fix an issue with actions/python-versions, that was already corrected in 2020 -> actions/python-versions@f75c0e6

The directions in the readme make using actions/setup-python far more complex than they need to be, and you will find that most consumers of this action will miss that step as it "just works" for hosted.

Personally it will be easy for me to adjust our runner permissions, but if I didn't have that level of control over them, I would have to go to every work flow and reset the runner cache back using an environment variable attached pointing to the runner context.

    steps:
      - uses: actions/checkout@v3
      - name: Setup Python
        env:
          AGENT_TOOLSDIRECTORY: ${{ runner.tool_cache }}
        uses: actions/setup-python@v4
        with:
          python-version: "3.11.0-beta.4"
          cache: pip

Adding to this, to be clear. This does not change the behaviour of hosted runners. RUNNER_TOOL_CACHE points to /opt/hostedtoolcache on hosted runners. This persists that behaviour, and means that self-hosted runners "just work" rather than requiring a string of extra steps to use an action.

I will apologise if there is something glaringly obvious I've missed, but an improvement in user experience shouldn't be excluded just because it's documented in the readme (short of breaking, backwards incompatible changes).

@techman83
Copy link
Contributor Author

techman83 commented Jul 18, 2022

I won't spam open PRs, but I did realise setting the AGENT_TOOLSDIRECTORY was unnecessary, but overriding the RUNNER_TOOL_CACHE is necessary when it is set. I have the diff here and a demonstration here

Edit: Updated, as an assumption was incorrect.

@IvanZosimov
Copy link
Contributor

Hi, @techman83 👋 First of all I'd like to thank you for your contribution and willingness to improve our action ❤️ Your questions made us to make deeper investigation. May I ask you to revive this PR?

@techman83
Copy link
Contributor Author

@IvanZosimov sure thing! I'll raise a new PR, I learned more about what was happening and I had definitely had a misunderstanding about what the #394 was intended to fix. So I have made some improvements with that in mind 🙂

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.

Cannot create directory ‘/opt/hostedtoolcache’ after recent release
2 participants