-
Notifications
You must be signed in to change notification settings - Fork 95
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
feat: add finch-daemon #1181
base: main
Are you sure you want to change the base?
feat: add finch-daemon #1181
Conversation
Turns out After that gets deployed to the runners, it should resolve the Windows build failures we're seeing. |
Signed-off-by: Justin Alvarez <alvajus@amazon.com>
196257f
to
a5e77d5
Compare
Signed-off-by: Justin Alvarez <alvajus@amazon.com>
Signed-off-by: Justin Alvarez <alvajus@amazon.com>
Signed-off-by: Justin Alvarez <alvajus@amazon.com>
1da9e94
to
86d2c8a
Compare
Signed-off-by: Justin Alvarez <alvajus@amazon.com>
Signed-off-by: Justin Alvarez <alvajus@amazon.com>
Signed-off-by: Justin Alvarez <alvajus@amazon.com>
Signed-off-by: Justin Alvarez <alvajus@amazon.com>
Signed-off-by: Justin Alvarez <alvajus@amazon.com>
Signed-off-by: Justin Alvarez <alvajus@amazon.com>
Signed-off-by: Justin Alvarez <alvajus@amazon.com>
Signed-off-by: Justin Alvarez <alvajus@amazon.com>
[Unit] | ||
Description=Finch daemon %I | ||
Documentation=https://runfinch.com https://github.com/runfinch/finch-daemon | ||
After=network.target local-fs.target containerd.service buildkit.service |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make use of finch-daemon socket activation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not blocking. We can do it as a follow up.
finch.yaml.d/common.yaml
Outdated
sudo systemctl restart containerd.service | ||
# Set a default ulimit for number of files in containerd | ||
sudo mkdir -p /usr/local/lib/systemd/system/containerd.service.d/ | ||
printf '[Service]\nLimitNOFILE=1048576\n' | sudo tee /usr/local/lib/systemd/system/containerd.service.d/finch.conf |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this needed?
@@ -75,7 +78,7 @@ endif | |||
|
|||
FINCH_CORE_DIR := $(CURDIR)/deps/finch-core | |||
|
|||
remote-all: arch-test finch install.finch-core-dependencies finch.yaml networks.yaml config.yaml | |||
remote-all: arch-test finch install.finch-core-dependencies finch.yaml networks.yaml config.yaml $(OUTDIR)/finch-daemon/finch@.service |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need separate targets for networks.yaml
, config.yaml
, $(OUTDIR)/finch-daemon/finch@.service
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think anything that writes a file should probably have its own rule so that its not re-created unless the file is deleted
Signed-off-by: Justin Alvarez <alvajus@amazon.com>
Signed-off-by: Justin Alvarez <alvajus@amazon.com>
Issue #, if available:
Description of changes
Adds finch-daemon, so that it starts inside the VM and is exposed to processes inside the VM (via
/run/finch.sock
and/var/run/docker.sock
), and processes on the native macOS host (via/path/to/lima/data/finch/sock/finch.sock
).To make this possible, these changes had to be made:
./finch.yaml.d/mac.yaml
finch-daemon
binary and service file are copied into the VM, and then starts the daemon using systemdMakefile.darwin
, changed the order so that the finch-daemon specific provisioning script would be the final script, after being mergedadd-daemon-mount
target, which makes it so that duplicate mounts are not added to Lima. This is done to avoid the default lima provisioning scripts failing due to trying to mount/unmount a directory twice (the end result is users may seecloud-final.service
fail after shelling into the VM. although this should only ever really impact developers, it was easy enough to fix)This change also currently relies on a fork of finch-core, which actually has the code necessary for building finch-daemon. Once runfinch/finch-core#424 is merged, I'll update this PR to remove the use of a different finch-core upstream.
Testing done
License Acceptance
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.