-
Notifications
You must be signed in to change notification settings - Fork 29
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
Add jit config #163
Add jit config #163
Conversation
CC @maigl This change will work with GHES |
Okay thx, we're not at 3.10 yet, so the fallback is important to us. |
5a891ae
to
b5797f9
Compare
runner/pool/pool.go
Outdated
labels := r.getLabelsForInstance(pool) | ||
// Attempt to create JIT config | ||
jitConfig, runner, err := r.helper.GetJITConfig(ctx, instance, pool, labels) | ||
if err == nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@maigl This should fall back to the old way of registering a runner. Any error in creating a JIT config should make it fall back to the old registration method.
08b9bf1
to
bd50c05
Compare
@maigl with the latest commit, fallback should work. The logs will show something like this: Aug 24 13:37:36 garm garm[3161868]: 2023/08/24 13:37:36 [Pool mgr ghadmin/test-repo] failed to get JIT config, falling back to registration token: failed to get JIT config: POST https://ghe.example.com/api/v3/repos/ghadmin/test-repo/actions/runners/generate-jitconfig: 404 Not Found [] If you have some cycles, it would be great if you could test it against your env. Note, you will also need to update the openstack external provider to latest [default]
webhook_url = "https://garm.example.com/webhooks"
enable_webhook_management = true in your garm config. Once your instance updates to This PR will most likely linger in draft until there is an upstream |
CC @mkulke I know you were looking into adding JIT at some point. |
Awesome, thanks. I'll take a look |
5eb76d7
to
09405dd
Compare
Signed-off-by: Gabriel Adrian Samfira <[email protected]>
Signed-off-by: Gabriel Adrian Samfira <[email protected]>
Signed-off-by: Gabriel Adrian Samfira <[email protected]>
Signed-off-by: Gabriel Adrian Samfira <[email protected]>
When using JIT runners, we register the runner on GitHub before we get a chance to spin up the instance in the provider. In such cases, we end up with a runner in "offline" state while we're creating the actual resource that will embody the runner. This change will give runners a chance to come online before garm tries to clean them up. Signed-off-by: Gabriel Adrian Samfira <[email protected]>
Signed-off-by: Gabriel Adrian Samfira <[email protected]>
Signed-off-by: Gabriel Adrian Samfira <[email protected]>
Signed-off-by: Gabriel Adrian Samfira <[email protected]>
Signed-off-by: Gabriel Adrian Samfira <[email protected]>
Signed-off-by: Gabriel Adrian Samfira <[email protected]>
Signed-off-by: Gabriel Adrian Samfira <[email protected]>
Signed-off-by: Gabriel Adrian Samfira <[email protected]>
Signed-off-by: Gabriel Adrian Samfira <[email protected]>
We must create the DB entry for a runner with a JIT config included. Adding it later via an update runs the risk of having the consolidate loop pick up the incomplete instance. Signed-off-by: Gabriel Adrian Samfira <[email protected]>
09405dd
to
019948a
Compare
This PR changes the way GARM registers runners. With this change, GARM will now attempt to use Just-In-Time configuration instead of passing a runner registration token to the instance to run against the
./config.sh
script.This adds a number of advantages:
self-hosted
,x64
,linux
)This change is currently blocked until the following PRs merge and a release is cut: