oxidecomputer / oxidecomputer/omicron
sled-agent should handle Propolis zones being deleted before the service has come up
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 572
- Forks
- 97
- Avg merge
- 2d 12h
- Merged PRs (30d)
- 96
Description
Thanks to @jmpesp chaos-monkeying the control plane in the Canada region, we have discovered that sled-agent does not seem to properly handle the abrupt disappearance of a Propolis zone (e.g. due to a zoneadm halt and zonecfg delete -F) before the Propolis service in that zone has actually started.
Previously, we merged #7794 in an attempt to fix sled-agent's confusion in the event of forceful deletion of Propolis zones (see #7563). However, this change only worked by making the InstanceStateMonitor task check whether the zone still exists when it sees an HTTP communication error while trying to check on the instance's state. This is what would occur if a Propolis zone had started successfully, sled-agent had observed it starting and began monitoring the instance, and then the zone was deleted. However, in the case where the zone is deleted before the propolis service has started, we don't see that happen.
For instance, James posts the following sled-agent log fragment:
19:22:18.438Z WARN SledAgent (InstanceManager): wait for service svc:/milestone/single-user:default in zone Some("oxz_propolis-server_17398ae4-34dc-4e5b-89e7-18bf70d4526d") failed: Property not found. retry in 745.248441ms
file = illumos-utils/src/svc.rs:36
instance_id = f5a2b1fe-c737-4334-be88-0d7eb8d3451c
propolis_id = 17398ae4-34dc-4e5b-89e7-18bf70d4526d
zone = oxz_propolis-server_17398ae4-34dc-4e5b-89e7-18bf70d4526d
19:22:18.602Z WARN SledAgent (InstanceManager): wait for service svc:/milestone/single-user:default in zone Some("oxz_propolis-server_fce209bb-e6b7-4935-85f8-b5c2c041f44c") failed: Property not found. retry in 689.749231ms
file = illumos-utils/src/svc.rs:36
instance_id = b8519513-c81d-4e01-ab44-f0da6ab17876
propolis_id = fce209bb-e6b7-4935-85f8-b5c2c041f44c
zone = oxz_propolis-server_fce209bb-e6b7-4935-85f8-b5c2c041f44c
This is logged by the wait_for_service function in illumos-utils.
I'm pretty sure that means we were in RunningZone::boot, which calls wait_for_service with the FMRI svc:/milestone/single-user:default:
That, in turn, was called by InstanceManager here: https://github.com/oxidecomputer/omicron/blob/26b33a11962d82b29fde9a2e3233d038d5495c44/sled-agent/src/instance.rs#L2250-L2251
I think we can fix this by making wait_for_service also check whether the zone exists at all when we see a "property not found" error, and return a non-retryable error in that case. However, we should be careful to make sure there isn't a possible race there --- can we also see that error in a case where a zone hasn't yet been created? I'm not sure.
As an aside, I'll also note that the higher-level code that calls wait_for_service seems to expect that there to be an eventual timeout around the retry loop, which should also implicitly handle this case (see the error types returned by RunningZone::boot and the code in instance_manager that waits for the Propolis service). However, as far as I can tell, it looks like there is no actual timeout here. Note that wait_for_service just calls backoff::retry_notify_ext with retry_policy_local, which configures the backoff policy thusly: https://github.com/oxidecomputer/omicron/blob/26b33a11962d82b29fde9a2e3233d038d5495c44/common/src/backoff.rs#L97-L102
This never calls the ExponentialBackoffBuilder::with_max_elapsed_time method, which is how one sets a timeout in the backoff crate's API. So, as far as I can tell, there will never be a timeout, and this will retry indefinitely. So, we might want to fix that as well, or at least change up the error returned in that case.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with illumos-utils/src/svc.rs, especially wait_for_service, then trace its callers in illumos-utils/src/running_zone.rs and sled-agent/src/instance.rs. Reproduce or test the case where a Propolis zone disappears before the service starts, and verify that the retry path handles the missing zone without retrying indefinitely or introducing a race for zones not yet created.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- backend, operating-systems
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100