Merge pull request 'A one-shot at rest satisfies a stopped declaration, so the kernel's boot cleanup applies (issue 1126)' (#162) from fix/ace-tun-cleanup into main
This commit was merged in pull request #162.
This commit is contained in:
+31
-2
@@ -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)
|
||||
}
|
||||
|
||||
@@ -350,6 +350,42 @@ 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, whatever its last run did (novox/hq issue 1126). The kernel module's
|
||||
// linux-modules-cleanup.service failed on ace with "was asked to be stopped and is running": an
|
||||
// inactive one-shot with ExecMainStatus=0 reads "running", and stopping it changes nothing.
|
||||
func TestABootOneShotDeclaredStoppedIsSatisfiedAtRest(t *testing.T) {
|
||||
for _, c := range []struct{ name, status string }{
|
||||
{"exited 0: never ran, ran at boot, or unloaded", "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 + "\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
|
||||
|
||||
+73
-36
@@ -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,88 @@ 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 == "" {
|
||||
//
|
||||
// It cannot be told from one that never ran: systemd says ExecMainStatus=0 and a start timestamp
|
||||
// of 0 for a one-shot that never ran, and also for one that ran and was unloaded because nothing
|
||||
// referenced it, then loaded again. So a one-shot declared running that must be seen to have run
|
||||
// lingers (RemainAfterExit) or is enabled at boot; one declared stopped is judged at rest by the
|
||||
// apply (ServiceAtRest, novox/hq issue 1126).
|
||||
if u.oneShotAtRest() {
|
||||
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 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", "--", 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
|
||||
}
|
||||
}
|
||||
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" {
|
||||
|
||||
@@ -5,47 +5,51 @@ 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.
|
||||
//
|
||||
// **At rest** (novox/hq issue 1126, the kernel's boot-time cleanup on ace) 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",
|
||||
"running", true},
|
||||
{"failed", "ActiveState=inactive\nType=oneshot\nRemainAfterExit=no\nExecMainStatus=1\n",
|
||||
"stopped", true},
|
||||
{"running now", "ActiveState=activating\nType=oneshot\nRemainAfterExit=no\nExecMainStatus=0\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)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user