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 fd:// for socket activation #1924

Merged
merged 1 commit into from Dec 30, 2020

Conversation

afbjorklund
Copy link
Contributor

Used go-systemd code from moby/moby daemon

Only added buildkitd --addr fd:// for now.

Closes #1923

@tonistiigi
Copy link
Member

You need to run ./hack/update-vendor

Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Can we have documented this in README?

@afbjorklund
Copy link
Contributor Author

afbjorklund commented Dec 28, 2020

Can we have documented this in README?

Sure. I didn't get any feedback on what you wanted to do with the systemd units, in general ?

Just stick them in some "contrib/init" or something, or where should such files be located ?

But can write some general text for the README.md, like the issue text (or like the tcp://)

@AkihiroSuda
Copy link
Member

Maybe examples/systemd

@tonistiigi
Copy link
Member

cross is failing

#65 117.7 # github.com/coreos/go-systemd/v22/activation
#65 117.7 vendor/github.com/coreos/go-systemd/v22/activation/files.go:57:22: cannot use fd (type int) as type syscall.Handle in argument to syscall.CloseOnExec
#65 ...

@afbjorklund
Copy link
Contributor Author

afbjorklund commented Dec 29, 2020

cross is failing

systemd on windows ? probably should just skip it there

(didn't know you were building buildkitd for Windows)

@afbjorklund
Copy link
Contributor Author

afbjorklund commented Dec 29, 2020

I think the test failure is unrelated to this ?

2020-12-29T08:49:53.9205191Z --- FAIL: TestClientGatewayIntegration/TestClientGatewayContainerMounts/worker=containerd-snapshotter-stargz (16.31s)

2020-12-29T08:52:48.5944387Z FAIL github.com/moby/buildkit/client 251.113s

Probably more related to 2f4c15d

@AkihiroSuda
Copy link
Member

LGTM but please squash commits, it will also restart CI

Used go-systemd code from moby/moby daemon

Only added `buildkitd --addr fd://` for now.

Don't do systemd fds for windows buildkitd

Add buildkit systemd units README/examples

Signed-off-by: Anders F Björklund <anders.f.bjorklund@gmail.com>
@afbjorklund
Copy link
Contributor Author

LGTM but please squash commits, it will also restart CI

Squashed and rebased, as requested.

Have you considered adding an idle timeout to buildkitd ?

So that it will shut down the service automatically, when done.
i.e. when all builds are completed, only listener socket remains

@tonistiigi tonistiigi merged commit 5efd74b into moby:master Dec 30, 2020
@tonistiigi
Copy link
Member

So that it will shut down the service automatically, when done.
i.e. when all builds are completed, only listener socket remains

Not against it if we can guarantee that there is no race between daemon shutting down and a new request coming in.

Copy link

@liveDC liveDC left a comment

Choose a reason for hiding this comment

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

Merging

@afbjorklund afbjorklund deleted the addr-fd branch January 1, 2021 12:25
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.

Systemd socket activation fds
4 participants