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

[RFC] implement systemd socket activation #5514

Closed
wants to merge 3 commits into from

Conversation

Stebalien
Copy link
Member

@Stebalien Stebalien commented Sep 24, 2018

It works but it could probably be prettier. Thoughts?

Basically, this patch checks for sockets passed via systemd socket-activation and, if present, ignores all other configured listen addresses and uses those. Currently, it only supports socket activation of the gateway and the API.

Motivation: this allows me to auto-start IPFS when I actually need it (but not until then).

@Stebalien Stebalien requested a review from Kubuxu as a code owner September 24, 2018 04:49
@ghost ghost assigned Stebalien Sep 24, 2018
@ghost ghost added the status/in-progress In progress label Sep 24, 2018
@Stebalien Stebalien changed the title implement systemd socket activation [RFC] implement systemd socket activation Sep 24, 2018
@Stebalien Stebalien added kind/feature A new feature need/community-input Needs input from the wider community labels Sep 24, 2018

var log = logging.Logger("socket-activation")

var socketsMu sync.Mutex
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick:

var (
    socketsMu sync.Mutex
    sockets map[string][]manet.Listener
)

Makes the association of the mutex a little clearer.

if err != nil {
return nil, fmt.Errorf("serveHTTPApi: invalid API address: %q (err: %s)", apiAddr, err)
return nil, fmt.Errorf("serveHTTPGateway: socket activation failed: %s", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

s/serveHTTPGateway/serveHTTPApi

@@ -0,0 +1,9 @@
[Unit]
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 configuration work, in terms of users vs system config? Where is the repo initialized when enabling this service, if this enabled as a system service instead of as a user one? For example, the Arch Linux Wiki suggests starting ipfs as a systemd user service. This probably doesn't need to be solved here but I feel if we are going to be putting out an 'official' service file it is something that we should take into consideration.

Copy link
Member Author

Choose a reason for hiding this comment

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

I just put these here as starting points. The one I use is:

[Unit]
Description=IPFS Daemon

[Service]
User=ipfs
Group=ipfs
NoNewPrivileges=true
ProtectHome=true
ProtectSystem=full
PrivateTmp=true
PrivateDevices=true
SystemCallArchitectures=native
MemoryDenyWriteExecute=true
Environment=IPFS_PATH=/var/lib/ipfs
Environment=HOME=/var/lib/ipfs
ExecStart=/usr/bin/ipfs daemon --routing=dhtclient --migrate=true
KillSignal=SIGINT

[Install]
WantedBy=default.target
Also=ipfs-api.socket
Also=ipfs-gateway.socket

Service=ipfs.service
FileDescriptorName=io.ipfs.api
BindIPv6Only=true
ListenStream=127.0.0.1:5001
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be great if the port values were retrieved from the ~/.ipfs/config or even better ~/.config/ipfs/config or some suitable configuration directory. But whatever the solution, this will be confusing for users, i.e. enable systemd sockets, at some point in the future want to change port values, update ~/.ipfs/config, does nothing, restart sockets, still does nothing, google it/remembers to change in systemd files.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah but there's really nothing we can do about that. systemd doesn't let one read a config file.

Well... technically I can write a generator and generate these socket files dynamically. However, that probably isn't worth it.


We could, alternatively, listen on the union of addresses.

Copy link
Contributor

Choose a reason for hiding this comment

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

You could use an EnvironmentFile?

License: MIT
Signed-off-by: Steven Allen <[email protected]>
License: MIT
Signed-off-by: Steven Allen <[email protected]>
@Stebalien Stebalien force-pushed the feat/systemd-activation branch from 30cf7d5 to 39ff597 Compare September 24, 2018 16:10
License: MIT
Signed-off-by: Steven Allen <[email protected]>
@ghost
Copy link

ghost commented Sep 24, 2018

I like the use case -- but how about a multiaddr? :) Could be systemd-specific, or something more general about file descriptors.

@lanzafame
Copy link
Contributor

@lgierth wouldn't that require systemd to understand what a multiaddr is?

@ghost
Copy link

ghost commented Sep 25, 2018

The systemd unit wouldn't change, but you'd have go-ipfs explicitly configured to listen on e.g. /fd/42 or /systemd/fd/io.ipfs.go.

@lanzafame
Copy link
Contributor

The systemd unit wouldn't change, but you'd have go-ipfs explicitly configured to listen on e.g. /fd/42 or /systemd/fd/io.ipfs.go.

Sorry, I am only two coffees into my day, would /systemd/fd/io.ipfs.go be passed to which service file directive?

@ghost
Copy link

ghost commented Sep 25, 2018

would /systemd/fd/io.ipfs.go be passed to which service file directive?

It would be the same as this PR, except instead of just optimistically looking for systemd file descriptors, go-ipfs would have e.g. ipfs config Addresses.Gateway /systemd/io.ipfs.api set. We wouldn't add a new way of overriding the go-ipfs config, and we'd have something that'd also work for transports later (e.g. /systemd/fd/io.ipfs.ws/tcp/443/wss)

@Stebalien Stebalien mentioned this pull request Oct 11, 2018
@Stebalien
Copy link
Member Author

This was a random "let's try this PR". I'll try to revive it at some future point but I'll close it for now.

@Stebalien Stebalien closed this Oct 18, 2018
@ghost ghost removed the status/in-progress In progress label Oct 18, 2018
@Stebalien Stebalien mentioned this pull request Sep 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature A new feature need/community-input Needs input from the wider community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants