diff --git a/config/example-runner-1.yaml b/config/example-runner-1.yaml index c7709e3..0a84522 100644 --- a/config/example-runner-1.yaml +++ b/config/example-runner-1.yaml @@ -9,7 +9,7 @@ control_plane: manager_version: v0.2.1-nddev.88 scheduling_mode: scale-set provider: incus - provider_version: v0.1.5-nddev.119 + provider_version: v0.1.5-nddev.120 provider_interface: v0.1.0 worker_kind: incus-container runner: actions/runner diff --git a/config/example-runner-2.yaml b/config/example-runner-2.yaml index da10a2b..ccc022f 100644 --- a/config/example-runner-2.yaml +++ b/config/example-runner-2.yaml @@ -9,7 +9,7 @@ control_plane: manager_version: v0.2.1-nddev.88 scheduling_mode: scale-set provider: incus - provider_version: v0.1.5-nddev.119 + provider_version: v0.1.5-nddev.120 provider_interface: v0.1.0 worker_kind: incus-container runner: actions/runner diff --git a/config/example-runner-3.yaml b/config/example-runner-3.yaml index fa5b0c7..f60e5e7 100644 --- a/config/example-runner-3.yaml +++ b/config/example-runner-3.yaml @@ -9,7 +9,7 @@ control_plane: manager_version: v0.2.1-nddev.88 scheduling_mode: scale-set provider: incus - provider_version: v0.1.5-nddev.119 + provider_version: v0.1.5-nddev.120 provider_interface: v0.1.0 worker_kind: incus-container runner: actions/runner diff --git a/config/example-runner-4.yaml b/config/example-runner-4.yaml index c2b353c..1a9734b 100644 --- a/config/example-runner-4.yaml +++ b/config/example-runner-4.yaml @@ -9,7 +9,7 @@ control_plane: manager_version: v0.2.1-nddev.88 scheduling_mode: scale-set provider: incus - provider_version: v0.1.5-nddev.119 + provider_version: v0.1.5-nddev.120 provider_interface: v0.1.0 worker_kind: incus-container runner: actions/runner diff --git a/config/example-services.yaml b/config/example-services.yaml index 5aad152..0da6525 100644 --- a/config/example-services.yaml +++ b/config/example-services.yaml @@ -27,7 +27,7 @@ control_plane: manager_version: v0.2.1-nddev.88 scheduling_mode: scale-set provider: incus - provider_version: v0.1.5-nddev.119 + provider_version: v0.1.5-nddev.120 provider_interface: v0.1.0 worker_kind: incus-container runner: actions/runner diff --git a/config/provider-derivative.yaml b/config/provider-derivative.yaml index f6c3455..4f10b5d 100644 --- a/config/provider-derivative.yaml +++ b/config/provider-derivative.yaml @@ -16,7 +16,7 @@ artifact: garm-provider-incus # state all move together, because all three derive from here. A provider change # that does not bump it ships under the previous version, which is exactly how # runner-1 and runner-2 diverged. -derivative_version: v0.1.5-nddev.119 +derivative_version: v0.1.5-nddev.120 # The external-provider protocol GARM speaks to this binary. It moves on its own # schedule -- a provider release does not imply an interface release -- so it is @@ -37,8 +37,8 @@ runtime: queue_intent_schema_version: 6 build: - source_commit: de15f14e23425201b4c18cfd989beffe3ccb5d95 - binary_sha256: c3aaa8e9e17f19d1cf81d5c7e99686a10980e2f9260a5d7e00186f5f09e81069 + source_commit: 3a77ae2562015ad006857deaf2823a84575e3e3c + binary_sha256: 3c07b99b8d11f1bcded6208147540522aaa37302bf2203da78937bdbd2c70368 go_version: go1.26.7 cgo_enabled: false target_os: linux diff --git a/internal/garmproviderincus/provider/incus.go b/internal/garmproviderincus/provider/incus.go index 617ddef..9b5621c 100644 --- a/internal/garmproviderincus/provider/incus.go +++ b/internal/garmproviderincus/provider/incus.go @@ -85,10 +85,14 @@ var ( const ( createOperationTimeout = 2 * time.Minute deleteOperationTimeout = time.Minute - stateOperationTimeout = time.Minute defaultPlacementLock = "/var/lib/gha-fleet/placement.lock" ) +// stateOperationTimeout bounds a wait on an instance's own state, the warm +// readiness poll among them. It is a var so a test can shorten it; nothing +// outside a test writes it. +var stateOperationTimeout = time.Minute + const ( directJITPhasePath = "/home/runner/actions-runner/_diag/nddev-direct-jit-phase.log" directJITPhaseMaxBytes = 4096 diff --git a/internal/garmproviderincus/provider/warm.go b/internal/garmproviderincus/provider/warm.go index 62fe10b..b679ab6 100644 --- a/internal/garmproviderincus/provider/warm.go +++ b/internal/garmproviderincus/provider/warm.go @@ -53,8 +53,12 @@ type WarmPoolResult struct { AdmissionDecision *admission.Decision `json:"admission_decision,omitempty"` Created []string `json:"created"` Consumed []string `json:"consumed_during_create"` - Promoted []string `json:"promoted"` - DeletedExcess []string `json:"deleted_excess"` + // Abandoned names instances that were created but never published their + // readiness evidence in time. They are deleted and the deficit refills on + // the next pass. One instance that does not arrive is not a failed pool. + Abandoned []string `json:"abandoned_during_create,omitempty"` + Promoted []string `json:"promoted"` + DeletedExcess []string `json:"deleted_excess"` } type WarmDrainResult struct { @@ -200,6 +204,11 @@ func (l *Incus) ReconcileWarm(ctx context.Context, flavor string, apply bool) (W for _, name := range preparing { promoted, err := l.promoteWarmReady(ctx, name, flavor) if err != nil { + if warmInstanceRetiredDuringCreate(err) { + // It was claimed or deleted while this pass read it; the + // next pass sees the resulting inventory. + continue + } return result, err } if promoted { @@ -232,6 +241,10 @@ func (l *Incus) ReconcileWarm(ctx context.Context, flavor string, apply bool) (W for range deficit { name, consumed, decision, err := l.createWarm(ctx, flavor) if err != nil { + if errors2.Is(err, errWarmNotReady) { + result.Abandoned = append(result.Abandoned, name) + continue + } return result, err } if consumed { @@ -318,6 +331,17 @@ func (l *Incus) createWarm(ctx context.Context, flavor string) (name string, con return "", false, nil, errors.Wrap(err, "waiting for warm instance network") } if err = l.waitWarmReady(ctx, name, flavor, 2*time.Second); err != nil { + if errors2.Is(err, runnerErrors.ErrTimeout) { + // The instance exists but never published readiness. It will never + // be claimed, and leaving it costs the pool a slot, so it is + // deleted and the deficit refills on the next pass. Failing here + // instead aborted the whole pool's reconcile -- so the depth was + // not restored during exactly the bursts that consume it. + if deleteErr := l.DeleteInstance(ctx, name); deleteErr != nil { + return "", false, nil, errors.Wrapf(deleteErr, "deleting warm instance %q that never became ready", name) + } + return name, false, nil, errWarmNotReady + } if warmInstanceRetiredDuringCreate(err) { err = nil return name, true, nil, nil @@ -336,7 +360,18 @@ func warmInstanceRetiredDuringCreate(err error) bool { // hides inside "attempt count exceeded: fetching instance: ...". A warm // instance that vanished during create was consumed by useful concurrent // work, whatever the wrapper says. - return err != nil && strings.Contains(err.Error(), "Instance not found") + // + // Incus does not always answer 404 for an instance that is going away. + // Reading a file from one whose delete has begun answers "Failed getting + // instance pool: Instance storage pool not found", and that read is + // exactly what the readiness poll does: on 2026-09-02 it failed the whole + // pool's reconcile eleven times. + message := "" + if err != nil { + message = err.Error() + } + return strings.Contains(message, "Instance not found") || + strings.Contains(message, "Instance storage pool not found") } // isPlacementRefusal recognizes the scriptlet's per-member capacity refusal @@ -350,6 +385,10 @@ func isPlacementRefusal(err error) bool { strings.Contains(message, "no fleet member has room") } +// errWarmNotReady marks an instance that was created but never published its +// readiness evidence: the reconciler deletes it and carries on with the pool. +var errWarmNotReady = errors2.New("warm instance never became ready") + func (l *Incus) waitWarmReady(ctx context.Context, name, flavor string, pollInterval time.Duration) error { if pollInterval <= 0 { return fmt.Errorf("warm readiness poll interval must be positive") diff --git a/internal/garmproviderincus/provider/warm_recycle_test.go b/internal/garmproviderincus/provider/warm_recycle_test.go index a493a67..6970b52 100644 --- a/internal/garmproviderincus/provider/warm_recycle_test.go +++ b/internal/garmproviderincus/provider/warm_recycle_test.go @@ -2,8 +2,14 @@ package provider import ( "context" + "fmt" + "io" + "strings" "testing" + "time" + "github.com/cloudbase/garm-provider-common/errors" + incus "github.com/lxc/incus/v7/client" "github.com/lxc/incus/v7/shared/api" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -122,3 +128,71 @@ func TestReconcileWarmRecyclesAnInstanceWhosePoolCapabilityMoved(t *testing.T) { require.Equal(t, 0, result.ReadyBefore) cli.AssertNotCalled(t, "DeleteInstance", mock.Anything) } + +// One warm instance that never publishes readiness is deleted and recorded, +// and the pool's reconcile carries on: failing it instead left the depth +// unrestored during exactly the bursts that consume it (2026-09-02, eleven +// aborted reconciles). +func TestWarmInstanceThatNeverBecomesReadyIsAbandonedNotFatal(t *testing.T) { + previous := stateOperationTimeout + stateOperationTimeout = 20 * time.Millisecond + t.Cleanup(func() { stateOperationTimeout = previous }) + + cli := new(MockIncusServer) + provider := newTestProvider(cli) + setWarmTarget(provider, "nddev-linux-standard", 1) + cli.On("GetInstancesFull", api.InstanceTypeAny).Return([]api.InstanceFull{}, nil).Once() + prepareCreateMocks(cli, testImageDigest) + operation := new(MockOperation) + operation.On("WaitContext", mock.Anything).Return(nil) + cli.On("CreateInstance", mock.Anything).Return(operation, nil).Once() + cli.On("UpdateInstanceState", mock.Anything, api.InstanceStatePut{Action: "start", Timeout: -1}, "").Return(operation, nil).Maybe() + // The instance is created and reachable, but its readiness evidence never + // appears, so every poll finds nothing and the wait times out. + created := warmInstance("warm-standard-unready") + created.ExpandedConfig[lifecycleKey] = lifecycleWarmPreparing + created.ExpandedConfig[warmReadyKey] = "" + created.State = &api.InstanceState{Status: "Running", Network: map[string]api.InstanceStateNetwork{ + "eth0": {Addresses: []api.InstanceStateNetworkAddress{{Family: "inet", Scope: "global", Address: "10.0.0.5"}}}, + }} + cli.On("GetInstanceFull", mock.Anything).Return(created, "etag", nil).Maybe() + cli.On("GetInstanceFile", mock.Anything, warmReadyGuestPath). + Return(io.NopCloser(strings.NewReader("")), (*incus.InstanceFileResponse)(nil), fmt.Errorf("evidence absent: %w", errors.ErrNotFound)).Maybe() + cli.On("DeleteInstance", mock.Anything).Return(operation, nil).Maybe() + cli.On("UpdateInstanceState", mock.Anything, api.InstanceStatePut{Action: "stop", Timeout: -1, Force: true}, "").Return(operation, nil).Maybe() + + result, err := provider.ReconcileWarm(context.Background(), "nddev-linux-standard", true) + require.NoError(t, err, "one unready instance must not fail the pool") + require.Len(t, result.Abandoned, 1) + require.Empty(t, result.Created) +} + +// Incus answers a file read on an instance whose delete has begun with +// "Instance storage pool not found", not with a 404; that is the instance +// going away, not a broken pool. +func TestAVanishingInstanceIsRecognisedByItsStoragePoolError(t *testing.T) { + require.True(t, warmInstanceRetiredDuringCreate(fmt.Errorf("reading warm readiness evidence: Failed getting instance pool: Instance storage pool not found"))) + require.True(t, warmInstanceRetiredDuringCreate(fmt.Errorf("attempt count exceeded: fetching instance: Instance not found"))) + require.False(t, warmInstanceRetiredDuringCreate(fmt.Errorf("storage pool is full"))) + require.False(t, warmInstanceRetiredDuringCreate(nil)) +} + +// The preparing loop reads each instance's readiness evidence. One that is +// being deleted answers with the storage-pool error; the pass must skip it +// and keep reconciling, not abort the pool (2026-09-02, 10:59Z). +func TestPreparingInstanceThatVanishesMidReadIsSkipped(t *testing.T) { + cli := new(MockIncusServer) + provider := newTestProvider(cli) + setWarmTarget(provider, "nddev-linux-standard", 0) + preparing := warmInstance("warm-standard-vanishing") + preparing.ExpandedConfig[lifecycleKey] = lifecycleWarmPreparing + cli.On("GetInstancesFull", api.InstanceTypeAny).Return([]api.InstanceFull{*preparing}, nil).Once() + cli.On("GetInstanceFull", preparing.Name).Return(preparing, "etag", nil).Maybe() + cli.On("GetInstanceFile", preparing.Name, warmReadyGuestPath). + Return(io.NopCloser(strings.NewReader("")), (*incus.InstanceFileResponse)(nil), + fmt.Errorf("Failed getting instance pool: Instance storage pool not found")).Once() + + result, err := provider.ReconcileWarm(context.Background(), "nddev-linux-standard", true) + require.NoError(t, err, "an instance that vanished mid-read must not fail the pool") + require.Empty(t, result.Promoted) +}