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

http/sftp: support listening on passed FDs #7801

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

flokli
Copy link

@flokli flokli commented Apr 24, 2024

Instead of the listening addresses specified above, rclone will listen to all FDs passed by the service manager, if any (and ignore any arguments passed by --{{ .Prefix }}addr.

This allows rclone to be a socket-activated service. It can be configured as described in https://www.freedesktop.org/software/systemd/man/latest/systemd.socket.html

It's possible to test this interactively through systemd-socket-activate, firing of a request in a second terminal:

❯ systemd-socket-activate -l 8088 -l 8089 --fdname=foo:bar -- ./rclone serve webdav :local:test/
Listening on [::]:8088 as 3.
Listening on [::]:8089 as 4.
Communication attempt on fd 3.
Execing ./rclone (./rclone serve webdav :local:test/)
2024/04/24 18:14:42 NOTICE: Local file system at /home/flokli/dev/flokli/rclone/test: WebDav Server started on [sd-listen:bar-0/ sd-listen:foo-0/]

What is the purpose of this change?

Closes #7783

Was the change discussed in an issue or in the forum before?

#7783
https://forum.rclone.org/t/systemd-socket-activation-for-rclone-serve/45644

Checklist

  • I have read the contribution guidelines.
  • I have added tests for all changes in this PR if appropriate.
  • I have added documentation for the changes if appropriate.
  • All commit messages are in house style.
  • I'm done, this Pull Request is ready for review :-)

@flokli flokli marked this pull request as ready for review April 24, 2024 14:08
@flokli flokli force-pushed the socket-activation branch 2 times, most recently from 1b6a661 to caa8067 Compare April 24, 2024 15:17
@flokli
Copy link
Author

flokli commented May 2, 2024

poke @ncw :-)

@flokli
Copy link
Author

flokli commented May 3, 2024

Thanks for kicking CI! That seems to be coreos/go-systemd#440. Adding a replace directive to my go.mod gets a GOOS=plan9 go build ./cmd/serve to pass.

@flokli flokli marked this pull request as draft May 3, 2024 16:25
@ncw
Copy link
Member

ncw commented May 4, 2024

Can you stub it out on plan 9?

@flokli
Copy link
Author

flokli commented May 4, 2024

That's what I did upstream, in coreos/go-systemd#440.

If this doesn't get merged there, I could add a little wrapper package in rclone, and do the plan9 stubbing there, but it'd be more code on the rclone side.

Is this the only thing blocking this, or do you have other feedback on the code?

@ncw
Copy link
Member

ncw commented May 5, 2024

Well you could do the same thing in the rclone code with build tags - that is what I was suggesting. Though if your pr is merged that is the neatest way for rclone!

I have looked at the code which looks ok (I'll do a formal review at some point). I would like some explanation of how it works in the docs and example config of how you would use it with systemd would be very useful.

Ones thing that wasn't clear to me - does systemd start a new rclone instance for every incoming connection? Or does it keep things running for a while. I guess this might be in the systemd docs!

@flokli
Copy link
Author

flokli commented May 5, 2024

Yeah, I'm also hoping PR gets merged into go-systemd, as essentially everyone linking against the activation package and building or this arch would do stubbing on their own, and there already is a windows stub...

Regarding systemd configuration, there's a lot of different config knobs, which is why I only linked to the systemd docs. You can bind on Unix sockets, TCP ports, ...

rclone keeps running, and normally gets started lazily on the first connection. If rclone had a parameter to exit the process after a specific time of inactivity, it would also get fully shut down, but that's a bit out of scope for this PR.

@ncw
Copy link
Member

ncw commented May 13, 2024

Hmm, looks like we might have to go for build tags - not a lot of action on that PR...

@flokli
Copy link
Author

flokli commented May 13, 2024

Alright, let me take a look at that

Instead of the listening addresses specified above, rclone will listen to all
FDs passed by the service manager, if any (and ignore any arguments passed by
`--{{ .Prefix }}addr`.

This allows rclone to be a socket-activated service. It can be configured as described in
https://www.freedesktop.org/software/systemd/man/latest/systemd.socket.html

It's possible to test this interactively through `systemd-socket-activate`,
firing of a request in a second terminal:

```
❯ systemd-socket-activate -l 8088 -l 8089 --fdname=foo:bar -- ./rclone serve webdav :local:test/
Listening on [::]:8088 as 3.
Listening on [::]:8089 as 4.
Communication attempt on fd 3.
Execing ./rclone (./rclone serve webdav :local:test/)
2024/04/24 18:14:42 NOTICE: Local file system at /home/flokli/dev/flokli/rclone/test: WebDav Server started on [sd-listen:bar-0/ sd-listen:foo-0/]
```
This was missing the fact rclone also supports listening on Unix Domain
Sockets.
@flokli
Copy link
Author

flokli commented May 13, 2024

I rebased this, and moved the wrapping into our own wrapper package, linking to the upstream issue. also added socket activation to the serve sftp command (which only has a single listener).

@flokli flokli marked this pull request as ready for review May 13, 2024 19:16
@flokli flokli changed the title http: support listening on passed FDs http/sftp: support listening on passed FDs May 14, 2024
@flokli
Copy link
Author

flokli commented May 15, 2024

@ncw sorry for the noise, but can you kick off CI once again?

It fails to build on plan9, which is part of the rclone CI matrix, and
the PR fixing it upstream doesn't seem to be getting traction.

Stub it on our side, we can still remove this once it gets merged.
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.

rclone serve: support systemd socket activation
2 participants