From 9949cfe5b04770052cbd21f41a9da057722f70c4 Mon Sep 17 00:00:00 2001 From: chick Date: Fri, 18 Sep 2026 01:09:15 +0800 Subject: [PATCH] =?UTF-8?q?fix(install):=20=E5=88=9A=E8=A3=85=E5=AE=8C?= =?UTF-8?q?=E7=9A=84=E6=9C=8D=E5=8A=A1=E4=BC=9A=E5=A4=B1=E8=B4=A5=EF=BC=8C?= =?UTF-8?q?=E5=8D=95=E5=85=83=E8=A6=81=E8=AF=B4=E6=B8=85=E6=A5=9A=E8=80=8C?= =?UTF-8?q?=E4=B8=8D=E6=98=AF=E6=AF=8F=205=20=E7=A7=92=E9=87=8D=E5=90=AF?= =?UTF-8?q?=E4=B8=80=E6=AC=A1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 我在真内核上把安装序列完整走了一遍(容器里补了 useradd/systemctl 的替身,其余全是真的), 暴露出一个只有真跑才看得见的缺陷: 装完 → `install` 启动服务 → 但那时**还没有配对的手机**,而守护进程**故意**拒绝空的设备列表 (一个什么都不能授权的 agent 不该坐在那里显得健康)。于是 `Restart=always` + `RestartSec=5` 变成每 5 秒重启一次,永远,journal 里刷同一行。实测:exit 1,"devices is empty, so no phone could ever be authorised"。 配对发生在安装**之后**(运维在节点上跑 `--pair`,然后在手机上配对),所以这不是异常状态, 而是流程的必经一步。修法两条: - 单元加 `StartLimitIntervalSec=300` / `StartLimitBurst=10`:十次之后 systemd 放弃,单元进入 `failed`——一个运维看得见、能处理的状态,而不是无限重启。 - `install` 的结束语改成说清这一步:服务会退出并停住,下一步是 `--pair`,用 `systemctl status` 看状态。此前它说"installed and running",而那是假的。 同一轮演练确认了此前所有修复在真内核上成立:服务用户建了、配置与日志目录交给它了、配置对 该用户可读、单元来自二进制且内容一致、卸载按序拆干净(用户/目录/单元全无残留)。 Go 全包通过(新增一项守着"单元必须有启动上限,且 install 必须说出下一步")。 --- README.md | 15 ++++++++++ internal/install/msa-agent.service | 11 +++++++ internal/install/service.go | 10 +++++-- internal/install/uninstall_test.go | 48 ++++++++++++++++++++++++++++++ systemd/msa-agent.service | 11 +++++++ 5 files changed, 92 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 1446c9b..8bdc64b 100644 --- a/README.md +++ b/README.md @@ -39,6 +39,20 @@ relay's `agents.json` by hand. Without it, `enroll` prints the entry to add. Removing it again is `sudo msa-agent uninstall`; `--dry-run` shows the plan first, and `--keep-logs` keeps the audit trail. +### After installing, pair + +The daemon refuses to start with no paired devices, on purpose: an agent that could authorise nothing +must not sit there looking healthy. Pairing happens after install, so a freshly installed node's service +will fail and stop — the unit's start limit makes systemd give up rather than restart every five seconds +forever, and `systemctl status msa-agent` shows it as failed rather than flapping. + +```sh +sudo /opt/msa-agent/msa-agent --pair # opens the LAN window; then pair from the app +``` + +Then `systemctl start msa-agent` (or just wait — it is enabled) and check `journalctl -u msa-agent -n 20` +for the policy digest and the device count. + ## Subcommands | Command | What it does | @@ -49,6 +63,7 @@ keeps the audit trail. | `uninstall` | Tells the relay first, then removes the service, unit, config, logs and user | | `version` | Prints the version stamped into the binary | | `--pair` | Opens the temporary LAN window a phone uses to pair; closes itself | +| `--pair-bind` | The address that window binds to (an IP literal; the default is all interfaces) | | `--check` | Validates the config and policy, then exits | | `--once` | Runs a single poll cycle and exits | diff --git a/internal/install/msa-agent.service b/internal/install/msa-agent.service index f13f226..39c401e 100644 --- a/internal/install/msa-agent.service +++ b/internal/install/msa-agent.service @@ -11,6 +11,17 @@ Restart=always # Backing off on our own is not enough: a relay that is down for an hour should not be hammered by # every node in the fleet the moment it comes back. RestartSec=5 +# A freshly installed node has no paired devices, and the daemon refuses to run in that state on +# purpose: an agent that could authorise nothing must not sit there looking healthy. But pairing happens +# *after* install — the operator runs `--pair` on the node and then pairs the phone — so between those +# two moments this unit would restart every five seconds, forever, filling the journal with the same +# line. Measured on a real kernel: exit 1, "devices is empty, so no phone could ever be authorised". +# +# StartLimitIntervalSec/StartLimitBurst make systemd give up instead: ten attempts inside five minutes +# and the unit enters `failed`, which is a state an operator can see and act on. The install command +# prints the next step precisely so that state is expected rather than surprising. +StartLimitIntervalSec=300 +StartLimitBurst=10 TimeoutStopSec=20 KillSignal=SIGTERM diff --git a/internal/install/service.go b/internal/install/service.go index 304ced2..114c82c 100644 --- a/internal/install/service.go +++ b/internal/install/service.go @@ -141,9 +141,13 @@ func Install(opts InstallOptions) error { opts.Log("dry run: nothing was changed") return nil } - opts.Log("msa-agent installed and running") - opts.Log("check it with: journalctl -u %s -n 20", paths.ServiceName) - opts.Log("next: run `msa-agent --pair` once, then pair the phone from the app") + // The service will fail until a phone is paired, and that is expected rather than a fault: the daemon + // refuses an empty device list so it cannot sit there looking healthy while authorising nothing. The + // unit's start limit turns that into a `failed` state instead of a restart loop, and this line is + // what makes the state expected. + opts.Log("msa-agent installed and enabled") + opts.Log("next: run `msa-agent --pair` once and pair the phone; until then the service exits and stops") + opts.Log("check it with: systemctl status %s; journalctl -u %s -n 20", paths.ServiceName, paths.ServiceName) return nil } diff --git a/internal/install/uninstall_test.go b/internal/install/uninstall_test.go index a0a68a0..fde2340 100644 --- a/internal/install/uninstall_test.go +++ b/internal/install/uninstall_test.go @@ -426,3 +426,51 @@ func TestTheRemovalHintIsTheOneItWasGiven(t *testing.T) { t.Fatalf("the uninstall still claims every node is in agents.json:\n%s", output) } } + +// The install ends by starting a service that cannot run yet, and the unit has to say what to do about +// that. A freshly installed node has no paired devices — pairing happens after install — and the daemon +// refuses to run in that state on purpose, because an agent that could authorise nothing must not look +// healthy. With `Restart=always` and `RestartSec=5` and no start limit, that is a restart every five +// seconds until a phone pairs. +// +// Found by rehearsing the install on a real kernel: exit 1, "devices is empty, so no phone could ever be +// authorised", and the journal filling with the same line. +func TestTheUnitGivesUpInsteadOfRestartingForever(t *testing.T) { + unit := string(UnitFile()) + if !strings.Contains(unit, "Restart=always") { + t.Skip("the unit no longer restarts, so there is nothing to bound") + } + if !strings.Contains(unit, "StartLimitIntervalSec=") || !strings.Contains(unit, "StartLimitBurst=") { + t.Fatal("the unit restarts without a start limit: an unpaired node would restart every RestartSec forever") + } + // And the install tells the operator what to do next, so the failed state is expected rather than + // surprising. + var lines []string + root := t.TempDir() + paths := Paths{ + ServiceName: "msa-agent", + UnitPath: filepath.Join(root, "etc/systemd/system/msa-agent.service"), + ConfigPath: filepath.Join(root, "etc/msa-agent/config.json"), + LogDir: filepath.Join(root, "var/log/msa-agent"), + BinaryPath: filepath.Join(root, "opt/msa-agent/msa-agent"), + User: "msa-agent", + } + for _, path := range []string{paths.BinaryPath, paths.ConfigPath} { + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + } + if err := Install(InstallOptions{ + Paths: paths, + Runner: &recorder{failOn: "getent"}, + Log: func(format string, args ...any) { lines = append(lines, fmt.Sprintf(format, args...)) }, + }); err != nil { + t.Fatalf("Install: %v", err) + } + if !strings.Contains(strings.Join(lines, "\n"), "--pair") { + t.Fatalf("the install does not say to pair next:\n%s", strings.Join(lines, "\n")) + } +} diff --git a/systemd/msa-agent.service b/systemd/msa-agent.service index f13f226..39c401e 100644 --- a/systemd/msa-agent.service +++ b/systemd/msa-agent.service @@ -11,6 +11,17 @@ Restart=always # Backing off on our own is not enough: a relay that is down for an hour should not be hammered by # every node in the fleet the moment it comes back. RestartSec=5 +# A freshly installed node has no paired devices, and the daemon refuses to run in that state on +# purpose: an agent that could authorise nothing must not sit there looking healthy. But pairing happens +# *after* install — the operator runs `--pair` on the node and then pairs the phone — so between those +# two moments this unit would restart every five seconds, forever, filling the journal with the same +# line. Measured on a real kernel: exit 1, "devices is empty, so no phone could ever be authorised". +# +# StartLimitIntervalSec/StartLimitBurst make systemd give up instead: ten attempts inside five minutes +# and the unit enters `failed`, which is a state an operator can see and act on. The install command +# prints the next step precisely so that state is expected rather than surprising. +StartLimitIntervalSec=300 +StartLimitBurst=10 TimeoutStopSec=20 KillSignal=SIGTERM