feat(server): Add socket-activation support #3283
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #3283 +/- ##
=======================================
Coverage 75.67% 75.68%
=======================================
Files 460 460
Lines 36807 36852 +45
=======================================
+ Hits 27855 27891 +36
- Misses 7029 7036 +7
- Partials 1923 1925 +2
☔ View full report in Codecov by Sentry. |
b51b4ea to
ef60900
Compare
|
Looks like there are some build issues, in principle I like the change, happy to merge when it passes all checks. |
ef60900 to
74e9005
Compare
|
Thanks for the feedback. I didn't want to spend too much time on it if there wasn't interest. I don't have access to the failing platforms, so I need to rely on the ci tests to validate. I tried to make the socket-activation tests linux only. The code itself doesn't need to be gated, as it should disable itself on unsupported platforms, but the tests rely on linux-only behavior due to the ugly syscall thing. An alternative would be to fork the go-systemd package (or extract the socket activation into local module) to allow it to be properly tested on platforms that will likely never use it. To me, this seemed like overkill just for testing, but I can go this way if you prefer. |
|
It looks like all tests passed except the code-coverage percent. line 58 is not new (just a reformatting of the existing code). I can write a test for that if you want, but it is mostly unrelated to this change line 63 is also difficult to check as it requires also duping fd=4 and I'm already scared of the code that handled fd=3. If fixing the code-coverage is critical to merging, I probably need to roll my own implementation of socket-activation, as the go-systemd module is not very amenable to this type of testing. |
|
we're not very rigid about code coverage for cases like this, thank you! |
Objective: make the tests more robust and reduce random failures. Preliminary refactoring: - Accept testing.TB in testenv helpers. This is needed to use `require.EventuallyWithT` in socket activation tests. - Rename parameters for clarity Tests refactoring: - use t.Cleanup instead of defer where appropriate - create file handlers in test routine instead of go routines - remove unnecessary var declaration - increased wait time to 30 seconds. - allow running socket activation test on Darwin Ref: - #3283 - #3313 - #3318
I'm not sure how much interest there is in this, but the attached patch adds systemd socket-activation support for Kopia. Normally socket-activation is used for on-demand application startup, but that probably isn't an ideal setup for Kopia as it could prevent the running of maintenance tasks. However socket activation has a few other uses:
My personal use is the 1st one, as it provides a race-free way to ensure my reverse-proxy can access the unix-domain-socket I use for Kopia.
The test is a little ugly, as the
go-systemd/activationmodule provides no means of changing the starting FD when scanning for socket-activated sockets, which requires that we enable activation on fd 3 which may already be in use when the test runs.