system: op-mode: T3334: allow delayed getty restart when configuring serial ports

[talmakion]

Change Summary

Reconfiguration of serial console settings from the serial console is not currently possible - the restart at commit terminates the login shell and does not complete the commit.

This PR relocates the serial-getty service control portion from the conf-mode handler into an op-mode user command "restart serial-console", wrapping it in additional checks and helpful messages. The conf handler will still call the op command, which will either silently succeed by restarting units (when no active sessions) or provide instructions to complete the process.

Types of changes

• [x] Bug fix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds functionality)
• [ ] Code style update (formatting, renaming)
• [ ] Refactoring (no functional changes)
• [ ] Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
• [ ] Other (please describe):

Related Task(s)

https://vyos.dev/T3334

Related PR(s)

Component(s) name

• op-mode
• system

Proposed changes

• Relocated service control to op-mode command "restart serial-console"
• Checking for logged-in serial sessions that may be affected by getty reconfig (extracted and adapted from ticket patch)
• Warning the user when changes are committed and serial sessions are active, otherwise restart services as normal. No prompts issued during commit, all config gen/commit steps still occur except for the service restarts (everything remains consistent)
• To apply committed changes, user will need to run "restart serial-console" to complete the process or reboot the whole router

How to test

• Starting with no serial ports, configuring new ones, all OK
• Adding more ports with existing commited ports, all OK
• Removing ports, working as expected
• Removing all ports, all OK
• Rebooting with working serial devices, boot-time configuration applies as expected
• Rebooting with missing serial devices (shutdown, remove VM device, power on), boot-time configuration applies as expected (missing device units created, sitting in failed state, does not affect working units)

One sample run through from the serial session itself:

vyos@TEST-VYOS-LEFT# set system console device ttyS1
[edit]
vyos@TEST-VYOS-LEFT# commit
There are user sessions connected via serial console that will be terminated
when serial console settings are changed.

Please ensure all settings are committed and saved before issuing a
"restart serial-console" command to apply new configuration.
[edit]
vyos@TEST-VYOS-LEFT# save
[edit]
vyos@TEST-VYOS-LEFT# exit
exit
vyos@TEST-VYOS-LEFT:~$ restart serial-console
There are user sessions connected via serial console that will be terminated
when serial console settings are changed.

Any uncommitted changes from these sessions will be lost and in-progress actions
may be left in an inconsistent state. Continue? [y/N] y

Welcome to VyOS - TEST-VYOS-LEFT ttyS0

TEST-VYOS-LEFT login: vyos
Password:
Welcome to VyOS!
[...banner snipped...]
vyos@TEST-VYOS-LEFT:~$ configure
[edit]
vyos@TEST-VYOS-LEFT# show system console
 device ttyS0 {
 }
 device ttyS1 {
 }
[edit]
vyos@TEST-VYOS-LEFT#

Additional run through when nobody is logged into a serial console:

vyos@TEST-VYOS-LEFT# delete  system console device ttyS1 
[edit]
vyos@TEST-VYOS-LEFT# commit
[edit]
vyos@TEST-VYOS-LEFT# save
[edit]

No interruption or messages, as expected. Service confirmed restarted.

Smoketest result

There does not seem to be an appropriate smoketest for this change.

Checklist:

• [x] I have read the CONTRIBUTING document
• [x] I have linked this PR to one or more Phabricator Task(s)
• [ ] I have run the components SMOKETESTS if applicable
• [x] My commit headlines contain a valid Task id
• [ ] My change requires a change to the documentation
• [ ] I have updated the documentation accordingly

[github-actions[bot]]

❌ PR title 'system: op-mode: T3334: allow delayed getty restart when configuring serial ports' does not match the required format!. Valid title example: T99999: make IPsec secure

[talmakion]

Apologies about the rapid fire force push followups - added a systemctl daemon-reload to the top of the op-mode command just in case, also managed to revert a previous minor cleanup edit from my private tree at the same time.

[talmakion]

Updated PR with requested changes.

[github-actions[bot]]

❌ PR title 'system: op-mode: T3334: allow delayed getty restart when configuring serial ports' does not match the required format!. Valid title example: T99999: make IPsec secure

[talmakion]

Re-wrote opmode script in the new style & re-tested.

[talmakion]

Changes made in latest revision:

• Centralised common code between completion, op-mode and conf-mode scripts in vyos.utils.serial
  • utmpdump example output with some additional brief notes included in source
• Filtering of devices is now possible in the helpers
• Selecting 1 specific device is now possible in the op-mode side with a device tagNode
  • Created completion script for device param

There are some more things that could be done to make this nicer, such as using the filter to only restart changed devices from conf-mode. That would be a bulk logic change in the existing conf-mode impl, which just wipes and replaces the systemd units every time it processes. I still need to put some effort into figuring out how get_config() detects changes and deletions in order to do that properly (there's plenty of examples I've seen in conf, but I haven't taken the time to pick them apart). For this PR, it would be a minor convenience (automatically skipping logged in sessions if they're not changed), so I haven't taken it that far yet.

What's the feeling on these revisions?

[github-actions[bot]]

❌ warning: Unused import pprint in src/op_mode/ipsec.py:16.

[talmakion]

Updated opmode func names as recommended, quickly re-tested the opmode side.

[sever-sever]

@c-po Can you recheck the PR?

[c-po]

I change a print to a Warning for the users to better indicate the severity of the action!

[c-po]

@MergifyIo backport sagitta circinus

[mergify[bot]]

backport sagitta circinus

✅ Backports have been created

• #3910 system: op-mode: T3334: allow delayed getty restart when configuring serial ports (backport #3698) has been created for branch sagitta
• #3911 system: op-mode: T3334: allow delayed getty restart when configuring serial ports (backport #3698) has been created for branch circinus

[github-actions[bot]]

CI integration 👍 passed!

Details

CI logs

• CLI Smoketests 👍 passed
• Config tests 👍 passed
• RAID1 tests 👍 passed