Skip to content

Commit 2063d1b

Browse files
committed
refactor(webconfig): collapse update UI to one system-update path
Webconfig, feed, and mlat now ship and update together through the runtime overlay, so the per-step apt-only update is redundant with the orchestrator's own apt stage. Remove that path end-to-end — the /api/system-upgrade endpoint, its sudoers grant and helper script, the log slug, and the per-step buttons — leaving the Updates card with just Update System and Update System log.
1 parent e399fcf commit 2063d1b

16 files changed

Lines changed: 62 additions & 293 deletions

File tree

.claude/CLAUDE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ web/
5757
files/ rootfs payload installed by install.sh (tarred at release time)
5858
etc/sudoers.d/ 010 (base privilege)
5959
etc/systemd/system/ airplanes-webconfig.service + reset oneshot
60-
usr/local/lib/airplanes-webconfig/ reset, system-upgrade.sh
60+
usr/local/lib/airplanes-webconfig/ reset, identity-export.sh, identity-import.sh
6161
usr/local/lib/airplanes/ wifi-validators.sh, wifi-keyfile.sh (sourced by apl-wifi + airplanes-first-run)
6262
usr/local/bin/apl-wifi privileged Wi-Fi management helper
6363
install.sh build-mode entrypoint (image build only)

files/etc/sudoers.d/010_airplanes-webconfig

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,15 +34,14 @@ airplanes-webconfig ALL=(root) NOPASSWD: /usr/local/bin/apl-wifi status --json
3434
airplanes-webconfig ALL=(root) NOPASSWD: /usr/bin/systemctl reboot
3535
airplanes-webconfig ALL=(root) NOPASSWD: /usr/bin/systemctl poweroff
3636
airplanes-webconfig ALL=(root) NOPASSWD: /usr/bin/systemctl start --no-block airplanes-claim.service
37-
airplanes-webconfig ALL=(root) NOPASSWD: /usr/bin/systemd-run --unit=airplanes-system-upgrade --collect /usr/local/lib/airplanes-webconfig/system-upgrade.sh
3837

39-
# Unified update orchestrator (apt → feed → webconfig → runtime overlay).
38+
# Unified update orchestrator (apt → runtime overlay).
4039
# The trampoline at /usr/local/lib/airplanes-webconfig/start-orchestrator.sh
4140
# is shipped by airplanes-live/image (pi-gen stage 06d); it execs into the
4241
# orchestrator binary inside the active runtime release. --collect drops
4342
# the transient unit record on exit so a repeat invocation doesn't 409 on
4443
# a stale unit name. ExecStopPost HUPs this service so the schema cache
45-
# reloads after the orchestrator's feed leg may have rewritten
44+
# reloads after the orchestrator's overlay leg may have rewritten
4645
# /etc/airplanes/*.
4746
airplanes-webconfig ALL=(root) NOPASSWD: /usr/bin/systemd-run --unit=airplanes-update-orchestrator.service --collect --property=ExecStopPost=/usr/bin/systemctl kill -s HUP airplanes-webconfig.service /usr/local/lib/airplanes-webconfig/start-orchestrator.sh
4847

files/usr/local/lib/airplanes-webconfig/system-upgrade.sh

Lines changed: 0 additions & 60 deletions
This file was deleted.

internal/devfakes/devfakes_test.go

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -444,18 +444,17 @@ func TestSystemctl_IsActiveMixedUnits(t *testing.T) {
444444
runner := Runner(s, priv)
445445
// maintenanceUnitActive calls all maintenance units in one
446446
// is-active. They must be inactive so the maintenance guard lets
447-
// reboot/system-upgrade through.
447+
// reboot / Update System through.
448448
res, err := runner(context.Background(), []string{
449449
"/usr/bin/systemctl", "is-active",
450-
"airplanes-system-upgrade.service",
451450
"airplanes-update-orchestrator.service",
452451
})
453452
if err != nil {
454453
t.Fatalf("runner: %v", err)
455454
}
456455
lines := strings.Split(strings.TrimRight(string(res.Stdout), "\n"), "\n")
457-
if len(lines) != 2 {
458-
t.Fatalf("expected 2 state lines, got %d (out=%q)", len(lines), res.Stdout)
456+
if len(lines) != 1 {
457+
t.Fatalf("expected 1 state line, got %d (out=%q)", len(lines), res.Stdout)
459458
}
460459
for _, l := range lines {
461460
if l != "inactive" {
@@ -474,25 +473,24 @@ func TestSystemdRun_PinsMaintenanceUnitActivating(t *testing.T) {
474473
s := mustNewState(t)
475474
priv := StubPrivilegedArgv()
476475
runner := Runner(s, priv)
477-
// Fire the system-upgrade transient — the maintenance unit must flip
476+
// Fire the orchestrator transient — the maintenance unit must flip
478477
// to `activating` so a follow-up handlers.maintenanceUnitActive guard
479478
// returns the unit name and the second click sees 409.
480-
if _, err := runner(context.Background(), priv.StartSystemUpgrade); err != nil {
479+
if _, err := runner(context.Background(), priv.StartOrchestrator); err != nil {
481480
t.Fatalf("runner: %v", err)
482481
}
483-
if got := s.ServiceState("airplanes-system-upgrade.service"); got != "activating" {
484-
t.Fatalf("after StartSystemUpgrade: airplanes-system-upgrade.service=%q want activating", got)
482+
if got := s.ServiceState("airplanes-update-orchestrator.service"); got != "activating" {
483+
t.Fatalf("after StartOrchestrator: airplanes-update-orchestrator.service=%q want activating", got)
485484
}
486-
// Fan-out is-active over the maintenance units; the system-upgrade
487-
// one must come back activating, the others inactive.
485+
// Fan-out is-active over the maintenance units; the orchestrator
486+
// must come back activating.
488487
res, _ := runner(context.Background(), []string{
489488
"/usr/bin/systemctl", "is-active",
490-
"airplanes-system-upgrade.service",
491489
"airplanes-update-orchestrator.service",
492490
})
493491
lines := strings.Split(strings.TrimRight(string(res.Stdout), "\n"), "\n")
494-
if len(lines) != 2 || lines[0] != "activating" || lines[1] != "inactive" {
495-
t.Fatalf("is-active fan-out=%v want [activating inactive]", lines)
492+
if len(lines) != 1 || lines[0] != "activating" {
493+
t.Fatalf("is-active fan-out=%v want [activating]", lines)
496494
}
497495
}
498496

internal/devfakes/runners.go

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,6 @@ func StubPrivilegedArgv() server.PrivilegedArgv {
7171
SchemaFeed: []string{"dev-stub", "apl-feed", "schema"},
7272
Reboot: []string{"dev-stub", "systemctl", "reboot"},
7373
Poweroff: []string{"dev-stub", "systemctl", "poweroff"},
74-
StartSystemUpgrade: []string{"dev-stub", "systemd-run", "airplanes-system-upgrade"},
7574
StartOrchestrator: []string{"dev-stub", "systemd-run", "airplanes-update-orchestrator"},
7675
RegisterClaim: []string{"dev-stub", "systemctl", "claim-register"},
7776
WifiList: []string{"dev-stub", "apl-wifi", "list"},
@@ -98,7 +97,7 @@ func StubPrivilegedArgv() server.PrivilegedArgv {
9897
// The fake returns "0" so the dashboard never renders an exit-status
9998
// warning over the simulated feed.
10099
// - dev-stub systemctl {reboot,poweroff} — log the intent and exit 0.
101-
// - dev-stub systemd-run airplanes-system-upgrade
100+
// - dev-stub systemd-run airplanes-update-orchestrator
102101
// — log the intent and exit 0. The HTTP handler writes 202.
103102
// - dev-stub systemctl claim-register — calls state.RegisterClaim()
104103
// so the next GET /api/identity reports claim_secret_present=true.
@@ -644,12 +643,8 @@ var unitLogLines = map[string][]string{
644643
"webconfig: dev-mode active",
645644
"webconfig: /api/status served in 1.2ms",
646645
},
647-
"airplanes-system-upgrade.service": {
648-
"system-upgrade: apt-get update (simulated)",
649-
"system-upgrade: 0 packages to upgrade",
650-
},
651646
"airplanes-update-orchestrator.service": {
652-
"update-orchestrator: sequencing apt -> feed -> webconfig -> runtime",
647+
"update-orchestrator: sequencing apt -> runtime",
653648
"update-orchestrator: idle",
654649
},
655650
}

internal/devfakes/state.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -164,10 +164,9 @@ func NewState(p Paths) *State {
164164
"airplanes-978.service": "active",
165165
"lighttpd.service": "active",
166166
"airplanes-webconfig.service": "active",
167-
// Maintenance units — must be inactive so the pre-flight
167+
// Maintenance unit — must be inactive so the pre-flight
168168
// guard at handlers.go:maintenanceUnitActive lets the user click
169-
// reboot / system-upgrade / update-system without a 409.
170-
"airplanes-system-upgrade.service": "inactive",
169+
// reboot / Update System without a 409.
171170
"airplanes-update-orchestrator.service": "inactive",
172171
// Claim unit — never reported active; the SPA pulls progress from
173172
// the claim SSE log instead.

internal/logs/logs.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ var Whitelist = map[string]string{
3333
"uat": "airplanes-978.service",
3434
"claim": "airplanes-claim.service",
3535
"webconfig": "airplanes-webconfig.service",
36-
"system-upgrade": "airplanes-system-upgrade.service",
3736
"update-orchestrator": "airplanes-update-orchestrator.service",
3837
}
3938

internal/logs/logs_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ func TestResolve_Whitelist(t *testing.T) {
2525
"uat": "airplanes-978.service",
2626
"claim": "airplanes-claim.service",
2727
"webconfig": "airplanes-webconfig.service",
28-
"system-upgrade": "airplanes-system-upgrade.service",
2928
"update-orchestrator": "airplanes-update-orchestrator.service",
3029
}
3130
for slug, want := range cases {

internal/server/handlers.go

Lines changed: 3 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -721,12 +721,10 @@ func (s *Server) runSudo(ctx context.Context, argv []string, timeout time.Durati
721721
}
722722

723723
// maintenanceUnits is the set of transient maintenance units that must not
724-
// overlap each other or be interrupted by a reboot. Each touches apt/dpkg,
725-
// release artefacts, or shells out to the others, and would deadlock on
726-
// the dpkg lock or leave half-configured state if any overlapped with
727-
// another or with a shutdown.
724+
// be interrupted by a reboot. The orchestrator touches apt/dpkg and release
725+
// artefacts, and would deadlock on the dpkg lock or leave half-configured
726+
// state if it overlapped with a shutdown.
728727
var maintenanceUnits = []string{
729-
"airplanes-system-upgrade.service",
730728
"airplanes-update-orchestrator.service",
731729
}
732730

@@ -752,51 +750,6 @@ func (s *Server) maintenanceUnitActive(ctx context.Context) string {
752750
return ""
753751
}
754752

755-
// startTransientUnit kicks off a transient systemd unit via the supplied
756-
// pinned argv (sudo systemd-run ...). It refuses with 409 if any maintenance
757-
// unit is already busy, and maps systemd-run's "already exists" stderr to a
758-
// 409 as well. On success it writes 202 + the unit name.
759-
//
760-
// The is-active guard and the systemd-run call are serialized via
761-
// maintenanceMu so two concurrent POSTs can't both observe an idle state and
762-
// then both kick off — by the time the second contender acquires the lock,
763-
// the first's transient unit is already registered and is-active reports it
764-
// as activating.
765-
func (s *Server) startTransientUnit(w http.ResponseWriter, r *http.Request, argv []string, unit, label string) {
766-
s.maintenanceMu.Lock()
767-
defer s.maintenanceMu.Unlock()
768-
if busy := s.maintenanceUnitActive(r.Context()); busy != "" {
769-
writeJSONError(w, http.StatusConflict, label+" refused: "+busy+" is in progress")
770-
return
771-
}
772-
cctx, cancel := context.WithTimeout(r.Context(), systemctlTimeout)
773-
defer cancel()
774-
res, err := s.runner(cctx, argv)
775-
if err != nil {
776-
stderr := strings.TrimSpace(string(res.Stderr))
777-
log.Printf("%s: %v stderr=%q", label, err, stderr)
778-
if strings.Contains(stderr, "already exists") || strings.Contains(stderr, "already running") {
779-
writeJSONError(w, http.StatusConflict, label+" is already in progress")
780-
return
781-
}
782-
writeJSONError(w, http.StatusInternalServerError, label+" start failed")
783-
return
784-
}
785-
writeJSON(w, http.StatusAccepted, map[string]string{
786-
"status": "running",
787-
"unit": unit,
788-
"started_at": time.Now().UTC().Format(time.RFC3339),
789-
})
790-
}
791-
792-
// /api/system-upgrade (POST): kicks off a transient
793-
// airplanes-system-upgrade.service that runs apt-get update + upgrade.
794-
// Returns 202 + the unit name so the SPA can stream
795-
// /api/log/system-upgrade for live output.
796-
func (s *Server) handleSystemUpgrade(w http.ResponseWriter, r *http.Request) {
797-
s.startTransientUnit(w, r, s.priv.StartSystemUpgrade, "airplanes-system-upgrade.service", "system upgrade")
798-
}
799-
800753
// /api/reboot (POST): refuses with 409 if a maintenance unit is active
801754
// (rebooting mid-dpkg would brick the device). Otherwise writes 202 + flushes,
802755
// then triggers reboot from a goroutine after a brief delay so the response

internal/server/orchestrator.go

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,8 @@ func orchestratorRunning(path string) (bool, error) {
134134
// SPA polls /api/orchestrator/state for progress and a terminal step.
135135
//
136136
// The wire envelope on success is `{"status":"running","unit":...,
137-
// "started_at":...}` — identical to /api/system-upgrade so the SPA's
138-
// transient-unit response shape is uniform across update flows.
137+
// "started_at":...}`, which the SPA polls against after navigating to
138+
// the progress view.
139139
func (s *Server) handleOrchestratorStart(w http.ResponseWriter, r *http.Request) {
140140
if !s.orchestratorCapableFunc() {
141141
writeJSON(w, http.StatusServiceUnavailable, map[string]string{"reason": "orchestrator_unavailable"})
@@ -196,12 +196,10 @@ func (s *Server) handleOrchestratorStart(w http.ResponseWriter, r *http.Request)
196196

197197
// handleOrchestratorState is GET /api/orchestrator/state.
198198
//
199-
// The endpoint is informational — the UI uses it both to decide whether
200-
// to render the unified "Update System" button (orchestrator capable =
201-
// yes) and to render progress while a run is in flight. So the
202-
// capability-failure response is a 200 with `{"step":"unavailable"}`,
203-
// not a 503; only POST /api/orchestrator/start hard-refuses on
204-
// capability failure.
199+
// The endpoint is informational — the UI polls it to render progress
200+
// while a run is in flight. The capability-failure response is a 200
201+
// with `{"step":"unavailable"}`, not a 503; only POST
202+
// /api/orchestrator/start hard-refuses on capability failure.
205203
//
206204
// Missing state file → `{"step":"idle"}` (capable but no run yet on
207205
// this boot — /run is tmpfs so this is the normal post-boot state).

0 commit comments

Comments
 (0)