Skip to content

test(blackbox): compute drift end states deterministically instead of tolerating a sync window - #656

Merged
JeroenSoeters merged 3 commits into
mainfrom
fix/property-drift-determinism
Aug 18, 2026
Merged

test(blackbox): compute drift end states deterministically instead of tolerating a sync window#656
JeroenSoeters merged 3 commits into
mainfrom
fix/property-drift-determinism

Conversation

@JeroenSoeters

@JeroenSoeters JeroenSoeters commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator

Why

TestProperty_FullChaos fails roughly half of PR runs and blocks merges (#653, #654, #655 are all currently red on it). Every recorded violation lands on a resource the suite itself mutated out-of-band, and both recurring signatures (phantom resource, inventory=... but model expects ...) are one harness defect seen through the delete and modify paths: the suite recorded out-of-band drift in a ManagedDriftedResources exemption map that the invariants consulted to skip slots, and that record was retired before the divergence was actually reconciled, so assertions ran with the exemption gone. Keyed by native id, a second drift op on one resource also aliased onto the first op's snapshot.

What

The exemption never needed to exist. Background synchronization, discovery, and stack expiry are disabled in the harness config, so every sync is harness-triggered and observable, and the suite can always compute a deterministic end state from generated inputs and API responses. This change deletes the drift-tolerance mechanism rather than repairing it:

  • Absorb managed drift synchronously at the drift op. OpCloudModify/OpCloudDelete on a managed resource now pick a target untouched by in-flight commands (a command on a stack can create, update, or reconcile-delete any slot on it), shape the drifted properties to the slot's schema (keys outside it split into ReadOnlyProperties and would diverge inventory from cloud), force a sync, and wait for the inventory row to converge. The model records the absorbed end state directly: row gone on delete, cloud properties on modify.
  • Same for discovered unmanaged resources, which had the identical latent window.
  • Delete the mechanism wholesale: ManagedDriftedResources, PendingSync, HasPendingManagedDriftAffectingSlot, ApplySyncToManagedDrift, ApplySyncCommand, reconcileManagedDrift*, ClearManagedDriftForResource, CheckManagedDriftVsInventory, and the invariant skips in the phantom check and model-vs-inventory check. No invariant is loosened; two skips are removed.
  • Model applies its own predicted unmanaged sync transition (ApplySyncToUnmanaged) at every observed sync instead of reading the sync command's resource updates back into the model.
  • Make drift-target selection actually deterministic: candidates were enumerated by Go map iteration, so the generated sequence number picked a random resource; they are now enumerated in stable slot order.
  • Crash-restart hygiene: once a managed modify is absorbed, the inventory row is the restart re-injection source, so the cloud-state mirror entry is dropped; a lingering entry would overwrite later plugin writes with stale properties after a crash.

Making the syncs strict surfaced four latent harness defects that the old tolerant logging had been hiding; all are fixed here:

  1. The awaited-sync baseline race. WaitForNextSyncCommand captured its latest-sync baseline after ForceSync had already fired, so the just-created command could be taken as pre-existing and the wait never matched. This is why TriggerSyncAndWait: no sync command observed kept appearing even when the sync ran. The baseline is now captured before triggering.
  2. No-change syncs are unobservable by design. The persister deletes a completed sync command whose reads absorbed nothing (shouldDeleteSyncCommand), so waiting long for one is pure burn: the sync waits are now a short appearance window plus a completion window, and a command that appears and then vanishes counts as an observed no-change sync. This is also what restores the suite's runtime: Sequential iterations are back to ~6s from the 15s-per-trigger burns (green-main baseline ~3s), and FullChaos runs at ~12s/iteration against a 7.2s baseline.
  3. Discovery never ingests in this suite. The formas never mark the target Discoverable, so ForceDiscover always ends at "no discoverable targets" and the unmanaged-discovery expectations were dead weight: the old trigger applied the ingestion transition whenever any sync command happened to be observed in its 10-second window, setting expectations for rows that were never ingested. The trigger is now fire-and-forget with no model transition, and surprise ingestion still fails loudly as an unexpected unmanaged inventory row.
  4. A canceled changeset's resources stay sync-excluded past the cancel. The synchronizer kept logging Excluding resource from sync (in-progress operation) for a canceled command's resource 25+ seconds after the command was terminal, so drift injected on it could never absorb. The window is unobservable from outside the agent, so stacks touched by a canceled command leave the drift-target pool for the rest of the iteration. The agent-side behavior is filed separately (internal: PLA-712) since in production it means drift on those resources silently stops being absorbed.

Net -130 lines.

Verification

  • New model-level tests (written first, watched fail): absorbed end states for modify/delete, in-flight-command / cross-stack / canceled-stack eligibility exclusions, deterministic candidate ordering.
  • TestProperty_FullChaos local: green at 25 checks (23m, pre-perf-fix), 30 checks (23m), 10 checks (2m), and 25 checks (5m) after the performance fixes; each earlier failing run root-caused one of the four defects above rather than being re-rolled.
  • The four other property suites green locally at 15 checks each; Sequential re-measured at ~6s/iteration after the fixes.
  • go vet clean under property and integration tags; golangci-lint below main's baseline with no new issues.

Fixes the flake tracked in PLA-448 (internal).

… tolerating a sync window

The chaos suite tracked out-of-band changes to managed resources in a
ManagedDriftedResources record and taught the invariants to skip any slot
with pending drift. The record was retired at several sites before the
divergence was actually reconciled, so assertions ran with the exemption
gone and failed on exactly the resources the suite had drifted; keyed by
native id, a second drift op on one resource also aliased onto the first
op's snapshot.

The suite never needed the exemption: every sync is harness-triggered
(background synchronization is disabled), so drift can be absorbed
synchronously at the drift operation itself and the model can compute the
absorbed end state directly from generated inputs.

- OOB modify/delete of a managed resource now targets only resources whose
  stacks have no in-flight commands, shapes the drifted properties to the
  slot's schema (keys outside it would split into ReadOnlyProperties),
  forces a sync, and waits for the inventory row to converge before
  continuing; the model records the absorbed state at the operation
- OOB modify/delete of a discovered unmanaged resource absorbs the same
  way, closing the same window on the unmanaged path
- delete the drift-tolerance mechanism and the invariant skips wholesale;
  CheckInvariants loses its managed-drift ignore set and
  CheckManagedDriftVsInventory is gone
- capture the sync-command baseline before ForceSync: taking it after the
  trigger could race the created command and made awaited syncs degrade
  into silent no-ops
- apply the model's own predicted unmanaged sync transition at every
  observed sync, instead of reading the sync command's updates back into
  the model
- drop the cloud-state mirror entry once a managed modify is absorbed so a
  crash restart cannot resurrect stale properties over later plugin writes
- enumerate drift candidates in stable slot order so the generated
  sequence number selects the same resource on every run
@JeroenSoeters
JeroenSoeters force-pushed the fix/property-drift-determinism branch from c7185cf to 7896284 Compare August 17, 2026 20:49
…waiting on impossible commands

A sync command whose reads produce no changes is deleted from the datastore
on completion, so a no-op sync is unobservable by design. The harness's
awaited syncs burned their full timeout on every no-op sync, and the
discovery trigger waited ten seconds for an ingestion command that can never
exist: the suite's formas never mark the target Discoverable, so discovery
finds no discoverable targets and ingests nothing.

- split the sync wait into a short appearance window (command creation is
  milliseconds) and a separate completion window, and treat a command that
  appears and then vanishes as an observed no-change sync
- make the discovery trigger fire-and-forget with no model transition: with
  no discoverable targets the deterministic end state is no change, and any
  surprise ingestion still surfaces as an unexpected unmanaged inventory row
- previously the discovery trigger applied the ingestion transition whenever
  any sync command happened to be observed within its window, setting
  expectations for rows that were never ingested
A canceled changeset's resources can stay registered as in-progress with the
synchronizer past the command's terminal state, during which sync skips them
and injected drift cannot absorb. The window is not observable from outside
the agent, so stacks touched by a canceled command leave the drift-target
pool for the rest of the iteration.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant