Stop nothing the mesh cannot replace, give the tunnel back on failure, and take it over after enrolment

Review of the ADR 0105 build (hq ADR 0105). The takeover stopped the found
unit and then found out whether the mesh's interface would do; a start that
failed left the machine with no tunnel at all.

Now nothing is stopped until the declared interface listens on the found port
at the found address and the key file it names holds the found key — the
refusal names the remedy — and a mesh interface that fails to start after the
takeover has the found unit started again, with the account saying so. The
account has three states (not taken, taken, down) and is given on every
takeover, failure included. An interface raised by hand is looked at again
for a moment and then refused naming `wg-quick down`. A found unit started
again by hand beside the mesh's is said, not stopped: on the hub it cannot
hold the port, and on a spoke two interfaces with one key would fight.

`mesh-host overlay take --tunnel <iface>` is the path for a node that
enrolled before the mesh knew to take a tunnel over: the found key becomes its
overlay key — identity, sealing and serving keys untouched, so nothing sealed
to the node is remade — and the mesh is told with a rekey signed by the
identity key, over the key left, the key taken and the tunnel. Told first,
written second, so a run again puts right whichever half did not happen.

internal/apply/apply.go

@@ -339,22 +339,32 @@ func ApplyKeeping(
 		// it (novox/hq ADR 0105): its configuration kept, its unit stopped and disabled, never
 		// flushed. A failure here fails the service too — the mesh's interface is not started on a
 		// port the found one still holds.
+		stoppedFound := false
 		if svc, ok := resource.(*declaration.Service); ok && svc.TakesOver != nil {
 			var outcome Outcome
 			var facts TakenTunnel
 			var err error
 			if d.Adoption == nil {
 				err = errNotAdopted
+				facts = TakenTunnel{Interface: svc.TakesOver.Interface, State: NotTaken}
 			} else {
-				outcome, facts, err = takeOver(ctx, sys, svc, d, &known, run, keep, time.Now().UTC())
+				outcome, facts, stoppedFound, err = takeOver(ctx, sys, svc, d, &known, run, keep, time.Now().UTC())
 			}
+			// Always an account, failure included: the last account standing must never be an
+			// older "taken" over a machine whose takeover has since gone wrong.
+			report.Tunnel = &facts
 			if err != nil {
+				report.Tunnel.Note = err.Error()
+				if stoppedFound {
+					// The found unit is down and the mesh's not up: the one state where the
+					// peers reach nothing. Started again, and said.
+					restoreFound(ctx, sys, svc.TakesOver.Unit, run, report.Tunnel)
+				}
 				failures = append(failures, &Error{Resource: svc.Identity(), Err: err, Done: report})
 				log(fmt.Sprintf("  failed %s (%s): %v", svc.Identity(), svc.Unit, err))
 				continue
 			}
 			report.Outcomes = append(report.Outcomes, outcome)
-			report.Tunnel = &facts
 			log(fmt.Sprintf("  held %s (%s): %s", outcome.ID, outcome.Target, outcome.Detail))
 		}
 
@@ -377,6 +387,17 @@ func ApplyKeeping(
 			failed := &Error{Resource: resource.Identity(), Err: err, Done: report}
 			failures = append(failures, failed)
 			log(fmt.Sprintf("  failed %s (%s): %v", resource.Identity(), outcome.Target, err))
+			if svc, ok := resource.(*declaration.Service); ok && svc.TakesOver != nil && report.Tunnel != nil {
+				// The mesh's interface did not come up after the found one was stopped: no
+				// tunnel at all. The found unit is started again — the machine goes back to
+				// what it had — and the account says so (novox/hq ADR 0105).
+				report.Tunnel.Note = "the mesh's interface did not come up: " + err.Error()
+				if stoppedFound {
+					restoreFound(ctx, sys, svc.TakesOver.Unit, run, report.Tunnel)
+				} else {
+					report.Tunnel.State = tunnelState(ctx, sys, svc.TakesOver.Unit, svc.Unit, run)
+				}
+			}
 
 			// **A failed action stops what follows. Nothing else does.**
 			//
@@ -426,8 +447,8 @@ func ApplyKeeping(
 		}
 		if svc, ok := resource.(*declaration.Service); ok && svc.TakesOver != nil && report.Tunnel != nil {
 			// The found interface is down and the mesh's is up in its place: the tunnel changed
-			// hands (novox/hq ADR 0105).
-			report.Tunnel.Taken = true
+			// hands (novox/hq ADR 0105). Read from the machine, not assumed.
+			report.Tunnel.State = tunnelState(ctx, sys, svc.TakesOver.Unit, svc.Unit, run)
 		}
 		report.Outcomes = append(report.Outcomes, outcome)
 		if outcome.Action != "unchanged" {

internal/apply/hold_test.go

@@ -31,6 +31,8 @@ type machine struct {
 
 type fakeUnit struct {
 	active, enabled string
+	// wontStart is a unit that accepts `start` and stays inactive — one that starts and dies.
+	wontStart bool
 	// fragment is where systemd loads the unit from; empty means /etc/systemd/system, where an
 	// administrator installs one.
 	fragment string
@@ -65,7 +67,9 @@ func (m *machine) systemctl(args []string) (string, error) {
 		}
 		return u.enabled + "\n", nil
 	case "start":
-		u.active = "active"
+		if !u.wontStart {
+			u.active = "active"
+		}
 	case "stop":
 		u.active = "inactive"
 	case "enable":

internal/apply/takeover.go

@@ -34,26 +34,53 @@ type TakenTunnel struct {
 	Port      int
 	Range     string
 	Peers     int
-	// Taken is whether the found interface is down and disabled and the mesh's up in its place.
-	Taken bool
+	// State is "not-taken" (the found interface still up, the mesh's not), "taken" (the found one
+	// down and disabled, the mesh's up with its key) or "down" (the found one down and the mesh's
+	// not up: the peers reach nothing). Note is what this apply did about it.
+	State string
+	Note  string
 	Kept  string
 }
 
+// The states, as the link says them.
+const (
+	NotTaken   = "not-taken"
+	Taken      = "taken"
+	TunnelDown = "down"
+)
+
+// takeoverRecheck is how often, and takeoverRechecks how many times, a found interface still up
+// after its unit stopped is looked at again before the takeover is refused: `wg-quick down` by a
+// person takes a moment. Variables so a test need not wait.
+var (
+	takeoverRecheck  = 2 * time.Second
+	takeoverRechecks = 3
+)
+
 // takeOverID is the held record's id for the found configuration: the service's own with a suffix,
 // so it is declared for as long as the service is and never mistaken for the service itself.
 func takeOverID(svc *declaration.Service) string { return svc.ID + ".takes-over" }
 
 // takeOver keeps the found tunnel's configuration and stops its unit, ahead of the service that
 // replaces it. Returned is the hold's outcome, and what was found for the report.
+//
+// **Nothing is stopped until the mesh's interface is known to be able to replace it** (the record's
+// option 2 is exactly this going wrong): the declared configuration must listen on the found port
+// at the found address, and the key file it points at must hold the found key. Only then is the
+// found unit stopped — and `stopped` says whether this apply did, so a mesh interface that then
+// fails to start can have the found unit started again.
 func takeOver(ctx context.Context, sys system.System, svc *declaration.Service, d *declaration.Declaration,
-	known *store.State, run Runner, keep Keep, now time.Time) (Outcome, TakenTunnel, error) {
+	known *store.State, run Runner, keep Keep, now time.Time) (out Outcome, facts TakenTunnel, stopped bool, err error) {
 	t := svc.TakesOver
 	id := takeOverID(svc)
 	module, _ := d.Adoption.UntakenModuleOf(svc.ID)
 	if module == "" {
 		module = "the private network"
 	}
-	facts := TakenTunnel{Interface: t.Interface}
+	facts = TakenTunnel{Interface: t.Interface, State: NotTaken}
+
+	// 0. What the found configuration says, before anything: the checks below are against it.
+	found, ferr := readFoundTunnel(t.Config)
 
 	// 1. The configuration, kept like any held file. A synthetic file resource stands for it, so
 	//    the same code keeps its original, digests it and notices it changing.
@@ -62,65 +89,97 @@ func takeOver(ctx context.Context, sys system.System, svc *declaration.Service,
 	out, held, err := hold(ctx, sys, file, module, was, already,
 		"the configuration of the tunnel "+t.Interface+", taken over by "+svc.Unit, run, keep, now)
 	if err != nil {
-		return begin(file), facts, fmt.Errorf("keeping the found tunnel's configuration: %w", err)
+		return begin(file), facts, false, fmt.Errorf("keeping the found tunnel's configuration: %w", err)
 	}
 	known.RecordHeld(held)
 	facts.Kept = held.Kept
-	// What the file says, for the report: read from the machine, or from the kept original when
-	// the machine's copy is gone. The private key stays in the file; nothing here keeps it.
+	// What the file says, for the report: from the machine, or from the kept original when the
+	// machine's copy is gone. The private key stays in the file; nothing here keeps it.
 	unread := ""
-	raw, err := os.ReadFile(t.Config)
-	if err != nil && held.Kept != "" {
-		raw, err = os.ReadFile(held.Kept)
+	if ferr != nil && held.Kept != "" {
+		found, ferr = readFoundTunnel(held.Kept)
 	}
-	if err == nil {
-		if found, perr := tunnel.Parse(raw); perr == nil {
-			facts.Port, facts.Range, facts.Peers = found.Port, found.Range, len(found.Peers)
-		} else {
-			unread = perr.Error()
-		}
+	if ferr == nil {
+		facts.Port, facts.Range, facts.Peers = found.Port, found.Range, len(found.Peers)
 	} else {
-		unread = err.Error()
+		unread = ferr.Error()
 	}
 
-	// 2. The found unit: stopped if it runs, disabled if it starts at boot. A unit that is not
-	//    there is not an error — the interface may have been raised another way, which the check
-	//    below catches — and neither is one already down.
+	// 2. Where things stand: the found unit, and the mesh's.
+	foundState, unitErr := sys.ServiceState(ctx, run, t.Unit)
+	meshState, _ := sys.ServiceState(ctx, run, svc.Unit)
+	if foundState == "running" && meshState == "running" {
+		// Both up. On the hub this cannot last — the found unit cannot bind the port the mesh's
+		// holds — and on a spoke two interfaces with one key flap between them. Not stopped again
+		// by the mesh: what is found on an adopted node is reported, and the first takeover was
+		// the one act (the PR note says why). Said, so a person sees it.
+		facts.Note = t.Unit + " is running again beside the mesh's interface; not stopped by the mesh — " +
+			"`systemctl stop " + t.Unit + "` on the machine"
+	}
+
+	// 3. Before the found unit is stopped: can the mesh's interface replace it? Its declared
+	//    configuration must listen on the found port at the found address, and the key file it
+	//    points at must hold the found key, or the peers would be dropped the moment it came up.
+	if foundState == "running" && meshState != "running" {
+		if ferr != nil {
+			return out, facts, false, fmt.Errorf("the found tunnel's configuration at %s cannot be read as a "+
+				"tunnel's (%v), so nothing says what the mesh's interface must match; %s is left running",
+				t.Config, ferr, t.Unit)
+		}
+		if err := replaces(d, svc, found); err != nil {
+			return out, facts, false, fmt.Errorf("%w; %s is left running", err, t.Unit)
+		}
+	}
+
+	// 4. The found unit: stopped if it runs and the mesh's does not, disabled if it starts at
+	//    boot. A unit that is not there is not an error — the interface may have been raised
+	//    another way, which the check below catches — and neither is one already down.
 	var did []string
-	state, err := sys.ServiceState(ctx, run, t.Unit)
 	switch {
-	case err != nil:
+	case unitErr != nil:
 		did = append(did, t.Unit+" is not a unit here")
-	case state == "running":
+	case foundState == "running" && meshState != "running":
 		if err := sys.SetServiceState(ctx, run, t.Unit, "stopped"); err != nil {
-			return out, facts, fmt.Errorf("stopping the found %s: %w", t.Unit, err)
+			return out, facts, false, fmt.Errorf("stopping the found %s: %w", t.Unit, err)
 		}
 		after, err := sys.ServiceState(ctx, run, t.Unit)
 		if err != nil {
-			return out, facts, err
+			return out, facts, true, err
 		}
 		if after != "stopped" {
-			return out, facts, fmt.Errorf("%s was asked to stop and is %s", t.Unit, after)
+			return out, facts, true, fmt.Errorf("%s was asked to stop and is %s", t.Unit, after)
 		}
+		stopped = true
 		did = append(did, "stopped "+t.Unit)
 	}
-	if err == nil {
+	if unitErr == nil {
 		if boot, err := sys.ServiceBoot(ctx, run, t.Unit); err == nil && boot == "enabled" {
 			if err := sys.SetServiceBoot(ctx, run, t.Unit, "disabled"); err != nil {
-				return out, facts, fmt.Errorf("disabling the found %s at boot: %w", t.Unit, err)
+				return out, facts, stopped, fmt.Errorf("disabling the found %s at boot: %w", t.Unit, err)
 			}
 			did = append(did, "disabled it at boot")
 		}
 	}
 
-	// 3. The interface is gone. If it is still up, something other than its unit raised it, and
-	//    starting the mesh's on the same port and address would fail or, worse, half work.
-	if up, err := run(ctx, "wg", "show", "interfaces"); err == nil {
-		for _, iface := range strings.Fields(up) {
-			if iface == t.Interface {
-				return out, facts, fmt.Errorf("%s is still up after its unit %s was stopped: something other "+
-					"than that unit raises it, and the mesh's interface cannot take its port and address "+
-					"while it does. Nothing was flushed", t.Interface, t.Unit)
+	// 5. The interface is gone. If it is still up, something other than its unit raised it —
+	//    the predecessor brings its up by hand — and the mesh's interface cannot take its port
+	//    and address while it is. Looked at again for a moment, since a person taking it down
+	//    takes a moment; then refused, naming what to do.
+	if meshState != "running" {
+		for try := 0; ; try++ {
+			if !interfaceUp(ctx, run, t.Interface) {
+				break
+			}
+			if try >= takeoverRechecks {
+				return out, facts, stopped, fmt.Errorf("%s is still up although its unit %s is not running: it was "+
+					"raised by hand, not by its unit, and the mesh's interface cannot take its port and "+
+					"address while it is. On the machine: `wg-quick down %s` — the next reconcile takes it "+
+					"over. Nothing was flushed", t.Interface, t.Unit, t.Interface)
+			}
+			select {
+			case <-ctx.Done():
+				return out, facts, stopped, ctx.Err()
+			case <-time.After(takeoverRecheck):
 			}
 		}
 	}
@@ -140,7 +199,140 @@ func takeOver(ctx context.Context, sys system.System, svc *declaration.Service,
 		// peers was carried, which reads as a tunnel that was not one.
 		out.Detail += "; what it says could not be read as a tunnel's: " + unread
 	}
-	return out, facts, nil
+	return out, facts, stopped, nil
+}
+
+// readFoundTunnel is the found configuration as a tunnel.
+func readFoundTunnel(path string) (tunnel.Found, error) {
+	raw, err := os.ReadFile(path)
+	if err != nil {
+		return tunnel.Found{}, err
+	}
+	return tunnel.Parse(raw)
+}
+
+// interfaceUp is whether a WireGuard interface is up on the machine.
+func interfaceUp(ctx context.Context, run Runner, iface string) bool {
+	up, err := run(ctx, "wg", "show", "interfaces")
+	if err != nil {
+		return false
+	}
+	for _, name := range strings.Fields(up) {
+		if name == iface {
+			return true
+		}
+	}
+	return false
+}
+
+// replaces holds the mesh's declared interface configuration against the found tunnel it is to
+// replace: same port, same address, and a key file holding the found key. The configuration is
+// the file the service restarts on; its `PostUp = wg set %i private-key <path>` names the key.
+func replaces(d *declaration.Declaration, svc *declaration.Service, found tunnel.Found) error {
+	var conf *declaration.File
+	for _, r := range d.Resources {
+		f, ok := r.(*declaration.File)
+		if !ok {
+			continue
+		}
+		for _, id := range svc.RestartOn {
+			if f.ID == id {
+				conf = f
+			}
+		}
+	}
+	if conf == nil {
+		return fmt.Errorf("%s takes over %s and restarts on no declared file, so the interface it would "+
+			"raise cannot be checked against the found one", svc.Unit, found.Interface)
+	}
+	port, address, keyPath := "", "", ""
+	for _, line := range strings.Split(conf.Content, "\n") {
+		key, value, ok := strings.Cut(strings.TrimSpace(line), "=")
+		if !ok {
+			continue
+		}
+		key, value = strings.ToLower(strings.TrimSpace(key)), strings.TrimSpace(value)
+		switch key {
+		case "listenport":
+			port = value
+		case "address":
+			address = strings.TrimSpace(strings.Split(value, ",")[0])
+		case "postup":
+			if _, after, ok := strings.Cut(value, "private-key "); ok {
+				keyPath = strings.Fields(after)[0]
+			}
+		}
+	}
+	var wrong []string
+	if port != fmt.Sprint(found.Port) {
+		wrong = append(wrong, fmt.Sprintf("it listens on port %q and the tunnel on %d", port, found.Port))
+	}
+	if host(address) != host(found.Address) {
+		wrong = append(wrong, fmt.Sprintf("its address is %q and the tunnel's %s", address, found.Address))
+	}
+	switch raw, err := os.ReadFile(keyPath); {
+	case keyPath == "":
+		wrong = append(wrong, "it names no key file")
+	case err != nil:
+		wrong = append(wrong, fmt.Sprintf("its key file %s cannot be read (%v)", keyPath, err))
+	default:
+		public, perr := tunnel.PublicKeyOf(strings.TrimSpace(string(raw)))
+		if perr != nil || public != found.PublicKey {
+			wrong = append(wrong, fmt.Sprintf("the key at %s is not the tunnel's — `mesh-host overlay take "+
+				"--tunnel %s` on this machine takes it, then push again", keyPath, found.Interface))
+		}
+	}
+	if len(wrong) > 0 {
+		return fmt.Errorf("the mesh's interface would not replace the tunnel on %s: %s — the peers would be "+
+			"dropped the moment it came up. Re-place the hub on the tunnel's address and port and push again",
+			found.Interface, strings.Join(wrong, "; "))
+	}
+	return nil
+}
+
+// host is an address without its prefix length.
+func host(address string) string {
+	if i := strings.Index(address, "/"); i >= 0 {
+		return address[:i]
+	}
+	return address
+}
+
+// tunnelState is where the tunnel stands, read from the machine: the found unit or interface up
+// and the mesh's not is not taken; the mesh's up and the found one down is taken; neither up is
+// down — the peers reach nothing.
+func tunnelState(ctx context.Context, sys system.System, foundUnit, meshUnit string, run Runner) string {
+	foundState, _ := sys.ServiceState(ctx, run, foundUnit)
+	meshState, _ := sys.ServiceState(ctx, run, meshUnit)
+	foundUp := foundState == "running" || interfaceUp(ctx, run, strings.TrimPrefix(foundUnit, "wg-quick@"))
+	switch {
+	case meshState == "running" && !foundUp:
+		return Taken
+	case meshState == "running":
+		// Both up: not a takeover that holds, and said as not taken so nobody reads it as one.
+		return NotTaken
+	case foundUp:
+		return NotTaken
+	default:
+		return TunnelDown
+	}
+}
+
+// restoreFound starts the found unit again after the mesh's interface failed to replace it, so the
+// machine has the tunnel it had rather than none, and says so in the account.
+func restoreFound(ctx context.Context, sys system.System, unit string, run Runner, facts *TakenTunnel) {
+	if err := sys.SetServiceState(ctx, run, unit, "running"); err != nil {
+		facts.State = TunnelDown
+		facts.Note += "; " + unit + " could not be started again (" + err.Error() + ") — on the machine: systemctl start " + unit
+		return
+	}
+	if state, err := sys.ServiceState(ctx, run, unit); err != nil || state != "running" {
+		facts.State = TunnelDown
+		facts.Note += "; " + unit + " was started again and is not running — on the machine: systemctl start " + unit
+		return
+	}
+	facts.State = NotTaken
+	facts.Note += "; " + unit + " was started again, so the machine has the tunnel it had"
 }
 
 // takesOver is the service in a declaration that takes over a tunnel, if any: one per node, since

internal/apply/takeover_test.go

@@ -14,53 +14,63 @@ import (
 )
 
 // novox/hq ADR 0105: the host raises the mesh's interface with the found key and peers, stops the
-// found interface without flushing it, and keeps its configuration.
+// found interface without flushing it, and keeps its configuration — and stops nothing until the
+// mesh's interface is known to be able to replace it.
 
-// foundConf is the predecessor's configuration, with a real key made once per run: the key is
-// what the takeover must never print or copy, so it had better be one.
-var foundConf = func() string {
+// foundKey is the predecessor's private key, a real one made once per run: the key is what the
+// takeover must never print or copy, so it had better be one.
+var foundKey = func() string {
 	k, err := ecdh.X25519().GenerateKey(rand.Reader)
 	if err != nil {
 		panic(err)
 	}
-	return "[Interface]\nPrivateKey = " + base64.StdEncoding.EncodeToString(k.Bytes()) + "\n" +
-		"ListenPort = 51900\nAddress = 192.0.2.1/24\n\n[Peer]\nPublicKey = PEER-A=\nAllowedIPs = 192.0.2.2/32\n" +
-		"\n[Peer]\nPublicKey = PEER-B=\nAllowedIPs = 192.0.2.3/32\n"
+	return base64.StdEncoding.EncodeToString(k.Bytes())
 }()
 
+var foundConf = "[Interface]\nPrivateKey = " + foundKey + "\n" +
+	"ListenPort = 51900\nAddress = 192.0.2.1/24\n\n[Peer]\nPublicKey = PEER-A=\nAllowedIPs = 192.0.2.2/32\n" +
+	"\n[Peer]\nPublicKey = PEER-B=\nAllowedIPs = 192.0.2.3/32\n"
+
 // aTakeover is the private network's declaration for an adopted hub whose interface takes over
-// the found tunnel: the mesh's configuration — with the found key set from the node's own key file
-// and the found peers in its list — and the interface's service naming what it replaces.
-func aTakeover(t *testing.T, config, mesh string) *declaration.Declaration {
+// the found tunnel: the mesh's configuration — on the found port and address, its key set from the
+// node's own key file, the found peers in its list — and the interface's service naming what it
+// replaces. Port and address are parameters so a test can declare a wrong one.
+func aTakeover(t *testing.T, config, mesh, keyFile, port, address string) *declaration.Declaration {
 	t.Helper()
 	return adopted(t,
 		`{"taken":[],"untaken":{"mesh-wireguard":["mesh-wireguard.overlay-config","mesh-wireguard.overlay-up"]}}`,
 		`{"id":"mesh-wireguard.overlay-config","type":"file","path":"`+mesh+`","mode":"0600",
-		  "content":"[Interface]\nAddress = 192.0.2.1/32\nListenPort = 51900\nPostUp = wg set %i private-key /var/lib/mesh-host/overlay.key\n\n[Peer]\nPublicKey = PEER-A=\nAllowedIPs = 192.0.2.2/32\n"},
+		  "content":"[Interface]\nAddress = `+address+`/32\nListenPort = `+port+`\nPostUp = wg set %i private-key `+keyFile+`\n\n[Peer]\nPublicKey = PEER-A=\nAllowedIPs = 192.0.2.2/32\n"},
 		 {"id":"mesh-wireguard.overlay-up","type":"service","unit":"wg-quick@mesh0","state":"running","boot":"enabled",
 		  "restart-on":["mesh-wireguard.overlay-config"],
 		  "takes-over":{"interface":"wg0","unit":"wg-quick@wg0","config":"`+config+`"}}`)
 }
 
-// aHubInUse is a machine with the predecessor's tunnel up and the mesh's not yet.
-func aHubInUse(t *testing.T) (dir, config, mesh string, m *machine) {
+// aHubInUse is a machine with the predecessor's tunnel up and the mesh's not yet: the found
+// configuration on disk, and the node's key file holding the found key, as enrolment left it.
+func aHubInUse(t *testing.T) (dir, config, mesh, keyFile string, m *machine) {
 	t.Helper()
 	dir = t.TempDir()
 	config = filepath.Join(dir, "wg0.conf")
 	mesh = filepath.Join(dir, "mesh0.conf")
+	keyFile = filepath.Join(dir, "overlay.key")
 	if err := os.WriteFile(config, []byte(foundConf), 0o600); err != nil {
 		t.Fatal(err)
 	}
+	if err := os.WriteFile(keyFile, []byte(foundKey+"\n"), 0o600); err != nil {
+		t.Fatal(err)
+	}
 	m = &machine{containers: map[string]*fakeContainer{}, units: map[string]*fakeUnit{
 		"wg-quick@wg0":   {active: "active", enabled: "enabled"},
 		"wg-quick@mesh0": {active: "inactive", enabled: "disabled", fragment: "/usr/lib/systemd/system/wg-quick@.service"},
 	}}
-	return dir, config, mesh, m
+	takeoverRecheck = 0
+	return dir, config, mesh, keyFile, m
 }
 
 func TestTheFoundTunnelIsStoppedNeverFlushedAndItsConfigurationKept(t *testing.T) {
-	dir, config, mesh, m := aHubInUse(t)
-	report, state := applyAdopted(t, aTakeover(t, config, mesh), store.State{}, m, dir)
+	dir, config, mesh, keyFile, m := aHubInUse(t)
+	report, state := applyAdopted(t, aTakeover(t, config, mesh, keyFile, "51900", "192.0.2.1"), store.State{}, m, dir)
 
 	// The found interface: its unit stopped and disabled, and nothing else done to it.
 	if u := m.units["wg-quick@wg0"]; u.active != "inactive" || u.enabled != "disabled" {
@@ -93,16 +103,18 @@ func TestTheFoundTunnelIsStoppedNeverFlushedAndItsConfigurationKept(t *testing.T
 		t.Fatalf("the mesh's configuration does not carry the found peer, or carries a key:\n%s", got)
 	}
 	// And the report says so, with what was found — port, range, peers — and never the key.
-	if report.Tunnel == nil || !report.Tunnel.Taken || report.Tunnel.Port != 51900 ||
+	if report.Tunnel == nil || report.Tunnel.State != Taken || report.Tunnel.Port != 51900 ||
 		report.Tunnel.Range != "192.0.2.0/24" || report.Tunnel.Peers != 2 || report.Tunnel.Kept != held.Kept {
 		t.Fatalf("the report does not say what was carried: %+v", report.Tunnel)
 	}
-	private := strings.TrimSpace(strings.SplitN(strings.SplitN(foundConf, "PrivateKey = ", 2)[1], "\n", 2)[0])
 	for _, o := range report.Outcomes {
-		if strings.Contains(o.Detail, private) {
+		if strings.Contains(o.Detail, foundKey) {
 			t.Errorf("the found key was printed in an outcome: %+v", o)
 		}
 	}
+	if strings.Contains(report.Tunnel.Note, foundKey) {
+		t.Error("the found key was printed in the account")
+	}
 	if o := outcomeOf(report, "mesh-wireguard.overlay-up.takes-over"); o.Action != "held" ||
 		!strings.Contains(o.Detail, "stopped wg-quick@wg0") || !strings.Contains(o.Detail, "never flushed") {
 		t.Errorf("the takeover was not reported as a hold that stopped the found unit: %+v", o)
@@ -112,9 +124,67 @@ func TestTheFoundTunnelIsStoppedNeverFlushedAndItsConfigurationKept(t *testing.T
 	}
 }
 
-func TestATakeoverIsSteadyAndTheFoundUnitStaysDown(t *testing.T) {
-	dir, config, mesh, m := aHubInUse(t)
-	d := aTakeover(t, config, mesh)
+func TestNothingIsStoppedUntilTheMeshsInterfaceCanReplaceTheFoundOne(t *testing.T) {
+	dir, config, mesh, keyFile, m := aHubInUse(t)
+	otherKey := filepath.Join(dir, "other.key")
+	k, _ := ecdh.X25519().GenerateKey(rand.Reader)
+	if err := os.WriteFile(otherKey, []byte(base64.StdEncoding.EncodeToString(k.Bytes())+"\n"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	cases := map[string]*declaration.Declaration{
+		"another port":    aTakeover(t, config, mesh, keyFile, "51821", "192.0.2.1"),
+		"another address": aTakeover(t, config, mesh, keyFile, "51900", "10.42.0.1"),
+		"another key":     aTakeover(t, config, mesh, otherKey, "51900", "192.0.2.1"),
+		"no key file":     aTakeover(t, config, mesh, filepath.Join(dir, "missing.key"), "51900", "192.0.2.1"),
+	}
+	for name, d := range cases {
+		m.asked = nil
+		report, state, err := ApplyKeeping(t.Context(), archHost(t), d, store.State{},
+			store.OriginDeclared, m.run, nil, nil, KeepIn(dir))
+		if err == nil || !strings.Contains(err.Error(), "would not replace the tunnel") {
+			t.Fatalf("%s: the takeover was not refused: %v", name, err)
+		}
+		if name == "another key" && !strings.Contains(err.Error(), "overlay take") {
+			t.Errorf("%s: the refusal does not name the remedy: %v", name, err)
+		}
+		if m.units["wg-quick@wg0"].active != "active" || m.did("systemctl stop wg-quick@wg0") {
+			t.Fatalf("%s: the found unit was stopped although the mesh's interface could not replace it", name)
+		}
+		if m.units["wg-quick@mesh0"].active == "active" {
+			t.Fatalf("%s: the mesh's interface was started on top of the found one", name)
+		}
+		if report.Tunnel == nil || report.Tunnel.State != NotTaken || !strings.Contains(report.Tunnel.Note, "would not replace") {
+			t.Fatalf("%s: the account does not say the tunnel is not taken and why: %+v", name, report.Tunnel)
+		}
+		if _, held := state.HeldAt("mesh-wireguard.overlay-up.takes-over"); !held {
+			t.Errorf("%s: the found configuration was not kept before the refusal", name)
+		}
+	}
+}
+
+func TestAMeshInterfaceThatFailsToStartGivesTheFoundOneBack(t *testing.T) {
+	dir, config, mesh, keyFile, m := aHubInUse(t)
+	m.units["wg-quick@mesh0"].wontStart = true
+	report, _, err := ApplyKeeping(t.Context(), archHost(t), aTakeover(t, config, mesh, keyFile, "51900", "192.0.2.1"),
+		store.State{}, store.OriginDeclared, m.run, nil, nil, KeepIn(dir))
+	if err == nil {
+		t.Fatal("a mesh interface that did not come up was reported as applied")
+	}
+	if !m.did("systemctl stop wg-quick@wg0") || !m.did("systemctl start wg-quick@wg0") {
+		t.Fatalf("the found unit was not stopped and then started again: %v", m.asked)
+	}
+	if m.units["wg-quick@wg0"].active != "active" {
+		t.Fatal("the machine was left with no tunnel at all")
+	}
+	if report.Tunnel == nil || report.Tunnel.State != NotTaken ||
+		!strings.Contains(report.Tunnel.Note, "did not come up") || !strings.Contains(report.Tunnel.Note, "started again") {
+		t.Fatalf("the account does not say the mesh's interface failed and the found one was given back: %+v", report.Tunnel)
+	}
+}
+
+func TestATakeoverIsSteadyAndAFoundUnitUpAgainIsSaidNotStopped(t *testing.T) {
+	dir, config, mesh, keyFile, m := aHubInUse(t)
+	d := aTakeover(t, config, mesh, keyFile, "51900", "192.0.2.1")
 	_, state := applyAdopted(t, d, store.State{}, m, dir)
 	m.asked = nil
 
@@ -128,28 +198,49 @@ func TestATakeoverIsSteadyAndTheFoundUnitStaysDown(t *testing.T) {
 	if m.did("systemctl stop wg-quick@wg0") {
 		t.Error("a found unit already down was stopped again")
 	}
+	if report.Tunnel == nil || report.Tunnel.State != Taken {
+		t.Errorf("a steady takeover does not read as taken: %+v", report.Tunnel)
+	}
 
-	// Somebody starts the found unit again: it would take the port back, so it is stopped again
-	// — the one thing on an adopted node the mesh undoes, because the tunnel is the mesh's now.
+	// Somebody starts the found unit again beside the mesh's interface. Not stopped by the mesh —
+	// on the hub it cannot hold the port, on a spoke stopping it would be a fight — but said.
 	m.units["wg-quick@wg0"].active = "active"
 	m.asked = nil
-	_, _ = applyAdopted(t, d, again, m, dir)
-	if m.units["wg-quick@wg0"].active != "inactive" || !m.did("systemctl stop wg-quick@wg0") {
-		t.Error("a found unit started again was left holding the mesh's port")
+	report, _ = applyAdopted(t, d, again, m, dir)
+	if m.did("systemctl stop wg-quick@wg0") {
+		t.Error("a found unit started again by hand was stopped by the mesh")
+	}
+	if report.Tunnel == nil || report.Tunnel.State != NotTaken || !strings.Contains(report.Tunnel.Note, "running again beside") {
+		t.Errorf("the account does not say the found unit is up again: %+v", report.Tunnel)
 	}
 }
 
-func TestAFoundInterfaceStillUpAfterItsUnitStoppedRefusesTheTakeover(t *testing.T) {
-	dir, config, mesh, m := aHubInUse(t)
+func TestAFoundInterfaceRaisedByHandIsRefusedNamingTheRemedy(t *testing.T) {
+	dir, config, mesh, keyFile, m := aHubInUse(t)
+	// The unit is not running, yet the interface is up: the predecessor raised it by hand.
+	m.units["wg-quick@wg0"].active = "inactive"
 	m.wgUp = "wg0 mesh0\n"
-	_, state, err := ApplyKeeping(t.Context(), archHost(t), aTakeover(t, config, mesh), store.State{},
-		store.OriginDeclared, m.run, nil, nil, KeepIn(dir))
-	if err == nil || !strings.Contains(err.Error(), "still up") || !strings.Contains(err.Error(), "Nothing was flushed") {
-		t.Fatalf("an interface something else raises was taken over anyway: %v", err)
+	report, state, err := ApplyKeeping(t.Context(), archHost(t), aTakeover(t, config, mesh, keyFile, "51900", "192.0.2.1"),
+		store.State{}, store.OriginDeclared, m.run, nil, nil, KeepIn(dir))
+	if err == nil || !strings.Contains(err.Error(), "wg-quick down wg0") || !strings.Contains(err.Error(), "Nothing was flushed") {
+		t.Fatalf("an interface raised by hand was not refused naming the remedy: %v", err)
 	}
 	if m.units["wg-quick@mesh0"].active == "active" {
 		t.Error("the mesh's interface was started on a port the found one still holds")
 	}
+	// Looked at more than once before giving up: a person taking it down takes a moment.
+	shows := 0
+	for _, a := range m.asked {
+		if a == "wg show interfaces" {
+			shows++
+		}
+	}
+	if shows < takeoverRechecks+1 {
+		t.Errorf("the interface was looked at %d time(s) before the refusal; a person needs a moment", shows)
+	}
+	if report.Tunnel == nil || report.Tunnel.State != NotTaken {
+		t.Errorf("the account does not say the tunnel is not taken: %+v", report.Tunnel)
+	}
 	if _, held := state.HeldAt("mesh-wireguard.overlay-up.takes-over"); !held {
 		t.Error("the found configuration was not kept before the refusal")
 	}