kbfs service: init #25610

cmacrae · 2017-05-08T08:01:21Z

Motivation for this change

It'd be nice to have a Keybase File System service on NixOS.
There's already been interest expressed (#22603).

These changes will lend themselves to those using any Keybase clients (CLI or GUI) by providing a kbfs mount.

Things done

Tested using sandboxing
(nix.useSandbox on NixOS,
or option build-use-sandbox in nix.conf
on non-NixOS)
Built on platform(s)
- NixOS
- macOS
- Linux
Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
Tested execution of all binary files (usually in ./result/bin/)
Fits CONTRIBUTING.md.

peterhoeg · 2017-05-09T09:35:53Z

Have you actually tried this? I was trying some time ago to get this running (you also need a keybase service service by the way) but I was unable to launch kbfs in a systemd user service due to mounts not working:

May 09 17:31:56 mildred kbfsfuse[13251]: 2017/05/09 17:31:56 mount helper error: fusermount: mount failed: Operation not permitted

peterhoeg · 2017-05-09T10:04:12Z

Ahhh, the trick was /run/wrappers in the path - I had pkgs.fuse which doesn't work.

cmacrae · 2017-05-09T10:15:23Z

@peterhoeg Yeah, fusermount has a wrapper which needs to be called instead of the standard binary :)
Sorry, what do you mean I need a keybase service? These commits provide a kbfs service purely for running the Keybase Filesystem as a user service - for use with Keybase clients.

Mic92 · 2017-05-09T21:43:52Z

nixos/modules/services/network-filesystems/kbfs.nix

+
+    systemd.user.services.kbfs = {
+      description = "Keybase File System";
+      path = [ "/run/wrappers" ];


I think this should be /run/wrappers/bin instead

bin gets appended automatically. Using /run/wrappers/bin results in /run/wrappers/bin/bin

Mic92 · 2017-05-09T21:45:43Z

nixos/modules/services/network-filesystems/kbfs.nix

+        description = "Mountpoint for the Keybase filesystem.";
+      };
+
+      createMountPoint = mkOption {


In which case the user does not wants to create the mountpoint? It will not work if the directory does not exists.

In other words, I don't think this needs to be an option. If the directory exists, everything is ok. If not, mkdir will fail with an error message.

Sure, I can understand your point. I just figured it might be useful for people who might want more control over the mountpoint's permissions. But if you feel it's unnecessary, I can take that out.

I'm with @Mic92 on this.

Mic92 · 2017-05-09T21:53:26Z

nixos/modules/services/network-filesystems/kbfs.nix

+      serviceConfig = {
+        ExecStart = "${cfg.package}/bin/kbfsfuse ${toString cfg.extraFlags} ${cfg.mountPoint}";
+        RestartSec = 3;
+        Restart = "always";


Restart = "on-failure" ?

Yeah, on-failure should probably be used here.

Now set to on-failure

Mic92 · 2017-05-09T21:54:00Z

nixos/modules/services/network-filesystems/kbfs.nix

+      postStop = "fusermount -u ${cfg.mountPoint}";
+      serviceConfig = {
+        ExecStart = "${cfg.package}/bin/kbfsfuse ${toString cfg.extraFlags} ${cfg.mountPoint}";
+        RestartSec = 3;


What is the purpose of three seconds? Defaults to 100ms.

I sometimes found that fusermount would not have executed cleanly without this. Though, it was intermittent (sometimes it worked fine) - can do some thorough testing without this. If it seems fine, I'll just remove it.

Okay, I've removed the RestartSec param

peterhoeg · 2017-05-11T03:03:30Z

nixos/modules/services/network-filesystems/kbfs.nix

+      ${optionalString cfg.createMountPoint
+      "test -d ${cfg.mountPoint} || mkdir -p ${cfg.mountPoint}"
+      }'';
+      postStop = "fusermount -u ${cfg.mountPoint}";


This doesn't work - systemd requires a full path, so it will have to be /run/wrappers/bin/fusermount.

Works fine for me? I've been using it the past week with no issues.
With the exec call in the kbfsfuse binary to fusermount, and this postStop operation, that's why I have path = [ "/run/wrappers" ]; defined.

Have you actually tested and confirmed this is the case? Because, if so, you'd actually see that any parameters for a systemd service that execute something (preStart, ExecStart, postStop, etc.) are in fact written to a shell script in the nix store, then the resulting systemd unit actually uses a full path to these scripts:

[Unit] Description=Keybase File System [Service] Environment="LOCALE_ARCHIVE=/nix/store/mry6hpzxg44idk6pmqhcjvxzr665y43p-glibc-locales-2.25/lib/locale/locale-archive" Environment="PATH=/run/wrappers/bin:/nix/store/k8dyhz9hrjydl23ny3131am6cws1rc07-coreutils-8.26/bin:/nix/store/dqzqrrampaf665laggc4ng33dfbxv5q7-findutils-4.6.0/bin:<*SNIP*> Environment="TZDIR=/nix/store/ib356ccx25xixzjmj0fmgdv61kj1f398-tzdata-2016j/share/zoneinfo" ExecStart=/nix/store/sqv84hq2j879z92pg71h2pmjr0kg58cc-kbfs-20170209.d1db463-bin/bin/kbfsfuse -label kbfs /home/cmacrae/keybase ExecStartPre=/nix/store/n44y06vk1b18knzng7w9dswkr0dnbm38-unit-script/bin/kbfs-pre-start ExecStopPost=/nix/store/0ag4fi7jgh1x23y3f7mxpvvihhkbqza5-unit-script/bin/kbfs-post-stop Restart=always RestartSec=3

I have tested it but it was using ExecStopPost instead of postStop and all the ExecXXXX require full paths. Sorry about the confusion.

Ah that makes sense then. No worries at all 👍

So here we need /run/wrappers/bin/fusermount inside serviceConfig.ExecStopPost.

peterhoeg · 2017-05-11T03:51:59Z

Sorry, what do you mean I need a keybase service? These commits provide a kbfs service purely for running the Keybase Filesystem as a user service - for use with Keybase clients.

If you want to run the Keybase GUI app, the keybase service is also needed. I mistakenly thought kbfs also required that service but that doesn't seem to be the case. If you're not planning on adding it as part of this, no problem, I'll do it later.

…rvice

cmacrae · 2017-05-11T10:11:08Z

Cheers @peterhoeg - no I personally don't plan on using the GUI app at this moment.
If there's interest in it, someone can add it, or perhaps if I end up using it in the near future, I'll contribute a service for that also :)

peterhoeg · 2017-05-11T14:36:11Z

@cmacrae, if I try running kbfs as a service without having launched the keybase server, I get a lot of these:

May 11 22:32:42 dolores kbfsfuse[12295]: 2017-05-11T22:32:42.545579 ▶ [WARN kbfs connection.go:370] 023 (CONN KeybaseDaemonRPC e41ff7d2) Connection: error dialing transport: dial unix /run/user/1000/keybase/keybased.sock: connect: no such file or directory
May 11 22:32:42 dolores kbfsfuse[12295]: 2017-05-11T22:32:42.545627 ▶ [WARN kbfs connection.go:535] 025 KeybaseDaemonRPC: connection error: "dial unix /run/user/1000/keybase/keybased.sock: connect: no such file or directory"; retrying in 2s
May 11 22:32:44 dolores kbfsfuse[12295]: 2017-05-11T22:32:44.545793 ▶ [WARN kbfs connection.go:370] 026 (CONN CryptoClient f336a259) Connection: error dialing transport: dial unix /run/user/1000/keybase/keybased.sock: connect: no such file or directory
May 11 22:32:44 dolores kbfsfuse[12295]: 2017-05-11T22:32:44.545835 ▶ [WARN kbfs connection.go:535] 027 CryptoClient: connection error: "dial unix /run/user/1000/keybase/keybased.sock: connect: no such file or directory"; retrying in 2s

cmacrae · 2017-05-11T15:48:20Z

@peterhoeg You're absolutely right... that's poor testing from me; I guess it was just working because I'd always had a keybase process forked in the background from using the CLI.

I'll implement this now :)

cmacrae · 2017-05-11T16:34:21Z

Cool, so with the above changes, I've added a keybase service - there wasn't much to be done here.
In light of that, I've then added a requirement of the keybase service for the kbfs service, such that:

If kbfs starts, but keybase isn't up, it'll start it (kbfs automatically retries in a loop if keybase isn't up initially)
If keybase stops gracefully/ungracefully, kbfs is automatically stopped (as it can't survive without it).

I've also enabled the keybase service at the end of the kbfs implementation, is this good practice?

Cheers for pointing this out @peterhoeg - bit silly of me not to notice 😉

cmacrae · 2017-05-11T17:11:30Z

I'm not sure the failure Travis is reporting is related ☝️
error: attribute ‘docker_17_04’ missing, at /Users/travis/.nox/nixpkgs/pkgs/top-level/all-packages.nix:13:2

Don't see how I could have impacted that with the changes I made 🙂

peterhoeg · 2017-05-12T03:20:05Z

I'm not sure the failure Travis is reporting is related

It wasn't you. Fixed in 49d3b43

cmacrae · 2017-05-12T09:01:11Z

@peterhoeg 👍🏻

peterhoeg · 2017-05-12T09:36:26Z

I have a few comments but I will only have time over the weekend to go through it. I'll get back to you then.

cmacrae · 2017-05-12T09:37:56Z

Alright, thanks @peterhoeg 👍

peterhoeg · 2017-05-17T15:55:31Z

nixos/modules/services/network-filesystems/kbfs.nix

+
+      mountPoint = mkOption {
+        type = types.str;
+        default = "/keybase";


As this service is expected to be used by multiple users, defaulting to /keybase may not be the best thing. I would recommend "%h/keybase" instead.

Cool, I didn't know about %h! Okay, sounds good
EDIT: Ah, I see it's a systemd thing

peterhoeg · 2017-05-17T15:56:12Z

nixos/modules/services/network-filesystems/kbfs.nix

+        description = "Mountpoint for the Keybase filesystem.";
+      };
+
+      createMountPoint = mkOption {


I'm with @Mic92 on this.

peterhoeg · 2017-05-17T15:56:48Z

nixos/modules/services/network-filesystems/kbfs.nix

+      };
+
+      package = mkOption {
+        type = types.package;


There is only one package that provides kbfs, so it seems redundant to make it configurable.

Cool, I'll strip the option out 👍

peterhoeg · 2017-05-17T15:57:47Z

nixos/modules/services/network-filesystems/kbfs.nix

+
+    systemd.user.services.kbfs = {
+      description = "Keybase File System";
+      requires = [ "keybase.service" ];


We should also have after = [ "keybase.service" ]; although kbfs WILL still throw an error in the beginning because the keybase service isn't up.

Sure, makes sense - will add this.

peterhoeg · 2017-05-17T15:59:25Z

nixos/modules/services/network-filesystems/kbfs.nix

+      requires = [ "keybase.service" ];
+      path = [ "/run/wrappers" ];
+      preStart = ''
+      ${optionalString cfg.createMountPoint


Ref my note above about the mountpoint, we cannot use preStart and postStop scripts any longer because they don't know anything about %h or %t which we might use to specify the mount point. Should be ExecStartPre inside serviceConfig.

Okay, I'll switch to using ExecStartPre :)

What does %t evaluate to? Is there a page of documentation noting these (%h, %t, etc.) and their value? I'd like to read up on them

EDIT: Ah, I see it's a systemd thing

peterhoeg · 2017-05-17T15:59:55Z

nixos/modules/services/network-filesystems/kbfs.nix

+      ${optionalString cfg.createMountPoint
+      "test -d ${cfg.mountPoint} || mkdir -p ${cfg.mountPoint}"
+      }'';
+      postStop = "fusermount -u ${cfg.mountPoint}";


So here we need /run/wrappers/bin/fusermount inside serviceConfig.ExecStopPost.

peterhoeg · 2017-05-17T16:00:25Z

nixos/modules/services/network-filesystems/kbfs.nix

+      };
+    };
+
+    environment.systemPackages = [ cfg.package ];


We don't need to pollute the global environment with the kbfs package.

Oh right? Okay. I only did this as I saw it implemented in other services (compton).
What with the abstraction layer providing an enable param, I think it's the expected behavior that it will take care of also ensuring the package is present on the system, no?

The service will pull in the package anyway, so it will be installed - you just will not be able to run kbfs directly from the command line. This is why it makes sense to put keybase into environment.systemPackages as you will use the keybase binary directly, but not kbfs.

Okay, I see. Is that inferred from the use of things like ${pkgs.kbfs}/bin/kbfsfuse?

Such that, Nix see's we want to evaluate ${pkgs.kbfs} and thus ensures it's present?

peterhoeg · 2017-05-17T16:00:48Z

nixos/modules/services/networking/keybase.nix

+        description = "Whether to start the Keybase service.";
+      };
+
+      package = mkOption {


Same as above, there is only one package.

Sure, I'll remove this param

peterhoeg · 2017-05-17T16:01:50Z

nixos/modules/services/networking/keybase.nix

+      description = "Keybase service";
+      serviceConfig = {
+        ExecStart = ''
+          ${cfg.package}/bin/keybase service --auto-forked


Are you sure about ---auto-forked? What does that give us?

I don't think it's necessary, really. I only chucked it in there as that's what you get when running from a shell - so, yeah we can probably drop this. I'll remove it and test it, along with all the other changes.

peterhoeg · 2017-05-17T16:02:31Z

nixos/modules/services/networking/keybase.nix

+          ${cfg.package}/bin/keybase service --auto-forked
+        '';
+        Restart = "on-failure";
+      };


We should probably harden things a little for both keybase and kbfs. PrivateTmp = true springs to mind.

Yep, good catch

cmacrae · 2017-05-18T08:25:28Z

Okay, so I've implemented the changes outlined above, all seems to work good.

One weird side affect of changing to using serviceConfig.ExecStartPre/Post is that now every time the service is started... it seems to create a '$HOME' directory in the user's home directory, so for me that's:
/home/cmacrae/'$HOME'

I'm guessing this is a result of using mkdir -p %h/keybase (%h/keybase now being the default mountPoint value).

peterhoeg · 2017-05-18T08:26:33Z

nixos/modules/services/network-filesystems/kbfs.nix

+        ExecStart = "${pkgs.kbfs}/bin/kbfsfuse ${toString cfg.extraFlags} ${cfg.mountPoint}";
+        ExecStopPost = "/run/wrappers/bin/fusermount -u ${cfg.mountPoint}";
+        Restart = "on-failure";
+	PrivateTmp = true;


There's some padding missing here. When fixing that, please also squash all the commits and push again.

Done :) I hope that will suffice

kbfs service: Use `%h/keybase` as the default mountpoint kbfs service: remove createMountPoint opt & move pre/post steps to serviceConfig kbfs service: Remove package option kbfs service: Add `after = keybase.service` to systemd unit kbfs service: Use private tmp keybase service: Remove package option keybase service: Use private tmp kbfs service: Fix `PrivateTmp` indentation

cmacrae · 2017-05-18T08:51:40Z

Hopefully that'll do it :) Only thing now is to work out the above (the creation of this $HOME directory)

peterhoeg · 2017-05-22T00:41:00Z

@cmacrae, it works perfectly fine here without any odd directories showing up. Did you by chance set the mountpoint to "$HOME/keybase" instead of "%h/keybase" ?

cmacrae · 2017-05-22T07:14:56Z

@peterhoeg Nope, as the default is now set to %h/keybase I just left that and didn't specify a mountpoint. When I look at this '$HOME' directory, it's only contents are: .local/share/keybase (with all the keybase files residing in that directory).

peterhoeg · 2017-05-23T07:45:52Z

That's so odd - it's working for me.

cmacrae · 2017-05-25T12:13:49Z

@peterhoeg So, it looks like it's the keybase service that's actually creating this directory

peterhoeg · 2017-05-30T00:28:45Z

What does systemctl --user show keybase | grep HOME show?

mpcsh · 2017-07-14T01:01:33Z

@cmacrae I'm interested in running the GUI app, and it requires the mountpoint to be /keybase, so I think it would be worth it to change the mountpoint to that. Also, the keybase service needs to have a requires and after - I think it should be network-online.service - because as it stands, even using services.keybase.enable = true and services.kbfs.enable = true in configuration.nix does nothing

peterhoeg · 2017-07-18T07:10:15Z

it requires the mountpoint to be /keybase

The keybase-gui launch script assumes the fs mounted at /keybase but you can simply launch the binary directly. That's what I do when running keybase-gui as a systemd service:

  systemd.user.services.keybase-gui =
    let
      services = [ "keybase.service" "kbfs.service" ];
    in {
      description = "Keybase GUI";
      requires = services;
      after = services;
      serviceConfig = {
        ExecStart = "${pkgs.keybase-gui}/share/keybase/Keybase";
        PrivateTmp = true;
      };
    };
  };
}

Also, the keybase service needs to have a requires and after - I think it should be network-online.service

That is incorrect. keybase and kbfs both run as user services while network.service and network-online.service are system services. User services don't know anything about system services.

because as it stands, even using services.keybase.enable = true and services.kbfs.enable = true in configuration.nix does nothing

That's a different issue. Can you dump your configuration somewhere and we'll see how to get it working?

mpcsh · 2017-07-31T03:55:11Z

@peterhoeg sorry for the delay! You're right about user services, I wasn't thinking of that. Regardless, here's the relevant portion of my configuration.nix:

environment.systemPackages = with pkgs; [
  keybase
  keybase-gui
  kbfs
];

services.kbfs = {
  enable = true;
  mountPoint = "/keybase";
};

The problem I'm having is that neither keybase.service nor kbfs.service actually start when I log in.

peterhoeg · 2017-08-03T03:25:56Z

Here is my entire keybase.nix config:

{ config, lib, pkgs, ... }:

let
  slice = "keybase.slice";

in {
  services.kbfs = {
    enable = true;
    mountPoint = "%t/kbfs";
    extraFlags = [
      "-label %u"
    ];
  };

  systemd.user.services = {
    keybase.serviceConfig.Slice = slice;
    kbfs = {
      environment = {
        KEYBASE_RUN_MODE = "prod";
      };
      serviceConfig.Slice = slice;
    };

    keybase-gui =
    let
      services = [ "keybase.service" "kbfs.service" ];
    in {
      description = "Keybase GUI";
      requires = services;
      after = services;
      serviceConfig = {
        ExecStart = "${pkgs.keybase-gui}/share/keybase/Keybase";
        PrivateTmp = true;
        Slice = slice;
      };
    };
  };
}

Then systemctl --user start keybase-gui brings up the GUI and starts whatever needs to be started.

kbfs service: init

e72c5b1

cmacrae mentioned this pull request May 8, 2017

Implement a NixOS module for the Keybase Filesystem #22603

Closed

pSub added the 6.topic: nixos label May 9, 2017

Mic92 reviewed May 9, 2017

View reviewed changes

peterhoeg reviewed May 11, 2017

View reviewed changes

cmacrae added 2 commits May 11, 2017 10:59

kbfs service: Use 'on-failure' restart policy for user service

Verified

This commit was signed with the committer’s verified signature.

vcunat Vladimír Čunát

GPG key ID: E747DF1F9575A3AA

Verified
Learn about vigilant mode

8e1316b

kbfs service: Remove explicit 'RestartSec' property from kbfs user se…

Verified

This commit was signed with the committer’s verified signature.

vcunat Vladimír Čunát

GPG key ID: E747DF1F9575A3AA

Verified
Learn about vigilant mode

5e6a4d4

…rvice

cmacrae added 2 commits May 11, 2017 17:20

keybase service: init

f06b503

kbfs service: Require and enable 'keybase' service

aa16f9e

keybase service: Fix indentation for ExecStart

ca18759

peterhoeg self-assigned this May 12, 2017

peterhoeg reviewed May 17, 2017

View reviewed changes

peterhoeg reviewed May 18, 2017

View reviewed changes

peterhoeg merged commit abe0da4 into NixOS:master May 22, 2017

Janik-Haag added the 12. first-time contribution label Jun 12, 2023

kbfs service: init #25610

kbfs service: init #25610

Conversation

cmacrae commented May 8, 2017

Motivation for this change

Things done

peterhoeg commented May 9, 2017

peterhoeg commented May 9, 2017

cmacrae commented May 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mic92 May 9, 2017 • edited Loading

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

Choose a reason for hiding this comment

Mic92 May 9, 2017 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peterhoeg commented May 11, 2017

cmacrae commented May 11, 2017

peterhoeg commented May 11, 2017

cmacrae commented May 11, 2017

cmacrae commented May 11, 2017

cmacrae commented May 11, 2017

peterhoeg commented May 12, 2017

cmacrae commented May 12, 2017

peterhoeg commented May 12, 2017

cmacrae commented May 12, 2017

Choose a reason for hiding this comment

cmacrae May 18, 2017 • edited Loading

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

Choose a reason for hiding this comment

cmacrae May 18, 2017 • edited Loading

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmacrae commented May 18, 2017

peterhoeg May 18, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmacrae commented May 18, 2017

peterhoeg commented May 22, 2017

cmacrae commented May 22, 2017

peterhoeg commented May 23, 2017

cmacrae commented May 25, 2017

peterhoeg commented May 30, 2017

mpcsh commented Jul 14, 2017

peterhoeg commented Jul 18, 2017

mpcsh commented Jul 31, 2017

peterhoeg commented Aug 3, 2017

Mic92 May 9, 2017 •

edited

Loading

Mic92 May 9, 2017 •

edited

Loading

cmacrae May 18, 2017 •

edited

Loading

cmacrae May 18, 2017 •

edited

Loading

peterhoeg May 18, 2017 •

edited

Loading