scaleset: adopt existing GitHub scale sets

[rm-you]

A database reset removes GARM's local scale-set records but leaves the GitHub scale sets. Recreating the configuration then fails with RunnerScaleSetExistsException before a worker exists to recover the association.

Look up an exact name in the selected runner group before creating a GitHub scale set, and retry the lookup after a duplicate-create race. Apply the same recovery when a worker finds a local scale set with an unset GitHub ID. Group-scoped list results may omit runnerGroupId, so accept an omitted value while rejecting populated mismatches and ambiguous matches.

Tests cover API and worker adoption, duplicate-create recovery, cleanup ownership, nested duplicate errors, unrelated bad requests, escaped names, missing scale-set creation, and known-ID conflicts.

[gabriel-samfira]

Hi @rm-you

Foreword: Apologies for the wall of text. Let's discuss the situation that lead to the need for this change, in the hopes of finding a clean solution.

---

A database reset removes GARM's local scale-set records but leaves the GitHub scale sets.

This is a situation that should not happen unless something catastrophic occurs. Resetting the database also resets the controller ID, which invalidates webhooks (in the case of pools), makes tags for instances no longer match the controller, etc. So this is a failure mode which results in a lot of orphaned resources not only in github but in the IaaS that the previous GARM controller managed resources in.

Resetting the database will in essence, give you a brand new controller. For situations such as this, if it is indeed a failure mode you're planning for, you must have a disaster recovery (DR) strategy. Especially if you're dealing with many runners/pools/scale sets.

Recreating the configuration then fails with RunnerScaleSetExistsException before a worker exists to recover the association.

That is by design. And it is so because we cannot know if existing scalesets began their existence via GARM or any other auto scaler like ARC or some other custom solution. And sadly, there is (or at least was at the time of initial support) no way to tag the scaleset in such a way that we know it used to belong to our controller.

If the scaleset has active or idle runners, there is no way to know the IaaS they were created in or what IaaS provider ID those runners have. So we would adopt the scale set, but would not be able to reliably adopt the existing runners.

Another reason for not adding scale set/runner adoption was that only one auto scaler may subscribe to a scaleset message queue. If another auto scaler is actively servicing a scale set we would potentially try to "adopt", GARM would fail in opaque ways.

Syncing state is somewhat brittle even in ideal conditions, given that forges have a tendency as of late to have outages. Adopting existing scale sets and runners would add a large failure surface to GARM which would generate all sorts of support requests.

---

All that being said, a more correct recovery path is to:

• remove runners from github
• remove associated instances from the IaaS
• remove scale set from github
• recreate the scale set in GARM

Otherwise, adopting the scaleset is just one part in the DB surgery and runner instance tag updates (in the provider of choice) you'd have to do to get in a consistent state from which GARM can continue to function reliably, in non ideal cases (scaleset still has runner instances at the time of DB reset).

[rm-you]

Thanks for the thorough response!
I'll admit my circumstances are a bit wonky -- my enterprise doesn't allow us access to the GitHub runner settings, so we can't actually do stuff to the groups by hand. Our GitHub app has access, so we can write scripts and such to do it, but it's very much a manual process. In looking at ARC (which we used to run), it DOES seem to support adoption, and I figured that was proof it was generally acceptable without looking that much deeper into it. Obviously that is not the case and likely their implementation is just naive. 🥲

We ran into this primarily in our staging and development ArgoCD/k8s clusters where we frequently end up blowing away the entire environment destructively. That's obviously not something that should occur in a real production system, and most users likely wouldn't need to deal with it. I've been running this patch internally and it works wonders for making our full environment come back online correctly from scratch every time. It's because we nuke the entire environment that I also hadn't really given much consideration to the existing resources, since it is assumed those are dead too. I'm actually not sure if they continue to persist in some way on the GitHub side -- as I said, we are blocked from basically any admin-like activity on GitHub by our enterprise and that includes everything with runners. It was like pulling teeth just to get them to allow us a runner group at all (almost didn't happen) and to grant our app access to it. This is unusual (and frustrating even for my team) so I wouldn't expect this to be a normal situation for most folks and I understand why it seems like "just go do this in the GitHub UI" would be a reasonable expectation.

If this isn't something we can consider upstreaming, we do have the infrastructure to continue carrying the patch downstream, I just generally prefer to avoid that, and figured others might find this functionality useful. Maybe it could be behind some explicit configuration variable or runtime flag that makes it clear the danger?
No hard feelings if you'd rather just decline though! I also have a few other PRs up now (we just got approval recently to upstream our patches so I've been going through and rebasing and cleaning up quite a few things over the past few days) so I can focus on those instead.

Thanks again for your time (and equally sorry about my response-wall-of-text). 😇

[gabriel-samfira]

In looking at ARC (which we used to run), it DOES seem to support adoption, and I figured that was proof it was generally acceptable without looking that much deeper into it.

ARCs model is narrower. It currently only creates runners in the k8s cluster its deployed in, so matching a runner registered in github to a runner running in the k8s cluster is simple, as it's referenced by name, guaranteed to be unique within a NS, etc.

GARM has plug-able providers and each IaaS has its own idea of unique identifiers for the resources it creates. Names are not always unique. It is why we have the runner ID that represents the runner within the GARM database, but we also have the provider ID which keeps track of the runner within the IaaS. We do have a fallback to search by name if a provider ID is never return by the IaaS or if we error out, but that is just a fallback, not guaranteed to always work. Think of it as a last ditch effort at a cleanup attempt.

So in an environment where you mix pools/scale sets in multiple providers (OpenStack, Azure, GCP, LXD, OCI), trying to adopt runners from a scale set means also adopting the compute instance created in one of the providers.

There is another limitation. GARM was designed to be deployable by multiple teams. Each team with their own instance of GARM, potentially sharing the same IaaS. Compute instances are tagged with the controller ID of GARM so we don't accidentally reap/delete runners from other controllers. When GARM consolidates and lists runners, it searches for the controller ID tag on the compute instance in the target IaaS. If the controller ID is not found, we ignore the instance, as it may belong to someone else. And the last thing we want to do is clobber someone else's runner while it's potentially running a 6 hour workflow.

ARC owns the runners in its own cluster. No risk of clobbering someone else. A cloud account may be a shared resource.

If you do have the option, I recommend that before clobbering your cluster, you disable all pools/scale sets, remove all runners, pools and scale sets by calling the GARM API, then you can safely reset the DB.

If that is not an option, you can still create a small script that cleans up github runners using your github app, compute instances using the IaaS credentials you use to configure the pools and them you should be in good shape.

I would love to add something like this to GARM, but it needs to work correctly with all the providers and with pools as well as scale sets. Right now, I am not sure how to best do that, given the constraints GARM has.

Maybe it could be behind some explicit configuration variable or runtime flag that makes it clear the danger?

That sounds like a good compromise, until you have a huge list of knobs, each turning functionality on or off. If we do add a feature, it needs to fit in with everything else and not be a special case that works on just a few providers or just on scale sets (unless it's a scale set only feature).

It was like pulling teeth just to get them to allow us a runner group at all (almost didn't happen) and to grant our app access to it.

I can empathize 😄

This is unusual (and frustrating even for my team) so I wouldn't expect this to be a normal situation for most folks and I understand why it seems like "just go do this in the GitHub UI" would be a reasonable expectation.

Not much you can do from the UI to be honest. Last time I checked, you can't delete a scale set from the UI anyway. The best option is a cleanup script.

---

As a side note, I just finished a large project and I can now re-focus for a while on GARM. So once I get some sleep, I will review all current PRs. Feel free to propose 😁

[rm-you]

Ok, thanks a lot for your time and feedback. To be honest I could probably just replace what this does with an "always wipe out all scalesets on the group" script and put it in the entrypoint script in our operator deployment. We DO also use OpenStack VM runners, and I have a whole other series of PRs to enable that correctly for other reasons (some still need to be posted after I get gophercloud fixes/features merged) so I get that too, but wiping out our environment (which includes all of OpenStack) also means the VMs are gone. 😂

So yes, I see the complications you're talking about and I agree this can't merge as is, and likely isn't something that can be reasonably added as a feature without a ton of planning and heavy lifting, if ever. I'm fine with closing this and not making you spend a whole lot more time on the subject, the other PRs I have will be way more important for us anyway.
Is that what you'd recommend?

[gabriel-samfira]

Yup. Let's focus on the other PRs for now. We can turn this PR into a discussion so we don't forget about it and we can potentially pick it up again later in a way it may work (if we figure it out).

I am keen on knowing more about how you use GARM with OpenStack. $DAYJOB for me has been designing and deploying OpenStack clouds, so I am curious if you've hit any pain points.

Just as a heads up, it may take a couple of weeks more for me to get to actually review PRs. I am winding down from a large project, going on vacation to recuperate and will be back at the end of the month.

[rm-you]

Ok, no worries! Yes, I've been working on OpenStack for about 12 years now, started the Octavia LB project with a bunch of folks back when I was at RAX, have been deploying it places since. :)

We should chat, I'm working on a lot of cool stuff that I'd love to see more people adopt / put eyes on. It's off-topic for here, but if you've ever been frustrated with RMQ, I've got a sales pitch for an alternative for you. 🤣

[gabriel-samfira]

we have a slack channel for GARM (you can see it in the README). Feel free to join and ping me directly there.