unkin/tomswall
4
0
Fork 0
Code Issues 5 Pull Requests Actions Packages Projects Releases 5 Wiki Activity

Revert agent generations that cut off the control plane #35

Merged
benvin merged 3 commits from benvin/agent-safe-apply into main 2026-10-05 21:46:08 +11:00
Conversation 3 Commits 3 Files Changed 9
+716 -77
unkin-agent commented 2026-10-05 13:46:28 +11:00
Member
Copy Link
Copy Source

Why: a generation that cuts the agent off from the control plane must roll back durably, never flap.

  • Fetched generations apply like tomswall try: on-disk snapshot, systemd revert timer, shared flock.
  • The cached config applies plainly (no timer), still under the flock; it never fails closed.
  • After apply the agent reports applied over a fresh connection; transport failure reverts.
  • Reverted generations are persisted to reverted.json (and kept in memory) before restoring.
  • Apply or arm errors report failed; a failed restore keeps the snapshot and timer armed.
  • State files are written with fsync + rename + dir fsync.
Why: a generation that cuts the agent off from the control plane must roll back durably, never flap. - Fetched generations apply like `tomswall try`: on-disk snapshot, systemd revert timer, shared flock. - The cached config applies plainly (no timer), still under the flock; it never fails closed. - After apply the agent reports `applied` over a fresh connection; transport failure reverts. - Reverted generations are persisted to `reverted.json` (and kept in memory) before restoring. - Apply or arm errors report `failed`; a failed restore keeps the snapshot and timer armed. - State files are written with fsync + rename + dir fsync.
unkin-agent added 1 commit 2026-10-05 13:46:28 +11:00
Revert agent generations that cut off the control plane
ci/woodpecker/pr/build Pipeline was successful
Details
ci/woodpecker/pr/test Pipeline was successful
Details
ci/woodpecker/pr/pre-commit Pipeline was successful
Details
4ad55fc65e
unkin-agent commented 2026-10-05 13:48:08 +11:00
Author
Member
Copy Link
Copy Source
  • internal/agent/agent.go:136 — failed restore after an unreachable verify returns before writing reverted.json or reporting; the bad generation is neither marked nor reported, and is re-applied on the next reachable fetch (apply/cut/revert flapping; with no cache the device stays on the bad ruleset) → on restore failure still persist the reverted marker and report failed/reverted with the restore error.
  • internal/agent/agent.go:116 — apply-error path reports failed even when restore also failed, so the control plane cannot tell the device is left half-applied → include a distinct restore-failure marker in the reported Error (or status).
  • internal/agent/safeapply_test.go — no test where restore() returns an error (after apply error or after unreachable verify) → add both.
  • nit: internal/agent/agent.go:233 — reverted.json written via Cache.Write without fsync; a power loss after revert can lose the marker the PR says survives restart → fsync tmp before rename.
- internal/agent/agent.go:136 — failed restore after an unreachable verify returns before writing reverted.json or reporting; the bad generation is neither marked nor reported, and is re-applied on the next reachable fetch (apply/cut/revert flapping; with no cache the device stays on the bad ruleset) → on restore failure still persist the reverted marker and report `failed`/`reverted` with the restore error. - internal/agent/agent.go:116 — apply-error path reports `failed` even when restore also failed, so the control plane cannot tell the device is left half-applied → include a distinct restore-failure marker in the reported Error (or status). - internal/agent/safeapply_test.go — no test where restore() returns an error (after apply error or after unreachable verify) → add both. - nit: internal/agent/agent.go:233 — reverted.json written via Cache.Write without fsync; a power loss after revert can lose the marker the PR says survives restart → fsync tmp before rename.
unkin-agent added 1 commit 2026-10-05 13:52:40 +11:00
Apply agent generations as a pending try with a revert timer
ci/woodpecker/pr/build Pipeline was successful
Details
ci/woodpecker/pr/test Pipeline was successful
Details
ci/woodpecker/pr/pre-commit Pipeline was successful
Details
9092b463a0
unkin-agent commented 2026-10-05 13:54:21 +11:00
Author
Member
Copy Link
Copy Source
  • internal/agent/agent.go:~108 (applyConfig → EngineApplier.Apply → tryapply.Arm) — cached/boot path (raw==nil) now needs a working systemd-run; if Arm fails (unit sandboxing, no systemd) the cached config is never loaded after reboot, so the "never fails closed" guarantee in cmd/tomswall/agent.go is lost → on Arm failure in the cached path, apply without the timer (or document the requirement); and add a test for Arm failure on both fetched and cached paths (setup-error report/retry is untested).
  • nit: internal/agent/agent.go:~190 (revertGeneration) — a failed writeReverted before restore is only logged, then the restore proceeds and the generation re-applies next cycle (flap) → return the error / keep the timer pending instead of continuing silently.
  • nit: internal/tryapply/tryapply.go:27-45 — Run/Restore are exported mutable package vars reachable from production code → acceptable, but add a "tests only" doc line, or move the override behind a small SetRunner helper.
- internal/agent/agent.go:~108 (applyConfig → EngineApplier.Apply → tryapply.Arm) — cached/boot path (raw==nil) now needs a working `systemd-run`; if Arm fails (unit sandboxing, no systemd) the cached config is never loaded after reboot, so the "never fails closed" guarantee in cmd/tomswall/agent.go is lost → on Arm failure in the cached path, apply without the timer (or document the requirement); and add a test for Arm failure on both fetched and cached paths (setup-error report/retry is untested). - nit: internal/agent/agent.go:~190 (revertGeneration) — a failed `writeReverted` before restore is only logged, then the restore proceeds and the generation re-applies next cycle (flap) → return the error / keep the timer pending instead of continuing silently. - nit: internal/tryapply/tryapply.go:27-45 — `Run`/`Restore` are exported mutable package vars reachable from production code → acceptable, but add a "tests only" doc line, or move the override behind a small `SetRunner` helper.
unkin-agent added 1 commit 2026-10-05 13:56:24 +11:00
Apply the cached config without safe-apply and keep reverted generations in memory
ci/woodpecker/pr/build Pipeline was successful
Details
ci/woodpecker/pr/test Pipeline was successful
Details
ci/woodpecker/pr/pre-commit Pipeline was successful
Details
c7e02c089a
unkin-agent commented 2026-10-05 13:57:35 +11:00
Author
Member
Copy Link
Copy Source

No findings.

No findings.
benvin merged commit f593f7625d into main 2026-10-05 21:46:08 +11:00
benvin deleted branch benvin/agent-safe-apply 2026-10-05 21:46:08 +11:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
bug
Something is not working
 duplicate
This issue or pull request already exists
 enhancement
New feature
 help wanted
Need some help
 invalid
Something is wrong
 question
More information is needed
 wontfix
This won't be fixed
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
benvin (BenVincent) unkin-agent (Unkin Agent) unkinben (Ben Vincent)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: unkin/tomswall#35
Delete Branch "benvin/agent-safe-apply"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?