novox/mesh-host
18
0
Fork 0
Code Issues Pull Requests 1 Releases Wiki Activity

Read a one-shot that never ran as stopped, and one at rest as stopped to a stopped declaration (issue 1126)

Browse Source 
The kernel module's boot-time cleanup, linux-modules-cleanup.service (Type=oneshot,
declared stopped and enabled at boot), failed its delivery on ace with 'was asked to
be stopped and is running': ServiceState read every inactive one-shot with
ExecMainStatus=0 as running, and systemd says 0 for one that never ran too. Stopping
an inactive unit changes nothing, so the read back said running again; after a boot
that ran it, the same declaration would fail every apply.

ServiceState now reads ExecMainStartTimestampMonotonic: 0 means never ran, read as
stopped (a package's one-shot declared running is then started, and the hold no
longer counts it as in use). The service apply reads a one-shot at rest as stopped
to a declaration that it be stopped (ServiceAtRest), whatever its last run did.

Quick fix weighed: declare the cleanup running in the catalogue. Thorough chosen:
the reading was wrong for every one-shot, and this is its second fault of the kind
(the firewall's was the first). Unit tests: three arch one-shot tests merged into one
table (7 rows), one apply test (2 rows); 1263 to 1262.
This commit is contained in:
jschoubben
2026-10-28 14:29:16 +01:00
parent 7a2af23764
commit cf738cdb90
4 changed files with 203 additions and 79 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/apply/apply.go
+31 -2
View File
@@ -2002,6 +2002,35 @@ type unitReloader interface {
ReloadUnits(ctx context.Context, run system.Runner) error
}
// restReader is a service manager that can say whether a one-shot has nothing running now
// (novox/hq issue 1126).
type restReader interface {
ServiceAtRest(ctx context.Context, run system.Runner, unit string) (bool, error)
}
// declaredState is a unit's state as a declaration of want reads it. **A one-shot at rest is
// stopped to a declaration that it be stopped** (novox/hq issue 1126): a one-shot that does not
// linger reads "running" once it ran and worked — that is what a declaration that it run asks — but
// it has no process between its runs, so it is also as stopped as it can be, and stopping it changes
// nothing. Read only as "running", a boot-time cleanup declared stopped and enabled failed every
// apply after the boot that ran it: "was asked to be stopped and is running".
func declaredState(ctx context.Context, sys system.System, run Runner, unit, want string) (string, error) {
state, err := sys.ServiceState(ctx, run, unit)
if err != nil || want != "stopped" || state != "running" {
return state, err
}
if r, ok := sys.(restReader); ok {
rest, err := r.ServiceAtRest(ctx, run, unit)
if err != nil {
return "", err
}
if rest {
return "stopped", nil
}
}
return state, nil
}
// serviceSettle is how long the host waits before looking at a unit a second time. A test sets it
// to nothing; on a machine it is the window in which a daemon that refuses its configuration dies.
var serviceSettle = 2 * time.Second
@@ -2154,7 +2183,7 @@ func applyServiceIn(ctx context.Context, sys system.System, r *declaration.Servi
// own stopping from anybody else's (novox/hq ADR 0269).
out.declared = &store.FoundUnit{Unit: r.Unit, State: r.State, Boot: r.Boot}
before, err := sys.ServiceState(ctx, run, r.Unit)
before, err := declaredState(ctx, sys, run, r.Unit, r.State)
if err != nil {
return out, err
}
@@ -2165,7 +2194,7 @@ func applyServiceIn(ctx context.Context, sys system.System, r *declaration.Servi
// Read back. A service manager accepting a command says the transaction was accepted,
// not that the unit is running — one that starts and immediately dies satisfies it.
after, err := sys.ServiceState(ctx, run, r.Unit)
after, err := declaredState(ctx, sys, run, r.Unit, r.State)
if err == nil && r.State == "running" {
after, err = stayedRunning(ctx, sys, run, r.Unit)
}
internal/apply/apply_test.go
+38
View File
@@ -350,6 +350,44 @@ func TestAnUnknownServiceStateIsRefusedNotGuessed(t *testing.T) {
}
}
// A boot-time one-shot a package ships, declared stopped and enabled at boot, is satisfied while
// nothing of it runs: before its first boot run and after it (novox/hq issue 1126). The kernel
// module's linux-modules-cleanup.service failed on ace with "was asked to be stopped and is
// running" for a unit that had never run, and every apply after a reboot would have failed the
// same way for one that had.
func TestABootOneShotDeclaredStoppedIsSatisfiedAtRest(t *testing.T) {
for _, c := range []struct{ name, started, status string }{
{"never ran", "0", "0"},
{"ran at boot", "5123456", "0"},
} {
t.Run(c.name, func(t *testing.T) {
var asked []string
run := func(ctx context.Context, name string, args ...string) (string, error) {
args = oldShape(name, args)
switch args[0] {
case "show":
return "LoadState=loaded\nActiveState=inactive\nType=oneshot\nRemainAfterExit=no\n" +
"ExecMainStatus=" + c.status + "\nExecMainStartTimestampMonotonic=" + c.started + "\n", nil
case "is-enabled":
return "enabled\n", nil
}
asked = append(asked, strings.Join(args, " "))
return "", nil
}
d := parse(t, `{"declaration":1,"resources":[
{"id":"cleanup","type":"service","unit":"linux-modules-cleanup.service","state":"stopped","boot":"enabled"}
]}`)
_, _, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginCarried, run, nil, nil)
if err != nil {
t.Fatalf("a one-shot at rest declared stopped failed: %v", err)
}
if len(asked) > 0 {
t.Errorf("the service manager was asked %q for a unit already as declared", asked)
}
})
}
}
func TestADroppedServiceIsGivenBackTheStateItWasFoundIn(t *testing.T) {
// novox/hq ADR 0118: undeclaring removes what the mesh made, gives back what it changed, and
// leaves what was the machine's. A service resource never installs a unit — so undeclaring it
internal/system/arch.go
+80 -36
View File
@@ -187,38 +187,11 @@ func describeAge(when, now time.Time) string {
// so LoadState is what is read — and it is the thing an interface spanning systemd and OpenRC
// would have had to drop.
func (arch) ServiceState(ctx context.Context, run Runner, unit string) (string, error) {
out, asked := run(ctx, "systemctl", "show",
"--property=LoadState", "--property=ActiveState", "--property=Type",
"--property=RemainAfterExit", "--property=ExecMainStatus", "--", unit)
var load, active, kind, remains, exited string
for _, line := range strings.Split(out, "\n") {
key, value, found := strings.Cut(strings.TrimSpace(line), "=")
if !found {
continue
}
switch key {
case "LoadState":
load = value
case "ActiveState":
active = value
case "Type":
kind = value
case "RemainAfterExit":
remains = value
case "ExecMainStatus":
exited = value
}
u, err := showUnit(ctx, run, unit)
if err != nil {
return "", err
}
switch load {
case "":
// With why, where it said why: an account's manager that could not be reached says so
// here, and nowhere else (novox/hq ADR 0177).
if asked != nil {
return "", fmt.Errorf("the service manager said nothing about %s: %w", unit, asked)
}
return "", fmt.Errorf("the service manager said nothing about %s", unit)
switch u.load {
case "not-found":
return "", fmt.Errorf(
"%s does not exist on this machine. A declaration naming a unit that is not "+
@@ -227,7 +200,7 @@ func (arch) ServiceState(ctx context.Context, run Runner, unit string) (string,
case "masked":
return "", fmt.Errorf("%s is masked, so its state cannot be declared", unit)
case "error", "bad-setting":
return "", fmt.Errorf("%s is installed but its unit file cannot be loaded (%s)", unit, load)
return "", fmt.Errorf("%s is installed but its unit file cannot be loaded (%s)", unit, u.load)
}
// **A one-shot that finished is not stopped.** A unit whose whole job is to apply something
@@ -243,24 +216,95 @@ func (arch) ServiceState(ctx context.Context, run Runner, unit string) (string,
// wrong with.
//
// So for that shape, what "running" means is "it ran, and it worked".
if kind == "oneshot" && remains != "yes" && active == "inactive" {
if exited == "0" || exited == "" {
//
// **And one that never ran is not one that worked** (novox/hq issue 1126). systemd says
// ExecMainStatus=0 for a unit that has not run since the manager loaded it exactly as for one
// that succeeded; only its start timestamp, 0, separates them. Read as "running", a package's
// boot-time one-shot declared stopped could never be satisfied — stopping an inactive unit
// changes nothing, and the read back said running again — and one declared running was never
// started. A timestamp the manager did not say keeps the reading it had before.
if u.oneShotAtRest() {
if u.started == "0" {
return "stopped", nil
}
if u.exited == "0" || u.exited == "" {
return "running", nil
}
return "stopped", nil
}
switch active {
switch u.active {
case "active", "activating", "reloading":
return "running", nil
case "inactive", "failed", "deactivating":
return "stopped", nil
default:
return "", fmt.Errorf(
"the service manager reports %s as %q, which is neither running nor stopped", unit, active)
"the service manager reports %s as %q, which is neither running nor stopped", unit, u.active)
}
}
// ServiceAtRest is whether unit is a one-shot that does not linger and has nothing running now
// (novox/hq issue 1126). Such a unit has no state in which a process is up between its runs, so a
// declaration that it be stopped is satisfied whatever its last run did: there is nothing to stop,
// and asking the manager to stop it changes nothing. A boot-time cleanup a package ships is declared
// this way — enabled at boot, never started by an apply.
func (arch) ServiceAtRest(ctx context.Context, run Runner, unit string) (bool, error) {
u, err := showUnit(ctx, run, unit)
if err != nil {
return false, err
}
return u.oneShotAtRest(), nil
}
// shownUnit is what `systemctl show` says of a unit, as far as its state is read.
type shownUnit struct {
load, active, kind, remains, exited, started string
}
// oneShotAtRest is a one-shot that does not linger and is not running now.
func (u shownUnit) oneShotAtRest() bool {
return u.kind == "oneshot" && u.remains != "yes" && u.active == "inactive"
}
// showUnit reads a unit's state properties. A manager that said nothing is an error, with why
// where it said why: an account's manager that could not be reached says so here, and nowhere
// else (novox/hq ADR 0177).
func showUnit(ctx context.Context, run Runner, unit string) (shownUnit, error) {
out, asked := run(ctx, "systemctl", "show",
"--property=LoadState", "--property=ActiveState", "--property=Type",
"--property=RemainAfterExit", "--property=ExecMainStatus",
"--property=ExecMainStartTimestampMonotonic", "--", unit)
var u shownUnit
for _, line := range strings.Split(out, "\n") {
key, value, found := strings.Cut(strings.TrimSpace(line), "=")
if !found {
continue
}
switch key {
case "LoadState":
u.load = value
case "ActiveState":
u.active = value
case "Type":
u.kind = value
case "RemainAfterExit":
u.remains = value
case "ExecMainStatus":
u.exited = value
case "ExecMainStartTimestampMonotonic":
u.started = value
}
}
if u.load == "" {
if asked != nil {
return u, fmt.Errorf("the service manager said nothing about %s: %w", unit, asked)
}
return u, fmt.Errorf("the service manager said nothing about %s", unit)
}
return u, nil
}
func (arch) SetServiceState(ctx context.Context, run Runner, unit, state string) error {
verb := "start"
if state == "stopped" {
internal/system/arch_test.go
+54 -41
View File
@@ -5,47 +5,60 @@ import (
"testing"
)
// A one-shot unit that did its work and exited is satisfied, not stopped.
// How a one-shot unit's state is read: what it did, not whether a process is up.
//
// **This made the firewall permanently unsatisfiable.** Its unit loads a rule set and exits, so it
// is inactive the instant it succeeds — the host started it, it worked, the host read back
// "stopped" and reported the machine as not doing what it was told. On every apply, for ever, with
// the rules correctly in place the whole time.
func TestAOneShotThatFinishedIsRunning(t *testing.T) {
run := func(_ context.Context, _ string, _ ...string) (string, error) {
return "LoadState=loaded\nActiveState=inactive\nType=oneshot\nRemainAfterExit=no\nExecMainStatus=0\n", nil
}
state, err := arch{}.ServiceState(context.Background(), run, "nftables.service")
if err != nil {
t.Fatalf("a one-shot that succeeded was an error: %v", err)
}
if state != "running" {
t.Fatalf("a one-shot that did its work reads as %q, so it can never be satisfied", state)
}
}
// And one that failed is still stopped, or the host would report success for work that did not
// happen — which is the opposite mistake and the worse one.
func TestAOneShotThatFailedIsStopped(t *testing.T) {
run := func(_ context.Context, _ string, _ ...string) (string, error) {
return "LoadState=loaded\nActiveState=inactive\nType=oneshot\nRemainAfterExit=no\nExecMainStatus=1\n", nil
}
state, err := arch{}.ServiceState(context.Background(), run, "nftables.service")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if state != "stopped" {
t.Fatalf("a one-shot that failed reads as %q, so a broken firewall reports success", state)
}
}
// A unit that lingers deliberately is unaffected: it says so, and its active state is the answer.
func TestAOneShotThatRemainsIsJudgedByItsActiveState(t *testing.T) {
run := func(_ context.Context, _ string, _ ...string) (string, error) {
return "LoadState=loaded\nActiveState=active\nType=oneshot\nRemainAfterExit=yes\nExecMainStatus=0\n", nil
}
state, _ := arch{}.ServiceState(context.Background(), run, "thing.service")
if state != "running" {
t.Fatalf("a lingering one-shot reads as %q", state)
// **A one-shot that did its work and exited is satisfied, not stopped.** Its unit loads a rule set
// and exits, so it is inactive the instant it succeeds; reading that as stopped made the firewall
// permanently unsatisfiable, on every apply, with the rules correctly in place the whole time.
//
// **One that failed is still stopped**, or the host would report success for work that did not
// happen — the opposite mistake and the worse one.
//
// **One that never ran is stopped** (novox/hq issue 1126, the kernel's boot-time cleanup on ace):
// systemd says ExecMainStatus=0 for a unit that never ran exactly as for one that succeeded, and
// only its start timestamp separates them. Read as "running", a package's one-shot declared
// running was never started, and one declared stopped could never be.
//
// **At rest** is whether a one-shot that does not linger has nothing running now: a declaration
// that it be stopped is then satisfied, whatever it last did, because there is nothing to stop.
func TestAOneShotIsReadByWhatItDid(t *testing.T) {
for _, c := range []struct {
name, show, want string
atRest bool
}{
{"finished", "ActiveState=inactive\nType=oneshot\nRemainAfterExit=no\nExecMainStatus=0\n" +
"ExecMainStartTimestampMonotonic=5123456\n", "running", true},
{"finished, no timestamp read", "ActiveState=inactive\nType=oneshot\nRemainAfterExit=no\n" +
"ExecMainStatus=0\n", "running", true},
{"failed", "ActiveState=inactive\nType=oneshot\nRemainAfterExit=no\nExecMainStatus=1\n" +
"ExecMainStartTimestampMonotonic=5123456\n", "stopped", true},
{"never ran", "ActiveState=inactive\nType=oneshot\nRemainAfterExit=no\nExecMainStatus=0\n" +
"ExecMainStartTimestampMonotonic=0\n", "stopped", true},
{"running now", "ActiveState=activating\nType=oneshot\nRemainAfterExit=no\nExecMainStatus=0\n" +
"ExecMainStartTimestampMonotonic=5123456\n", "running", false},
{"lingers", "ActiveState=active\nType=oneshot\nRemainAfterExit=yes\nExecMainStatus=0\n",
"running", false},
{"a daemon, stopped", "ActiveState=inactive\nType=simple\nRemainAfterExit=no\nExecMainStatus=0\n",
"stopped", false},
} {
t.Run(c.name, func(t *testing.T) {
run := func(_ context.Context, _ string, _ ...string) (string, error) {
return "LoadState=loaded\n" + c.show, nil
}
state, err := arch{}.ServiceState(context.Background(), run, "thing.service")
if err != nil {
t.Fatalf("read as an error: %v", err)
}
if state != c.want {
t.Errorf("reads as %q, want %q", state, c.want)
}
rest, err := arch{}.ServiceAtRest(context.Background(), run, "thing.service")
if err != nil {
t.Fatalf("at rest read as an error: %v", err)
}
if rest != c.atRest {
t.Errorf("at rest is %v, want %v", rest, c.atRest)
}
})
}
}