From bd0182785cbb952152ef8f9a7af52c300dee9889 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 9 Oct 2026 01:40:02 +0200 Subject: [PATCH] Run the scrub schedule as a scheduled process, never pause a scrub, and keep the pool's name the only setting in a unit The second review of #147 found three blockers before the home server. 1. ${setting:} was not filled in a unit's name, so the distribution timers would have reached the machine as "zfs-scrub-weekly@${setting:scrub-pool}.timer". mesh-controller #171 fills it and refuses any value that is not a unit-name part. The module's own schedule needs no setting in a name now: the node-engine's timer for the scheduled process zfs-mesh-scrub fires `zfs-tools scrub-run` every quarter of an hour, and the run reads the pool, cadence, day and time from the configuration file. 2. The run does what upstream's zfs-scrub@.service does: if a scrub is running it waits for it with zpool wait -t scrub, otherwise it starts one and waits. It never pauses or stops a scrub, and the unit has no ExecStop. A resilver defers the scrub. The served slot is kept in /var/lib/zfs-tools. 3. Unassigning left the pool with no schedule. mesh-host #66 (hq ADR 0269, proposed) gives the distribution timer back running and enabled. The README says not to unassign before it is merged. Also: the run comes before the distribution timers in the declaration; the day and time are checked against fixed sets and reach no unit file (an invalid value runs nothing); a due scrub not started an hour after its time is a warning; the error counts outlive a restart in a small state file. --- modules/zfs/README.md | 74 ++++--- modules/zfs/cmd/zfs-tools/health.go | 8 +- modules/zfs/cmd/zfs-tools/helpers_test.go | 4 + modules/zfs/cmd/zfs-tools/machine.go | 49 ++++- modules/zfs/cmd/zfs-tools/main.go | 14 +- modules/zfs/cmd/zfs-tools/manifest_test.go | 106 +++++++--- modules/zfs/cmd/zfs-tools/review_test.go | 20 ++ modules/zfs/cmd/zfs-tools/schedule.go | 198 ++++++++++++++++-- modules/zfs/cmd/zfs-tools/scrub.go | 7 +- modules/zfs/cmd/zfs-tools/scrubrun_test.go | 168 +++++++++++++++ .../zfs/cmd/zfs-tools/scrubschedule_test.go | 16 +- modules/zfs/module.json | 56 ++--- 12 files changed, 596 insertions(+), 124 deletions(-) create mode 100644 modules/zfs/cmd/zfs-tools/scrubrun_test.go diff --git a/modules/zfs/README.md b/modules/zfs/README.md index caf68bc0..d7975f4a 100644 --- a/modules/zfs/README.md +++ b/modules/zfs/README.md @@ -1,7 +1,7 @@ # zfs The mesh's view of ZFS on a machine: every pool's health, layout, space, scrubs, I/O, cache and events, -and every dataset with its space and snapshots. It owns the pool's scrub schedule. It has four acts: a named +and every dataset with its space and snapshots. It owns the pool's scrub schedule, run by a scheduled process of its own. It has four acts: a named snapshot of one dataset, and a scrub's start, pause and stop. It is for the home server, whose pool holds the operator's media library, which the mesh does not own (novox/hq ADR 0051). So the module reads, and what it does only adds a snapshot or reads the pool through. @@ -17,7 +17,8 @@ This is checked by `TestTheBundleRunsOnlyReadingVerbsSnapshotAndScrub`. Every co built by one of its argument functions, and each function is held to its program and verb: - `zfs`: `list`, `get` and `snapshot`; -- `zpool`: `list`, `status`, `iostat`, `events`, `history` and `scrub` (with `-p` or `-s` only); +- `zpool`: `list`, `status`, `iostat`, `events`, `history`, `wait -t scrub` and `scrub` (with `-p` or `-s` + only, and those only from the person's acts, never from the scheduled run); - `systemctl`: `show`. The test fails on any call site that builds its own argument list, through `ask`, `AsRoot`, `Escalated`, @@ -86,34 +87,47 @@ The module owns the schedule of the pool its `scrub-pool` setting names. |---|---|---| | `scrub-pool` | none: the operator's value | the pool to scrub | | `scrub-cadence` | `weekly` | `weekly`, or `monthly` (the first given weekday of each month) | -| `scrub-day` | `Mon` | `Mon` to `Sun` | -| `scrub-time` | `00:00` | `HH:MM`, in the machine's time zone | +| `scrub-day` | `Mon` | `Mon`, `Tue`, `Wed`, `Thu`, `Fri`, `Sat` or `Sun`, nothing else | +| `scrub-time` | `00:00` | `HH:MM` from `00:00` to `23:59`, in the machine's time zone | The defaults are what the distribution's weekly timer did on the home server before the module, so taking -it over does not move the schedule. +it over does not move the schedule. The operator chose monthly for the home server. -The mesh writes two timer templates of the module's own, `zfs-mesh-scrub-weekly@.timer` and -`zfs-mesh-scrub-monthly@.timer`, and the service they start, `zfs-mesh-scrub@.service`. The service runs -`/usr/bin/zpool scrub -w -- ` with no shell; stopping it while a scrub runs pauses the scrub, as the -distribution's unit does. The mesh enables the one timer the cadence names, for the one pool. +**The scheduled run.** The node-engine runs the module's scheduled process `zfs-mesh-scrub` (`zfs-tools +scrub-run`, as root) from its own timer `zfs-mesh-scrub.timer`, every quarter of an hour. The run reads the +configuration the mesh writes from the settings (`/etc/zfs-tools/scrub.conf`). It finds the slot, which is +the last scheduled time not after now. If the slot is not yet served, the run acts as upstream's +`zfs-scrub@.service` does: -**Exactly one schedule per pool.** In the same declaration, the distribution's own -`zfs-scrub-weekly@.timer` and `zfs-scrub-monthly@.timer` are held stopped and disabled. On the -home server the weekly one was enabled by hand, outside the mesh. A scrub already running is not stopped by -this; only its timer is. The day and time cannot be expressed with the distribution's templates, which fire -at "weekly" or "monthly" with a random delay, so the module's own timer replaces them. +- if a scrub is running, it waits for it with `zpool wait -t scrub -- `; +- otherwise it starts one with `zpool scrub -- ` and waits for it the same way; +- if the start is refused because a scrub has just begun, it waits for that one. -**Unassigning the module leaves the pool with no scrub schedule.** The node-engine gives a unit back as it -found it but never starts one on the way out: the distribution's timer stays stopped and disabled, and the -module's own timer and units go. Re-enable a schedule by hand, or assign the module again. +It never pauses or stops a scrub. The unit has no ExecStop, so stopping the unit ends only the wait. A +resilver that is running defers the scrub to the next run after it. The served slot is kept in +`/var/lib/zfs-tools/scrub-served.json`. Without that file, a scrub that status shows started in the slot +serves it. So a scrub starts within 15 minutes of its time, and after the machine was off, within 15 +minutes of its coming back. + +**No setting value reaches a unit file.** The day, the time and the cadence go only into the +configuration. The run checks them against fixed sets (the seven weekday names, and `HH:MM`). It refuses +any other value and runs nothing; a value like `*:*` or `Mon..Fri` is impossible. The pool's name reaches +the names of the two distribution timers. The controller fills it there and refuses anything but letters, +digits, `:`, `_`, `.` and `-` (mesh-controller #171). + +**Exactly one schedule per pool.** The distribution's `zfs-scrub-weekly@.timer` and +`zfs-scrub-monthly@.timer` are held stopped and disabled. They come after the scheduled run in the +declaration, so a run that cannot be installed stops the apply before the timer that scrubs the pool today +is touched. On the home server the weekly one was enabled by hand, outside the mesh. A scrub already +running is not stopped by this; only its timer is. + +**Unassigning the module gives the distribution's timer back running and enabled**, as the node-engine +found it, once mesh-host #66 (hq ADR 0269, proposed) is merged. Until then the node-engine never starts a +unit on the way out, and unassigning would leave the pool with no scrub schedule. Do not unassign the +module before #66 is merged. **One pool per assignment in this version.** A manifest's resources are fixed, and a setting cannot make -one timer per pool in a list. The home server has one pool. A second pool needs either a second set of -resources or a controller feature that makes resources per item of a list. - -The setting values reach the timer file as they are. The tools check them (`zfs_scrub` and the health say -an invalid one), and the service manager refuses a calendar it cannot read. The values are the operator's -own, so they are not escaped. +one timer per pool in a list. The home server has one pool. ### The scrub acts @@ -137,14 +151,15 @@ module watches and reports it: ## Health -The module runs two long-running processes of its own binary, and the timer: +The module runs two long-running processes of its own binary, and the scheduled run: - **The watch** (`zfs-tools watch`) runs as the operator account and carries the health check, `zfs_health` (novox/hq ADR 0240). Each minute it reads the pools and writes a line in its journal whenever their health changes. - **The event follower** (`zfs-tools events`) runs as root and follows `zpool events -f`. It is described in the next section. -- **The scrub timer** is judged by its unit. +- **The scheduled run** (`zfs-tools scrub-run`) is not long-running and has no health check of its own; + `zfs_health` says when it failed or did not start a due scrub. `zfs_health` is **unhealthy** for a fault now: @@ -163,9 +178,13 @@ These are **warnings**, which leave it healthy: - a resilver running; - a last finished scrub older than the schedule allows (weekly 9 days, monthly 38 days), or 35 days for a pool without a schedule; -- a scheduled scrub that failed, with no scrub finished since; +- a scheduled scrub that has not started an hour after its time; +- a scheduled run that failed, with no scrub finished since; - a schedule whose timer is not active and enabled, or whose settings are not valid. +The error counts last seen are kept in `/var/lib/zfs-tools/errors.json`, so a restart of the bundle does +not take every count as past again. + ## The event journal ZFS keeps its events in a queue in the kernel, in memory, from the moment its module loads, so a reboot @@ -194,7 +213,8 @@ The tests cover: paused scrubs) and `iostat -H` without section lines; - a timed-out command ended with its children; - the snapshot's refusals, read-back, space statement and unknown outcome; -- the scrub acts and the schedule; +- the scrub acts, the schedule's slots, and the scheduled run (start, wait, never pause, defer to a + resilver, refuse an invalid setting); - the follower's cursor, dropped events, kill before wait, and exit after repeated failures; - the health verdicts, past against new errors; - a machine without ZFS; diff --git a/modules/zfs/cmd/zfs-tools/health.go b/modules/zfs/cmd/zfs-tools/health.go index 32d20d1b..076d0d96 100644 --- a/modules/zfs/cmd/zfs-tools/health.go +++ b/modules/zfs/cmd/zfs-tools/health.go @@ -35,6 +35,8 @@ type Look struct { Schedule *Schedule // ScheduleErr is a schedule whose settings are not valid. ScheduleErr error + // Served is the last slot the scheduled run served, per pool. + Served map[string]int64 } // errorFinding judges one line of status's error counts against the last look: a count that rose is a @@ -166,12 +168,13 @@ func (mem *ErrorMemory) Judge(l Look) Health { warn = append(warn, fmt.Sprintf("Pool %s was never scrubbed.", p.Name)) } if l.Schedule != nil && l.Schedule.Pool == p.Name { - warn = append(warn, scheduleFindings(l.Schedule, ended)...) + warn = append(warn, scheduleFindings(l.Schedule, s, l.Now, l.Served)...) } } if l.Schedule != nil && statusOf(l.Status, l.Schedule.Pool) == nil { warn = append(warn, fmt.Sprintf("The scrub schedule names pool %s, which is not imported here.", l.Schedule.Pool)) } + mem.save() if len(found) > 0 { return Health{Healthy: false, Why: strings.Join(found, " "), Found: found, Warnings: warn} } @@ -201,7 +204,8 @@ func (m *Machine) Health(ctx context.Context) Health { return Health{Healthy: false, Why: sentence("zpool status did not answer: " + err.Error()), Found: []string{err.Error()}} } sched, serr := m.ScheduleNow(ctx) - return m.Errors.Judge(Look{Pools: pools, Status: status, Now: m.Now(), Schedule: sched, ScheduleErr: serr}) + return m.Errors.Judge(Look{Pools: pools, Status: status, Now: m.Now(), Schedule: sched, ScheduleErr: serr, + Served: m.readServed(ServedFile)}) } func sentence(s string) string { diff --git a/modules/zfs/cmd/zfs-tools/helpers_test.go b/modules/zfs/cmd/zfs-tools/helpers_test.go index e9da5b0c..3cc1be18 100644 --- a/modules/zfs/cmd/zfs-tools/helpers_test.go +++ b/modules/zfs/cmd/zfs-tools/helpers_test.go @@ -66,6 +66,10 @@ func (f *fake) machine() *Machine { ReadFile: func(p string) ([]byte, error) { s, ok := f.files[p] if !ok { + if strings.HasPrefix(p, os.TempDir()) { + // What the code under test wrote into a test's own directory. + return os.ReadFile(p) + } return nil, os.ErrNotExist } return []byte(s), nil diff --git a/modules/zfs/cmd/zfs-tools/machine.go b/modules/zfs/cmd/zfs-tools/machine.go index 3a163a64..234523ed 100644 --- a/modules/zfs/cmd/zfs-tools/machine.go +++ b/modules/zfs/cmd/zfs-tools/machine.go @@ -3,6 +3,7 @@ package main import ( "bytes" "context" + "encoding/json" "errors" "fmt" "os" @@ -119,7 +120,7 @@ type Machine struct { // Here is the machine this process runs on. func Here() *Machine { - return &Machine{Run: ExecRunner, UID: os.Getuid(), Now: time.Now, ReadFile: os.ReadFile, Errors: NewErrorMemory()} + return &Machine{Run: ExecRunner, UID: os.Getuid(), Now: time.Now, ReadFile: os.ReadFile, Errors: LoadErrorMemory(ErrorsFile)} } // NotApplicable is the answer of a machine without ZFS: the module does not apply here. @@ -160,6 +161,7 @@ type ErrorMemory struct { mu sync.Mutex counts map[string][3]uint64 rose map[string]time.Time + path string } // NewErrorMemory remembers nothing yet: every count on the first look is past. @@ -167,6 +169,51 @@ func NewErrorMemory() *ErrorMemory { return &ErrorMemory{counts: map[string][3]uint64{}, rose: map[string]time.Time{}} } +// ErrorsFile is where the error counts last seen are kept, so a restart of the bundle does not take every +// count as past again. +const ErrorsFile = "/var/lib/zfs-tools/errors.json" + +type keptErrors struct { + Counts map[string][3]uint64 `json:"counts"` + Rose map[string]time.Time `json:"rose"` +} + +// LoadErrorMemory is the memory kept in path; an empty one when there is none, and either way it is kept +// there from now on. +func LoadErrorMemory(path string) *ErrorMemory { + mem := NewErrorMemory() + mem.path = path + if b, err := os.ReadFile(path); err == nil { + var k keptErrors + if json.Unmarshal(b, &k) == nil { + if k.Counts != nil { + mem.counts = k.Counts + } + if k.Rose != nil { + mem.rose = k.Rose + } + } + } + return mem +} + +// save keeps the memory, atomically; a failure only costs the next restart its memory. +func (mem *ErrorMemory) save() { + if mem == nil || mem.path == "" { + return + } + mem.mu.Lock() + b, err := json.Marshal(keptErrors{Counts: mem.counts, Rose: mem.rose}) + mem.mu.Unlock() + if err != nil { + return + } + tmp := mem.path + ".new" + if os.WriteFile(tmp, b, 0o644) == nil { + _ = os.Rename(tmp, mem.path) + } +} + // ask runs a read-only command as this account and answers its output, or why it could not. func (m *Machine) ask(ctx context.Context, name string, args ...string) (string, error) { r := m.Run(ctx, name, args...) diff --git a/modules/zfs/cmd/zfs-tools/main.go b/modules/zfs/cmd/zfs-tools/main.go index 47d93c4e..4d3c9535 100644 --- a/modules/zfs/cmd/zfs-tools/main.go +++ b/modules/zfs/cmd/zfs-tools/main.go @@ -2,11 +2,12 @@ // scrubs, I/O, cache and events, every dataset with its space and snapshots — and one act, taking a named // snapshot of one dataset. // -// One binary, three ways to run. With no argument it is the bundle the node's tool runner launches and +// One binary, four ways to run. With no argument it is the bundle the node's tool runner launches and // speaks MCP to over stdio (novox/hq ADR 0188, ADR 0193), as the operator account. With `watch` and // `events` it is one of the module's two long-running processes, run by the node-engine: the watch, which // the health check zfs_health stands on (watch.go), and the event follower, as root, judged by its unit -// (follow.go). +// (follow.go). With `scrub-run` it is the scheduled scrub, run as root by the node-engine's timer +// (schedule.go). package main import ( @@ -26,6 +27,13 @@ func main() { switch os.Args[1] { case "watch": (&Watch{M: Here(), Out: os.Stdout}).Loop(ctx) + case "scrub-run": + said, err := Here().ScrubRun(ctx, ServedFile) + if err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } + fmt.Println(said) case "events": if err := Follower(ctx, os.Stdout); err != nil { fmt.Fprintln(os.Stderr, err) @@ -33,7 +41,7 @@ func main() { } default: fmt.Fprintf(os.Stderr, "zfs-tools: %q is not a way to run; with no argument it serves its tools, with watch it "+ - "watches the pools, with events it follows ZFS's events\n", os.Args[1]) + "watches the pools, with events it follows ZFS's events, with scrub-run it runs the scheduled scrub\n", os.Args[1]) os.Exit(2) } return diff --git a/modules/zfs/cmd/zfs-tools/manifest_test.go b/modules/zfs/cmd/zfs-tools/manifest_test.go index e3774375..a008e039 100644 --- a/modules/zfs/cmd/zfs-tools/manifest_test.go +++ b/modules/zfs/cmd/zfs-tools/manifest_test.go @@ -8,6 +8,7 @@ import ( "sort" "strings" "testing" + "time" ) type manifest struct { @@ -75,8 +76,9 @@ func TestTheManifestNamesTheToolsTheBundleServesAndWhatEachReplaces(t *testing.T } } -// ADR 0240: zfs_health on the watch, which runs as the operator account; the follower and the scrub -// timer judged by their units. The module installs nothing, so it never puts ZFS on a machine. +// ADR 0240: zfs_health on the watch, which runs as the operator account; the follower judged by its +// unit. The scheduled run is not long-running and declares none. The module installs nothing, so it never +// puts ZFS on a machine. func TestTheHealthChecksAndWhoRunsWhat(t *testing.T) { m, raw := readManifest(t) byID := map[string]map[string]any{} @@ -91,53 +93,62 @@ func TestTheHealthChecksAndWhoRunsWhat(t *testing.T) { if h, _ := e["health"].(map[string]any); h["kind"] != "unit" || e["user"] != nil { t.Errorf("the follower runs as root, judged by its unit: %v", e) } - if h, _ := byID["scrub-schedule"]["health"].(map[string]any); h["kind"] != "unit" { - t.Errorf("the scrub timer is judged by its unit: %v", byID["scrub-schedule"]) + if d := byID["state-dir"]; d["path"] != "/var/lib/zfs-tools" || d["owner"] != "${machine:account}" || + filepath.Dir(ErrorsFile) != d["path"] || filepath.Dir(ServedFile) != d["path"] { + t.Errorf("the state the watch and the run keep: %v", d) } if strings.Contains(raw, `"package"`) { t.Errorf("the module installs no package: ZFS is the machine's, and its kernel module is built against its kernel") } } -// One schedule per pool: the module's timer for the cadence the settings name is running and enabled, -// and the distribution's weekly and monthly timers for the same pool are stopped and disabled in the same -// declaration. The service runs zpool with fixed arguments and no shell. -func TestExactlyOneScrubScheduleAndItsCommandIsFixed(t *testing.T) { - m, _ := readManifest(t) +// One schedule per pool, and no setting in any unit's name but the distribution's timers', which the +// controller fills (mesh-controller #171) and checks. The module's own schedule is the node-engine's timer +// for the scheduled run, a fixed name; the day and time reach only the configuration, which the run checks. +// The run comes before the distribution's timers are stopped, so a run that cannot be installed stops the +// apply before the timer that scrubs the pool today is touched. +func TestExactlyOneScrubScheduleAndNoSettingInAUnitFile(t *testing.T) { + m, raw := readManifest(t) byID := map[string]map[string]any{} - for _, r := range m.Resources { + order := map[string]int{} + for i, r := range m.Resources { byID[r["id"].(string)] = r + order[r["id"].(string)] = i } - mine := byID["scrub-schedule"] - if mine["unit"] != "zfs-mesh-scrub-${setting:scrub-cadence}@${setting:scrub-pool}.timer" || mine["state"] != "running" || mine["boot"] != "enabled" { - t.Errorf("the module's timer: %v", mine) + run := byID["scrub-run"] + args, _ := run["run"].([]any) + if run["type"] != "process" || run["name"] != "zfs-mesh-scrub" || run["schedule"] != "*/15 * * * *" || run["user"] != nil || + len(args) != 2 || args[1] != "scrub-run" { + t.Errorf("the scheduled run: %v", run) + } + if ScrubTimer != run["name"].(string)+".timer" || ScrubService != run["name"].(string)+".service" { + t.Errorf("the tools read %s and %s; the node-engine names them after %v", ScrubTimer, ScrubService, run["name"]) } for _, c := range []string{"weekly", "monthly"} { d := byID["distribution-"+c] if d["unit"] != "zfs-scrub-"+c+"@${setting:scrub-pool}.timer" || d["state"] != "stopped" || d["boot"] != "disabled" { t.Errorf("the distribution's %s timer: %v", c, d) } - timer, _ := byID["scrub-"+c]["content"].(string) - if byID["scrub-"+c]["path"] != "/etc/systemd/system/zfs-mesh-scrub-"+c+"@.timer" || !strings.Contains(timer, "Unit=zfs-mesh-scrub@%i.service") || - !strings.Contains(timer, "Persistent=true") || !strings.Contains(timer, "OnCalendar=${setting:scrub-day} ") { - t.Errorf("the %s timer: %v", c, byID["scrub-"+c]) + if order["scrub-run"] > order["distribution-"+c] { + t.Errorf("the distribution's %s timer is stopped before the run is installed", c) } } - svc, _ := byID["scrub-service"]["content"].(string) - var execs []string - for _, l := range strings.Split(svc, "\n") { - if strings.HasPrefix(l, "Exec") { - execs = append(execs, l) + for _, r := range m.Resources { + p, _ := r["path"].(string) + if strings.HasPrefix(p, "/etc/systemd/") { + t.Errorf("the module writes a unit file of its own: %s", p) } } - if strings.Join(execs, "|") != "ExecStart=/usr/bin/zpool scrub -w -- %i|ExecStop=-/usr/bin/zpool scrub -p -- %i" { - t.Errorf("the scrub service runs: %v", execs) + if strings.Contains(raw, "scrub-day}") && !strings.Contains(byID["scrub-config"]["content"].(string), "${setting:scrub-day}") { + t.Errorf("the day reaches something other than the configuration") + } + if n := strings.Count(raw, "${setting:scrub-day}") + strings.Count(raw, "${setting:scrub-time}") + strings.Count(raw, "${setting:scrub-cadence}"); n != 3 { + t.Errorf("the day, time and cadence are written %d times; only into the configuration", n) } var settings struct { Settings map[string]map[string]any `json:"settings"` } - raw, _ := os.ReadFile("../../module.json") - json.Unmarshal(raw, &settings) + json.Unmarshal([]byte(raw), &settings) if settings.Settings["scrub-cadence"]["default"] != "weekly" || settings.Settings["scrub-day"]["default"] != "Mon" || settings.Settings["scrub-time"]["default"] != "00:00" { t.Errorf("defaults: %v", settings.Settings) @@ -145,10 +156,41 @@ func TestExactlyOneScrubScheduleAndItsCommandIsFixed(t *testing.T) { if _, has := settings.Settings["scrub-pool"]; has { t.Errorf("the pool is the operator's value, with no default (ADR 0112, ADR 0262)") } - conf, _ := byID["scrub-config"]["content"].(string) - if _, err := ParseScrubConfig(strings.NewReplacer("${setting:scrub-pool}", "storage", "${setting:scrub-cadence}", "weekly", - "${setting:scrub-day}", "Mon", "${setting:scrub-time}", "00:00").Replace(conf)); err != nil || byID["scrub-config"]["path"] != ScrubConfig { - t.Errorf("the configuration the tools read: %v", err) +} + +// The home server, composed as the controller fills its settings (pool storage, cadence monthly, the +// operator's choice): the units the node-engine is sent and the configuration the run reads. +func TestTheHomeServersComposedUnitsAndConfiguration(t *testing.T) { + m, _ := readManifest(t) + fill := strings.NewReplacer("${setting:scrub-pool}", "storage", "${setting:scrub-cadence}", "monthly", + "${setting:scrub-day}", "Mon", "${setting:scrub-time}", "00:00") + var units []string + var conf string + for _, r := range m.Resources { + switch { + case r["type"] == "service": + units = append(units, fill.Replace(r["unit"].(string))+" "+r["state"].(string)+"/"+r["boot"].(string)) + case r["type"] == "process" && r["schedule"] != nil: + units = append(units, r["name"].(string)+".timer running/enabled (the node-engine's, every quarter of an hour)") + case r["id"] == "scrub-config": + conf = fill.Replace(r["content"].(string)) + } + } + want := []string{"zfs-mesh-scrub.timer running/enabled (the node-engine's, every quarter of an hour)", + "zfs-scrub-weekly@storage.timer stopped/disabled", "zfs-scrub-monthly@storage.timer stopped/disabled"} + if strings.Join(units, "\n") != strings.Join(want, "\n") { + t.Errorf("units:\n%s", strings.Join(units, "\n")) + } + for _, u := range units { + t.Log(u) + } + s, err := ParseScrubConfig(conf) + if err != nil || s.Pool != "storage" || s.Cadence != "monthly" { + t.Fatalf("configuration %q: %v", conf, err) + } + t.Logf("next scrub of storage after 2026-10-09 12:00: %s", s.NextSlot(time.Date(2026, 10, 9, 12, 0, 0, 0, time.Local)).Format("2006-01-02 15:04")) + if next := s.NextSlot(time.Date(2026, 10, 9, 12, 0, 0, 0, time.Local)); !next.Equal(time.Date(2026, 11, 2, 0, 0, 0, 0, time.Local)) { + t.Errorf("next monthly slot %s, want the first Monday of November", next) } } @@ -162,13 +204,13 @@ func TestTheBundleRunsOnlyReadingVerbsSnapshotAndScrub(t *testing.T) { "SnapshotArgv": {"zfs", "snapshot"}, "PoolsArgv": {"zpool", "list"}, "StatusArgv": {"zpool", "status"}, "IOArgv": {"zpool", "iostat"}, "EventsArgv": {"zpool", "events"}, "HistoryArgv": {"zpool", "history"}, "FollowArgv": {"zpool", "events"}, - "ScrubArgv": {"zpool", "scrub"}, + "ScrubArgv": {"zpool", "scrub"}, "WaitArgv": {"zpool", "wait"}, "TimerShowArgv": {"systemctl", "show"}, "ServiceShowArgv": {"systemctl", "show"}, } verbs := map[string][]string{"DatasetsArgv": DatasetsArgv(), "SnapshotsArgv": SnapshotsArgv(), "PoolsArgv": PoolsArgv(), "StatusArgv": StatusArgv(), "IOArgv": IOArgv(3), "EventsArgv": EventsArgv(), "HistoryArgv": HistoryArgv("tank"), "PropertiesArgv": PropertiesArgv("tank"), "SnapshotArgv": SnapshotArgv("tank", "x"), "FollowArgv": FollowArgv(), - "TimerShowArgv": TimerShowArgv("x.timer"), "ServiceShowArgv": ServiceShowArgv("x.service")} + "TimerShowArgv": TimerShowArgv("x.timer"), "ServiceShowArgv": ServiceShowArgv("x.service"), "WaitArgv": WaitArgv("tank")} for fn, argv := range verbs { if argv[0] != allowed[fn][1] { t.Errorf("%s runs %q, not %q", fn, argv[0], allowed[fn][1]) diff --git a/modules/zfs/cmd/zfs-tools/review_test.go b/modules/zfs/cmd/zfs-tools/review_test.go index 46e0b8ff..6fb40191 100644 --- a/modules/zfs/cmd/zfs-tools/review_test.go +++ b/modules/zfs/cmd/zfs-tools/review_test.go @@ -317,3 +317,23 @@ func TestIOWithoutSectionLines(t *testing.T) { t.Errorf("rows: %+v", rows) } } + +// The error counts outlive a restart of the bundle: what was seen before is kept in a small file, so a +// count that rose while the bundle was down is told from one that was there before. +func TestTheErrorMemoryIsKeptAcrossARestart(t *testing.T) { + path := filepath.Join(t.TempDir(), "errors.json") + f := newFake(t) + f.errors = LoadErrorMemory(path) + f.on(line("zpool", PoolsArgv()), "slow\t1000\t100\t900\t10\t1\tONLINE\t1.00\n") + f.on(line("zpool", StatusArgv()), fixtureSection(t, "zpool-status-rebuild.txt", "slow")) + if h := f.machine().Health(context.Background()); !h.Healthy { + t.Fatalf("first look: %+v", h) + } + // Restarted: a new memory from the file, and the count rose meanwhile. + f.errors = LoadErrorMemory(path) + f.now = f.now.Add(time.Hour) + f.on(line("zpool", StatusArgv()), strings.Replace(fixtureSection(t, "zpool-status-rebuild.txt", "slow"), "0 0 2", "0 0 9", 1)) + if h := f.machine().Health(context.Background()); h.Healthy || !strings.Contains(h.Why, "checksum errors rose from 2 to 9") { + t.Errorf("after a restart: %+v", h) + } +} diff --git a/modules/zfs/cmd/zfs-tools/schedule.go b/modules/zfs/cmd/zfs-tools/schedule.go index 7dd0bb86..acd521b2 100644 --- a/modules/zfs/cmd/zfs-tools/schedule.go +++ b/modules/zfs/cmd/zfs-tools/schedule.go @@ -2,7 +2,10 @@ package main import ( "context" + "encoding/json" "fmt" + "os" + "path/filepath" "regexp" "strconv" "strings" @@ -11,12 +14,29 @@ import ( // The scrub schedule the module owns. // -// The mesh writes ScrubConfig from the assignment's settings, and with it two timer templates of the -// module's own, weekly and monthly, and the service they start, which runs `zpool scrub -w ` with a -// fixed argument list. It enables the one timer the cadence names for the one pool the settings name, and -// holds the distribution's own scrub timers for that pool stopped and disabled, so exactly one schedule -// scrubs the pool. The tools read the configuration and ask the service manager what the timer and the -// service say; they never change either. +// The mesh writes ScrubConfig from the assignment's settings. The node-engine runs the module's +// scheduled process, `zfs-tools scrub-run`, from its own timer (ScrubTimer) every quarter of an hour, and +// the run decides from the configuration whether a scrub is due: the last scheduled time not after now +// (the slot) not yet served. Due, it starts the pool's scrub, or waits for one already running, as +// upstream's zfs-scrub@.service does; it never pauses or stops one, and nothing stops the unit to pause +// it (the node-engine writes no ExecStop). So no setting value ever reaches a unit file: the day and the +// time are read and checked here, and an invalid one is refused before anything runs. +// +// The mesh holds the distribution's own scrub timers for the pool stopped and disabled, so one schedule +// scrubs it; the node-engine gives them back running and enabled when the module goes (novox/hq ADR 0269). + +// ScrubTimer and ScrubService are the node-engine's units for the scheduled run (the process's name). +const ( + ScrubTimer = "zfs-mesh-scrub.timer" + ScrubService = "zfs-mesh-scrub.service" +) + +// ServedFile is where the scheduled run keeps the last slot it served, per pool. +const ServedFile = "/var/lib/zfs-tools/scrub-served.json" + +// MissedAfter is how long after its slot a scrub that has not started is said to be missed: the run +// fires every quarter of an hour. +const MissedAfter = time.Hour // ScrubConfig is the schedule as the mesh wrote it from the settings. const ScrubConfig = "/etc/zfs-tools/scrub.conf" @@ -37,9 +57,12 @@ type Schedule struct { Timer string `json:"timer"` Service string `json:"service"` // TimerState and Enabled are the timer's ActiveState and UnitFileState. - TimerState string `json:"timer_state,omitempty"` - Enabled string `json:"enabled,omitempty"` + TimerState string `json:"timer_state,omitempty"` + Enabled string `json:"enabled,omitempty"` + // Next is the next scheduled scrub, from the settings; TimerNext when the timer next fires the run that + // decides (every quarter of an hour). Next *time.Time `json:"next_run,omitempty"` + TimerNext *time.Time `json:"timer_next,omitempty"` LastTrigger *time.Time `json:"last_triggered,omitempty"` // LastResult is the service's Result of its last run (success, exit-code, …), LastExit when it ended. LastResult string `json:"last_result,omitempty"` @@ -79,11 +102,151 @@ func ParseScrubConfig(text string) (*Schedule, error) { case !clock.MatchString(s.Time): return nil, fmt.Errorf("the scrub-time setting is %q; it is HH:MM, 00:00 to 23:59", s.Time) } - s.Timer = "zfs-mesh-scrub-" + s.Cadence + "@" + s.Pool + ".timer" - s.Service = "zfs-mesh-scrub@" + s.Pool + ".service" + s.Timer, s.Service = ScrubTimer, ScrubService return s, nil } +var weekdays = map[string]time.Weekday{"Sun": time.Sunday, "Mon": time.Monday, "Tue": time.Tuesday, + "Wed": time.Wednesday, "Thu": time.Thursday, "Fri": time.Friday, "Sat": time.Saturday} + +// at is the scheduled time on one day. +func (s *Schedule) at(day time.Time) time.Time { + h, _ := strconv.Atoi(s.Time[:2]) + mi, _ := strconv.Atoi(s.Time[3:]) + return time.Date(day.Year(), day.Month(), day.Day(), h, mi, 0, 0, day.Location()) +} + +// scheduledOn says whether a scrub is scheduled on a day: its weekday, and for a monthly one the first +// such day of the month. +func (s *Schedule) scheduledOn(day time.Time) bool { + return day.Weekday() == weekdays[s.Day] && (s.Cadence == "weekly" || day.Day() <= 7) +} + +// Slot is the last scheduled time not after now. +func (s *Schedule) Slot(now time.Time) time.Time { + day := time.Date(now.Year(), now.Month(), now.Day(), 0, 0, 0, 0, now.Location()) + for i := 0; i < 45; i++ { + if s.scheduledOn(day) && !s.at(day).After(now) { + return s.at(day) + } + day = day.AddDate(0, 0, -1) + } + return time.Time{} +} + +// NextSlot is the first scheduled time after now. +func (s *Schedule) NextSlot(now time.Time) time.Time { + day := time.Date(now.Year(), now.Month(), now.Day(), 0, 0, 0, 0, now.Location()) + for i := 0; i < 45; i++ { + if s.scheduledOn(day) && s.at(day).After(now) { + return s.at(day) + } + day = day.AddDate(0, 0, 1) + } + return time.Time{} +} + +// WaitArgv waits for a pool's running scrub to end. +func WaitArgv(pool string) []string { return []string{"wait", "-t", "scrub", "--", pool} } + +// WaitBound is how long the scheduled run waits for a scrub: past it the run ends and leaves the scrub +// running; a scrub never pauses for it. +const WaitBound = 7 * 24 * time.Hour + +// readServed is the last slot served per pool; none when the file is not there or not readable. +func (m *Machine) readServed(path string) map[string]int64 { + out := map[string]int64{} + b, err := m.ReadFile(path) + if err != nil { + return out + } + _ = json.Unmarshal(b, &out) + return out +} + +// startedSince says whether status shows the pool's scrub started at or after a time. +func startedSince(p *PoolStatus, slot time.Time) bool { + if p == nil { + return false + } + sc := p.Scrub() + return sc != nil && sc.Started != nil && !sc.Started.Before(slot) +} + +// ScrubRun is the scheduled run: a due scrub started, or one running waited for. It answers what it did in +// a line, and fails on an invalid schedule, which it never acts on. +func (m *Machine) ScrubRun(ctx context.Context, servedPath string) (string, error) { + raw, err := m.ReadFile(ScrubConfig) + if err != nil { + return "", fmt.Errorf("%s could not be read: %v", ScrubConfig, err) + } + s, err := ParseScrubConfig(string(raw)) + if err != nil { + return "", fmt.Errorf("refused, nothing run: %v", err) + } + now := m.Now() + slot := s.Slot(now) + served := m.readServed(servedPath) + if served[s.Pool] >= slot.Unix() { + return fmt.Sprintf("the scrub of %s scheduled %s was served", s.Pool, slot.Format("2006-01-02 15:04")), nil + } + st, err := m.Status(ctx) + if err != nil { + return "", err + } + p := statusOf(st, s.Pool) + if p == nil { + return "", fmt.Errorf("the scheduled pool %s is not imported here", s.Pool) + } + mark := func() error { + served[s.Pool] = slot.Unix() + b, _ := json.Marshal(served) + if err := os.MkdirAll(filepath.Dir(servedPath), 0o755); err != nil { + return err + } + tmp := servedPath + ".new" + if err := os.WriteFile(tmp, b, 0o644); err != nil { + return err + } + return os.Rename(tmp, servedPath) + } + if startedSince(p, slot) && p.Scrub().State != "running" { + return fmt.Sprintf("the scrub of %s scheduled %s ran already", s.Pool, slot.Format("2006-01-02 15:04")), mark() + } + for _, r := range p.Resilvers() { + if r.State == "running" { + return fmt.Sprintf("a resilver of %s runs; the scrub scheduled %s waits for the next run after it", s.Pool, + slot.Format("2006-01-02 15:04")), nil + } + } + started := false + if sc := p.Scrub(); sc == nil || sc.State != "running" { + _, err := m.AsRoot(ctx, "zpool", ScrubArgv("start", s.Pool)...) + switch { + case err == nil: + started = true + case strings.Contains(err.Error(), "currently scrubbing"): + // One began since status was read: it serves the slot, and is waited for like any other. + default: + return "", fmt.Errorf("the scheduled scrub of %s could not start: %v", s.Pool, err) + } + } + if err := mark(); err != nil { + return "", fmt.Errorf("the scrub of %s runs, and the served slot could not be kept: %v", s.Pool, err) + } + wctx, cancel := context.WithTimeout(ctx, WaitBound) + defer cancel() + _, werr := m.AsRoot(wctx, "zpool", WaitArgv(s.Pool)...) + what := "waited for the scrub of " + s.Pool + " already running" + if started { + what = "started the scrub of " + s.Pool + " scheduled " + slot.Format("2006-01-02 15:04") + ", and waited for it" + } + if werr != nil { + return what + "; the wait ended without its end (" + werr.Error() + "), and the scrub goes on", nil + } + return what + " to finish", nil +} + // TimerShowArgv asks the service manager what a timer says: whether it runs and is enabled, when it // fires next and when it last did, as Unix times. func TimerShowArgv(unit string) []string { @@ -133,8 +296,10 @@ func (m *Machine) ScheduleNow(ctx context.Context) (*Schedule, error) { } else { v := parseShow(out) s.TimerState, s.Enabled = v["ActiveState"], v["UnitFileState"] - s.Next, s.LastTrigger = unixStamp(v["NextElapseUSecRealtime"]), unixStamp(v["LastTriggerUSec"]) + s.TimerNext, s.LastTrigger = unixStamp(v["NextElapseUSecRealtime"]), unixStamp(v["LastTriggerUSec"]) } + next := s.NextSlot(m.Now()) + s.Next = &next if out, err := m.ask(ctx, "systemctl", ServiceShowArgv(s.Service)...); err == nil { v := parseShow(out) s.ServiceState, s.LastResult, s.LastExit = v["ActiveState"], v["Result"], unixStamp(v["ExecMainExitTimestamp"]) @@ -143,9 +308,16 @@ func (m *Machine) ScheduleNow(ctx context.Context) (*Schedule, error) { } // scheduleFindings are the schedule's warnings: a timer not waiting, a last run that failed with no scrub -// finished since. -func scheduleFindings(s *Schedule, scrubEnded *time.Time) []string { +// finished since, a slot past by MissedAfter that no scrub served. +func scheduleFindings(s *Schedule, p *PoolStatus, now time.Time, served map[string]int64) []string { var warn []string + var scrubEnded *time.Time + if p != nil { + scrubEnded = LastScrubEnded(*p) + } + if slot := s.Slot(now); now.Sub(slot) > MissedAfter && served[s.Pool] < slot.Unix() && !startedSince(p, slot) { + warn = append(warn, fmt.Sprintf("The scheduled scrub of %s due %s has not started.", s.Pool, slot.Format("2006-01-02 15:04"))) + } if s.TimerState != "" && (s.TimerState != "active" || s.Enabled != "enabled") { warn = append(warn, fmt.Sprintf("The scrub schedule's timer %s is %s and %s, so no scheduled scrub of %s will start.", s.Timer, s.TimerState, s.Enabled, s.Pool)) diff --git a/modules/zfs/cmd/zfs-tools/scrub.go b/modules/zfs/cmd/zfs-tools/scrub.go index 6d455b6d..85e9a3b4 100644 --- a/modules/zfs/cmd/zfs-tools/scrub.go +++ b/modules/zfs/cmd/zfs-tools/scrub.go @@ -144,8 +144,13 @@ func (m *Machine) Scrubs(ctx context.Context, history bool) (any, error) { a.Said = append(a.Said, "The module has no scrub schedule on this machine (no "+ScrubConfig+").") default: a.Schedule = sched - line := fmt.Sprintf("%s scrub of %s, %s at %s, by %s (%s, %s)", sched.Cadence, sched.Pool, sched.Day, sched.Time, + line := fmt.Sprintf("%s scrub of %s, %s at %s (monthly: the first %s of the month), decided by the run %s fires "+ + "every quarter of an hour (%s, %s)", sched.Cadence, sched.Pool, sched.Day, sched.Time, sched.Day, sched.Timer, orUnknown(sched.TimerState), orUnknown(sched.Enabled)) + if sched.Cadence == "weekly" { + line = fmt.Sprintf("weekly scrub of %s, %s at %s, decided by the run %s fires every quarter of an hour (%s, %s)", + sched.Pool, sched.Day, sched.Time, sched.Timer, orUnknown(sched.TimerState), orUnknown(sched.Enabled)) + } if sched.Next != nil { line += "; next scheduled scrub of " + sched.Pool + ": " + sched.Next.Local().Format("2006-01-02 15:04") } diff --git a/modules/zfs/cmd/zfs-tools/scrubrun_test.go b/modules/zfs/cmd/zfs-tools/scrubrun_test.go new file mode 100644 index 00000000..7b402362 --- /dev/null +++ b/modules/zfs/cmd/zfs-tools/scrubrun_test.go @@ -0,0 +1,168 @@ +package main + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +// The scheduled run (`zfs-tools scrub-run`, second review of mesh-catalog #147): the node-engine's timer +// fires it every quarter of an hour, and it decides from the configuration whether a scrub is due. It +// starts one, or waits for one already running, as upstream's zfs-scrub@.service does, and never pauses or +// stops a scrub. + +func monthlyConf(pool string) string { + return "pool=" + pool + "\ncadence=monthly\nday=Mon\ntime=00:00\n" +} + +func TestTheSlotIsTheLastScheduledTimeNotAfterNow(t *testing.T) { + loc := time.Local + at := func(y int, m time.Month, d, h, min int) time.Time { return time.Date(y, m, d, h, min, 0, 0, loc) } + cases := []struct { + conf string + now time.Time + want time.Time + }{ + {"pool=tank\ncadence=weekly\nday=Mon\ntime=00:00\n", at(2026, 10, 9, 12, 0), at(2026, 10, 5, 0, 0)}, + {"pool=tank\ncadence=weekly\nday=Fri\ntime=13:30\n", at(2026, 10, 9, 12, 0), at(2026, 10, 2, 13, 30)}, + {"pool=tank\ncadence=weekly\nday=Fri\ntime=11:30\n", at(2026, 10, 9, 12, 0), at(2026, 10, 9, 11, 30)}, + {monthlyConf("tank"), at(2026, 10, 9, 12, 0), at(2026, 10, 5, 0, 0)}, + {monthlyConf("tank"), at(2026, 10, 4, 23, 0), at(2026, 9, 7, 0, 0)}, + {"pool=tank\ncadence=monthly\nday=Thu\ntime=06:00\n", at(2026, 10, 1, 6, 0), at(2026, 10, 1, 6, 0)}, + } + for _, c := range cases { + s, err := ParseScrubConfig(c.conf) + if err != nil { + t.Fatal(err) + } + if got := s.Slot(c.now); !got.Equal(c.want) { + t.Errorf("%q at %s: slot %s, want %s", c.conf, c.now, got, c.want) + } + if next := s.NextSlot(c.now); !next.After(c.now) || !s.Slot(next).Equal(next) { + t.Errorf("%q at %s: next slot %s", c.conf, c.now, next) + } + } +} + +// root runs the scheduled run: no sudo. +func rootFake(t *testing.T, conf string) (*fake, string) { + f := withZFS(t) + f.uid = 0 + f.files[ScrubConfig] = conf + dir := t.TempDir() + return f, filepath.Join(dir, "scrub-served.json") +} + +func TestADueScrubIsStartedThenWaitedForAndMarkedServed(t *testing.T) { + f, served := rootFake(t, monthlyConf("tank")) + f.on("zpool scrub -- tank", "") + f.on("zpool wait -t scrub -- tank", "") + said, err := f.machine().ScrubRun(context.Background(), served) + if err != nil { + t.Fatal(err) + } + if got := strings.Join(ran(f, "zpool scrub"), "|") + "/" + strings.Join(ran(f, "zpool wait"), "|"); got != "zpool scrub -- tank/zpool wait -t scrub -- tank" { + t.Errorf("ran %q", got) + } + if !strings.Contains(said, "started") { + t.Errorf("said %q", said) + } + // Served: the next fire within the same slot runs nothing. + f.calls = nil + if _, err := f.machine().ScrubRun(context.Background(), served); err != nil { + t.Fatal(err) + } + if n := len(ran(f, "zpool scrub")) + len(ran(f, "zpool wait")); n != 0 { + t.Errorf("a served slot ran %v", f.calls) + } +} + +func TestAScrubInProgressIsWaitedForNeverPausedOrStarted(t *testing.T) { + f, served := rootFake(t, monthlyConf("tank")) + f.on(line("zpool", StatusArgv()), fixture(t, "zpool-status-scrubbing.txt")) + f.on(line("zpool", PoolsArgv()), "tank\t1000\t100\t900\t10\t1\tONLINE\t1.00\n") + f.on("zpool wait -t scrub -- tank", "") + if _, err := f.machine().ScrubRun(context.Background(), served); err != nil { + t.Fatal(err) + } + if strings.Join(f.calls, "|") == "" || len(ran(f, "zpool scrub")) != 0 || len(ran(f, "zpool wait -t scrub -- tank")) != 1 { + t.Errorf("ran %v", f.calls) + } +} + +func TestAStartRefusedBecauseAScrubJustBeganIsWaitedForNotPaused(t *testing.T) { + f, served := rootFake(t, monthlyConf("tank")) + f.answers["zpool scrub -- tank"] = Ran{Code: 1, Err: "cannot scrub tank: currently scrubbing; use 'zpool scrub -s' to cancel the current scrub"} + f.on("zpool wait -t scrub -- tank", "") + if _, err := f.machine().ScrubRun(context.Background(), served); err != nil { + t.Fatal(err) + } + for _, c := range f.calls { + if strings.Contains(c, "scrub -p") || strings.Contains(c, "scrub -s") { + t.Errorf("the run paused or stopped a scrub: %s", c) + } + } + if len(ran(f, "zpool wait -t scrub -- tank")) != 1 { + t.Errorf("not waited for: %v", f.calls) + } +} + +func TestAResilverDefersTheScrubWithoutServingTheSlot(t *testing.T) { + f, served := rootFake(t, monthlyConf("tank")) + f.on(line("zpool", StatusArgv()), fixture(t, "zpool-status-degraded.txt")) + f.on(line("zpool", PoolsArgv()), "tank\t1000\t100\t900\t10\t1\tDEGRADED\t1.00\n") + said, err := f.machine().ScrubRun(context.Background(), served) + if err != nil { + t.Fatal(err) + } + if len(ran(f, "zpool scrub")) != 0 || !strings.Contains(said, "resilver") { + t.Errorf("ran %v, said %q", f.calls, said) + } + if _, err := os.Stat(served); err == nil { + t.Errorf("the slot was marked served while the scrub waits for the resilver") + } +} + +func TestAScrubAlreadyStartedInTheSlotServesItWithoutAStateFile(t *testing.T) { + // tank's scrub started 2026-09-13 00:24 (finished 09-14); a weekly Sunday-00:00 slot of 09-13 is served + // by it, so a run on 09-15 starts nothing. + f, served := rootFake(t, "pool=tank\ncadence=weekly\nday=Sun\ntime=00:00\n") + f.now = time.Date(2026, 9, 15, 12, 0, 0, 0, time.Local) + if _, err := f.machine().ScrubRun(context.Background(), served); err != nil { + t.Fatal(err) + } + if n := len(ran(f, "zpool scrub")) + len(ran(f, "zpool wait")); n != 0 { + t.Errorf("ran %v", f.calls) + } +} + +func TestAnInvalidScheduleRunsNothingAndFails(t *testing.T) { + for _, conf := range []string{ + "pool=tank\ncadence=monthly\nday=Mon\ntime=*:*\n", + "pool=tank\ncadence=monthly\nday=Mon..Fri\ntime=00:00\n", + "pool=tank\ncadence=monthly\nday=*\ntime=00:00\n", + "pool=tank\ncadence=hourly\nday=Mon\ntime=00:00\n", + "pool=tank\ncadence=monthly\nday=Mon\ntime=0:00\n", + } { + f, served := rootFake(t, conf) + if _, err := f.machine().ScrubRun(context.Background(), served); err == nil { + t.Errorf("%q: not refused", conf) + } + if len(ran(f, "zpool scrub"))+len(ran(f, "zpool wait")) != 0 { + t.Errorf("%q ran %v", conf, f.calls) + } + } +} + +func TestAScheduledScrubThatDidNotStartIsAWarning(t *testing.T) { + f := withZFS(t) + f.files[ScrubConfig] = "pool=tank\ncadence=weekly\nday=Mon\ntime=00:00\n" + f.on(line("systemctl", TimerShowArgv(ScrubTimer)), "ActiveState=active\nUnitFileState=enabled\nNextElapseUSecRealtime=@1791842400\nLastTriggerUSec=@1791237600\n") + h := f.machine().Health(context.Background()) + if !strings.Contains(strings.Join(h.Warnings, " "), "The scheduled scrub of tank due 2026-10-05 00:00 has not started") { + t.Errorf("warnings: %v", h.Warnings) + } +} diff --git a/modules/zfs/cmd/zfs-tools/scrubschedule_test.go b/modules/zfs/cmd/zfs-tools/scrubschedule_test.go index 3d7bbce8..eaa8acf6 100644 --- a/modules/zfs/cmd/zfs-tools/scrubschedule_test.go +++ b/modules/zfs/cmd/zfs-tools/scrubschedule_test.go @@ -14,9 +14,9 @@ const scrubConf = "pool=tank\ncadence=weekly\nday=Mon\ntime=00:00\n" func scheduled(t *testing.T) *fake { f := withZFS(t) f.files[ScrubConfig] = scrubConf - f.on(line("systemctl", TimerShowArgv("zfs-mesh-scrub-weekly@tank.timer")), + f.on(line("systemctl", TimerShowArgv(ScrubTimer)), "ActiveState=active\nUnitFileState=enabled\nNextElapseUSecRealtime=@1791842400\nLastTriggerUSec=@1791237600\n") - f.on(line("systemctl", ServiceShowArgv("zfs-mesh-scrub@tank.service")), + f.on(line("systemctl", ServiceShowArgv(ScrubService)), "ActiveState=inactive\nResult=success\nExecMainStartTimestamp=@1791237600\nExecMainExitTimestamp=@1791331200\n") return f } @@ -29,11 +29,11 @@ func TestTheScheduleIsTheConfigAndWhatTheTimerSays(t *testing.T) { } a := got.(ScrubAnswer) s := a.Schedule - if s == nil || s.Pool != "tank" || s.Cadence != "weekly" || s.Timer != "zfs-mesh-scrub-weekly@tank.timer" || s.TimerState != "active" || - s.Enabled != "enabled" || s.Next == nil || !s.Next.Equal(time.Unix(1791842400, 0)) || s.LastResult != "success" { + if s == nil || s.Pool != "tank" || s.Cadence != "weekly" || s.Timer != "zfs-mesh-scrub.timer" || s.TimerState != "active" || + s.Enabled != "enabled" || s.Next == nil || !s.Next.Equal(time.Date(2026, 10, 12, 0, 0, 0, 0, time.Local)) || s.LastResult != "success" { t.Fatalf("schedule: %+v", s) } - if !strings.Contains(strings.Join(a.Said, "\n"), "next scheduled scrub of tank: "+time.Unix(1791842400, 0).Local().Format("2006-01-02 15:04")) { + if want := s.NextSlot(f.now).Format("2006-01-02 15:04"); !strings.Contains(strings.Join(a.Said, "\n"), "next scheduled scrub of tank: "+want) || want != "2026-10-12 00:00" { t.Errorf("said: %v", a.Said) } } @@ -67,12 +67,12 @@ func TestTheScheduleWarnsOfAnOldScrubAFailedRunAndAStoppedTimer(t *testing.T) { if !strings.Contains(w, "Pool tank was last scrubbed 25 days ago, more than its weekly schedule allows (9 days)") { t.Errorf("warnings: %v", h.Warnings) } - f.on(line("systemctl", ServiceShowArgv("zfs-mesh-scrub@tank.service")), + f.on(line("systemctl", ServiceShowArgv(ScrubService)), "ActiveState=failed\nResult=exit-code\nExecMainStartTimestamp=@1791237600\nExecMainExitTimestamp=@1791237601\n") - f.on(line("systemctl", TimerShowArgv("zfs-mesh-scrub-weekly@tank.timer")), + f.on(line("systemctl", TimerShowArgv(ScrubTimer)), "ActiveState=inactive\nUnitFileState=disabled\nNextElapseUSecRealtime=\nLastTriggerUSec=@1791237600\n") w = strings.Join(f.machine().Health(context.Background()).Warnings, " ") - for _, want := range []string{"The last scheduled scrub of tank failed (exit-code)", "The scrub schedule's timer zfs-mesh-scrub-weekly@tank.timer is inactive and disabled"} { + for _, want := range []string{"The last scheduled scrub of tank failed (exit-code)", "The scrub schedule's timer zfs-mesh-scrub.timer is inactive and disabled"} { if !strings.Contains(w, want) { t.Errorf("lacks %q: %s", want, w) } diff --git a/modules/zfs/module.json b/modules/zfs/module.json index fc66eb6f..7b4de50e 100644 --- a/modules/zfs/module.json +++ b/modules/zfs/module.json @@ -12,17 +12,17 @@ "scrub-cadence": { "kind": "preference", "default": "weekly", - "why": "how often the pool named by scrub-pool is scrubbed, weekly or monthly: weekly is what the distribution's timer did on the home server before the module, so taking it over changes nothing; a pool whose scrub takes days may want monthly" + "why": "how often the pool named by scrub-pool is scrubbed, weekly or monthly (the first scrub-day of each month); weekly is what the distribution's timer did on the home server before the module, so taking it over moves nothing until the operator chooses" }, "scrub-day": { "kind": "preference", "default": "Mon", - "why": "the weekday the scrub starts (Mon to Sun); for a monthly scrub, the first such day of the month. Monday is when the distribution's weekly timer fired" + "why": "the weekday the scrub starts, Mon to Sun: for a monthly scrub, the first such day of the month. Checked by the scheduled run, which refuses any other value and runs nothing; Monday is when the distribution's weekly timer fired" }, "scrub-time": { "kind": "preference", "default": "00:00", - "why": "the time the scrub starts, HH:MM in the machine's time zone; midnight is when the distribution's weekly timer fired" + "why": "the time the scrub starts, HH:MM from 00:00 to 23:59 in the machine's time zone; the scheduled run checks it every quarter of an hour, so the scrub starts within 15 minutes of it. Midnight is when the distribution's weekly timer fired" } }, "tools": [ @@ -114,6 +114,13 @@ "grace": "60s" } }, + { + "id": "state-dir", + "type": "directory", + "path": "/var/lib/zfs-tools", + "mode": "0755", + "owner": "${machine:account}" + }, { "id": "cursor-dir", "type": "directory", @@ -147,25 +154,15 @@ "content": "# Written by the mesh (module zfs) from this machine's assignment. Replaced on every push; change the\n# settings scrub-pool, scrub-cadence, scrub-day and scrub-time, never this file. The module's tools read it.\npool=${setting:scrub-pool}\ncadence=${setting:scrub-cadence}\nday=${setting:scrub-day}\ntime=${setting:scrub-time}\n" }, { - "id": "scrub-service", - "type": "file", - "path": "/etc/systemd/system/zfs-mesh-scrub@.service", - "mode": "0644", - "content": "# Written by the mesh (module zfs). Replaced on every push; change the module's scrub settings, never this file.\n# The scrub of one pool, started by the module's timer: zpool with fixed arguments and no shell. Stopping the\n# unit while a scrub runs pauses it, as the distribution's own scrub unit does; a later start resumes it.\n[Unit]\nDescription=zpool scrub of %i (the mesh's zfs module)\nDocumentation=man:zpool-scrub(8)\nRequires=zfs.target\nAfter=zfs.target\nConditionPathIsDirectory=/sys/module/zfs\n\n[Service]\nExecStart=/usr/bin/zpool scrub -w -- %i\nExecStop=-/usr/bin/zpool scrub -p -- %i\n" - }, - { - "id": "scrub-weekly", - "type": "file", - "path": "/etc/systemd/system/zfs-mesh-scrub-weekly@.timer", - "mode": "0644", - "content": "# Written by the mesh (module zfs). Replaced on every push; change the module's scrub settings, never this file.\n# The weekly scrub schedule. The mesh enables it for the pool its scrub-pool setting names, and holds the\n# distribution's own scrub timers for that pool stopped and disabled, so one schedule scrubs the pool.\n[Unit]\nDescription=Weekly zpool scrub of %i (the mesh's zfs module)\nDocumentation=man:zpool-scrub(8)\n\n[Timer]\nOnCalendar=${setting:scrub-day} *-*-* ${setting:scrub-time}:00\nPersistent=true\nUnit=zfs-mesh-scrub@%i.service\n\n[Install]\nWantedBy=timers.target\n" - }, - { - "id": "scrub-monthly", - "type": "file", - "path": "/etc/systemd/system/zfs-mesh-scrub-monthly@.timer", - "mode": "0644", - "content": "# Written by the mesh (module zfs). Replaced on every push; change the module's scrub settings, never this file.\n# The monthly scrub schedule. The mesh enables it for the pool its scrub-pool setting names, and holds the\n# distribution's own scrub timers for that pool stopped and disabled, so one schedule scrubs the pool.\n[Unit]\nDescription=Monthly zpool scrub of %i (the mesh's zfs module)\nDocumentation=man:zpool-scrub(8)\n\n[Timer]\nOnCalendar=${setting:scrub-day} *-*-01..07 ${setting:scrub-time}:00\nPersistent=true\nUnit=zfs-mesh-scrub@%i.service\n\n[Install]\nWantedBy=timers.target\n" + "id": "scrub-run", + "type": "process", + "name": "zfs-mesh-scrub", + "artifact": "tools", + "run": [ + "./zfs-tools", + "scrub-run" + ], + "schedule": "*/15 * * * *" }, { "id": "distribution-weekly", @@ -180,21 +177,6 @@ "unit": "zfs-scrub-monthly@${setting:scrub-pool}.timer", "state": "stopped", "boot": "disabled" - }, - { - "id": "scrub-schedule", - "type": "service", - "unit": "zfs-mesh-scrub-${setting:scrub-cadence}@${setting:scrub-pool}.timer", - "state": "running", - "boot": "enabled", - "restart-on": [ - "scrub-service", - "scrub-weekly", - "scrub-monthly" - ], - "health": { - "kind": "unit" - } } ], "build": {