Skip to content
This repository has been archived by the owner on Dec 7, 2023. It is now read-only.

Expose firecracker sockets #132

Merged
merged 3 commits into from
Jul 12, 2019

Conversation

patrobinson
Copy link
Contributor

This exposes the Firecracker socket files (API, Metrics and Logs) to the host by mapping them into the (already mounted) /var/lib/firecracker/vm/$id/firecracker directory.

This is useful for our use case because we want to use the Firecracker metadata service to expose AWS Credentials. These credentials need to be refreshed so cannot be set statically.

Patrick Robinson added 2 commits July 11, 2019 16:24
This enables us to easily find the API Socket from within the host
Copy link
Contributor

@luxas luxas left a comment

Choose a reason for hiding this comment

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

Hi @patrobinson 👋!
Thanks for this PR, very valid use-case IMO :)

Just one comment, let's not make a new subdirectory, but just keep it in /var/lib/firecracker/vm/$uid/
Does this sound good to you?

@@ -29,13 +29,14 @@ const (
IGNITE_TIMEOUT = 10

// In-container path for the firecracker socket
SOCKET_PATH = "/tmp/firecracker.sock"
FIRECRACKER_SOCKET_PATH = "firecracker"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we please avoid creating an extra directory for now to keep the code a bit simpler?
Please attach these sockets at the /var/lib/firecracker/vm/$id/firecracker.sock etc. path directly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure thing

@@ -97,6 +97,10 @@ func processUID(obj meta.Object, c *client.Client) error {
if err := os.MkdirAll(dir, constants.DATA_DIR_PERM); err != nil {
return fmt.Errorf("failed to create directory for ID %q: %v", uid, err)
}

if err := os.MkdirAll(paths.Join(dir, FIRECRACKER_SOCKET_PATH), constants.DATA_DIR_PERM); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

this can be removed with the above design

Copy link
Contributor

@luxas luxas left a comment

Choose a reason for hiding this comment

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

Very well 👍

LGTM

@luxas luxas self-assigned this Jul 12, 2019
@luxas luxas merged commit ca7c3f0 into weaveworks:master Jul 12, 2019
@patrobinson patrobinson deleted the expose-firecracker-sockets branch July 15, 2019 00:57
@patrobinson
Copy link
Contributor Author

I realised I'm not cleaning up firecrackerSocketPath which may result in a VM failing to start again

@luxas
Copy link
Contributor

luxas commented Jul 18, 2019

I realised I'm not cleaning up firecrackerSocketPath which may result in a VM failing to start again

We do not officially (yet) claim we support restarting VMs, but we will (#196).
However, we'd be glad to take a patch which cleans up the firecracker socket before running again.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants