Add systemd socket activation support #2095
Open
seercat
wants to merge 3 commits from
seercat/continuwuity:cat/socket-activation into main
pull from: seercat/continuwuity:cat/socket-activation
merge into: continuwuation:main
continuwuation:main
continuwuation:renovate/cargo-bins-cargo-binstall-1.x
continuwuation:aranje/illegal-car-mods
continuwuation:renovate/rust-1.x
continuwuation:morgan/arm64-runner
continuwuation:renovate/rand_core-0.x
continuwuation:renovate/ipaddress-0.x
continuwuation:renovate/base64-0.x
continuwuation:renovate/ruma-digest
continuwuation:next/meta/disable-ci-build-cache
continuwuation:fix/26.7.3
continuwuation:nex/feat/msc-fed-bidi-ping
continuwuation:nex/feat/backwards-compat-invites
continuwuation:1235-add-arm-deb
continuwuation:nex/feat/admin-api
continuwuation:nex/meta/release-schedule
continuwuation:nex/feat/centralise-remote-memberships
continuwuation:nex/feat/pdu-versions
continuwuation:ginger/revert-direct-tls
continuwuation:nex/fix/federated-invite-revoke
continuwuation:nex/fix/plunger
continuwuation:jade/perf/gme-rebased
continuwuation:nex/perf/get-missing-events2
continuwuation:nex/backport/v0.5.11
continuwuation:nex/backport/v0.5.10-backport
continuwuation:nex/feat/msc4491-invite-reasons-in-room-creation
continuwuation:nex/feat/deprecated-room-versions
continuwuation:release/v0.5.9
continuwuation:nex/feat/enable-debug-log-release-builds
continuwuation:nex/feat/room-purging
continuwuation:nex/feat/room-shutdown
continuwuation:ginger/ruma-upstreaming
continuwuation:jade/tls-backends
continuwuation:ginger/email-fixes
continuwuation:jade/changelog-labels
continuwuation:nex/fix/v12-publishing
continuwuation:jade/build-info
continuwuation:jade/purge-sync-tokens
continuwuation:ginger/terms-and-conditions
continuwuation:ginger/remove-sliding-sync-proxy
continuwuation:nex/fix/pusher-association
continuwuation:ginger/email-support
continuwuation:jade/community-guidelines
continuwuation:nex/fix/federation-format
continuwuation:jade/git-deps-updates
continuwuation:jade/changelog-check
continuwuation:jade/rust-1-92
continuwuation:ginger/password-reset
continuwuation:nex/experiment/push-gateway-logs
continuwuation:ginger/msc3575-obliteration
continuwuation:nex/feat/block-busted-rooms
continuwuation:nex/fix/informative-startup-errs
continuwuation:ginger/no-left-room-initial-sync
continuwuation:jade/docker-entrypoint
continuwuation:jade/dehydrated-devices
continuwuation:ginger/complement-fixes
continuwuation:nex/fix/stale-destination-cache
continuwuation:nex/experiment/sync-mutex
continuwuation:tcpipuk/docker-docs
continuwuation:jade/snafu
continuwuation:jade/rand-update
continuwuation:nex/stateres-refactor
continuwuation:ginger/779-in-troubleshooting
continuwuation:jade/liveit-guide
continuwuation:jade/http3
continuwuation:nex/feat/admin-hide-empty-rooms
continuwuation:ginger/oobe
continuwuation:nex/fix/debian-thingy
continuwuation:jade/ldap-admin-check
continuwuation:nex/fix/remote-restricted-joins
continuwuation:nex/feat/msc4406-sender-ignored
continuwuation:jade/deadlock-detection
continuwuation:jade/get-started
continuwuation:jade/docs-guide
continuwuation:ginger/fix-local-invites
continuwuation:nex/fix/tpi
continuwuation:nex/feat/room-deletion
continuwuation:nex/feat/msc4322-media-redaction
continuwuation:ginger/stitched-order
continuwuation:ginger/deps/update-rspress
continuwuation:jade/admin-announce-improvements
continuwuation:ginger/xtask-improvements
continuwuation:jade/improve-admin-config-display
continuwuation:nex/fix/better-stateres-error-logs
continuwuation:jade/sender-timeouts
continuwuation:nex/feat/custom-v12-room-ids
continuwuation:ginger/update-metadata
continuwuation:nex/feat/admin-force-logout
continuwuation:tom/max-perf-docs
continuwuation:nex/fix/invalid-appservice-reg
continuwuation:nex/feat/antispam
continuwuation:nex/feat/account-locking
continuwuation:jade/logging-cleanup
continuwuation:jade/remove-legacy-appservice-auth
continuwuation:nex/fix/key-query
continuwuation:jade/update-prek
continuwuation:nex/fix/room-summaries
continuwuation:ginger/restrict-admin-commands
continuwuation:ginger/enable-console-by-default
continuwuation:jade/tag-fixes
continuwuation:jade/otlp
continuwuation:nex/meta/pull-req-template
continuwuation:nex/fix/fed-invite-compliance
continuwuation:nex/feat/build-commit
continuwuation:nex/feat/join-logging
continuwuation:jade/mailmap-updates
continuwuation:jade/hack-ci-tmp
continuwuation:jade/v12-stable
continuwuation:jade/relations
continuwuation:ginger/database-refactor
continuwuation:jade/fix-ldap-uiaa
continuwuation:nex/fix/validation
continuwuation:ginger/nuke-invalid-msc4133-fields-in-migration
continuwuation:ginger/downgrade-artifact-actions
continuwuation:oddlid/reload-fix
continuwuation:jade/fix-assert
continuwuation:ginger/sync-v3-cleanup
continuwuation:ginger/remove-absolute-action-urls
continuwuation:jade/website
continuwuation:nex/fix/backoff
continuwuation:ginger/fix-mdbook-for-0.5
continuwuation:ginger/no-docker-on-prs
continuwuation:backport/v0.5.0-rc.8-1
continuwuation:nex/fed-improvements
continuwuation:jade/rust-1.90
continuwuation:jade/mirror-dockerhub
continuwuation:jade/clippy-fixes
continuwuation:jade/fix-support
continuwuation:jade/clean-images
continuwuation:jade/wal-compression-type
continuwuation:jade/flake-clone
continuwuation:ginger/upload-rpms-on-schedule
continuwuation:nex/fix/incoming-fetch
continuwuation:nex/fix/upgrade
continuwuation:tom/ci-fedora-rpm
continuwuation:jade/ci-release-fix
continuwuation:jade/rocksdb-10-5
continuwuation:ginger/fix-msc4133-migration
continuwuation:ginger/migrate-busted-tz
continuwuation:hydra/public
continuwuation:nex/feat/manual-extremities
continuwuation:nex/feat/async-media
continuwuation:nex/feat/fast-joins-hack-do-not-use-DO-NOT-USE
continuwuation:nex/feat/better-logging
continuwuation:trigger-ci-so-latest-isnt-on-illegal-car-mods
continuwuation:nex/feat/pins-backfill
continuwuation:jade/tuwunel-2025-06-old
continuwuation:jade/ai-slop-db-docs
continuwuation:nex/fix-create-auth
continuwuation:jade/version-stats
continuwuation:jade/read-receipts
continuwuation:jade/rust-toolchain-no-targets
continuwuation:jade/logging-features
continuwuation:jade/syncv5-typing
continuwuation:jade/msc2815
continuwuation:morguldir/see-eye
continuwuation:jade/css-small-screen
continuwuation:nex/wip-751
continuwuation:tuwunel-rebase
continuwuation:test
continuwuation:oddlid/rename-admin-room-bot
continuwuation:strawberry/nix-ci-stuff
continuwuation:strawberry/valgrind
continuwuation:phonemain
continuwuation:strawberry/morgs-snake-sync-jason-main
continuwuation:newer-media-endpoints
continuwuation:folly-coroutines-async-io
continuwuation:federation-retry-timer-port
continuwuation:bad-attempt-at-extracting-homeserver-signing-key
continuwuation:room-deletion-attempt-do-not-use
Labels
Clear labels
Abandoned
This pull request appears to be abandoned by its author.
Blocked
This pull request or issue is currently blocked from being merged/closed
Bug
Something isn't working as intended
Changelog
Added
Changelog entry added
Changelog
Missing
No changelog when one is needed
Changelog
None
Changelog is unnecesary for this change
Cherry-picking
Commits picked from other conduit projects
Database
This requires or includes changes to the database
Dependencies
Something dependency related
Dependencies/Renovate
Automatic dependency upgrades by Renovate
Difficulty
Easy
Low difficulty to implement - touches few parts of the codebase, low complexity
Difficulty
Hard
High difficulty to implement - touches many parts of the codebase, high complexity
Difficulty
Medium
Medium difficulty to implement - touches more parts of the codebase, higher complexity
Documentation
Improvements or additions to documentation
Enhancement
New feature or request
Good first issue
Good for newcomers
Help wanted
Additional eyes and keyboards are required for this one
Inherited
Issues that have been inhereted from the project pre-fork
Matrix/Administration
Features pertaining to homeserver administration
Matrix/Appservices
Features pertaining to the appservice API
Matrix/Auth
Features pertaining to authentication
Matrix/Client
Features pertaining to client-to-server interactions
Matrix/Core
Issues relating to core matrix functionality, such as state resolution and PDU formats
Matrix/E2EE
Issues related to end to end encryption
Matrix/Federation
Features pertaining to server-to-server interactions
Matrix/Hydra
Issues related to room version 12 and related changes (temporary label)
Matrix/MSC
Features pertaining to unstable matrix features
Matrix/Media
Features pertaining to media interactions
Matrix/T&S
Changes or issues related to trust & safety tooling
Merge
This PR is ready to be merged
Merge/Manual
This PR should be manually merged
Merge/Squash
This PR should be squashed when it is merged
Meta
Related to housekeeping, maintenance, or other repo-meta.
Meta/CI
Issues related to CI changes
Meta/Packaging
Packaging
Priority
Blocking
This issue is blocking the next release
Priority
High
This issue is very important
Priority
Low
This issue is of a rather low priority
Security
This item is related to general security
Status
Confirmed
This issue has enough information and is confirmed
Status
Duplicate
This issue or pull request already exists
Status
Invalid
This issue doesn't seem right
Status
Needs Investigation
This issue needs further investigation
Support
Questions or support requests
bob the builder
pdu logic rewrite starring nexus nicholson
No labels
Abandoned
Blocked
Bug
Changelog
Added
Changelog
Missing
Changelog
None
Cherry-picking
Database
Dependencies
Dependencies/Renovate
Difficulty
Easy
Difficulty
Hard
Difficulty
Medium
Documentation
Enhancement
Good first issue
Help wanted
Inherited
Matrix/Administration
Matrix/Appservices
Matrix/Auth
Matrix/Client
Matrix/Core
Matrix/E2EE
Matrix/Federation
Matrix/Hydra
Matrix/MSC
Matrix/Media
Matrix/T&S
Merge
Merge/Manual
Merge/Squash
Meta
Meta/CI
Meta/Packaging
Priority
Blocking
Priority
High
Priority
Low
Security
Status
Confirmed
Status
Duplicate
Status
Invalid
Status
Needs Investigation
Support
bob the builder
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
4 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".
No due date set.
Dependencies
No dependencies set.
Reference
continuwuation/continuwuity!2095
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "seercat/continuwuity:cat/socket-activation"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
This pull request adds support for systemd socket activation. If systemd passes sockets, they will be used to listen in addition to the addresses/ports and unix socket set in the configuration.
This allows greater flexibility in managing the addresses on which continuwuity will listen, as systemd can use it's root privileges to bind to ports and paths which continuwuity itself cannot. Zero-downtime restarts may also be possible, as the listening socket will be kept open by systemd while continuwuity restarts.
I have also refactored the
router/servecode to allow for this addition. I have kept that in a separate commit (i.e. please review by commits), but I can split this into two MRs if preferable.Still to do:
direct_tlsand/orsystemdfeatures are disabledCloses: #2087
Pull request checklist:
mainbranch, and the branch is named something other thanmain.myself, if applicable. This includes ensuring code compiles.
619269ee06743ff8a5d1743ff8a5d11d5b448a7c1d5b448a7c4ba493fffeWIP: Add systemd socket activation supportto Add systemd socket activation supportAdd systemd socket activation supportto WIP: Add systemd socket activation support4ba493fffe5fc2e72c13@ -481,0 +481,4 @@[workspace.dependencies.sd-listen-fds]version = "0.2.0"default-features = falseI just noticed that the existing
sd-notifydependency also provides a helper for supporting socket activation: https://docs.rs/sd-notify/0.5.0/sd_notify/fn.listen_fds.html (weird, because I wouldn't expect that from the name).It can be used instead of adding
sd-listen-fdsThanks for pointing this out, I'll swap to using that.
@ -0,0 +1,8 @@[Socket]ListenStream=127.0.0.1:8008ListenStream=[::1]:8008It might make sense to explicitly disable ipv4 compatibility on the ipv6 socket if using two separate sockets:
BindIPv6Only=ipv6-only(though it's kind of a misleading name when also listening on an ipv4 address 😅).Alternatively, this could just listen on the ipv6 address, and set
BindIPv6Only=both(but then I think any ipv4 addresses displayed in the logs would show up as ipv6 addresses).When running this on my own server, where I have nginx pointed at
http://127.0.0.1:8008, I found that using justBindIPv6Only=bothandListenStream=[::1]:8008prevented nginx from being able to connect to c10y, whileListenStream=127.0.0.1:8008works fine.I'm leaning towards changing the recommended/example to the following, but I would appreciate input from anyone with more thoughts on this matter.
@ -0,0 +2,4 @@ListenStream=127.0.0.1:8008ListenStream=[::1]:8008Accept=noAccept=nois the default, this can be removed to simplify the file.Thanks.
5fc2e72c134c2c56d77eI've updated this to address those review comments (thanks!), although I would still appreciate input on the
ListenStream=matter (unresolved above). I'm running this on my server at the moment, and I think the code is ready for review.WIP: Add systemd socket activation supportto Add systemd socket activation support4c2c56d77e2e84b83c742e84b83c7473104e633373104e63330797faf5f10797faf5f138d892935fRight sorry! It looks like the
BindIPv6Onlyoption only makes a difference when binding on the wildcard ipv6 address[::](I thought it would also apply to[::1]), so I was wrong to suggest it. There's no harm in keepingBindIPv6Only, but it would probably make sense to remove to reduce the noise in the file.@ -18,3 +34,1 @@handle: ServerHandle<std::net::SocketAddr>,mut shutdown: broadcast::Receiver<()>,) -> Result {mut shutdown_rx: broadcast::Receiver<Duration>,I'm not a maintainer for the project, but it seems like this file is handling too many cases now. Previously it would just determine what kind of listener it would need to run from the config, and delegate to the specific implementation. Now it seems like it needs to be aware of implementation details for every possible type of listener, which makes it harder to understand.
I don't have enough context of the project to know the best approach to decouple everything, but I think unix socket specific code & tcp specific code should be split off into in separate files (and ideally only have a single point of entry from this file).
I also find it strange that the direct tls layer only applies to tcp listeners. I think it would make things easier to decouple if it was added as an extra layer on top of all listeners (but it also makes sense why it's done this way for backwards compatibility with the previous implementation).
Ooh, it looks like
axum-server-dual-protocoldoesn't support unix listeners, even thoughaxum-serverdoes.Upstream
axum-server-dual-protocolappears abandoned, could probably fork it given we're already relying on a patchI agree that I could probably split things up a bit more, but the dual-protocol TLS is certainly part of the problem.
Out of curiosity, how important is that feature? (I know, hard question to answer)
I am not interested in maintaining such a fork, fwiw. I also think that having no-tls vs. just-tls vs. dual is always going to be more complicated than a binary of TLS or non-TLS.
Having the ability is needed to run some test suites.
@ -43,0 +53,4 @@let mut bound_ip_addrs = Vec::new();#[cfg(all(target_os = "linux", feature = "systemd"))]let mut bound_unix_socket_paths = Vec::new();It also seems like the implementation can be generalized to avoid needing
bound_ip_addrs&bound_unix_socket_paths.From what I understand, they're used to allow the server to skip binding sockets that would conflict with sockets allocated by systemd (essentially reducing errors into warnings for a narrow use-case).
To generalize it, I think it would make sense to allow the server to continue running after any bind error (as long as one listener remains active). Then maybe it would also make sense to add a "strict" listening option that would enforce all listeners be active.
My intention in doing this was to make it easier to use socket activation, as it wouldn't be strictly necessary to explicitly unset your listeners if not using
use_exclusively_socket_activation, so that c10y would be able to bind sockets itself if socket activation didn't provide them for some reason.Other than that case, I feel that it makes sense to fail completely if anything fails to bind, since other downstream stuff might otherwise break in confusing ways: "continuwuity is running, so why can't nginx get at it?"
I'd appreciate a maintainer's opinion on this since it's more of a design decision.
Aside from my comments - I've applied the changes from
9c714035a9..38d892935fon one of my servers, and it's been working great for socket activation! Thanks for working on this! 😊@ -122,4 +13,1 @@.expect("failed to extract configured unix socket path");if path.exists() {warn!("Removing existing UNIX socket {:#?} (unclean shutdown?)...", path.display());It looks like this warning was accidentally removed?
Intentionally, although this is something I need to look further into.
I also removed the code that deletes/unlinks the filesystem path when shutting down the unix socket listener, since I couldn't see a good way to fit it in and I'm not sure that it's strictly necessary. As such, I removed the check that complains if there was already a socket there, since that would now be the expected case.
@ -2677,4 +2700,2 @@.map_err(|e| err!("There was a problem with your configuration file: {e}"))?;// don't start if we're listening on both UNIX sockets and TCP at same timecheck::is_dual_listening(raw_config)?;Justification for removing this check?
With the refactoring done to support multiple fds passed by systemd socket activation, the server can now listen on both unix & tcp sockets at the same time.
The check could be left in, but it would be unnecessarily restrictive now that it's supported.
That is correct.
@ -0,0 +13,4 @@/// Given a file descriptor, classify it and convert to a `TcpListener`,/// `UnixListener`, or neither (in which case the fd is returned).fn classify_fd(fd: OwnedFd) -> std::io::Result<Result<Listener, OwnedFd>> {Wrapping errors is bad, either use thiserror/anyhow to make a specific error or use the global generic error type
You're also
not adding any context to the errorsremoving the original errors, which may make issues harder to debug@ -45,0 +216,4 @@server.runtime().spawn(graceful_shutdown_handler(shutdown_rx, server_handle.clone()));Couldn't everything that's done by
shutdown_rxbe done with aServerHandlepassed down fromsrc/router/run.rs? It seems wasteful to create a newServerHandle& spawn a task for every socket just to handle shutdowns.Unfortunately not, though I did try initially.
ServerHandleis generic over the type of address used by the listener bound, for some reason, and I can't find a nice way to work around that, especially considering that...ServerHandlefor all of them if multiple tcp listeners existed) could have caused some subtle issues with the shutdown process, since the handles seems to encapsulate some mutable inner state for communication.I agree that it's not the nicest, but the generics mean that a "better" solution may still require keeping unix socket- and tcp socket-adjacent things separate like with
bound_ip_addrs/bound_unix_socket_paths.38d892935f186ec15170186ec151709e1e488679Not doing a functional review (yet), I think no one noticed this yet so that's why I am submitting this.
Do I understand correctly that this will be a non-default feature? I am thinking of making socket activation the default (if we end up merging this) on NixOS 26.11 (the upcoming release at the end of the year), as socket activation is the more regular approach on NixOS anyway.
@ -69,0 +69,4 @@# If a provided socket is bound to an address also configured here,# Continuwuity will not bind that address again.## Does nothing if Continuwuity was not build with the `systemd` feature.s/build/built
@ -123,0 +123,4 @@/// If a provided socket is bound to an address also configured here,/// Continuwuity will not bind that address again.////// Does nothing if Continuwuity was not build with the `systemd` feature.s/build/built
@bart wrote in #2095 (comment):
Thanks, and thanks for the typo fix! I'll include that when I make and push other changes.
I wouldn't agree that this will be a non-default feature, though I may misunderstand what specifically you are referring to.
By default, c10y is built with the
systemdfeature, anduse_socket_activationdefaults totrue. That means that if an appropriately-configured.socketunit is present, causing systemd to pass sockets to c10y when executing it, c10y will automatically use those sockets in addition to binding its own (with the exception that it won't bind anything itself if it doesn't need to).Effectively, the bulk of the socket activation functionality is enabled by default, if a socket unit is set up.
However,
use_exclusively_socket_activationdefaults tofalse. If set totrue, c10y will never try to bind its own listening sockets.Of course the design is up to you, but my input w/r/t using this for the NixOS module would be:
bikeshed_socket_activationmodule option is enabled, setuse_exclusively_socket_activation = trueand throw an error if any of c10y's listening-related config is set (perhaps non-exhaustively,address,port,unix_socket_path).By the way: I'll get to all the other review stuff from other people hopefully sometime soon, I got busier shortly after opening this PR. I just need to avoid making this code all overly-complicated since my goal here was to make things simpler.
@seercat I did end up playing around with your PR about a week ago to try to see if I could organize things the way I described in some of my comments.
I basically had it ready to publish (in my own fork), but then suddenly lost all motivation (hopefully I can get back into the hyper-fixation again 😅). I also didn't really want to distract too much from what you already started - but let me know if you're curious to see what I came up with.
@kira-bruneau wrote in #2095 (comment):
Cool!
I'd be very interested to see what you have, I'm not super happy with what I've tried so far. Would you be OK with me integrating it into my branch (i.e. this PR) if I agree with it? If you push it to a branch on your fork here I'll take a look.
@seercat Thanks for the response! I was/am away, so I'll do a functional review once you got to resolving all (other) reviews. Thanks for explaining your intention. I just thought the systemd feature would not be on by default, no need to explain the rest :p
I think we can just reuse the
services.matrix-continuwuity.settingsfreeform submodule and there's no need for a separate option that essentially just sets another option, and nothing else. This is better for introspection as well. The evaluation error is a good idea though, but I think an evaluation warning is nicer to the consumers of the module. Nothing breaks if bothservices.matrix-continuwuity.settings.global.address(or any of the other ones) andservices.matrix-continuwuity.settings.global.use_exclusively_socket_activation = true;are set.Thanks again, I'll shut up about NixOS now, it is better to defer that conversation to the nixpkgs PR for it.
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.