## Summary A job environment suspended on a fleeting-based executor (docker-autoscaler) cannot be resumed after the Runner process restarts. Both configurations fail: - **Without `state_storage`** (the default), the resume job fails with `acquisition "<key>" not found`. The stopped VM is left behind, and is removed only if another instance later causes a scale-down. - **With `state_storage`**, the acquisition is restored and the VM is started, but Runner's pre-existing instance check then removes it. The resume job waits for the full `instance_acquire_timeout` (15 minutes by default) before failing. Without a restart, suspend and resume work. Both failures reproduce on real AWS with Runner `main`. The `state_storage` failure also reproduces in an in-process test with the fleeting dummy plugin. The causes are in taskscaler and in how Runner finds a suspended acquisition, not in the cloud plugins. The proposed fix has two parts. First, the runtime environment key should name the VM and slot, so that a resume can find the VM after a restart without saved state. That makes the default configuration, with `state_storage` off, work. Second, the saved-state path needs fixing as well, so that `state_storage` doesn't make things worse. A prototype of the taskscaler fixes, with tests, is at the end of this issue. It covers the saved-state path and the fixes both paths share. The key change is not prototyped yet. Epic: https://gitlab.com/groups/gitlab-org/-/work_items/21159 ## Steps to reproduce Seen on real AWS with Runner `main`. The setup is under [Environment description](#environment-description). 1. Run a docker-autoscaler job with `suspend_options.suspend_on_success: true`, and keep the `runtime_environment_key` that Runner returns. 2. Wait until the VM is suspended. On AWS, that is when the instance is in Standby and stopped. 3. Restart Runner: send SIGQUIT, then start it again on the same host with the same config file, so that the system ID stays the same. Runner keeps the system ID in `.runner_system_id`, next to `config.toml`. 4. Run a job whose `suspend_options.runtime_environment_key` is the key from step 1. Do this once with `[runners.autoscaler.state_storage]` enabled and once without. The jobs, in runner-backbench format (GitLab sends these `suspend_options` itself; runner-backbench needs https://gitlab.com/gitlab-org/ci-cd/runner-tools/runner-backbench/-/merge_requests/16 to send them): <details> <summary>Jobs</summary> ```yaml # pass 1 sr_e2e_suspend: job: variables: - {key: GIT_STRATEGY, value: "none"} suspend_options: suspend_on_success: true steps: - name: script script: - 'echo "MARK=<token>" > /sr-marker.txt' # pass 2 sr_e2e_resume: job: variables: - {key: GIT_STRATEGY, value: "none"} suspend_options: runtime_environment_key: "<key from pass 1>" steps: - name: script script: - 'sed "s/MARK=/FOUND_/" /sr-marker.txt' ``` </details> Without a cloud, `TestProviderSuspendedEnvironmentSurvivesRestart` (under [Prototype](#prototype)) reproduces the `state_storage` case in-process. ## Actual behavior Pass 1 runs a `suspend_on_success` job that writes a marker file in the build container. Pass 2 runs a job with the returned key and reads the marker back. In the restart scenarios, Runner gets SIGQUIT once the VM is in Standby and stopped, then starts again with the same config and system ID. | Scenario | Pass 2 (resume) | |---|---| | No restart | Succeeds on the same instance. The marker is read back and `get_sources` is skipped. | | Restart, no `state_storage` | `runner_system_failure` after 9s, with `resuming acquisition: acquisition "sr-e2e-re385057c363fc8d5e" not found` on all three prepare attempts. taskscaler had rediscovered the VM (`instance discovery cause=preexisted state=suspended`) but had no slot for it. The VM stayed in Standby, stopped, for the 3+ minutes observed, past `idle_time = "1m"`. | | Restart, `state_storage` on | `runner_system_failure` after 309s. The saved state held the suspended slot, so the acquisition was found and the VM was started. Then the log shows `ready up preparation failed err="no data on pre-existing instance so removing for safety"` and `instance marked for removal`, followed 43s later by `removing orphaned acquisition for deleted instance`. The resume kept waiting until `instance_acquire_timeout` (5m in this run): `did not become ready after resume: context deadline exceeded`. The retries then failed with `acquisition ... not found`, which became the job's final error. | Not covered: GCP (the failing code does not depend on the plugin), `vm_isolation` and `keep_instance_with_acquisitions`. ## Expected behavior The resume job runs on the same VM, finds the marker file and skips `get_sources`, as it does without a restart. This should hold with `state_storage` on or off. ## Relevant logs and/or screenshots `TestProviderSuspendedEnvironmentSurvivesRestart` (see [Prototype](#prototype)) drives two Runner `provider`s over the real taskscaler and the fleeting dummy plugin. The two providers share one instance group and one state directory. On Runner `main` it fails the same way as the AWS run with `state_storage`: ```plaintext ready up preparation failed err="no data on pre-existing instance so removing for safety" instance marked for removal reason="instance ready up preparation failed" resuming acquisition: instance "63159a1b2ed57d7b" did not become ready after resume: context deadline exceeded --- FAIL: TestProviderSuspendedEnvironmentSurvivesRestart (32.17s) ``` With the taskscaler prototype, it passes and resumes the same instance in about 0.1s. ## Environment description - Runner `main` at ea3a974, which pins taskscaler 1f87885 and fleeting 6ed75a3. - fleeting-plugin-aws `main` at https://gitlab.com/gitlab-org/fleeting/plugins/aws/-/commit/07f5e7d85ad07e969d947a2f6b381994a871970b. - A sandbox ASG of t4g.small instances running Amazon Linux 2023 with Docker. - runner-backbench stood in for GitLab, built from https://gitlab.com/gitlab-org/ci-cd/runner-tools/runner-backbench/-/merge_requests/16 for `suspend_options` support. The ASG has scale-in protection on and `AZRebalance` suspended. Its launch template's user data installs and starts Docker. <details> <summary>config.toml contents (sanitized)</summary> ```toml [[runners]] id = 4242 executor = "docker-autoscaler" [runners.feature_flags] FF_SUSPENDABLE_ENVIRONMENTS = true [runners.docker] image = "alpine:3.20" [runners.autoscaler] plugin = "fleeting-plugin-aws" capacity_per_instance = 1 max_instances = 2 instance_acquire_timeout = "5m" instance_ready_command = "sudo cloud-init status --wait >/dev/null 2>&1; sudo docker info >/dev/null" [runners.autoscaler.state_storage] # only in the restart-with-storage scenario enabled = true [runners.autoscaler.plugin_config] name = "<asg-name>" region = "eu-west-2" [runners.autoscaler.connector_config] os = "linux" arch = "arm64" protocol = "ssh" username = "ec2-user" use_external_addr = true [[runners.autoscaler.policy]] idle_count = 0 idle_time = "1m" ``` </details> ### Used GitLab Runner version `main` at ea3a974660a030f0314aa1505a64792bf2633b33, which pins taskscaler 1f87885729aa827e971f9b5e029daf9f969f75b3 and fleeting 6ed75a33d303fb14a495af7e33bf519d3c588c18. ## Possible fixes Root causes, a proposed design, test scenarios and a prototype follow. ### How a resume finds its VM today 1. When a suspending job ends, the autoscaler executor's [`Suspend`](https://gitlab.com/gitlab-org/gitlab-runner/-/blob/ea3a974660a030f0314aa1505a64792bf2633b33/executors/internal/autoscaler/executor.go#L125-154) calls `taskscaler.Suspend(acqKey)` and adds `acquisition-key=<acqKey>` to the inner executor's fields. The docker executor contributes `build-container-id`, `helper-id` and `service-ids`. 2. Runner encodes these as `<runner-id>/<system-id>/<fields>` ([`RuntimeEnvironmentKey`](https://gitlab.com/gitlab-org/gitlab-runner/-/blob/ea3a974660a030f0314aa1505a64792bf2633b33/common/runtime_environment_key.go#L10-24)) and reports the result to GitLab. A key from the AWS run (container IDs shortened): `4242/s_08fe1be799c8/acquisition-key=sr-e2e-ba7a340e00ef664de2&build-container-id=308f1745f295…&helper-id=404f1b1bcb83…` The key does not name the instance. 3. The resume job carries the key. [`prepareResume`](https://gitlab.com/gitlab-org/gitlab-runner/-/blob/ea3a974660a030f0314aa1505a64792bf2633b33/executors/internal/autoscaler/executor.go#L87-123) checks the runner and system IDs, then calls `taskscaler.Resume(ctx, acqKey)`. 4. [`Resume`](https://gitlab.com/gitlab-org/fleeting/taskscaler/-/blob/1f87885729aa827e971f9b5e029daf9f969f75b3/taskscaler.go#L951-1002) looks the key up in `ts.acquisitions` ([`lookupSuspendedAcquisition`](https://gitlab.com/gitlab-org/fleeting/taskscaler/-/blob/1f87885729aa827e971f9b5e029daf9f969f75b3/taskscaler.go#L1006-1027)). That map lives in memory. After a restart, only `state_storage` refills it, in [`createInstanceOrRestore`](https://gitlab.com/gitlab-org/fleeting/taskscaler/-/blob/1f87885729aa827e971f9b5e029daf9f969f75b3/taskscaler.go#L636-684). The key that GitLab stores is enough to route the resume job back to the same runner and system, and to restore the containers once the VM is back. It is not enough to find the VM. ### Root causes #### 1. Without `state_storage`, nothing links the key to the VM After a restart, fleeting reports the stopped VM as `cause=preexisted state=suspended`. With no saved state, taskscaler adopts it with no slots, in the [`StateSuspended` branch of `updates`](https://gitlab.com/gitlab-org/fleeting/taskscaler/-/blob/1f87885729aa827e971f9b5e029daf9f969f75b3/taskscaler.go#L697-702). `Resume(acqKey)` then finds no acquisition, and the VM is orphaned (see cause 5). #### 2. The "restored" flag is lost when an instance is adopted while suspended [`createInstanceOrRestore`](https://gitlab.com/gitlab-org/fleeting/taskscaler/-/blob/1f87885729aa827e971f9b5e029daf9f969f75b3/taskscaler.go#L636-684) returns whether saved state was restored, but the `StateSuspended` branch discards that result. The cloud resume then brings the VM to `StateRunning`. The [`StateRunning` branch](https://gitlab.com/gitlab-org/fleeting/taskscaler/-/blob/1f87885729aa827e971f9b5e029daf9f969f75b3/taskscaler.go#L704-744) only sets `restored` when it creates the instance itself, which it did not here. So `UpFunc` gets `Restored=false`. Fleeting fixes the cause at discovery ([provisioner.go#L307-310](https://gitlab.com/gitlab-org/fleeting/fleeting/-/blob/6ed75a33d303fb14a495af7e33bf519d3c588c18/provisioner.go#L307-310)), so it stays `Cause=preexisted`. Runner's [`instanceReadyUp`](https://gitlab.com/gitlab-org/gitlab-runner/-/blob/ea3a974660a030f0314aa1505a64792bf2633b33/executors/internal/autoscaler/provider.go#L402-407) removes exactly that combination unless `vm_isolation` is on. The same loss affects an instance restored while it was **running**. Its first ready-up after the restart passes, with `Restored=true`. If a job on it is later suspended and resumed in the same process, the second ready-up gets `Restored=false` and the instance is removed. `TestRestartResume_RestoredRunningInstanceSurvivesSuspendResume` covers this case. It came up while writing the tests and has not been run on a cloud. #### 3. The resume wait does not notice that the instance was removed [`ensureInstanceResumed`](https://gitlab.com/gitlab-org/fleeting/taskscaler/-/blob/1f87885729aa827e971f9b5e029daf9f969f75b3/taskscaler.go#L1033-1061) waits first in `ResumeAndWait` and then in [`WaitUntilReady`](https://gitlab.com/gitlab-org/fleeting/taskscaler/-/blob/1f87885729aa827e971f9b5e029daf9f969f75b3/internal/instance.go#L647-661), both on the caller's context only. Removing an instance cancels the instance's group context ([`remove`](https://gitlab.com/gitlab-org/fleeting/taskscaler/-/blob/1f87885729aa827e971f9b5e029daf9f969f75b3/internal/instance.go#L729-740)), but the wait does not use that context. So the job holds its slot until `instance_acquire_timeout` runs out, and the error says `context deadline exceeded` instead of why the instance went away. #### 4. A restart while the VM is stopping or starting discards the saved state During Init, fleeting runs the first reconcile ([provisioner.go#L85](https://gitlab.com/gitlab-org/fleeting/fleeting/-/blob/6ed75a33d303fb14a495af7e33bf519d3c588c18/provisioner.go#L85)) and passes its result to taskscaler synchronously ([#L381-382](https://gitlab.com/gitlab-org/fleeting/fleeting/-/blob/6ed75a33d303fb14a495af7e33bf519d3c588c18/provisioner.go#L381-382)). The [`StateSuspending`/`StateResuming` case](https://gitlab.com/gitlab-org/fleeting/taskscaler/-/blob/1f87885729aa827e971f9b5e029daf9f969f75b3/taskscaler.go#L691-695) only marks an existing instance as not ready; it does not create one. Right after `fleeting.Init` returns, [`start`](https://gitlab.com/gitlab-org/fleeting/taskscaler/-/blob/1f87885729aa827e971f9b5e029daf9f969f75b3/taskscaler.go#L241-254) deletes every saved state that was not reattached, logging "failed to restore saved state". When the VM later reaches `StateSuspended`, it is adopted with no slots, as in cause 1. The suspend is requested when the job ends, and a cloud stop takes tens of seconds. A Runner restart during a deploy can land in that window, even with `state_storage` on. `TestRestartResume_RestartWhileSuspendingKeepsState` covers this case. It has not been run on a cloud. #### 5. A suspended VM that nothing can resume is only removed by an unrelated scale-down [`desired`](https://gitlab.com/gitlab-org/fleeting/taskscaler/-/blob/1f87885729aa827e971f9b5e029daf9f969f75b3/taskscaler.go#L795-806) leaves suspended instances out of `InstanceCount`, so they never cause a scale-down. An adopted VM with no slots (causes 1 and 4) cannot take jobs, because `needsCloudResume` blocks acquisitions. Scale-down still reaches it when something else causes one: [`scale`](https://gitlab.com/gitlab-org/fleeting/taskscaler/-/blob/1f87885729aa827e971f9b5e029daf9f969f75b3/taskscaler.go#L858-876) walks every instance in the order taskscaler adopted them, and [`RemoveIfExpired`](https://gitlab.com/gitlab-org/fleeting/taskscaler/-/blob/1f87885729aa827e971f9b5e029daf9f969f75b3/internal/instance.go#L678-718) removes an instance with no slots once `idle_time` has passed since taskscaler first saw it. That only happens when another instance is idle beyond `idle_count`. A shutdown with `delete_instances_on_shutdown` also removes it. Until then it stays stopped, keeps its disk and takes up a place in the group's maximum size. If no other instance goes idle, as in the AWS run, it stays indefinitely. The `state_storage` docs say that without it, "the existing fleeting instances are removed immediately for safety reasons" ([advanced-configuration.md](https://gitlab.com/gitlab-org/gitlab-runner/-/blob/ea3a974660a030f0314aa1505a64792bf2633b33/docs/configuration/advanced-configuration.md?plain=1#L1660-1661)). Suspended instances escape that rule, because they never reach the ready-up check. #### Related, from code reading only - **Token rotation.** The state directory is `<dir>/<ShortenToken(token)>` ([provider.go#L141-152](https://gitlab.com/gitlab-org/gitlab-runner/-/blob/ea3a974660a030f0314aa1505a64792bf2633b33/executors/internal/autoscaler/provider.go#L141-152)). Suppose the token rotates between suspend and resume, and Runner restarts after that. Runner then looks in a new directory and behaves as if `state_storage` were off. Not tested. - **The final error hides the cause.** Runner retries `Prepare`. Once the first attempt has failed and the instance is gone, later attempts fail with `acquisition ... not found`, and that becomes the job's error. - **Environments that are never resumed.** Even without a restart, a suspended slot that is never resumed keeps its VM forever. https://gitlab.com/gitlab-org/gitlab-runner/-/work_items/39468 tracks TTL cleanup. Its description assumes "suspension state is already persisted to disk", which is only true when `state_storage` is enabled. ### Proposed design: the key finds the VM, saved state adds to it GitLab already stores the runtime environment key and sends it back with the resume job. If the key also names the instance and slot, taskscaler can find a suspended VM after a restart from the key alone. This has to work without `state_storage`, because `state_storage` is Beta and off by default. Saved state stays useful on top of the key. It knows which other slots on a VM are free and how many jobs the VM has run, so after a resume the VM can go back into the pool. When both are present, the saved state is used, and the instance named in the key is checked against it. #### Part 1, required: the key names the VM Runner: - At suspend time, add `instance-id` and `slot` to the autoscaler's key fields. The acquisition already exposes both (`InstanceID()` and `Slot()`). - In [`parseEnvKeyFields`](https://gitlab.com/gitlab-org/gitlab-runner/-/blob/ea3a974660a030f0314aa1505a64792bf2633b33/executors/internal/autoscaler/environment_key_fields.go#L20-33), take the new fields out before passing the rest to the inner executor, and pass them to taskscaler's `Resume`. - Let `instanceReadyUp` accept an instance that taskscaler claimed for a resume, as it accepts a restored one. taskscaler: - Accept the instance and slot on resume, for example `Resume(ctx, key, WithInstance(id, slot))`. - If the acquisition is known, in memory or from saved state, use it. If it is on a different instance from the one in the key, fail with a clear error. - If it isn't known, claim the slot on the named instance, then resume as today. Only claim it if the instance belongs to the group, was adopted while suspending or suspended, and the slot isn't already held. - Tell `UpFunc` that the instance was claimed, for example with a `Claimed` field next to `Restored`. - Never give new jobs to an instance that was adopted while suspended without saved state. Nothing is known about how it was used before the restart. - With `capacity_per_instance = 1`, remove it once the resumed job releases it, unless that job is suspended again. - With `capacity_per_instance > 1`, other slots may hold suspended jobs that haven't been claimed yet. Keep the instance, suspend it again once no claimed slot is active, and remove it when the TTL (fix 5) passes with nothing claimed. This needs the most design. A first version could support only `capacity_per_instance = 1` without saved state, and fail other claims with a clear error. Points to settle: - **Scope of a claim.** A claim is limited to suspended instances in this runner's group, and `prepareResume` already checks the runner and system IDs in the key. - **Compatibility.** - A key issued before this change has no instance fields. It resumes through saved state only, and without saved state it fails at once with a clear error. - An older Runner given a new key passes the extra fields to the docker executor, which ignores fields it doesn't know. - Check whether GitLab limits the key's length. `&instance-id=i-03973f41f9d28d60a&slot=0` adds 39 characters for an AWS instance, and more for plugins with longer instance IDs. #### Part 2, needed on both paths | Fix | Change | Tests | |---|---|---| | 3 | In `ensureInstanceResumed`, derive the wait context from `inst.WithContext(ctx)` and report `context.Cause`. | `TestRestartResume_RemovedInstanceEndsWait` | | 4 | Adopt instances first seen in `StateSuspending` or `StateResuming` the same way as `StateSuspended`: create or restore, then `SetNeedsCloudResume`. With saved state, this keeps the state. Without it, an instance first seen at `StateSuspending` would be adopted anyway once it reaches `StateSuspended`, so fix 4 only lets a claim arrive during the stop. An instance first seen at `StateResuming` holds no slot, and part 1 does not allow a claim on it, so it is removed at its first ready-up. See the points to settle in review. | `TestRestartResume_RestartWhileSuspendingKeepsState` (with saved state only) | | 5 | Remove an instance that waits for a cloud resume but holds no slot (`Unclaimed`), once it is past the suspended-environment TTL from https://gitlab.com/gitlab-org/gitlab-runner/-/work_items/39468. Count from when taskscaler first saw it, because its suspend time is unknown. Until then it must stay, because a resume may still claim it. The existing scale-down path has to skip such an instance until then too, or an unrelated scale-down can remove a VM that a resume could still claim. | `TestRestartResume_UnclaimedSuspendedInstanceRemoved`, `TestRestartResume_RestoredSuspendedInstanceKept` | The prototype's fix 5 removes the instance after `idle_time`. That fits a design without the key fields, but here it would remove VMs that a resume could still claim, so it needs the TTL instead. https://gitlab.com/gitlab-org/gitlab-runner/-/work_items/39468 assumes the suspension state is on disk. Its TTL also has to cover instances adopted without saved state. #### Part 3, additional: saved state (`state_storage`) | Fix | Change | Tests | |---|---|---| | 2 | Record on `internal.Instance` that its state was restored (`MarkRestored`, `Restored`). Use it in the `StateRunning` branch when the instance already exists. | `TestRestartResume_WithStorageReadiesAsRestored`, `TestRestartResume_RestoredRunningInstanceSurvivesSuspendResume` | | - | Key the state directory by something that survives token rotation, with a fallback to the old path. `runnerScalerKey` already uses the URL and runner ID for this reason. With part 1, a resume after a rotation still works through the key, but the VM can't go back into the pool. | Runner unit test | Without fix 2, a restart with `state_storage` on still fails: the resume finds the VM and starts it, and Runner then removes it. #### Points to settle in review - **Fix 4, instance adopted at `StateResuming`.** With saved state, it reaches `StateRunning` with its slot still suspended and `needsCloudResume` set. A later `Resume` finds the restored acquisition. If no resume arrives, it runs idle, because nothing suspends it again. Without saved state, it holds no slot, and part 1 does not allow a claim on it, so it is removed at its first ready-up. Decide whether taskscaler should request a new suspend in the first case, and whether a claim should be allowed in the second. - **Fix 5 on AWS.** Removing a suspended instance calls `Decrease` on a Standby instance. On aws `main`, that lowers desired capacity a second time; https://gitlab.com/gitlab-org/fleeting/plugins/aws/-/merge_requests/181 fixes it. #### Prototype status The prototype covers fixes 2 to 5, with fix 5 on `idle_time`. Part 1 is not prototyped. Prototype verification, on taskscaler `main` at https://gitlab.com/gitlab-org/fleeting/taskscaler/-/commit/62fb4b698b91e4be8d2543e65435995027f8cafb (its Go code is the same as 1f87885): - All six new tests pass with the prototype. - On unpatched `main`, all four tests for fixes 2, 3 and 4 fail, each for the cause described above: - `did not become ready after resume: context deadline exceeded`, twice; - `acquisition "key-1" not found`; - a 30s wait instead of an early failure. The unclaimed-instance test also fails, because the instance is never removed. - The full `go test -race ./...` suite was also run, with and without the prototype. The only failures were `TestTaskscalerIdleProvision` and `TestPotentialCapacityWithOutOfBandScaling`, and both also fail locally on unpatched `main`. `TestPotentialCapacityWithOutOfBandScaling` failed 2 of 3 isolated runs, both with and without the prototype. #### Runner 1. The part 1 changes. 2. Bump taskscaler once the fixes merge. 3. Add `TestProviderSuspendedEnvironmentSurvivesRestart` (see [Prototype](#prototype)) to `executors/internal/autoscaler`, plus a variant with `state_storage` off. 4. Report the first resume error as the job's error, or stop retrying a resume once the instance is gone. 5. Document the restart behaviour, and what `state_storage` adds, wherever suspendable environments are documented. The `state_storage` section says instances are "removed immediately for safety reasons" without it. Suspended instances become an exception: they are kept until the TTL and never take new jobs. 6. The state directory change from part 3. ### Test scenarios Unit and in-process tests (no cloud, no GitLab): | # | Scenario | Expected | |---|---|---| | U1 | Restart with storage, then resume | Resumes the same instance. `UpFunc` sees `Restored=true`. | | U2 | Restart with storage while the instance is running, then suspend and resume a job on it | Both ready-ups see `Restored=true`. The instance is kept. | | U3 | Restart while the VM is `Suspending`, with and without storage | The resume succeeds on the same instance once the stop completes. With storage, the saved state is kept. | | U4 | Instance removed during a resume (ready-up fails) | `Resume` fails within seconds and reports why the instance was removed. | | U5 | Restart without storage, `capacity_per_instance = 1`, then resume | Resumes the same instance through the key. `UpFunc` sees it as claimed. No other job is placed on it, and it is removed once the job releases it. | | U6 | Restart with and without storage, and no resume arrives | The suspended instance is kept until the TTL, then removed. It takes no new jobs in the meantime. Without storage, fix 5 removes it. With storage, it still holds its suspended slot, so the TTL cleanup in https://gitlab.com/gitlab-org/gitlab-runner/-/work_items/39468 has to remove it. | | U7 | The key names a different instance from the saved state | Clear error. Nothing is claimed. | | U8 | The key names an instance that is not in the group or not suspended, or a slot already held | Clear error. Nothing is claimed. | | U9 | Key from before this change, with no instance fields | With storage, it resumes. Without storage, it fails at once with a clear error. | | U10 | `capacity_per_instance = 2` without storage, two suspended jobs on one VM | Matches the settled scope: both resume through their keys, or both claims fail with a clear error. No new job lands on the VM. | | U11 | Runner `parseEnvKeyFields` with the new fields | They reach taskscaler and not the inner executor. | | U12 | Runner provider restart, with and without storage | `Prepare` resumes the same instance, with no `Decrease`. | Cloud end to end, on AWS and GCP, each with `state_storage` on and off: | # | Restart point | Expected | |---|---|---| | E1 | After the VM has stopped | The resume succeeds on the same VM, the marker is present, and `get_sources` is skipped. | | E2 | While the VM is stopping | As E1, once the stop completes. | | E3 | While a resume is starting the VM | With storage, the resume job fails, and a retry or the next resume succeeds. Without storage, the outcome depends on the point to settle for fix 4 at `StateResuming`: as designed, the VM is removed at its first ready-up, and the retry fails with a clear error. In both cases, the VM does not run idle indefinitely. | | E4 | Two restarts before the resume | As E1. | | E5 | Restart, then `instance_ready_command` fails after the resume | The job fails within seconds, not after `instance_acquire_timeout`. | | E6 | Restart, and no resume arrives | The VM takes no new jobs, and is removed after the TTL. With storage, that removal comes from https://gitlab.com/gitlab-org/gitlab-runner/-/work_items/39468. | | E7 | Token rotation between suspend and restart, with storage | As E1, through the key. | | E8 | `capacity_per_instance = 2`, two suspended jobs on one VM, restart, resume both | With storage, both resume, and the VM is suspended again only once both jobs are suspended or done. Without storage, matches the settled scope. | On GCP, also re-check the plugin problems from the review of https://gitlab.com/gitlab-org/fleeting/plugins/googlecloud/-/merge_requests/78 that a restart triggers: - A failed capability check in `Init` deletes every suspended VM. - A restart after a start that was lost deletes the VM. Once restarts work, both of these lose suspended jobs. #### Running the end-to-end scenarios Use the config under [Environment description](#environment-description) and the jobs under [Steps to reproduce](#steps-to-reproduce). Between the passes: 1. Wait for the instance to be in Standby and stopped. 2. Send Runner SIGQUIT. 3. Start Runner again on the same host with the same config file, so that the system ID stays the same. For E2, restart as soon as the instance enters Standby. For E3, restart a few seconds after pass 2 starts. Notes: - runner-backbench gains `suspend_options` support in https://gitlab.com/gitlab-org/ci-cd/runner-tools/runner-backbench/-/merge_requests/16. Until that merges, build it from that branch. On a real GitLab, routing to runners is behind the `ci_suspendable_environment_runner_routing` feature flag. - An AWS instance terminated while in Standby stays listed in the ASG. To clean up, detach it with `--no-should-decrement-desired-capacity`. ### Acceptance criteria - With `state_storage` on or off, a suspended docker-autoscaler job resumes on the same VM after a Runner restart. This holds whether the restart comes after the stop completes or while it is in progress (E1, E2 and E4, on AWS and GCP, with `capacity_per_instance = 1`). - A VM adopted while suspended without saved state never takes new jobs. - When a resumed instance is removed, the resume fails within seconds and says why. - No suspended VM is kept beyond the TTL. With `state_storage`, this depends on https://gitlab.com/gitlab-org/gitlab-runner/-/work_items/39468. - Keys issued before the change still resume with `state_storage`, and fail with a clear error without it. - The restart behaviour, and what `state_storage` adds, is documented. - U1 to U12 run in CI. ### Prototype These are a prototype, not a finished change. They cover fixes 2 to 5, with fix 5 on `idle_time` rather than the TTL. Part 1 is not prototyped. The Runner test needs a `replace` directive pointing at the patched taskscaler until the fixes are released. <details><summary>taskscaler diff (against main at 62fb4b698b91e4be8d2543e65435995027f8cafb)</summary> ```diff diff --git a/internal/instance.go b/internal/instance.go index 80d8852..60e6341 100644 --- a/internal/instance.go +++ b/internal/instance.go @@ -40,6 +40,11 @@ type Instance struct { // acquisitions. needsCloudResume bool + // restored is true when the instance's state was loaded from storage + // when taskscaler adopted it. It outlives the first ready-up, so an + // adopted instance that is suspended and resumed still reports it. + restored bool + flight *singleflight.Group[string, any] heartbeatFn func(context.Context, provider.ConnectInfo) error heartbeating bool @@ -391,6 +396,32 @@ func (inst *Instance) SetNeedsCloudResume() { inst.state.Info.Expires = &now } +// MarkRestored records that the instance's state was loaded from storage. +func (inst *Instance) MarkRestored() { + inst.mu.Lock() + defer inst.mu.Unlock() + + inst.restored = true +} + +// Restored reports whether the instance's state was loaded from storage. +func (inst *Instance) Restored() bool { + inst.mu.Lock() + defer inst.mu.Unlock() + + return inst.restored +} + +// Unclaimed reports whether the instance waits for a cloud resume but holds +// no slot, so no acquisition can ever resume it. This is the case for a +// suspended instance adopted without saved state. +func (inst *Instance) Unclaimed() bool { + inst.mu.Lock() + defer inst.mu.Unlock() + + return inst.needsCloudResume && len(inst.state.Slots) == 0 && !inst.state.Removing +} + // ClearNeedsCloudResume atomically clears the needsCloudResume flag and // returns its previous value. func (inst *Instance) ClearNeedsCloudResume() bool { diff --git a/taskscaler.go b/taskscaler.go index 7def4bd..ca6b14c 100644 --- a/taskscaler.go +++ b/taskscaler.go @@ -267,6 +267,22 @@ func (ts *taskscaler) tick(ctx context.Context) { time.Sleep(time.Second) ts.scale(ts.desired()) + ts.removeUnclaimed() + } +} + +// removeUnclaimed removes suspended instances that no acquisition can +// resume once they exceed the idle time. Suspended instances are not +// counted as instances, so they never cause a scale-down themselves. +func (ts *taskscaler) removeUnclaimed() { + ts.mu.Lock() + idleTime := ts.active.IdleTime + ts.mu.Unlock() + + for _, instance := range ts.instances.List() { + if instance.Unclaimed() && instance.RemoveIfExpired(idleTime) { + ts.opts.logger.Info("removed suspended instance with no suspended slots", "instance", instance.ID()) + } } } @@ -658,6 +674,9 @@ func (ts *taskscaler) createInstanceOrRestore(instance fleeting.Instance) (*inte }) inst := internal.New(instance, ts.opts.logger, state, ts.opts.storage, &ts.flight, ts.opts.heartbeatFunc) + if restored { + inst.MarkRestored() + } // restore acquisitions var activeKeys []string @@ -688,13 +707,15 @@ func (ts *taskscaler) updates(ctx context.Context, instances []fleeting.Instance inst := ts.instances.Get(instance.ID()) switch instance.State() { - case provider.StateCreating, provider.StateDeleting, - provider.StateSuspending, provider.StateResuming: + case provider.StateCreating, provider.StateDeleting: if inst != nil { inst.Unready() } - case provider.StateSuspended: + case provider.StateSuspending, provider.StateResuming, provider.StateSuspended: + // An instance first seen mid-transition (e.g. Runner restarted + // while the VM was stopping) is adopted here too, so its saved + // state is restored rather than discarded after Init. if inst == nil { inst, _, _ = ts.createInstanceOrRestore(instance) inst.SetNeedsCloudResume() @@ -706,6 +727,11 @@ func (ts *taskscaler) updates(ctx context.Context, instances []fleeting.Instance var restored bool if inst == nil { inst, acquisitions, restored = ts.createInstanceOrRestore(instance) + } else { + // An instance adopted while suspended was created in the branch + // above; its ready-up after the cloud resume still comes from + // saved state. + restored = inst.Restored() } upFunc := internal.UpFunc(func(id string, info provider.ConnectInfo, cause fleeting.Cause) error { @@ -1031,6 +1057,11 @@ func (ts *taskscaler) lookupSuspendedAcquisition(key string) (*acquisition, *int // If needsCloudResume is set, issues a cloud resume via ResumeAndWait. // Then waits for the instance to become ready (no-op if already ready). func (ts *taskscaler) ensureInstanceResumed(ctx context.Context, inst *internal.Instance) error { + // End the wait as soon as the instance is removed (failed ready-up, + // external deletion), not only at the caller's deadline. + ctx, cancel := inst.WithContext(ctx) + defer cancel() + // Check-and-clear inside the callback so concurrent callers share the // result via singleflight. If we checked outside, a second caller could // observe the flag already cleared, skip flight.Do entirely (never @@ -1053,7 +1084,7 @@ func (ts *taskscaler) ensureInstanceResumed(ctx context.Context, inst *internal. } if !inst.WaitUntilReady(ctx) { - return fmt.Errorf("instance %q did not become ready after resume: %w", inst.ID(), ctx.Err()) + return fmt.Errorf("instance %q did not become ready after resume: %w", inst.ID(), context.Cause(ctx)) } return nil ``` </details> <details><summary>taskscaler tests: restart_resume_test.go</summary> ```go package taskscaler import ( "context" "errors" "sync" "testing" "time" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/fleeting/fleeting" dummy "gitlab.com/gitlab-org/fleeting/fleeting/plugin/fleeting-plugin-dummy" "gitlab.com/gitlab-org/fleeting/fleeting/provider" "gitlab.com/gitlab-org/fleeting/taskscaler/internal" "gitlab.com/gitlab-org/fleeting/taskscaler/storage" ) // upFuncRecorder records UpFunc calls and applies Runner's pre-existing // instance rule (gitlab-runner executors/internal/autoscaler/provider.go, // instanceReadyUp): a pre-existing instance without restored state is // rejected. type upFuncRecorder struct { mu sync.Mutex calls []UpFuncInstance err error // returned for every call when set } func (r *upFuncRecorder) fn(_ Taskscaler, inst UpFuncInstance) error { r.mu.Lock() defer r.mu.Unlock() r.calls = append(r.calls, inst) if r.err != nil { return r.err } if inst.Cause == fleeting.CausePreexisted && !inst.Restored { return errors.New("no data on pre-existing instance so removing for safety") } return nil } func (r *upFuncRecorder) seen() []UpFuncInstance { r.mu.Lock() defer r.mu.Unlock() return append([]UpFuncInstance(nil), r.calls...) } func newRestartTestScaler(t *testing.T, ctx context.Context, group provider.InstanceGroup, store storage.Storage, upFn UpFunc) Taskscaler { t.Helper() opts := []Option{ WithCapacityPerInstance(1), WithMaxInstances(1), WithUpdateInterval(100 * time.Millisecond), WithUpdateIntervalWhenExpecting(100 * time.Millisecond), } if store != nil { opts = append(opts, WithStorage(store)) } if upFn != nil { opts = append(opts, WithInstanceUpFunc(upFn)) } ts, err := New(ctx, group, opts...) require.NoError(t, err) return ts } // suspendAndStop acquires "key-1", suspends it, waits for the dummy to // report waitFor, then shuts the taskscaler down as a Runner restart would. func suspendAndStop(t *testing.T, ctx context.Context, group *dummy.InstanceGroup, store storage.Storage, waitFor provider.State) string { t.Helper() tctx, tcancel := context.WithCancel(ctx) defer tcancel() ts := newRestartTestScaler(t, ctx, group, store, nil) ts.ConfigureSchedule(Schedule{IdleCount: 0}) go configureScaleTransitions(tctx, group) acq, err := ts.Acquire(ctx, "key-1") require.NoError(t, err) id := acq.InstanceID() require.NoError(t, ts.Suspend("key-1")) require.Eventually(t, func() bool { return getInstanceState(group, id) == waitFor }, 10*time.Second, 20*time.Millisecond) sctx, scancel := context.WithTimeout(ctx, 2*time.Second) defer scancel() ts.Shutdown(sctx) return id } func completeTransitions(ctx context.Context, group *dummy.InstanceGroup) { for ctx.Err() == nil { for _, inst := range group.List() { group.CompleteTransition(inst.ID) } time.Sleep(20 * time.Millisecond) } } // After a restart with state storage, the resumed instance must reach // UpFunc as restored, or Runner removes it as an unknown pre-existing // instance. func TestRestartResume_WithStorageReadiesAsRestored(t *testing.T) { ctx, cancel := context.WithTimeout(t.Context(), time.Minute) defer cancel() store, err := storage.NewFileStorage(t.TempDir()) require.NoError(t, err) group := &dummy.InstanceGroup{MaxSize: 1, Suspendable: true} id := suspendAndStop(t, ctx, group, store, provider.StateSuspended) rec := &upFuncRecorder{} ts := newRestartTestScaler(t, ctx, group, store, rec.fn) defer shutdownWithin(ts, 2*time.Second) go configureScaleTransitions(ctx, group) rctx, rcancel := context.WithTimeout(ctx, 10*time.Second) defer rcancel() acq, err := ts.Resume(rctx, "key-1") require.NoError(t, err) require.Equal(t, id, acq.InstanceID()) calls := rec.seen() require.Len(t, calls, 1) require.Equal(t, fleeting.CausePreexisted, calls[0].Cause) require.True(t, calls[0].Restored) } // An instance restored while running, then suspended and resumed in the // new process, must still be readied as restored. func TestRestartResume_RestoredRunningInstanceSurvivesSuspendResume(t *testing.T) { ctx, cancel := context.WithTimeout(t.Context(), time.Minute) defer cancel() store, err := storage.NewFileStorage(t.TempDir()) require.NoError(t, err) group := &dummy.InstanceGroup{MaxSize: 1, Suspendable: true} // first process: create an instance and leave it idle tctx, tcancel := context.WithCancel(ctx) ts1 := newRestartTestScaler(t, ctx, group, store, nil) ts1.ConfigureSchedule(Schedule{IdleCount: 1}) go configureScaleTransitions(tctx, group) waitForIdleCapacity(t, ctx, ts1, 1, true) shutdownWithin(ts1, 2*time.Second) tcancel() rec := &upFuncRecorder{} ts := newRestartTestScaler(t, ctx, group, store, rec.fn) defer shutdownWithin(ts, 2*time.Second) ts.ConfigureSchedule(Schedule{IdleCount: 1}) go configureScaleTransitions(ctx, group) waitForIdleCapacity(t, ctx, ts, 1, true) acq, err := ts.Acquire(ctx, "key-2") require.NoError(t, err) id := acq.InstanceID() require.NoError(t, ts.Suspend("key-2")) require.Eventually(t, func() bool { return getInstanceState(group, id) == provider.StateSuspended }, 10*time.Second, 20*time.Millisecond) rctx, rcancel := context.WithTimeout(ctx, 10*time.Second) defer rcancel() _, err = ts.Resume(rctx, "key-2") require.NoError(t, err) calls := rec.seen() require.Len(t, calls, 2) for _, c := range calls { require.Equal(t, fleeting.CausePreexisted, c.Cause) require.True(t, c.Restored) } } // A restart while the VM is still stopping must keep its saved state, so // the suspended acquisition can be resumed once the stop completes. func TestRestartResume_RestartWhileSuspendingKeepsState(t *testing.T) { ctx, cancel := context.WithTimeout(t.Context(), time.Minute) defer cancel() store, err := storage.NewFileStorage(t.TempDir()) require.NoError(t, err) group := &dummy.InstanceGroup{MaxSize: 1, Suspendable: true, AsyncTransitions: true} id := suspendAndStop(t, ctx, group, store, provider.StateSuspending) rec := &upFuncRecorder{} ts := newRestartTestScaler(t, ctx, group, store, rec.fn) defer shutdownWithin(ts, 2*time.Second) go configureScaleTransitions(ctx, group) go completeTransitions(ctx, group) require.Eventually(t, func() bool { return getInstanceState(group, id) == provider.StateSuspended }, 10*time.Second, 20*time.Millisecond) rctx, rcancel := context.WithTimeout(ctx, 10*time.Second) defer rcancel() acq, err := ts.Resume(rctx, "key-1") require.NoError(t, err) require.Equal(t, id, acq.InstanceID()) } // When the resumed instance is removed (here: ready-up fails), Resume must // fail promptly instead of waiting for the caller's deadline. func TestRestartResume_RemovedInstanceEndsWait(t *testing.T) { ctx, cancel := context.WithTimeout(t.Context(), time.Minute) defer cancel() store, err := storage.NewFileStorage(t.TempDir()) require.NoError(t, err) group := &dummy.InstanceGroup{MaxSize: 1, Suspendable: true} suspendAndStop(t, ctx, group, store, provider.StateSuspended) rec := &upFuncRecorder{err: errors.New("ready command failed")} ts := newRestartTestScaler(t, ctx, group, store, rec.fn) defer shutdownWithin(ts, 2*time.Second) go configureScaleTransitions(ctx, group) rctx, rcancel := context.WithTimeout(ctx, 30*time.Second) defer rcancel() start := time.Now() _, err = ts.Resume(rctx, "key-1") require.Error(t, err) require.NotErrorIs(t, err, context.DeadlineExceeded) require.ErrorIs(t, err, internal.ErrInstanceReadyUpPreparationFailed) require.Less(t, time.Since(start), 10*time.Second) } // A suspended instance adopted without saved state cannot be resumed, so it // is removed once it exceeds the idle time instead of being kept forever. func TestRestartResume_UnclaimedSuspendedInstanceRemoved(t *testing.T) { ctx, cancel := context.WithTimeout(t.Context(), time.Minute) defer cancel() group := &dummy.InstanceGroup{MaxSize: 1, Suspendable: true} id := suspendAndStop(t, ctx, group, nil, provider.StateSuspended) ts := newRestartTestScaler(t, ctx, group, nil, nil) defer shutdownWithin(ts, 2*time.Second) ts.ConfigureSchedule(Schedule{IdleCount: 0, IdleTime: 2 * time.Second}) go configureScaleTransitions(ctx, group) time.Sleep(time.Second) require.Equal(t, provider.StateSuspended, getInstanceState(group, id), "kept within the idle time") require.Eventually(t, func() bool { s := getInstanceState(group, id) return s == "" || s == provider.StateDeleted }, 10*time.Second, 50*time.Millisecond) } // A restored suspended instance holds its suspended slot and is kept. func TestRestartResume_RestoredSuspendedInstanceKept(t *testing.T) { ctx, cancel := context.WithTimeout(t.Context(), time.Minute) defer cancel() store, err := storage.NewFileStorage(t.TempDir()) require.NoError(t, err) group := &dummy.InstanceGroup{MaxSize: 1, Suspendable: true} id := suspendAndStop(t, ctx, group, store, provider.StateSuspended) ts := newRestartTestScaler(t, ctx, group, store, nil) defer shutdownWithin(ts, 2*time.Second) ts.ConfigureSchedule(Schedule{IdleCount: 0, IdleTime: time.Second}) go configureScaleTransitions(ctx, group) time.Sleep(4 * time.Second) require.Equal(t, provider.StateSuspended, getInstanceState(group, id)) require.Equal(t, int64(0), group.CallCount("Decrease")) } func shutdownWithin(ts Taskscaler, d time.Duration) { ctx, cancel := context.WithTimeout(context.Background(), d) defer cancel() ts.Shutdown(ctx) } ``` </details> <details><summary>Runner test: executors/internal/autoscaler/provider_restart_resume_test.go</summary> ```go //go:build !integration package autoscaler import ( "context" "net/url" "testing" "time" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/fleeting/fleeting" dummy "gitlab.com/gitlab-org/fleeting/fleeting/plugin/fleeting-plugin-dummy" fleetingprovider "gitlab.com/gitlab-org/fleeting/fleeting/provider" "gitlab.com/gitlab-org/fleeting/taskscaler" "gitlab.com/gitlab-org/gitlab-runner/common" "gitlab.com/gitlab-org/gitlab-runner/common/spec" "gitlab.com/gitlab-org/gitlab-runner/helpers/featureflags" ) // TestProviderSuspendedEnvironmentSurvivesRestart suspends a job through one // provider, shuts it down as a Runner restart would, and resumes the job // through a second provider on the same instance group and state storage. // The real taskscaler drives an in-memory fleeting provider. func TestProviderSuspendedEnvironmentSurvivesRestart(t *testing.T) { ctx, cancel := context.WithTimeout(t.Context(), time.Minute) defer cancel() group := &dummy.InstanceGroup{MaxSize: 2, Suspendable: true} newProvider := func() *provider { p := New(common.NewMockExecutorProvider(t), Config{}).(*provider) p.taskscalerNew = taskscaler.New p.fleetingRunPlugin = func(string, []byte, ...fleeting.PluginOption) (fleetingPlugin, error) { return &dummyFleetingPlugin{group: group}, nil } return p } config := common.NewTestRunnerConfig(). WithAutoscalerConfig( common.NewTestAutoscalerConfig(). WithPolicies(common.AutoscalerPolicyConfig{IdleCount: 0}). AutoscalerConfig, ). WithURL("https://gitlab.example.com"). WithID(42). WithToken("glrt-restart-resume-token"). RunnerConfig config.SystemID = "s_restart" config.FeatureFlags = map[string]bool{featureflags.SuspendableEnvironments: true} config.Autoscaler.MaxInstances = 2 config.Autoscaler.CapacityPerInstance = 1 config.Autoscaler.UpdateInterval = 100 * time.Millisecond config.Autoscaler.UpdateIntervalWhenExpecting = 100 * time.Millisecond config.Autoscaler.InstanceAcquireTimeout = 30 * time.Second config.Autoscaler.StateStorage.Enabled = true config.Autoscaler.StateStorage.Dir = t.TempDir() // first process: run a job and suspend it p1 := newProvider() data1, err := p1.Acquire(config) require.NoError(t, err) build1 := newSuspendableBuild(spec.SuspendOptions{SuspendOnSuccess: true}) build1.Runner = config build1.ExecutorData = data1 inner1 := &mockSuspendableExecutor{ MockExecutor: common.NewMockExecutor(t), MockSuspendableExecutor: common.NewMockSuspendableExecutor(t), } inner1.MockExecutor.On("Prepare", mock.Anything).Return(nil).Once() inner1.MockSuspendableExecutor.On("Suspend", mock.Anything). Return(url.Values{"build-container-id": {"c1"}, "helper-id": {"h1"}}, nil).Once() e1 := &executor{Executor: inner1, provider: p1} require.NoError(t, e1.Prepare(common.ExecutorPrepareOptions{Config: config, Context: ctx, Build: build1})) instanceID := data1.(*acquisitionRef).acq.InstanceID() fields, err := e1.Suspend(ctx) require.NoError(t, err) envKey := common.RuntimeEnvironmentKey{RunnerID: config.ID, SystemID: config.GetSystemID(), Fields: fields}.String() p1.Release(config, data1) require.Eventually(t, func() bool { for _, inst := range group.List() { if inst.ID == instanceID { return inst.State == fleetingprovider.StateSuspended } } return false }, 10*time.Second, 50*time.Millisecond) p1.Shutdown(ctx, nil) // second process: resume the job from the key alone p2 := newProvider() t.Cleanup(func() { sctx, scancel := context.WithTimeout(context.Background(), 5*time.Second) //nolint:usetesting // t.Context() is canceled at Cleanup time defer scancel() p2.Shutdown(sctx, nil) }) data2, err := p2.Acquire(config) require.NoError(t, err) build2 := newSuspendableBuild(spec.SuspendOptions{RuntimeEnvironmentKey: envKey}) build2.Runner = config build2.ExecutorData = data2 inner2 := &mockSuspendableExecutor{ MockExecutor: common.NewMockExecutor(t), MockSuspendableExecutor: common.NewMockSuspendableExecutor(t), } inner2.MockExecutor.On("Prepare", mock.Anything).Return(nil).Once() inner2.MockSuspendableExecutor.On("Resume", mock.Anything, mock.MatchedBy(func(f url.Values) bool { return f.Get("build-container-id") == "c1" && f.Get("helper-id") == "h1" })).Return(nil).Once() e2 := &executor{Executor: inner2, provider: p2} start := time.Now() require.NoError(t, e2.Prepare(common.ExecutorPrepareOptions{Config: config, Context: ctx, Build: build2})) t.Logf("resume after restart took %s", time.Since(start)) require.Equal(t, instanceID, data2.(*acquisitionRef).acq.InstanceID()) require.Equal(t, int64(0), group.CallCount("Decrease"), "the resumed instance must not be removed") p2.Release(config, data2) } ``` </details>