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

Allow Running Container as Runner Host User #600

Merged
merged 18 commits into from
Nov 25, 2023
Merged

Conversation

AndrewKahr
Copy link
Member

@AndrewKahr AndrewKahr commented Nov 21, 2023

Changes

  • Added runAsHostUser to allow running the container as the same user as the host system. This fixes most permissions issues on self-hosted runners.
  • Perform android sdk setup during entrypoint.sh to ensure it has root permissions if the user switches to a non-root user
  • Automatically detect android sdk target version if parameters are not already provided to configure the sdk
  • Generate a new uuid for machineID to ensure separate containers are unique to reduce license activation errors
  • Add exponential retry strategy for Ubuntu license activations

Related PR

Related Issues

Checklist

  • Read the contribution guide and accept the
    code of conduct
  • Docs (If new inputs or outputs have been added or changes to behavior that should be documented. Please make a PR
    in the documentation repo)
  • Readme (updated or not needed)
  • Tests (added, updated or not needed)

Copy link

Cat Gif

@codecov-commenter
Copy link

codecov-commenter commented Nov 21, 2023

Codecov Report

Merging #600 (110228f) into main (8da77ac) will increase coverage by 0.04%.
The diff coverage is 100.00%.

❗ Current head 110228f differs from pull request most recent head 88edf4e. Consider uploading reports for the commit 88edf4e to get more accurate results

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #600      +/-   ##
==========================================
+ Coverage   37.29%   37.33%   +0.04%     
==========================================
  Files          76       76              
  Lines        3057     3059       +2     
  Branches      610      611       +1     
==========================================
+ Hits         1140     1142       +2     
  Misses       1917     1917              
Files Coverage Δ
src/model/build-parameters.ts 90.00% <ø> (ø)
src/model/image-environment-factory.ts 3.70% <ø> (ø)
src/model/input.ts 88.43% <100.00%> (+0.15%) ⬆️

Copy link
Member

@GabLeRoux GabLeRoux left a comment

Choose a reason for hiding this comment

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

I haven't tried this, it looks fine to me, but I left a few comments. I'm not exactly sure why we're changing permissions to stdout and stderr. I assume this does indeed solve issues with self-hosted runner, but I'm wondering if this should always be done or if it should be done only based on configuration 🤔

dist/platforms/ubuntu/entrypoint.sh Outdated Show resolved Hide resolved
dist/platforms/ubuntu/entrypoint.sh Outdated Show resolved Hide resolved
dist/platforms/ubuntu/steps/runsteps.sh Show resolved Hide resolved
@AndrewKahr AndrewKahr merged commit 8ca1282 into main Nov 25, 2023
39 checks passed
@AndrewKahr AndrewKahr deleted the fix-permissions branch November 25, 2023 07:24
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.

4 participants