pillar: Implement console disabling during runtime

[rene]

Description

Pillar starts on a fresh boot with USB, VGA and console enabled. This isn't an issue if no serial consoles are present in the cmdline because for video terminals, the TUI + VGA + usb inputs can be disabled during runtime. So as soon as pillar gets the global configuration from the controller, it can disabled all of them. However, if any serial console is present at the cmdline, like console=ttyAMA0, pillar will start getty on that console through the rungetty.sh script, which spawns a getty process on every console from /proc/cmdline. The script use to run a infinte loop, so even if pillar killed the getty process, it would be restarted again.

The behavior of rungetty.sh was changed so it really stops getty if it receives a USR1 signal. This commit implements the stopGetty() to send the signal and really stop the console, allowing disabling serial consoles during runtime if if debug.enable.console" is set to false. Also, it always check if device is onboarded during initialization, if device is onboarded, it will not enable console by default but wait until debug.enable.console is read.

How to test and validate this PR

I tested with QEMU, but it can be tested on any device with a working UART console and the console=<serial device> is present in the kernel command line.

1. Build and run live image: make live run-live or make live run-live-gui, although VGA is not required. The focus is the serial console.
2. First boot device will not be onboarded, so serial console must be enabled and functional
3. Onboard the device
4. Set device property debug.enable.console to false (if it's not false by default)
5. Serial console must be disabled during runtime for inputs
6. Set device property debug.enable.console to true
7. Serial console access should be recovered during runtime

Different combinations can also be tried:

1. Set debug.enable.console to false, reboot the device to ensure serial console is never enabled
2. Set debug.enable.console to true, reboot the device to ensure serial console is enabled

Changelog notes

Fix: allow disabling serial consoles at runtime when debug.enable.console is set to false and skip default console enablement on onboarded devices.

PR Backports

This fix will be required for users running 16.0.x, I think it makes sense to backport at least to 17.0 and 16.0-stable. I can backport to other branches as well, if required.

• 17.0-stable
• 16.0-stable

Checklist

• I've provided a proper description
• I've added the proper documentation
• I've tested my PR on amd64 device
• I've tested my PR on arm64 device
• I've written the test verification instructions
• I've set the proper labels to this PR
• I've checked the boxes above, or I've provided a good reason why I didn't
  check them.

[codecov]

Codecov Report

❌ Patch coverage is 0% with 45 lines in your changes missing coverage. Please review.
✅ Project coverage is 23.56%. Comparing base (b7d4463) to head (66a0c78).
⚠️ Report is 13 commits behind head on master.

Files with missing lines	Patch %	Lines
pkg/pillar/cmd/domainmgr/domainmgr.go	0.00%	26 Missing ⚠️
pkg/pillar/cmd/domainmgr/handlegetty.go	0.00%	19 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6220      +/-   ##
==========================================
+ Coverage   23.21%   23.56%   +0.35%     
==========================================
  Files         510      520      +10     
  Lines       93470    95227    +1757     
==========================================
+ Hits        21697    22442     +745     
- Misses      70025    70853     +828     
- Partials     1748     1932     +184     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[rene]

@claude

[claude]

Claude finished @rene's task in 3m 22s —— View job

---

Code Review

• Fetch and read the diff
• Analyze rungetty.sh signal handling
• Analyze stopGetty / handlegetty.go
• Analyze isDeviceOnboarded in domainmgr.go
• Post inline review comments

I reviewed the change and left 3 inline comments. Summary:

🔴 Likely blocking — stopGetty probably signals nothing (handlegetty.go)
p.Name() returns the kernel comm (interpreter basename, ≤15 chars), which for a #!/bin/sh script is sh/busybox, never rungetty.sh. The subshell holding the USR1 trap is also comm == "sh" (and orphaned to init after the INITGETTY parent exits). So strings.Contains(name, "rungetty.sh") matches no process and no SIGUSR1 is sent — the serial console wouldn't actually be disabled. Suggested matching on p.Cmdline() instead, and re-verifying the QEMU test with a count of signalled PIDs.

🟠 Console may not be fully released (rungetty.sh)
kill -9 $child SIGKILLs the setsid.getty wrapper; the agetty it started in a new session can be orphaned and keep holding the tty. Prefer TERM and/or killing the process group.

🟡 Backport hygiene (domainmgr.go)
The unrelated any → found rename widens the diff for a change slated for 16.0/17.0 backports.

Verified OK: isDeviceOnboarded uses the correct publisher (zedclient), and Activate() on a Persistent subscription synchronously calls populate(), so the subsequent GetAll() observes the on-disk OnboardingStatus — that logic is sound; failing to false (console enabled) is a safe default.
· branch fix-serial-console-access

[rene]

@claude

[claude]

Claude finished @rene's task in 1m 52s —— View job

---

Re-review of updated PR

• Fetch and read the current diff
• Re-check handlegetty.go (stopGetty, process matching)
• Re-check rungetty.sh signal handling (post -9 removal)
• Re-check domainmgr.go isDeviceOnboarded / init logic
• Post inline review comments

Thanks for the updates — the -9 removal and the 2-console test result address the earlier console-release and multi-console concerns. One inconsistency remains from the last round:

🟠 p.Name() vs p.Cmdline() mismatch (handlegetty.go:73)
The variable was renamed to cmdline, but the call is still p.Name(), not p.Cmdline(). So the "switch to command-line matching" wasn't actually applied — only the variable name changed. It works today because rungetty.sh (11 chars) fits within comm's 15-char cap, but it's misleading and less robust than the intended Cmdline(). Please either call p.Cmdline() or rename the variable back to name. Related: gettyStarted = false is set unconditionally even when no process matched, so a failed match could later cause a duplicate getty — another reason to prefer the more reliable match.

🟡 Discarded error (handlegetty.go:69)
log.Errorf("Cannot list processes") drops the actual err; include %v.

Verified OK / resolved:

• rungetty.sh USR1 trap + wait/kill $child (no -9) — confirmed working with 2 serial consoles per your test.
• isDeviceOnboarded — correct publisher (zedclient), persistent Activate() populates synchronously before GetAll(), and failing to false (console enabled) is a safe default. Sound.
• any → found rename — acknowledged as a Yetus fix, fine to keep here and drop during backport.

Nothing blocking beyond the Name()/Cmdline() cleanup.
· branch fix-serial-console-access

[rene]

@claude

[claude]

Claude finished @rene's task in 1m 0s —— View job

---

Re-review of updated PR

• Fetch and read the current diff
• Re-check handlegetty.go (stopGetty, process matching, error handling)
• Re-check rungetty.sh signal handling
• Re-check domainmgr.go isDeviceOnboarded / init logic
• Post review feedback

Thanks — both items from the last round are now resolved:

✅ p.Name() → p.Cmdline() — stopGetty now matches on p.Cmdline(), which isn't subject to the 15-char comm truncation and stays correct even if the script is renamed.

✅ Discarded error — log.Errorf("Cannot list processes: %v", err) now includes the underlying error.

Verified OK / resolved from earlier rounds:

• rungetty.sh — USR1 trap + wait "$child" / kill $child (no -9), confirmed working with 2 serial consoles per your test.
• isDeviceOnboarded — correct publisher (zedclient), persistent Activate() populates synchronously before GetAll(), and the DeviceUUID != nilUUID check plus failing to false (console enabled) on error are safe defaults. Sound.
• any → found rename — acknowledged as a Yetus fix; keep here, drop during backport.

One remaining minor note (non-blocking, left inline): gettyStarted = false is cleared unconditionally in stopGetty. Benign with the reliable Cmdline() match, but noted in case you want to be strict about the "no process matched" edge case.

Nothing blocking. LGTM.
· branch fix-serial-console-access