Make instance state transitions race-tolerant and bound provider execution time

apiserver/controllers/controllers.go

@@ -123,6 +123,9 @@ func handleError(ctx context.Context, w http.ResponseWriter, err error) {
 	case errors.Is(err, gErrors.ErrDuplicateEntity), errors.Is(err, &gErrors.ConflictError{}):
 		w.WriteHeader(http.StatusConflict)
 		apiErr.Error = "Conflict"
+	case errors.Is(err, gErrors.ErrUnprocessable):
+		w.WriteHeader(http.StatusUnprocessableEntity)
+		apiErr.Error = "Unprocessable Content"
 	default:
 		w.WriteHeader(http.StatusInternalServerError)
 		apiErr.Error = "Server error"

config/external.go

@@ -19,6 +19,7 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
+	"time"
 
 	"github.com/cloudbase/garm-provider-common/util/exec"
 )
@@ -48,6 +49,19 @@ type External struct {
 	// EnvironmentVariables is a list of environment variable names that will be
 	// passed to the external binary together with their values.
 	EnvironmentVariables []string `toml:"environment_variables" json:"environment-variables"`
+	// ExecTimeoutSeconds is the maximum number of seconds a single invocation
+	// of the provider binary may run. When exceeded, the binary is killed and
+	// the operation fails; for instance creation this marks the instance as
+	// error and normal cleanup takes over. This bounds how long an instance
+	// can sit in the creating/deleting states if a provider binary hangs.
+	// A value of 0 (the default) disables the timeout.
+	ExecTimeoutSeconds uint `toml:"exec_timeout_seconds" json:"exec-timeout-seconds"`
+}
+
+// ExecTimeout returns the configured provider binary execution timeout as a
+// time.Duration. A zero duration means no timeout is enforced.
+func (e *External) ExecTimeout() time.Duration {
+	return time.Duration(e.ExecTimeoutSeconds) * time.Second
 }
 
 func (e *External) GetEnvironmentVariables() []string {

database/sql/instances.go

@@ -31,6 +31,7 @@ import (
 	runnerErrors "github.com/cloudbase/garm-provider-common/errors"
 	commonParams "github.com/cloudbase/garm-provider-common/params"
 	"github.com/cloudbase/garm/database/common"
+	garmErrors "github.com/cloudbase/garm/internal/errors"
 	"github.com/cloudbase/garm/params"
 )
 
@@ -319,7 +320,7 @@ func (s *sqlDatabase) validateRunnerStatusTransition(current, newStatus params.R
 	}
 
 	if !slices.Contains(allowedTransitions, newStatus) {
-		return runnerErrors.NewBadRequestError("invalid runner status transition from %s to %s", current, newStatus)
+		return garmErrors.NewRunnerTransitionError(current, newStatus)
 	}
 	return nil
 }
@@ -338,7 +339,7 @@ func (s *sqlDatabase) validateInstanceStatusTransition(current, newStatus common
 	}
 
 	if !slices.Contains(allowedTransitions, newStatus) {
-		return runnerErrors.NewBadRequestError("invalid instance status transition from %s to %s", current, newStatus)
+		return runnerErrors.NewInstanceTransitionError(current, newStatus)
 	}
 	return nil
 }

doc/providers.md

@@ -50,6 +50,24 @@ However, some providers need access to host environment variables for authentica
 
 This passes all environment variables starting with `AWS_` to the provider. You can also specify exact variable names (e.g., `["AZURE_CLIENT_ID", "AZURE_TENANT_ID"]`).
 
+## Bounding provider execution time
+
+Instance creation is always bounded by the pool or scale set's `runner_bootstrap_timeout`. If the provider binary does not return within that window, it is killed, the instance is marked as `error` and it is cleaned up through the normal lifecycle. Past that point the runner would be considered failed anyway, so there is no reason to keep waiting on a hung create.
+
+All other provider operations (delete, list, etc.) run without a deadline by default. You can bound every invocation of a provider binary with `exec_timeout_seconds`:
+
+```toml
+[provider.external]
+  provider_executable = "/opt/garm/providers.d/garm-provider-aws"
+  config_file = "/etc/garm/garm-provider-aws.toml"
+  exec_timeout_seconds = 900
+```
+
+When the timeout is exceeded, the binary is killed and the operation fails.
+
+> [!IMPORTANT]
+> For instance creation, the effective deadline is the smaller of `exec_timeout_seconds` and the pool or scale set's `runner_bootstrap_timeout`. If you set `exec_timeout_seconds`, keep it below your bootstrap timeouts — a create that is allowed to outlive the bootstrap timeout only produces a runner that is already considered failed. A value of `0` (the default) leaves non-create operations unbounded.
+
 ## Listing configured providers
 
 ```bash

go.mod

@@ -6,7 +6,7 @@ require (
 	github.com/BurntSushi/toml v1.6.0
 	github.com/alecthomas/chroma/v2 v2.27.0
 	github.com/bradleyfalzon/ghinstallation/v2 v2.19.0
-	github.com/cloudbase/garm-provider-common v0.1.10-0.20260707095006-33396540b57b
+	github.com/cloudbase/garm-provider-common v0.1.10-0.20260707224136-4f5d9bf80949
 	github.com/felixge/httpsnoop v1.1.0
 	github.com/gdamore/tcell/v2 v2.13.10
 	github.com/go-gormigrate/gormigrate/v2 v2.1.6

go.sum

@@ -25,8 +25,8 @@ github.com/chzyer/test v1.0.0 h1:p3BQDXSxOhOG0P9z6/hGnII4LGiEPOYBhs8asl/fC04=
 github.com/chzyer/test v1.0.0/go.mod h1:2JlltgoNkt4TW/z9V/IzDdFaMTM2JPIi26O1pF38GC8=
 github.com/clipperhouse/uax29/v2 v2.7.0 h1:+gs4oBZ2gPfVrKPthwbMzWZDaAFPGYK72F0NJv2v7Vk=
 github.com/clipperhouse/uax29/v2 v2.7.0/go.mod h1:EFJ2TJMRUaplDxHKj1qAEhCtQPW2tJSwu5BF98AuoVM=
-github.com/cloudbase/garm-provider-common v0.1.10-0.20260707095006-33396540b57b h1:70W/+KXIfnNXy8KcT17CdnDG5U7yflS3rxvIgfIKzDw=
-github.com/cloudbase/garm-provider-common v0.1.10-0.20260707095006-33396540b57b/go.mod h1:9UvQ/bHlwgSAeLFV94by9M7kRg3d0H3kemkjcFzJMmc=
+github.com/cloudbase/garm-provider-common v0.1.10-0.20260707224136-4f5d9bf80949 h1:Ksf13uqPZhEcGwAHT1pmAXU3+LcAKl1yqyLWiKIxLTc=
+github.com/cloudbase/garm-provider-common v0.1.10-0.20260707224136-4f5d9bf80949/go.mod h1:9UvQ/bHlwgSAeLFV94by9M7kRg3d0H3kemkjcFzJMmc=
 github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=

internal/errors/errors.go

@@ -0,0 +1,104 @@
+// Copyright 2026 Cloudbase Solutions SRL
+//
+//	Licensed under the Apache License, Version 2.0 (the "License"); you may
+//	not use this file except in compliance with the License. You may obtain
+//	a copy of the License at
+//
+//	     http://www.apache.org/licenses/LICENSE-2.0
+//
+//	Unless required by applicable law or agreed to in writing, software
+//	distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+//	WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+//	License for the specific language governing permissions and limitations
+//	under the License.
+package errors
+
+import (
+	"fmt"
+
+	runnerErrors "github.com/cloudbase/garm-provider-common/errors"
+	commonParams "github.com/cloudbase/garm-provider-common/params"
+	"github.com/cloudbase/garm/params"
+)
+
+// NewRunnerTransitionError returns a RunnerTransitionError describing an invalid
+// runner status state machine transition.
+func NewRunnerTransitionError(from, to params.RunnerStatus) error {
+	return &RunnerTransitionError{
+		From: from,
+		To:   to,
+	}
+}
+
+// RunnerTransitionError is returned when a requested runner status transition is
+// rejected by the state machine. It carries the current (From) and requested
+// (To) statuses as their proper type, so callers can compare them directly (via
+// errors.As) without casting through strings. It also reports as a
+// BadRequestError, so it maps to an HTTP 400.
+type RunnerTransitionError struct {
+	From params.RunnerStatus
+	To   params.RunnerStatus
+}
+
+func (e *RunnerTransitionError) Error() string {
+	return fmt.Sprintf("invalid runner status transition from %s to %s", e.From, e.To)
+}
+
+func (e *RunnerTransitionError) Is(target error) bool {
+	if target == nil {
+		return false
+	}
+
+	switch target.(type) {
+	case *RunnerTransitionError, *runnerErrors.BadRequestError:
+		return true
+	default:
+		return false
+	}
+}
+
+// InstanceIsBeingDeleted reports whether an instance status is on the deletion
+// lane, meaning the runner is already on its way out. Note that the lane is
+// not strictly monotonic: a failed provider delete moves the instance from
+// deleting to error. Tolerating a status observed here is still safe because
+// reconciliation re-drives error back to pending_delete until the delete
+// succeeds — the system converges by retry, and the intent behind a rejected
+// transition to pending_delete (the runner should be removed) remains
+// satisfied. Pair it with InstanceTransitionError.From.
+func InstanceIsBeingDeleted(s commonParams.InstanceStatus) bool {
+	switch s {
+	case commonParams.InstancePendingDelete, commonParams.InstancePendingForceDelete,
+		commonParams.InstanceDeleting, commonParams.InstanceDeleted:
+		return true
+	default:
+		return false
+	}
+}
+
+// InstanceIsProvisioning reports whether an instance status indicates the
+// provider is still creating the instance (or is about to). The provider
+// worker owns this stage of the lifecycle: deletion cannot be requested until
+// the create call returns (creating only transitions to error or running).
+// Pair it with InstanceTransitionError.From to decide whether a rejected
+// transition to pending_delete should be deferred to a later reconciliation
+// pass instead of being treated as fatal.
+func InstanceIsProvisioning(s commonParams.InstanceStatus) bool {
+	switch s {
+	case commonParams.InstancePendingCreate, commonParams.InstanceCreating:
+		return true
+	default:
+		return false
+	}
+}
+
+// RunnerIsTerminal reports whether a runner status is terminal, meaning a
+// transition to active can no longer succeed and a late "started" message for
+// the runner is moot. Pair it with RunnerTransitionError.From.
+func RunnerIsTerminal(s params.RunnerStatus) bool {
+	switch s {
+	case params.RunnerTerminated, params.RunnerFailed:
+		return true
+	default:
+		return false
+	}
+}

params/params.go

@@ -280,12 +280,23 @@ var RunnerStatusTransitions = map[RunnerStatus][]RunnerStatus{
 		RunnerInstalling,
 		RunnerTerminated,
 		RunnerPending,
+		// The forge assigns jobs based on its own registration state; a
+		// "job started" event can arrive before the runner's own idle
+		// callback (or the image may never send callbacks at all), so a
+		// runner the forge considers busy is authoritatively active.
+		RunnerActive,
 	},
 	RunnerInstalling: {
 		RunnerFailed,
 		RunnerIdle,
 		RunnerTerminated,
 		RunnerInstalling,
+		// Same as RunnerPending: the forge saying a job started on this
+		// runner is proof it finished installing.
+		RunnerActive,
+		// When an agent manages the runner, the runner itself is offline,
+		// until the process is started and set online by the agent.
+		RunnerOffline,
 	},
 	RunnerIdle: {
 		RunnerOffline,

runner/pool/pool.go

@@ -39,6 +39,7 @@ import (
 	"github.com/cloudbase/garm/cache"
 	dbCommon "github.com/cloudbase/garm/database/common"
 	"github.com/cloudbase/garm/database/watcher"
+	garmErrors "github.com/cloudbase/garm/internal/errors"
 	"github.com/cloudbase/garm/locking"
 	"github.com/cloudbase/garm/params"
 	"github.com/cloudbase/garm/runner/common"
@@ -346,6 +347,12 @@ func (r *basePoolManager) handleCompletedJob(ctx context.Context, jobParams para
 		if errors.Is(err, runnerErrors.ErrNotFound) {
 			return nil
 		}
+		var te *runnerErrors.InstanceTransitionError
+		if errors.As(err, &te) && garmErrors.InstanceIsBeingDeleted(te.From) {
+			// Another code path already moved the instance into the deletion
+			// lane; the runner is being removed, which is what we wanted.
+			return nil
+		}
 		slog.With(slog.Any("error", err)).ErrorContext(
 			ctx, "failed to update runner status",
 			"runner_name", util.SanitizeLogEntry(jobParams.RunnerName))
@@ -1130,7 +1137,13 @@ func (r *basePoolManager) addInstanceToProvider(instance params.Instance) error
 			ProviderBaseParams: r.getProviderBaseParams(pool),
 		},
 	}
-	providerInstance, err := provider.CreateInstance(r.ctx, bootstrapArgs, createInstanceParams)
+	// Bound the create call by the pool's bootstrap timeout; past that point
+	// the runner is considered failed anyway. The provider-level exec timeout
+	// (if configured and smaller) still applies; the effective deadline is
+	// whichever is sooner.
+	createCtx, cancel := context.WithTimeout(r.ctx, time.Duration(pool.RunnerTimeout())*time.Minute)
+	defer cancel()
+	providerInstance, err := provider.CreateInstance(createCtx, bootstrapArgs, createInstanceParams)
 	if err != nil {
 		instanceIDToDelete = instance.Name
 		return fmt.Errorf("error creating instance: %w", err)

runner/providers/v0.1.0/external.go

@@ -70,6 +70,25 @@ type external struct {
 	environmentVariables []string
 }
 
+// execWithTimeout invokes the provider binary, bounding the call by the
+// provider's configured exec timeout (if any). On expiry the child process is
+// killed and a ProviderError is returned so the operation fails and normal
+// cleanup takes over, instead of the instance being stuck in a transient
+// state (e.g. creating) for as long as a hung binary sits there.
+func (e *external) execWithTimeout(ctx context.Context, stdinData []byte, environ []string) ([]byte, error) {
+	timeout := e.cfg.External.ExecTimeout()
+	if timeout > 0 {
+		var cancel context.CancelFunc
+		ctx, cancel = context.WithTimeout(ctx, timeout)
+		defer cancel()
+	}
+	out, err := garmExec.Exec(ctx, e.execPath, stdinData, environ)
+	if err != nil && errors.Is(ctx.Err(), context.DeadlineExceeded) {
+		return nil, garmErrors.NewProviderError("provider binary %s timed out (context deadline exceeded)", e.execPath)
+	}
+	return out, err
+}
+
 // CreateInstance creates a new compute instance in the provider.
 func (e *external) CreateInstance(ctx context.Context, bootstrapParams commonParams.BootstrapInstance, _ common.CreateInstanceParams) (commonParams.ProviderInstance, error) {
 	asEnv := []string{
@@ -90,7 +109,7 @@ func (e *external) CreateInstance(ctx context.Context, bootstrapParams commonPar
 		e.cfg.Name,       // label: provider
 	).Inc()
 
-	out, err := garmExec.Exec(ctx, e.execPath, asJs, asEnv)
+	out, err := e.execWithTimeout(ctx, asJs, asEnv)
 	if err != nil {
 		metrics.InstanceOperationFailedCount.WithLabelValues(
 			"CreateInstance", // label: operation
@@ -137,7 +156,7 @@ func (e *external) DeleteInstance(ctx context.Context, instance string, _ common
 		"DeleteInstance", // label: operation
 		e.cfg.Name,       // label: provider
 	).Inc()
-	_, err := garmExec.Exec(ctx, e.execPath, nil, asEnv)
+	_, err := e.execWithTimeout(ctx, nil, asEnv)
 	if err != nil {
 		var exitErr *exec.ExitError
 		if !errors.As(err, &exitErr) || exitErr.ExitCode() != commonExecution.ExitCodeNotFound {
@@ -168,7 +187,7 @@ func (e *external) GetInstance(ctx context.Context, instance string, _ common.Ge
 		"GetInstance", // label: operation
 		e.cfg.Name,    // label: provider
 	).Inc()
-	out, err := garmExec.Exec(ctx, e.execPath, nil, asEnv)
+	out, err := e.execWithTimeout(ctx, nil, asEnv)
 	if err != nil {
 		metrics.InstanceOperationFailedCount.WithLabelValues(
 			"GetInstance", // label: operation
@@ -212,7 +231,7 @@ func (e *external) ListInstances(ctx context.Context, poolID string, _ common.Li
 		e.cfg.Name,      // label: provider
 	).Inc()
 
-	out, err := garmExec.Exec(ctx, e.execPath, nil, asEnv)
+	out, err := e.execWithTimeout(ctx, nil, asEnv)
 	if err != nil {
 		metrics.InstanceOperationFailedCount.WithLabelValues(
 			"ListInstances", // label: operation
@@ -258,7 +277,7 @@ func (e *external) RemoveAllInstances(ctx context.Context, _ common.RemoveAllIns
 		e.cfg.Name,           // label: provider
 	).Inc()
 
-	_, err := garmExec.Exec(ctx, e.execPath, nil, asEnv)
+	_, err := e.execWithTimeout(ctx, nil, asEnv)
 	if err != nil {
 		metrics.InstanceOperationFailedCount.WithLabelValues(
 			"RemoveAllInstances", // label: operation
@@ -283,7 +302,7 @@ func (e *external) Stop(ctx context.Context, instance string, _ common.StopParam
 		"Stop",     // label: operation
 		e.cfg.Name, // label: provider
 	).Inc()
-	_, err := garmExec.Exec(ctx, e.execPath, nil, asEnv)
+	_, err := e.execWithTimeout(ctx, nil, asEnv)
 	if err != nil {
 		metrics.InstanceOperationFailedCount.WithLabelValues(
 			"Stop",     // label: operation
@@ -309,7 +328,7 @@ func (e *external) Start(ctx context.Context, instance string, _ common.StartPar
 		e.cfg.Name, // label: provider
 	).Inc()
 
-	_, err := garmExec.Exec(ctx, e.execPath, nil, asEnv)
+	_, err := e.execWithTimeout(ctx, nil, asEnv)
 	if err != nil {
 		metrics.InstanceOperationFailedCount.WithLabelValues(
 			"Start",    // label: operation