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

Support systemd socket activation #1378

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Kern--
Copy link
Contributor

@Kern-- Kern-- commented Sep 18, 2024

Issue #, if available:
Closes #1088

Description of changes:
This change adds 2 features:

  1. the --address flag of the soci snapshotter supports a network prefix
  2. The soci snapshotter can listen on a file descriptor passed by
    systemd socket activation.

If the user tells soci to listen on a file descriptor, but doesn't use systemd socket activation to launch the process, the snapshotter will fall back to the default address as a unix socket.

The example systemd service file is updated to use systemd activation if possible. There is an example systemd socket file, but no strict dependency from the service file so that users who are always starting SOCI on boot won't get a warning from systemd.

Testing performed:
Tested manually and verified that:

  1. socket activation works
  2. the example systemd unit file works without socket activation
  3. setting a custom socket address still works both with and without socket activation.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@Kern-- Kern-- requested a review from a team as a code owner September 18, 2024 17:12
@github-actions github-actions bot added the go Pull requests that update Go code label Sep 18, 2024
This change adds 2 features:
1) the --address flag of the soci snapshotter supports a network prefix
2) The soci snapshotter can listen on a file descriptor passed by
   systemd socket activation.

If the user tells soci to listen on a file descriptor, but doesn't use
systemd socket activation to launch the process, the snapshotter will
fall back to the default address as a unix socket.

The example systemd service file is updated to use systemd activation if
possible. There is an example systemd socket file, but no strict
dependency from the service file so that users who are always starting
SOCI on boot won't get a warning from systemd.

Signed-off-by: Kern Walster <walster@amazon.com>
Copy link
Contributor

@sondavidb sondavidb left a comment

Choose a reason for hiding this comment

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

Do we want to add an integration test to this? My understanding is that this doesn't change anything in the integration tests so maybe a quick one using socket activation. (Though actually, alpine doesn't support systemd, so maybe not something very feasible?)

return nil, err
}
if len(listeners) == 0 {
log.G(ctx).Info("Address was set to listen on a file descriptor, but no file descriptors were passed. Perhaps soci was launched directly without using systemd socket activation? Using the default address.")
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Can we separate the last sentence into a separate log message for clarity?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
go Pull requests that update Go code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] Support systemd socket activation for soci-snapshotter-grpc
3 participants