Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add systemd service file example #5583

Closed
wants to merge 2 commits into from
Closed

add systemd service file example #5583

wants to merge 2 commits into from

Conversation

sneak
Copy link

@sneak sneak commented Oct 10, 2018

No description provided.

@sneak sneak requested a review from Kubuxu as a code owner October 10, 2018 19:09
License: MIT
Signed-off-by: Jeffrey Paul <jp@eeqj.com>
Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is way too customized to be generally useful. We should probably provide systemd unit files, but they need to be general purpose.

@sneak
Copy link
Author

sneak commented Oct 11, 2018

It's not autoinstalled or anything, it's just an example file for the user to configure, with defaults that work on the most commonly deployed systemd distributions. It's not part of a distribution package or anything.

@sneak
Copy link
Author

sneak commented Oct 11, 2018

It took me some time to suss out all of these various systemd-related quirks and I am contributing it simply in the hopes that it will same someone else that amount of time. It's not exceedingly system-specific in anything other than perhaps the choice of StorageMax. It's rather aggressive in enabling experimental features but that's about it?

@sneak
Copy link
Author

sneak commented Oct 11, 2018

You've requested changes; could you please be more specific about what "way too customized" means? Is it the assumption of installation in /usr/local/bin? Is it the setting of StorageMax? Is it the experimental features? Is it the adduser? I can't make requested changes without this sort of feedback. I submitted this because I thought it was generally useful for the majority of cases.

Copy link
Contributor

@lanzafame lanzafame left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sneak The overall issue with this service file is that with many ways that ipfs can be configured to run it makes too many assumptions about how the ipfs binary has been installed and how the user wants to use it and the system that it is installed on. To be clear, the PR is appreciated but this service file is specific to how you use ipfs and how it is installed and configured on your machine.

ExecStartPre=-mkdir /var/lib/ipfs
ExecStartPre=-/bin/chown ipfs:ipfs /var/lib/ipfs
ExecStartPre=-/bin/chmod u+rwx /var/lib/ipfs
ExecStartPre=-chpst -u ipfs /usr/local/bin/ipfs init --profile=badgerds
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sneak chpst which is apart of runit isn't available by default on all systems.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no way to run some of the ExecStartPres as root (e.g. the adduser/chown/chmod) and others as the user in question (ipfs). Either all are run as root or all are run as the User specified (depending on PermissionsStartOnly). Because root is required for making sure the user exists and the directory has the correct owner/permissions, chpst is required to do the ipfs init as the correct user. The fact that it is not installed by default is not our concern; ipfs is also not installed by default and /usr/local/bin/ipfs will not be available unless the user installs that.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additionally, if someone tries to use this unit file and it fails, checking the systemd journal will show that they need chpst quite plainly.

misc/systemd/ipfs.service Outdated Show resolved Hide resolved
ExecStartPre=-/bin/chown ipfs:ipfs /var/lib/ipfs
ExecStartPre=-/bin/chmod u+rwx /var/lib/ipfs
ExecStartPre=-chpst -u ipfs /usr/local/bin/ipfs init --profile=badgerds
ExecStartPre=-chpst -u ipfs /usr/local/bin/ipfs config profile apply server
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line 26-27 are assumptions that don't aren't useful to specify in a service file as they don't need to be run on each time the service start and are specific to the user.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This being a systemwide service file assumes that the "user" is root and that there is one ipfs daemon running on the system - just like ssh, or httpd, or any other system service. They don't need to be run each time but otherwise there are additional steps to installing/starting ipfs and it doesn't hurt anything to run them each startup.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They don't need to be run each time but otherwise there are additional steps to installing/starting ipfs and it doesn't hurt anything to run them each startup.

They will run each time the service is started or restarted, which also means that the configuration as specified in $IPFS_PATH/config will be overwritten each time. Sure it is great for when you are installing ipfs the first time but is annoying everytime after that.

Copy link
Author

@sneak sneak Oct 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have not found this systemd unit file to be annoying in several weeks of use, and if an administrator that uses the example file does, it is trivial to remove any lines that they find to be cumbersome or unnecessary. If those lines are removed, the daemon will not start, or will potentially get the user's hosting account disabled automatically (see #4343 and #1226, the reason for the profile feature in the first place). They are essential to the correct functioning of the daemon's first run as ipfs is currently implemented.

[Unit]
description=ipfs p2p daemon
After=network.target
Requires=network.target
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A ipfs daemon doesn't actually need a network to start and the user can interact with several functions of ipfs without a network connection. Granted most people will want a network connection but part of the issue with putting examples in the repo is that people will take them as how things 'should' or 'must' work and that isn't necessarily the case.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now you're just being pedantic. There is zero point to using the ipfs daemon without a network connection; all of the local-only options work fine without a daemon. You certainly don't want the ipfs daemon starting up before the network as it discovers its network interfaces on startup.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The commands do work offline but, in this case, they'll only work if you happen to be the ipfs user or root.

Side note: Units generally should not use Requires=network.target, only After=network.target (see the documentation in man 7 systemd.special).

Environment=HOME=/var/lib/ipfs
LimitNOFILE=8192
Environment=IPFS_FD_MAX=8192
EnvironmentFile=-/etc/sysconfig/ipfs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The config file shouldn't exist at this path, see here

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dash means that this service file works fine even if the file doesn't exist.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, what I mean is /etc/sysconfig/<program-name> is not the convention for systemd service configuration.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall I remove this line? If not, to which value would you like it changed?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/etc/ipfs

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sysconfig is a a redhat thing. The recommended way to customize a systemd unit (or set an environment variable) is to put a drop-in in /etc/systemd/system/ipfs.service.d/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Stebalien That is for changes to the systemd unit, I was assuming this was referring to the location of the ipfs configuration itself, which as I am writing this I am realising that ipfs puts both config and data together in the IPFS_PATH so as ipfs currently works, there is no point defining a /etc/ipfs as it won't get used for the config file.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, that line is actually sourcing the environment variables in /etc/sysconfig/ipfs (not related to the IPFS config).

EnvironmentFile=-/etc/sysconfig/ipfs
StandardOutput=journal
WorkingDirectory=/var/lib/ipfs
ExecStartPre=-adduser --system --group --home /var/lib/ipfs ipfs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this work on a multi-user machine? Does it mean any non-admin user is stuck having ipfs running in the background and with the server profile enable with no way of changing it to a more appropriate profile or stop ipfs completely?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for a service started by root, which is why it uses /var/lib/ipfs - just like ssh, or httpd, or any other system service. This is entirely normal for a systemd service file. The "non-admin user" mode of a unix system is mostly outmoded these days anyway. This isn't a time-sharing mainframe from the 70s, this is a way to run a daemon on a standard distribution quickly for a sysadmin in 2018 with sane defaults.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "non-admin user" mode of a unix system is mostly outmoded these days anyway.

This is anyone who shares a linux machine with other people in a household? Not everyone has the luxury of having their own machine. But I will take the answer as this service file doesn't work in multi-user scenario by design.

Copy link
Author

@sneak sneak Oct 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is anyone who shares a linux machine with other people in a household

Is this the use case we are targeting? All 75 of those people?

Restart=always
PermissionsStartOnly=true
Nice=18
StateDirectory=/var/lib/ipfs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have an issue with /var/lib/ipfs being the place that the repo gets stored but considering the default StorageMax of 4TB, which is too large for a general purpose service file, but also most suggested partitioning schemes for linux distros when installing a new distro, places more storage towards the /home directory instead of the / directory.

Copy link
Author

@sneak sneak Oct 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The administrator of a service (or container) can mount storage wherever they'd like. This stores state data in /var/lib like every other service stores state data in /var/lib: /var/lib/httpd, /var/lib/ldap, /var/lib/mysql, et c. All of these services store as much data as they are instructed to in /var/lib. If you have a problem with this you have a problem with LSB. We didn't start the fire.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, this isn't quite the correct usage of StateDirectory. It's primarily used for the DynamicUser feature and should be relative (that is, just ipfs).

So, we should either drop this or switch this unit to a dynamic user. If we go with the latter, we'd want to:

  1. Not have a tmpfiles/sysusers config file and not create a user account.
  2. Set DynamicUser=true
  3. Set StateDirectory=ipfs

The issue with this is that, if there's a UID conflict, systemd will have to chown the entire directory tree (which, depending on the datastore, could be very expensive).

On the other hand, it means one config file versus 3.

ExecStartPre=-/bin/chmod u+rwx /var/lib/ipfs
ExecStartPre=-chpst -u ipfs /usr/local/bin/ipfs init --profile=badgerds
ExecStartPre=-chpst -u ipfs /usr/local/bin/ipfs config profile apply server
ExecStartPre=-chpst -u ipfs /usr/local/bin/ipfs config Datastore.StorageMax "4000GB"
Copy link
Contributor

@lanzafame lanzafame Oct 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4TB does not fit into sane defaults.

Copy link
Author

@sneak sneak Oct 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To what value would you like it changed? 4TB is well within the storage capacity of most small hard drives sold today. I have over 10TB in my backpack and 1TB in my pocket. It also does not immediately use 4TB as you well know, it simply approaches this limit as things are cached or pinned. For reference, other services such as MySQL or OpenLDAP or Postgres will happily use as much space in /var/lib as is required to store whatever they are asked to store.

I can see 100KiB or 100PiB being viewed as "insane", however, 50% of the size of the most commonly sold HDD today may be suboptimal or disagreeable, but is certainly sane.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lanzafame To what value would you like it changed?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really something that should be provided by the user via the config file. It is by is nature not something particularly generic. Thinking about this again, service files shouldn't need to be changed to adjust what is a config value.

@@ -0,0 +1,31 @@
[Unit]
description=ipfs p2p daemon
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/description/Description

EnvironmentFile=-/etc/sysconfig/ipfs
StandardOutput=journal
WorkingDirectory=/var/lib/ipfs
ExecStartPre=-adduser --system --group --home /var/lib/ipfs ipfs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I was testing this service file out and discovered that adduser is a debian specific cmd.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lanzafame Yes, this works on Ubuntu, Mint, Debian, et c. Do you want this line removed? It means that in the majority case it requires additional commands (beyond this service file) to make the daemon start correctly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It just needs to use useradd instead

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The correct way to do this with systemd is to write a sysusers.d file:

u ipfs - "ipfs daemon" /var/lib/ipfs

Running an add-user command on start is very unusual for a systemd unit.

License: MIT
Signed-off-by: Jeffrey Paul <jp@eeqj.com>
[Unit]
description=ipfs p2p daemon
After=network.target
Requires=network.target
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The commands do work offline but, in this case, they'll only work if you happen to be the ipfs user or root.

Side note: Units generally should not use Requires=network.target, only After=network.target (see the documentation in man 7 systemd.special).

Requires=network.target

[Service]
Type=simple
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to reiterate the default.

RestartSec=1
Restart=always
PermissionsStartOnly=true
Nice=18
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no reason to specify this in the default unit. The user can use a drop-in to set it if desired but it's better to leave defaults at their default values for predictability.

Environment=HOME=/var/lib/ipfs
LimitNOFILE=8192
Environment=IPFS_FD_MAX=8192
EnvironmentFile=-/etc/sysconfig/ipfs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sysconfig is a a redhat thing. The recommended way to customize a systemd unit (or set an environment variable) is to put a drop-in in /etc/systemd/system/ipfs.service.d/

LimitNOFILE=8192
Environment=IPFS_FD_MAX=8192
EnvironmentFile=-/etc/sysconfig/ipfs
StandardOutput=journal
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the default.

EnvironmentFile=-/etc/sysconfig/ipfs
StandardOutput=journal
WorkingDirectory=/var/lib/ipfs
ExecStartPre=-adduser --system --group --home /var/lib/ipfs ipfs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The correct way to do this with systemd is to write a sysusers.d file:

u ipfs - "ipfs daemon" /var/lib/ipfs

Running an add-user command on start is very unusual for a systemd unit.

StandardOutput=journal
WorkingDirectory=/var/lib/ipfs
ExecStartPre=-adduser --system --group --home /var/lib/ipfs ipfs
ExecStartPre=-mkdir /var/lib/ipfs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As with the user account creation, the systemd way to do this is to write a tmpfiles.d file:

d /var/lib/ipfs 0750 ipfs ipfs -

ExecStartPre=-/bin/chmod u+rwx /var/lib/ipfs
ExecStartPre=-chpst -u ipfs /usr/bin/env ipfs init --profile=badgerds
ExecStartPre=-chpst -u ipfs /usr/bin/env ipfs config profile apply server
ExecStartPre=-chpst -u ipfs /usr/bin/env ipfs config Datastore.StorageMax "4000GB"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep the example systemd config to the defaults. Users can customize it how they will.

ExecStartPre=-chpst -u ipfs /usr/bin/env ipfs init --profile=badgerds
ExecStartPre=-chpst -u ipfs /usr/bin/env ipfs config profile apply server
ExecStartPre=-chpst -u ipfs /usr/bin/env ipfs config Datastore.StorageMax "4000GB"
ExecStart=/usr/bin/env ipfs daemon --enable-namesys-pubsub --enable-pubsub-experiment --migrate=true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pass --init to initialize the repo if not already initialized. That way, we can skip the ExecStartPre command.

Restart=always
PermissionsStartOnly=true
Nice=18
StateDirectory=/var/lib/ipfs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, this isn't quite the correct usage of StateDirectory. It's primarily used for the DynamicUser feature and should be relative (that is, just ipfs).

So, we should either drop this or switch this unit to a dynamic user. If we go with the latter, we'd want to:

  1. Not have a tmpfiles/sysusers config file and not create a user account.
  2. Set DynamicUser=true
  3. Set StateDirectory=ipfs

The issue with this is that, if there's a UID conflict, systemd will have to chown the entire directory tree (which, depending on the datastore, could be very expensive).

On the other hand, it means one config file versus 3.

@Stebalien
Copy link
Member

(closing due to inactivity, please comment if you'd like this reopened)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants