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

add support for socket activation #111

Closed
wants to merge 1 commit into from

Conversation

flokli
Copy link
Contributor

@flokli flokli commented Apr 29, 2024

If nsncd is started using socket activation, use the passed FD instead of manually binding on SOCKET_PATH.

This doesn't change any current behaviour in the currently documented startup mode, but makes nsncd fit to run in a socket-activated environment.

If nsncd is started using socket activation, use the passed FD instead
of manually binding on SOCKET_PATH.
Copy link
Collaborator

@blinsay blinsay left a comment

Choose a reason for hiding this comment

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

Looks good, some comments in line. Have you tested it locally with socket activation yet?

// else bind manually on SOCKET_PATH.
let (listener, listen_address) = match ListenFd::from_env()
.take_unix_listener(0)
.expect("invalid socket type at index")
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it makes slightly more sense to log an error and exit here but I don't feel strongly about it.

{
Some(listener) => (listener, "sd-listen-unix"),
None => {
let path = Path::new(SOCKET_PATH);
Copy link
Collaborator

Choose a reason for hiding this comment

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

let's throw this in a fn

@flokli
Copy link
Contributor Author

flokli commented May 8, 2024

Looks good, some comments in line. Have you tested it locally with socket activation yet?

I did check it actually starting up, via systemd-socket-activate:

❯ systemd-socket-activate -l /tmp/foobar.sock target/debug/nsncd
Listening on /tmp/foobar.sock as 3.
Communication attempt on fd 3.
Execing target/debug/nsncd (target/debug/nsncd)

I wrote some spam to it, pointing curl to it, and (successfully) got back an error about the wrong protocol version:

May 08 11:43:02.917 DEBG accepted connection, stream: UnixStream { fd: FileDesc(OwnedFd { fd: 4 }), local: "/tmp/foobar.sock" (pathname), peer: (unnamed) }, thread: worker_2
May 08 11:43:02.917 DEBG parsing request, err: wrong protocol version 542393671, thread: worker_2

I have some systemd config to wire it up there, but I ran into some unrelated other issues and didn't get to fully test it yet. I will, happy to mark it as draft until then.

@flokli flokli marked this pull request as draft May 8, 2024 11:46
@flokli
Copy link
Contributor Author

flokli commented Jun 29, 2024

Shelving these efforts for now, let's revisit this once systemd/systemd#30360 has landed in a release, as it'd probably be revisited after that anyways.

@flokli flokli closed this Jun 29, 2024
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.

None yet

2 participants