[tor-bugs] #33015 [Internal Services/Tor Sysadmin Team]: turn rc.local into a systemd service

[Tor Bug Tracker & Wiki]

#33015: turn rc.local into a systemd service
-------------------------------------------------+-------------------------
 Reporter:  anarcat                              |          Owner:  anarcat
     Type:  task                                 |         Status:  closed
 Priority:  Low                                  |      Milestone:
Component:  Internal Services/Tor Sysadmin Team  |        Version:
 Severity:  Major                                |     Resolution:  fixed
 Keywords:                                       |  Actual Points:
Parent ID:                                       |         Points:
 Reviewer:                                       |        Sponsor:
-------------------------------------------------+-------------------------
Changes (by anarcat):

 * status:  accepted => closed
 * resolution:   => fixed


Old description:

> We have a somewhat strange `rc.local` configuration file which does this:
>
> {{{
> #!/bin/bash
>
> ##
> ## THIS FILE IS UNDER PUPPET CONTROL. DON'T EDIT IT HERE.
> ## USE: git clone
> git+ssh://$USER@puppet.debian.org/srv/puppet.debian.org/git/dsa-
> puppet.git
> ##
>
> if [ -e /proc/sys/kernel/modules_disabled ]; then
>         ( sleep 60;
>           echo 1 > /proc/sys/kernel/modules_disabled || true
>         ) & disown
> fi
>
> touch /var/run/reboot-lock
> }}}
>
> Here's what it does, from what I can gather.
>
>  1. If the kernel is so configured (if
> `/proc/sys/kernel/modules_disabled` is present) it will disable module
> loading, after a 60 seconds delay
>  2. it creates the /var/run/reboot-lock file, which is used by other
> components to forbid reboots through a molly-guard hook
>
> Those are two independent purposes, of course.
>
> This, or at least the first part, should be replaced by a systemd
> service. This will make it easier to disable, which is necessary when we
> want to actually load modules. This is done in the buster upgrade
> process, for example, which says:
>
>  5. Enable module loading (for ferm)
>
> {{{
> sed -i -e 's/.*modules_disabled/#&/' /etc/rc.local
>
> reboot
>
> export LC_ALL=C.UTF-8 &&
> sudo ttyrec -e screen /var/log/upgrade-buster.ttyrec.2
> }}}
>
> That `sed` line has a serious bug which will make `rc.local` crash during
> the reboot. Instead of just "disabling the disabling", it actually
> mangles the shell script and turns the `if` block into this:
>
> {{{
> #if [ -e /proc/sys/kernel/modules_disabled ]; then
>         ( sleep 60;
> #         echo 1 > /proc/sys/kernel/modules_disabled || true
>         ) & disown
> fi
> }}}
>
> And while that hack could be fixed, it would be much easier, logical and
> understandable if it was written as (say) `systemctl disable module-
> disable`. There might even be existing service files that do this which
> we could use.
>
> The reboot lock, on its part, could be created by the systemd.tmpfiles
> mechanism.

New description:

 We have a somewhat strange `rc.local` configuration file which does this:

 {{{
 #!/bin/bash

 ##
 ## THIS FILE IS UNDER PUPPET CONTROL. DON'T EDIT IT HERE.
 ## USE: git clone
 git+ssh://$USER@puppet.debian.org/srv/puppet.debian.org/git/dsa-puppet.git
 ##

 if [ -e /proc/sys/kernel/modules_disabled ]; then
         ( sleep 60;
           echo 1 > /proc/sys/kernel/modules_disabled || true
         ) & disown
 fi

 touch /var/run/reboot-lock
 }}}

 Here's what it does, from what I can gather.

  1. If the kernel is so configured (if `/proc/sys/kernel/modules_disabled`
 is present) it will disable module loading, after a 60 seconds delay
  2. it creates the /var/run/reboot-lock file, which is used by other
 components to forbid reboots through a molly-guard hook

 Those are two independent purposes, of course.

 This, or at least the first part, should be replaced by a systemd service.
 This will make it easier to disable, which is necessary when we want to
 actually load modules. This is done in the buster upgrade process, for
 example, which says:

  5. Enable module loading (for ferm)

 {{{
 sed -i -e 's/.*modules_disabled/#&/' /etc/rc.local

 reboot

 export LC_ALL=C.UTF-8 &&
 sudo ttyrec -e screen /var/log/upgrade-buster.ttyrec.2
 }}}

 That `sed` line has a serious bug which will make `rc.local` crash during
 the reboot. Instead of just "disabling the disabling", it actually mangles
 the shell script and turns the `if` block into this:

 {{{
 #if [ -e /proc/sys/kernel/modules_disabled ]; then
         ( sleep 60;
 #         echo 1 > /proc/sys/kernel/modules_disabled || true
         ) & disown
 fi
 }}}

 And while that hack could be fixed, it would be much easier, logical and
 understandable if it was written as (say) `systemctl disable module-
 disable`. There might even be existing service files that do this which we
 could use.

 The reboot lock, on its part, could be created by the systemd.tmpfiles
 mechanism. This, in turn, would make it effectively as soon as Puppet
 runs, as opposed to after the first reboot in the old mechanism.

--

Comment:

 reboot-lock is now populated by systemd-tmpfiles on boot (and on install!)
 thanks to cbc274ea and the unfortunate 6e733ee5.

 this is now done.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/33015#comment:3>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online