modules/supervisord: init, modules/openssh: init #203

zhaofengli · 2022-09-21T09:32:25Z

This PR adds support for running services with supervisord as well as an OpenSSH module. I wanted to run an SSH server (#32, #156) that gets automatically launched when Nix-on-Droid starts, complete with process supervision.

The supervisord.programs.<name> options are designed to resemble systemd.services.<name>:

{ pkgs, ... }:
{
  supervisord.programs.sshd = {
    path = [ pkgs.openssh ];
    autoRestart = true;
    script = ''
      ssh-keygen ...

      exec ${pkgs.openssh}/bin/sshd -D -f /etc/ssh/sshd_config
    '';
  };
}

Fixes #54.

Gerschtli

Looks cool, will continue the review later :)

modules/supervisord.nix

zhaofengli

Thanks for the review! I'll squash the commits once everything is ready.

modules/supervisord.nix

Gerschtli

Please also mention the new modules in the changelog :) Would be great to have some tests for these new options (if feasible with reasonable effort).

Wanted to implement something like that since a very long time, I am glad you took the effort. Thank you a lot :)

modules/environment/login/login-inner.nix

Gerschtli · 2022-09-21T19:45:22Z

modules/environment/login/login-inner.nix

+  ${lib.optionalString config.supervisord.enable ''
+    set +e
+    if [ ! -e "${config.supervisord.socketPath}" ]; then
+      ${config.supervisord.package}/bin/supervisord -c /etc/supervisord.conf


You are starting the daemon but you should also stop it, when every shell is closed.

That makes sense to do, but I'm not sure what the cleanest way to do that is.

I can see myself doing an 'Acquire wakelock', closing all shells and still expecting services to run, so not sure what's the ideal UX here. My vote is "notification is present <=> services are running".

What's the current situation?

I think it is worth providing solutions to both use cases, so there could be an option to disable automatic shutdown after last session closes (I think using automatic shutdown as default sounds safer than the other way around).

But also not quite sure where such a shutdown could be implemented..

We could replace exec proot in /bin/login with a fork and wait approach while redirecting process signals to the proot process, enabling us to run cleanup stuff after a session is closed. Counting of proot processes could then be used to shutdown supervisord. I can not think of a downside of this approach (and I don't see an alternative).

modules/services/openssh.nix

modules/supervisord.nix

modules/services/openssh.nix

Gerschtli · 2022-09-21T20:43:38Z

For completion, it would be great to have native options to add ssh pub keys in /etc/ssh/authorized_keys.d. But could be done in a separate PR, however you like :)

Gerschtli · 2022-09-21T21:21:49Z

Tested it on device: Running supervisorctl stop sshd does not stop the sshd process when there is an active ssh session.. Does not look like that would be expected behaviour.

zhaofengli

Tested it on device: Running supervisorctl stop sshd does not stop the sshd process when there is an active ssh session.. Does not look like that would be expected behaviour.

It's expected behavior because sshd forks a separate process to handle each connection. You can try by stopping sshd on NixOS (or any other distro): Existing sessions keep running but you can't establish new ones.

But yes, we do need to consider whether to want to terminate everything when all foreground Nix-on-Droid sessions exit. In regular Termux my manually started sshd processes still run when all sessions end.

modules/supervisord.nix

zhaofengli · 2022-09-21T21:55:49Z

modules/supervisord.nix

+        '';
+        type = types.str;
+      };
+      script = lib.mkOption {


command is the direct way of specifying what to run (systemd.services.<name>.serviceConfig.ExecStart) and works fine for simple services that don't require much setup. I think there's no need to involve a script when setting command directly is sufficient (also avoid the exec pitfalls).

modules/supervisord.nix

modules/services/openssh.nix

zhaofengli · 2022-09-21T22:27:37Z

modules/environment/login/login-inner.nix

+  ${lib.optionalString config.supervisord.enable ''
+    set +e
+    if [ ! -e "${config.supervisord.socketPath}" ]; then
+      ${config.supervisord.package}/bin/supervisord -c /etc/supervisord.conf


That makes sense to do, but I'm not sure what the cleanest way to do that is.

Gerschtli · 2022-09-22T20:45:49Z

It's expected behavior because sshd forks a separate process to handle each connection. You can try by stopping sshd on NixOS (or any other distro): Existing sessions keep running but you can't establish new ones.

Okay, but there is still a sshd process running even after the last ssh connection is closed when the sshd was stopped while session where open.

modules/services/openssh.nix

modules/supervisord.nix

modules/services/openssh.nix

Otherwise mkdir et al may silently fail.

This matches the behavior of makeJobScript in nixos/lib/systemd-lib.nix.

modules/supervisord.nix

Doesn't exist in stable.

This removes the 1-second delay between supervisord starting and forking into background.

zhaofengli · 2022-10-01T21:56:51Z

There is another UX issue with the current approach. Currently proot waits for all traced processes to terminate, so the first Nix-on-Droid session which starts supervisord won't terminate after exiting the shell. If this isn't desirable, we can start supervisord in a separate proot invocation.

Gerschtli · 2022-10-11T19:56:57Z

Why not detach the process from the invoking session with e.g. disown?

zhaofengli · 2022-10-11T20:03:41Z

Why not detach the process from the invoking session with e.g. disown?

Sadly this can't work because proot needs to ptrace all processes in the session to perform its hooking, so it has to be there as long as any child processes exist. There doesn't appear to be an option to have proot itself fork into the background if the "main process" exits.

t184256 · 2022-10-11T20:22:00Z

Random idea, feel free to disregard:

login -> proot -> login-inner -> session
       \
        -> (supervisord overseeing process) -> proot -> login-inner -> supervisord

zhaofengli · 2022-10-11T20:45:30Z

Random idea, feel free to disregard:

login -> proot -> login-inner -> session
       \
        -> (supervisord overseeing process) -> proot -> login-inner -> supervisord

That's what I meant by "start supervisord in a separate proot invocation." If this is desired I'll implement it later this week.

t184256 · 2022-10-11T20:50:08Z

I still somehow thought that meant starting it from proot with extra precautions. Sorry for the noise then.

As for desireability, I'm not sure. Sounds rather involved to me wrt starting/stopping it (via signals?). If you figure out some alternative that simplifies things at the expense of, say, some support from the app, I'm open for discussing it further.

Gerschtli · 2022-11-05T22:22:12Z

Hey @zhaofengli , is there anything where I could support you with this PR? :)

zhaofengli · 2022-11-07T06:51:17Z

Sorry, got distracted and forgot about this :/ Termux does appear to have a special type of session for background tasks:

One integration is implemented in Termux:Widgets, where you can drop a shell script in ~/.shortcuts/tasks and add a shortcut that launches the script in the background. We should look into whether it's possible to start such a task with /system/bin/am start or similar outside PRoot. If so, then things should be cleaner with the user still having a persistent notification even with no interactive sessions active.

diamondburned · 2023-01-07T08:23:35Z

Is there a way to test this PR without rebuilding the fork? Maybe a way to patch these modules into the config?

Gerschtli · 2023-01-07T09:02:44Z

Yes, either use @zhaofengli's fork as channel/flake input or create your own fork of nix-on-droid and rebase his branch onto latest master to keep all changes that were merged in the meantime. No need for a new bootstrap zip because there were no changes :)

diamondburned · 2023-01-07T11:33:45Z

create your own fork of nix-on-droid and rebase his branch onto latest master to keep all changes that were merged in the meantime

Sorry for being off-topic; how would I do this, exactly? I'm currently not using Flakes. Do I swap the Nix channel with one that points to this commit?

Gerschtli · 2023-01-07T11:39:22Z

nix-channel --list shows you the current nix-on-droid url. That's a url with repo owner, repo name and rev (probably master or release-22.11). Replace owner and rev and set within .nix-channels, afterwards run nix-channel --update and nix-on-droid switch :)

Shados · 2023-09-07T11:11:03Z

modules/services/openssh.nix

+          fi
+        '')}
+
+        exec ${cfg.package}/bin/sshd -D -f /etc/ssh/sshd_config


Should probably add the -e flag so log output goes to stderr, which supervisord can actually pick up (unlike syslog).

expenses · 2024-06-01T08:50:46Z

What's the status of this PR? I've been using it for some basic services and I'm pretty happy with it. Would be good to get it merged if possible.

Gerschtli requested changes Sep 21, 2022

View reviewed changes

modules/supervisord.nix Outdated Show resolved Hide resolved

modules/supervisord.nix Show resolved Hide resolved

modules/supervisord.nix Show resolved Hide resolved

modules/supervisord.nix Show resolved Hide resolved

modules/supervisord.nix Outdated Show resolved Hide resolved

zhaofengli commented Sep 21, 2022

View reviewed changes

modules/supervisord.nix Show resolved Hide resolved

modules/supervisord.nix Outdated Show resolved Hide resolved

modules/supervisord.nix Show resolved Hide resolved

modules/supervisord.nix Outdated Show resolved Hide resolved

modules/supervisord.nix Show resolved Hide resolved

Gerschtli requested changes Sep 21, 2022

View reviewed changes

zhaofengli commented Sep 21, 2022

View reviewed changes

Gerschtli reviewed Sep 22, 2022

View reviewed changes

modules/services/openssh.nix Show resolved Hide resolved

Gerschtli reviewed Sep 22, 2022

View reviewed changes

modules/supervisord.nix Outdated Show resolved Hide resolved

Gerschtli reviewed Sep 22, 2022

View reviewed changes

modules/services/openssh.nix Outdated Show resolved Hide resolved

zhaofengli added 20 commits September 24, 2022 21:19

modules/supervisord: init

dc61ae1

modules/openssh: init

3a02d09

fixup svd: remove cfg.configFile

88b5731

fixup svd: mdDoc-ify everything

a00dc02

fixup svd: make script-generated command have normal priority

7834088

fixup svd: add more docs

16107f5

fixup svd: make environment.PATH compose better

86248da

fixup svd: start svd during activation

4d93571

fixup ssh: just hardcode password auth to no

3278a9c

fixup svd: use enabledPrograms for numPrograms

269f6d1

fixup svd: license header

6e9389c

fixup ssh: license header

0faa47c

fixup svd: add message when reloading

bd44a10

fixup ssh: reduce lib inherits

c39cb39

fixup svd: dont do set +e

42b95bd

fixup svd: support dry-run

06d245c

fixup ssh: don't write to stdout

5ea968c

fixup svd: add autostart option

f861f45

fixup ssh: add autostart option

b416b58

fixup ssh: also add coreutils to path

2f92f24

Otherwise mkdir et al may silently fail.

zhaofengli added 4 commits September 24, 2022 21:19

fixup svd: add set -e to job script

5278b3d

This matches the behavior of makeJobScript in nixos/lib/systemd-lib.nix.

fixup svd: only accept the typed boolean form in nix config

edb3247

fixup svd: also accept boolean in extraConfig

82c42b3

fixup svd: disallow double quotes in environment

56f4e44

zhaofengli force-pushed the supervisord branch from 711e4f3 to 56f4e44 Compare September 24, 2022 21:19

Gerschtli reviewed Sep 25, 2022

View reviewed changes

modules/supervisord.nix Outdated Show resolved Hide resolved

zhaofengli added 4 commits October 1, 2022 21:52

fixup ssh: Remove mdDoc for now

ca13b29

Doesn't exist in stable.

fixup svd: Remove mdDoc for now

01a7447

fixup svd: use module assertions for double quote check

9222c03

fixup svd: start supervisord in background

ba1526a

This removes the 1-second delay between supervisord starting and forking into background.

Gerschtli mentioned this pull request Jan 2, 2023

Init daemon to manage all running processes #54

Open

Shados reviewed Sep 7, 2023

View reviewed changes

t184256 mentioned this pull request Feb 7, 2024

Create SSH module #325

Open

expenses mentioned this pull request Jun 8, 2024

X11 with VNC, Fatal server error: (EE) Failed to activate virtual core keyboard: 2(EE) , termux workaround #75

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

modules/supervisord: init, modules/openssh: init #203

modules/supervisord: init, modules/openssh: init #203

zhaofengli commented Sep 21, 2022

Gerschtli left a comment

zhaofengli left a comment

Gerschtli left a comment

Gerschtli Sep 21, 2022

zhaofengli Sep 21, 2022

t184256 Sep 22, 2022

Gerschtli Sep 22, 2022

Gerschtli Oct 11, 2022

Gerschtli commented Sep 21, 2022

Gerschtli commented Sep 21, 2022

zhaofengli left a comment

zhaofengli Sep 21, 2022

zhaofengli Sep 21, 2022

Gerschtli commented Sep 22, 2022

zhaofengli commented Oct 1, 2022

Gerschtli commented Oct 11, 2022

zhaofengli commented Oct 11, 2022

t184256 commented Oct 11, 2022

zhaofengli commented Oct 11, 2022

t184256 commented Oct 11, 2022

Gerschtli commented Nov 5, 2022

zhaofengli commented Nov 7, 2022

diamondburned commented Jan 7, 2023

Gerschtli commented Jan 7, 2023

diamondburned commented Jan 7, 2023

Gerschtli commented Jan 7, 2023

Shados Sep 7, 2023

expenses commented Jun 1, 2024

modules/supervisord: init, modules/openssh: init #203

Are you sure you want to change the base?

modules/supervisord: init, modules/openssh: init #203

Conversation

zhaofengli commented Sep 21, 2022

Gerschtli left a comment

Choose a reason for hiding this comment

zhaofengli left a comment

Choose a reason for hiding this comment

Gerschtli left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Gerschtli commented Sep 21, 2022

Gerschtli commented Sep 21, 2022

zhaofengli left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Gerschtli commented Sep 22, 2022

zhaofengli commented Oct 1, 2022

Gerschtli commented Oct 11, 2022

zhaofengli commented Oct 11, 2022

t184256 commented Oct 11, 2022

zhaofengli commented Oct 11, 2022

t184256 commented Oct 11, 2022

Gerschtli commented Nov 5, 2022

zhaofengli commented Nov 7, 2022

diamondburned commented Jan 7, 2023

Gerschtli commented Jan 7, 2023

diamondburned commented Jan 7, 2023

Gerschtli commented Jan 7, 2023

Choose a reason for hiding this comment

expenses commented Jun 1, 2024