cloudbase / garm-provider-common

Common functionality for GARM providers
https://github.com/cloudbase/garm-provider-common
Apache License 2.0
0 stars 5 forks source link

`config.sh` not executed with JIT tokens -> `.env` / `.path` not created #39

Open SystemKeeper opened 1 week ago

SystemKeeper commented 1 week ago

After "fixing" https://github.com/cloudbase/garm/issues/309 for us, I enabled JIT tokens again. Soon afterwards we saw failing runs in connection with android emulators. After investigation we found out that when setting up the runner with JIT tokens, garm does not execute config.sh, which in turn executes env.sh (https://github.com/actions/runner/blob/3d34a3c6d6dfd7599e489b3c04fcf95a8a428434/src/Misc/layoutroot/config.sh#L74). Therefore no .env file and no .path in /home/runner/actions-runner/ is created and the run is missing the necessary environment variables.

So question is, do we still need to run config.sh before we run the JIT runner? According to the docs, it seems it might not be needed: https://docs.github.com/en/actions/security-for-github-actions/security-guides/security-hardening-for-github-actions#using-just-in-time-runners ? But then I'd say it's a issue in the runner repo?

SystemKeeper commented 1 week ago

JIT runner: image

Non-JIT runner: image

gabriel-samfira commented 1 week ago

Hi @SystemKeeper

insert cat hissing at computer meme

So, there is this discussion I opened:

https://github.com/orgs/community/discussions/58031

Which went unanswered and was automatically closed......

For JIT config we had to reproduce what run.sh does when you pass it a JIT config. We did this because we still wanted to have the runner be installed as a service. We're not using containers in most providers (save the k8s one). So we need a service. The current userdata does not run the env.sh, because we never thought of it. But apparently it's needed.

To test it out, you can override the install template and above this line:

https://github.com/cloudbase/garm-provider-common/blob/5b7633dfb896fe1a8ddcff745290e5ef21c65729/cloudconfig/templates.go#L185

add the following:

pushd $RUN_HOME
source env.sh
popd

And you should have the needed files.

Let me know if this works, and I can create a PR.

SystemKeeper commented 1 week ago

insert cat hissing at computer meme

So, there is this discussion I opened:

https://github.com/orgs/community/discussions/58031

Which went unanswered and was automatically closed......

🤦 No comment 😅 Thanks you - I did not find that issue when searching.

I changed the template and added:

pushd $RUN_HOME
source env.sh
popd

(still with the old template btw, since I didn't build a new image yet)

Works perfectly fine now 👍

gabriel-samfira commented 1 week ago

Will pish a PR soon. I'm thinking of adding a new feature to garm to allow a template catalog which can be associated with pools. Defauls to whatever the provider has, but if set explicitly, it will be overwritten. Right now it's cumbersome to set a template override. Nobody likes to mess with json and yaml if they have a choice.

SystemKeeper commented 1 week ago

Right now it's cumbersome to set a template override. Nobody likes to mess with json and yaml if they have a choice.

Yea, I actually just changed /vendor/github.com/cloudbase/garm-provider-common/cloudconfig and recompiled garm-provider-lxd for the test 😅

gabriel-samfira commented 1 week ago

When people prefer to change vendor and recompile, you know you have a UX problem 😭 .

Ok. Will find some time to add that.

SystemKeeper commented 1 week ago

Haha, sorry about that 😅

Not sure what the best way is here, tbh. For me it doesn't feel intuitive because I need to first find the current template (I never find it..), modify it, look what other options I already have set in extra-specs, change the pool etc. Maybe a way of dumping and loading the installation template would be possible? Or something what LXD does when editing stuff (e.g. lxc profile edit default). Just talking out loud here 🤷

gabriel-samfira commented 1 week ago

Yeah. That's exactly what I was thinking. A new command for the providers that dumps the default template. Then GARM can record the template in the db and users can dump it, edit it and upload it as a new template.

Then users can associate a template to a pool or set one as default for a provider (or something). Needs some thoughy