From fb659ad21de5a3b3df769d6d24998364c5bdeac7 Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Fri, 4 Sep 2026 13:27:34 +0300 Subject: [PATCH] Migrate from a concatenated pseudo pool ID This change migrates from a pseudo pool ID composed of the entity ID and the name of the scale set, to a stable ID computed from the entity ID and the internal scaleset ID as an UUID V5. Both the entity ID and the internal scaleset ID are immutable. The UUID V5 will always be 36 characters long, regardless of the input data used to compute it. The downside of this change is that we now need to do two ListInstance() provider calls to list both instances tagged with the old shape of the ID and the new shape. Internally we record whether or not ListInstances() returned an empty list. If empty, we don't have instances in the provider for a scale set, so on each subsequent list, we only do one call. So in theory at least, we should only be abusing the IaaS API only as long as there are still old runners tagged with the old shape. Signed-off-by: Gabriel Adrian Samfira --- util/util.go | 15 +++++++++ workers/provider/instance_manager.go | 5 +-- workers/scaleset/scaleset.go | 48 ++++++++++++++++++++++++---- 3 files changed, 59 insertions(+), 9 deletions(-) diff --git a/util/util.go b/util/util.go index 170b4ae99..75c93d4f2 100644 --- a/util/util.go +++ b/util/util.go @@ -23,6 +23,7 @@ import ( "os" "unicode/utf8" + "github.com/google/uuid" "github.com/h2non/filetype" "github.com/cloudbase/garm-provider-common/cloudconfig" @@ -32,6 +33,20 @@ import ( "github.com/cloudbase/garm/runner/common" ) +// scaleSetPoolIDNamespace is the UUIDv5 namespace used to derive pseudo pool +// IDs for scale sets. Instances in providers are tagged with IDs derived from +// this namespace; changing it (or the derivation input format) orphans every +// instance created with the previous value. +var scaleSetPoolIDNamespace = uuid.MustParse("3f5bfba0-d82c-4f91-a5f6-ddc1c9209b01") + +// ScaleSetPseudoPoolID derives a stable, UUID-shaped pseudo pool ID for a +// scale set, from the forge entity ID and the internal scale set database ID. +// Both inputs are immutable, so the result never changes for the lifetime of +// the scale set and always fits within provider tag/label length limits. +func ScaleSetPseudoPoolID(entityID string, scaleSetID uint) string { + return uuid.NewSHA1(scaleSetPoolIDNamespace, fmt.Appendf(nil, "%s-%d", entityID, scaleSetID)).String() +} + func FetchTools(ctx context.Context, cli common.GithubClient) ([]commonParams.RunnerApplicationDownload, error) { tools, ghResp, err := cli.ListEntityRunnerApplicationDownloads(ctx) if err != nil { diff --git a/workers/provider/instance_manager.go b/workers/provider/instance_manager.go index f276bd404..2ba88d4d5 100644 --- a/workers/provider/instance_manager.go +++ b/workers/provider/instance_manager.go @@ -148,8 +148,9 @@ func (i *instanceManager) getEntity() (params.ForgeEntity, error) { } func (i *instanceManager) pseudoPoolID() string { - // This is temporary. We need to extend providers to know about scale sets. - return fmt.Sprintf("%s-%s", i.scaleSet.Name, i.scaleSetEntity.ID) + // nolint:golangci-lint,godox + // TODO(gabriel-samfira): extend providers to know about scale sets. + return garmUtil.ScaleSetPseudoPoolID(i.scaleSetEntity.ID, i.scaleSet.ID) } func (i *instanceManager) handleCreateInstanceInProvider(instance params.Instance) error { diff --git a/workers/scaleset/scaleset.go b/workers/scaleset/scaleset.go index 418756c0d..d6db786fc 100644 --- a/workers/scaleset/scaleset.go +++ b/workers/scaleset/scaleset.go @@ -62,6 +62,7 @@ func NewWorker(ctx context.Context, store dbCommon.Store, scaleSet params.ScaleS scaleSet: scaleSet, entity: entity, runners: make(map[string]params.Instance), + pseudoPoolID: garmUtil.ScaleSetPseudoPoolID(scalesetEntity.ID, scaleSet.ID), }, nil } @@ -76,6 +77,14 @@ type Worker struct { entity params.ForgeEntity runners map[string]params.Instance + // pseudoPoolID is the stable pool ID reported to providers for this scale + // set. It is derived from immutable IDs, so it is computed once. + pseudoPoolID string + // legacyPoolIDDrained indicates that the provider no longer has any + // instances tagged with the legacy name-based pseudo pool ID, so we can + // stop querying for it. + legacyPoolIDDrained bool + consumer dbCommon.Consumer listener *scaleSetListener @@ -551,10 +560,6 @@ func (w *Worker) consolidateRunnerState(runners []params.RunnerReference) error // DB runners with no provider instance are marked as pending_delete. Must be // called with w.mux held. func (w *Worker) consolidateProviderState() error { - pseudoPoolID, err := w.pseudoPoolID() - if err != nil { - return fmt.Errorf("getting pseudo pool ID: %w", err) - } listParams := common.ListInstancesParams{ ListInstancesV011: common.ListInstancesV011Params{ ProviderBaseParams: common.ProviderBaseParams{ @@ -563,7 +568,7 @@ func (w *Worker) consolidateProviderState() error { }, } - providerRunners, err := w.provider.ListInstances(w.ctx, pseudoPoolID, listParams) + providerRunners, err := w.provider.ListInstances(w.ctx, w.pseudoPoolID, listParams) if err != nil { return fmt.Errorf("listing instances: %w", err) } @@ -573,6 +578,27 @@ func (w *Worker) consolidateProviderState() error { providerRunnersByName[runner.Name] = runner } + if !w.legacyPoolIDDrained { + legacyID, err := w.legacyPseudoPoolID() + if err != nil { + return fmt.Errorf("getting legacy pseudo pool ID: %w", err) + } + legacyRunners, err := w.provider.ListInstances(w.ctx, legacyID, listParams) + if err != nil { + return fmt.Errorf("listing instances by legacy pseudo pool ID: %w", err) + } + if len(legacyRunners) == 0 { + w.legacyPoolIDDrained = true + } + for _, runner := range legacyRunners { + if _, ok := providerRunnersByName[runner.Name]; ok { + continue + } + providerRunnersByName[runner.Name] = runner + providerRunners = append(providerRunners, runner) + } + } + deleteInstanceParams := common.DeleteInstanceParams{ DeleteInstanceV011: common.DeleteInstanceV011Params{ ProviderBaseParams: common.ProviderBaseParams{ @@ -637,8 +663,16 @@ func (w *Worker) consolidateProviderState() error { return nil } -func (w *Worker) pseudoPoolID() (string, error) { - // This is temporary. We need to extend providers to know about scale sets. +// legacyPseudoPoolID returns the deprecated name-based pseudo pool ID. Older +// GARM versions tagged provider instances with this ID, so we keep listing by +// it until no instances tagged with it remain. It must be recomputed on every +// call, because the scale set name can change (which is bad and have failed to +// consider this until now). +// +// Deprecated: This function is "born" deprecated because it transitions from +// the old way to tag instances, to the new stable uuid V5 computed from the +// entity ID and the internal scaleset ID. +func (w *Worker) legacyPseudoPoolID() (string, error) { entity, err := w.scaleSet.GetEntity() if err != nil { return "", fmt.Errorf("getting entity: %w", err)