saucelabs / sauce-connect-action

A GitHub action to launch Sauce Connect Proxy.
https://github.com/marketplace/actions/sauce-connect-proxy-action
MIT License
13 stars 19 forks source link

refactor: install sc proxy using actions/tool-cache instead of docker #54

Closed ZauberNerd closed 2 years ago

ZauberNerd commented 2 years ago

This commit refactors the sauce-connect-action to use the @actions/tool-cache to download the sauce connect proxy binary for the current architecture and place it into the hosted tool cache.

This allows to use the action inside containers, for example by using the container directive or even inside the act tool.

References:

ZauberNerd commented 2 years ago

@waggledans any thoughts on this change?

waggledans commented 2 years ago

@ZauberNerd tnx for your contribution. Looks promising, will review as soon as I have time (probably tomorrow)

ZauberNerd commented 2 years ago

@waggledans did you find some time to take a look at this change?

waggledans commented 2 years ago

Merged https://github.com/saucelabs/sauce-connect-action/pull/53, will check this one tomorrow. Thank you for your contribution!

waggledans commented 2 years ago

The CI fails with

/opt/hostedtoolcache/sc/4.8.1/x64/sc-4.8.1-linux/bin/sc  ... --readyfile=/tmp/sauce-connect-actioncO0LVR/sc.ready

....

Sauce connect log:

2022-09-13 05:38:55.259 [1689] [CLI] [info] Sauce Connect 4.8.1, build ...
22-09-13 05:38:55.259 [1689] [CLI] [fatal] Error creating pidfile /srv/sauce-connect.pid: open /srv/sauce-connect.pid: permission denied

I think, you could hardcode the PID file to some arbitrary file, like /tmp/scXXX.pid. AFAICT, the PID file is created to support a non-ephemeral (ie long-running) server environment, I don't think it's useful in the setup that involves ephemeral docker containers.

ZauberNerd commented 2 years ago

As per my last commit: I don't think the pidfile is used at all, because the documentation notes, that the --pidfile argument is used to automatically stop the sauce connect proxy. But we are using saveState() and getState() to pass the pid between the main and post action and thus won't need the pid file.

waggledans commented 2 years ago

lgtm. Thank you @ZauberNerd !