Add systemd service to bitcoind in debian package #12769

ghost · 2018-03-23T13:46:07Z

On suggestion from @TheBlueMatt I have updated contrib/debian files to include a systemd service in the bitcoind build. Tested and working on Ubuntu 16.04 and 17.10.

This fixes Issue #12758

randolf · 2018-03-23T14:24:14Z

@ctp-tsteenholdt Please remember to squash/rebase.

ghost · 2018-03-23T14:57:42Z

@randolf That makes total sense. My apologies - looking into it right now...

laanwj · 2018-03-26T19:36:22Z

We already have a bitcoind.service in contrib/init/ - what is the difference with this one, can we avoid duplicating it?

ghost · 2018-03-26T19:37:39Z

@randolf The pull request should look better now - please let me know if I'm missing something.

ghost · 2018-03-27T11:02:36Z

@laanwj I'm completely with you on that question, but I don't think we can feasibly avoid multiple versions of a file like this. The different versions of the bitcoind.service in the contrib/ tree have subtle differences to make them work best for their particular purpose. The differences reflect distro-specifics such as location of environment files, dependency services etc.

Also, building a package for Ubuntu or Debian requires a full copy of the contrib/debian/ folder, including the bitcoind.service file, to allow the packaging helper dh to recognize, treat and install it correctly.

It think distro-specific files are probably better maintained within distro specific folders such as contrib/debian/ or contrib/rpm/, as part of the overall package maintenance for that distro. This avoids possible breakage from a change made for a different distro too.

To underline that the contrib/init/ version is still very relevant, it will be the goto file for people on platforms without a specific package or people who prefer to build from source. This file can be easily adapted to work well on most systems.

Please let me know what you think.

laanwj · 2018-03-27T14:16:11Z

@ctp-tsteenholdt If it is intentional, then IMO it's not a problem. Arguably the debian-specific file is easier to maintain because at least it aims for a specific environment and can be tested there, not 'all distributions in general', which has made the file under contrib/init/ a real pain in practice (see e.g. discussion in #12255).

ghost · 2018-03-27T15:03:50Z

@laanwj That's exactly right.

laanwj · 2018-03-28T11:30:20Z

Would be good to have an ACK from @TheBlueMatt here.

TheBlueMatt

Looks good, though I didnt test any of it.

TheBlueMatt · 2018-03-28T17:54:10Z

contrib/debian/bitcoind.postrm

+	echo "#  The bitcoin user (${BCUSER}) and data dir (${BCHOME})"
+	echo "#  were left intact."
+	echo "#"
+	echo "#  After backing up all vital data, cleanup can be completed"


Maybe make it explicit that they should check /var/lib/bitcoin for things like wallets?

TheBlueMatt · 2018-03-28T17:54:51Z

contrib/debian/bitcoind.service

+After=network.target
+
+[Service]
+ExecStart=/usr/bin/bitcoind -daemon -conf=/etc/bitcoin/bitcoin.conf -pid=/run/bitcoind/bitcoind.pid


May be nice to make the datadir explicit so that we dont have /var/lib/bitcoin/.bitcoin and instead just put stuff in /var/lib/bitcoin?

TheBlueMatt · 2018-03-28T17:56:14Z

contrib/debian/rules


 override_dh_auto_clean:
 	if [ -f Makefile ]; then $(MAKE) distclean; fi
 	rm -rf Makefile.in aclocal.m4 configure src/Makefile.in src/bitcoin-config.h.in src/build-aux src/qt/Makefile.in src/qt/test/Makefile.in src/test/Makefile.in

 QT=$(shell dpkg-vendor --derives-from Ubuntu && echo qt4 || echo qt5)
+# qt4 is very broken on arm


Can you pull the backporting of existing debian/ contents out into a separate commit to make it easier to track?

ghost · 2018-03-28T21:45:27Z

Thanks @TheBlueMatt. Each of your comments make perfect sense and I'll have them all addressed.

ghost · 2018-03-29T16:45:59Z

I've addressed the suggestions made by @TheBlueMatt, run a test dpkg-buildpackage using the updated contrib/debian/ tree and tested that the package changes work as expected, with success.

The commits have been updated to reflect the changes.

laanwj · 2018-04-07T13:55:49Z

@TheBlueMatt can you take another look?
Would be good to get this merged, I think.

TheBlueMatt · 2018-04-17T19:12:14Z

Hmm, if you're gonna pull some of the debian changes upstream, can you just pull all of them? Especially getting the changelog and other stuff out of sync seems not ideal.

TheBlueMatt · 2018-04-17T19:13:59Z

contrib/debian/bitcoind.postinst

+if [ "$1" = "configure" ]; then
+
+	# Add bitcoin user/group - this will gracefully abort if the user already exists.
+	# A homedir is never created.


Why not create the directory? If its empty it will be deleted on uninstall, but if ther user ever tries to use the service things will simply fail to start, no?

The reason for this approach is that, if for whatever reason the user has changed ownership or permissions on the /var/lib/bitcoin directory, we don't want to mess with that on operations like reconfigure, update etc.

adduser does not let me specify custom homedir permissions, so I'm using --no-create-home to work around that. A missing home directory becomes a stable indicator that this is a fresh install, making it safe for me to create the directory and set the restrictive default permissions.

Oh, I misread the script, sorry.

ghost · 2018-04-18T14:38:40Z

@TheBlueMatt I'm not sure what you mean by:

Hmm, if you're gonna pull some of the debian changes upstream, can you just pull all of them? Especially getting the changelog and other stuff out of sync seems not ideal.

Upstream in this case would essentially be your PPA right? Or is there somewhere else?

I can certainly take a look at any other differences between this tree and yours and see if it makes sense to include in this PR or create a separate one for getting those in sync? Is this what you mean?

TheBlueMatt · 2018-04-18T14:42:06Z

Yea, just pull the stuff from the PPA's debian tar.

ghost · 2018-04-18T15:22:21Z

Alright - I'm looking in to it...

ghost · 2018-04-18T19:26:47Z

@TheBlueMatt so I took all the changes from your upstream PPA debian tar and committed them as a single commit. After that, I committed the Systemd changes in a separate commit.

As a precaution I've run a local build and made sure that things still work as expected.

fanquake · 2018-04-20T01:50:42Z

contrib/debian/bitcoin-qt.desktop

@@ -10,5 +10,4 @@ Terminal=false
 Type=Application
 Icon=bitcoin128
 MimeType=x-scheme-handler/bitcoin;
-Categories=Office;Finance;P2P;Network;Qt;
-StartupWMClass=Bitcoin-qt
+Categories=Office;Finance;


These additional categories were just added in #12854

@fanquake thanks. I'll get that sorted out here...

Adding systemd service for bitcoind, to provide for a simpler out-of-the-box experience. Configuration file is /etc/bitcoin/bitcoin.conf. This file is a copy of the sample configuration file. The service user 'bitcoin' is added during install. Its homedir is in '/var/lib/bitcoin'. bitcoind.service is disabled by default to allow the user to configure it, before starting it the first time. On package purge, the 'bitcoin' user as well as its homedir is left intact, to not accidentally remove a wallet or something of equal importance. Instead the user is presented with information on how to perform the cleanup manually, after making sure all important data has been backed up.

ghost · 2018-04-20T10:40:50Z

So I re-resolved the conflict with #12854. This time without nuking the accepted changes of that PR. Sorry about that. Thanks @fanquake for the heads up.

laanwj · 2018-04-26T18:48:56Z

@TheBlueMatt is this now ready for merge?

ghost · 2018-04-26T21:43:08Z

Just tested on Ubuntu 18.04, with no issues related to this PR. There is an unrelated issue with the libdb++ version that needs to be sorted out, so I built with --with-incompatible-bdb to test things out.

TheBlueMatt

Looks good modulo the one restart question I had.

TheBlueMatt · 2018-04-27T18:21:28Z

contrib/debian/rules

+# Restart after upgrade
+override_dh_systemd_start:
+	dh_systemd_start \
+		--restart-after-upgrade


Why? I dont think anyone would be surprised by their bitcoind going down prior to upgrade instead of only being restarted, and this could break eg a lxc VM as you'll get a text file busy error.

@TheBlueMatt, good point.

--restart-after-upgrade is default in dh_systemd_start from compat level 10, so the reason is perhaps best described as emulating modern defaults. I agree that for bitcoind, there should not be any real issues with using the defaults.

I don't quite follow the problems you mention though. As far as I can tell, an upgrade should work perfectly, even when leaving the daemon running during the upgrade. Nothing runtime related is touched during the package upgrade, only the new bitcoind and bitcoin-cli binaries are put into place, making them ready for the late restart. Unless I'm missing something, this should be no different when running in a container?

If you feel there's a risk here, I'll happily remove this!

Eh, if its default whatever, but linux generally refuses to replace binaries on disk if they are running inside of an LXC/docker container (though will happily do so during normal operation).

Got it. I've never run into that problem, but haven't really come across a situation where such a setup would be needed. I tend to want to install apps like bitcoind inside my containers, in which case this would not be an issue.

But if I wanted this kind of setup, I imagine I'd leave bitcoind disabled on the host and only start the service inside the container, right?

If this is true, then the host level daemon would never be restarted on upgrade as the service is disabled, causing the restart to be skipped on upgrade. The container on the other hand would keep running the "old" in-memory binary, until it's restarted by some other means.

I could be missing something...

TheBlueMatt · 2018-04-27T18:53:53Z

utACK 2a87b1b with or without restart-after-upgrade.

luke-jr · 2018-04-27T21:05:08Z

How will this affect systems that don't have/use systemd?

ghost · 2018-04-28T00:07:45Z

@luke-jr this affects only debian-based systems (including Ubuntu) with systemd.

TheBlueMatt · 2018-04-28T00:20:43Z

I believe @luke-jr's question was "what about older but still supported Debian-based systems that do not have systemd", which is a good one. WIll this not result in them failing to build the packages?

ghost · 2018-04-28T00:47:54Z

Older debian systems that are missing the dh-systemd package, will fail to build the packages, yes. I believe the (supported) Debian/Ubuntu versions that are affected by this, are Debian 7 (LTS version is EOL at the end of May 2018) and Ubuntu 14.04 LTS (EOL in April 2019).

I'm running a test on Ubuntu 14.04 right now, to see what kind of issues we're running into on that platform.

ghost · 2018-04-28T01:57:36Z

As it turns out, Ubuntu 14.04 LTS actually does have a dh-systemd package and apart from the libdb4.8++-dev build dependency, that was also causing problems on Ubuntu 18.04, the build is working just fine.

Since the distro does not actually use systemd, the included bitcoind.service simply does nothing.

FWIW, Debian 7 also has a dh-systemd package (in the backports repository), so I guess it's the same story there.

luke-jr · 2018-04-28T02:59:16Z

I'm less concerned with merely old distros, as much as ones that have simply rejected the mandatory-systemd nonsense, such as Devuan.

ghost · 2018-04-28T08:28:00Z

@luke-jr to build the package, you'd need to have dh-systemd installed, but you do not need systemd to install or use it. If you don't have systemd on the system, you simply can't use the systemd service configuration that's included here.

ghost · 2018-05-01T13:45:28Z

Did that last comment answer your questions?

laanwj · 2018-05-01T14:52:37Z

I'm going to merge this as @TheBlueMatt utACKed this - he's the ppa maintainer.
I don't think other distributions are relevant here as this concerns explicitly the debian directory.
Other improvements can be made later.

TheBlueMatt · 2018-05-01T14:53:55Z

Err, no, Luke's point is good and would break several PPA-supported releases.

…

On May 1, 2018 2:52:47 PM UTC, "Wladimir J. van der Laan" ***@***.***> wrote: I'm going to merge this as @TheBlueMatt utACKed this - he's the ppa maintainer. I don't think other distributions are relevant here as this concerns explicitly the `debian` directory. Other improvements can be made later. -- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: #12769 (comment)

@TheBlueMatt

2a87b1b Add systemd service for bitcoind (ctp-tsteenholdt) 9085532 Sync contrib/debian from Matt Corallo's PPA (ctp-tsteenholdt) Pull request description: On suggestion from @TheBlueMatt I have updated `contrib/debian` files to include a systemd service in the `bitcoind` build. Tested and working on Ubuntu 16.04 and 17.10. This fixes Issue #12758 Tree-SHA512: b6137fafee940c7410df1242c8716a87f47c5bc60eb8df3ad0184a50c2d67ef3f2728761c742670a0ad546ab6e7ad60472a721350cd6280b3bcbdc582e50ee07

TheBlueMatt · 2018-05-01T15:04:44Z

Oh, hmm, I appears to be wrong, it looks like the dh-systemd package is on trusty, I suppose its fine, then.

After discussion with Matt on IRC, this is not ready for prime time until 2019 and shouldn't have been ACKed and merged. - Revert "Add systemd service for bitcoind" This reverts commit 2a87b1b. - Revert "Sync contrib/debian from Matt Corallo's PPA" This reverts commit 9085532. Tree-SHA512: 439f4ccc3e196011af448b220adf26b0e653ac589bf4cfbbc276c1500c9d08f209c9d6101e4d232857779d9f25164cfb222ed30e3d63de116f9121e6ebde31c3

laanwj · 2018-05-01T15:38:19Z

Had to revert this, please file again in 2019 (or whenever relevant).

ghost · 2018-05-01T15:38:20Z

@laanwj, as @TheBlueMatt mentions in his followup, dh-systemd is indeed available on the relevant distributions - It certainly is on all supported versions of Ubuntu, Debian and Devuan. I can't see a reason to hold off on the merge?

laanwj · 2018-05-01T16:15:40Z

After discussion on IRC we're switching to a different strategy, distribution packaging files will be under this repository from now on:
https://github.com/bitcoin-core/packaging
(see also #13137)

ghost · 2018-05-01T16:28:10Z

Okay... Does this mean I should rework the changes to go in the new path then?

maflcko · 2018-05-01T16:54:28Z

Indeed, just cherry-pick the commits and submit them to the other repo.

ghost · 2018-05-02T12:21:04Z

Thanks. I've created bitcoin-core/packaging#1 with exact same changes...

After discussion with Matt on IRC, this is not ready for prime time until 2019 and shouldn't have been ACKed and merged. - Revert "Add systemd service for bitcoind" This reverts commit 2a87b1b. - Revert "Sync contrib/debian from Matt Corallo's PPA" This reverts commit 9085532. Tree-SHA512: 439f4ccc3e196011af448b220adf26b0e653ac589bf4cfbbc276c1500c9d08f209c9d6101e4d232857779d9f25164cfb222ed30e3d63de116f9121e6ebde31c3

randolf approved these changes Mar 23, 2018

View reviewed changes

ghost closed this Mar 23, 2018

ghost reopened this Mar 23, 2018

laanwj added the Scripts and tools label Mar 27, 2018

TheBlueMatt reviewed Mar 28, 2018

View reviewed changes

TheBlueMatt reviewed Apr 17, 2018

View reviewed changes

fanquake reviewed Apr 20, 2018

View reviewed changes

ctp-tsteenholdt added 2 commits April 20, 2018 08:31

Sync contrib/debian from Matt Corallo's PPA

9085532

TheBlueMatt reviewed Apr 27, 2018

View reviewed changes

laanwj merged commit 2a87b1b into bitcoin:master May 1, 2018

ghost mentioned this pull request May 2, 2018

Add systemd service for bitcoind to debian package bitcoin-core/packaging#1

Merged

maflcko mentioned this pull request Apr 26, 2020

Add systemd service for debian bitcoind package (contrib) #12758

Closed

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

Add systemd service to bitcoind in debian package #12769

Add systemd service to bitcoind in debian package #12769

Conversation

ghost commented Mar 23, 2018 • edited by ghost

randolf commented Mar 23, 2018

ghost commented Mar 23, 2018

laanwj commented Mar 26, 2018

ghost commented Mar 26, 2018

ghost commented Mar 27, 2018

laanwj commented Mar 27, 2018

ghost commented Mar 27, 2018

laanwj commented Mar 28, 2018

TheBlueMatt 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

ghost commented Mar 28, 2018

ghost commented Mar 29, 2018

laanwj commented Apr 7, 2018 • edited

TheBlueMatt commented Apr 17, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost commented Apr 18, 2018

TheBlueMatt commented Apr 18, 2018 • edited

ghost commented Apr 18, 2018

ghost commented Apr 18, 2018

fanquake Apr 20, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost commented Apr 20, 2018

laanwj commented Apr 26, 2018

ghost commented Apr 26, 2018 • edited by ghost

TheBlueMatt 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

TheBlueMatt commented Apr 27, 2018

luke-jr commented Apr 27, 2018

ghost commented Apr 28, 2018

TheBlueMatt commented Apr 28, 2018

ghost commented Apr 28, 2018 • edited by ghost

ghost commented Apr 28, 2018 • edited by ghost

luke-jr commented Apr 28, 2018

ghost commented Apr 28, 2018

ghost commented May 1, 2018

laanwj commented May 1, 2018

TheBlueMatt commented May 1, 2018 via email

TheBlueMatt commented May 1, 2018

laanwj commented May 1, 2018

ghost commented May 1, 2018 • edited by ghost

laanwj commented May 1, 2018 • edited

ghost commented May 1, 2018

maflcko commented May 1, 2018

ghost commented May 2, 2018

ghost commented Mar 23, 2018 •

edited by ghost

laanwj commented Apr 7, 2018 •

edited

TheBlueMatt commented Apr 18, 2018 •

edited

fanquake Apr 20, 2018 •

edited

ghost commented Apr 26, 2018 •

edited by ghost

ghost commented Apr 28, 2018 •

edited by ghost

ghost commented Apr 28, 2018 •

edited by ghost

ghost commented May 1, 2018 •

edited by ghost

laanwj commented May 1, 2018 •

edited