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

Should not require socket path to be set in both firecracker.Config and firecracker.VMCommandBuilder #88

Open
samuelkarp opened this issue Mar 19, 2019 · 1 comment

Comments

@samuelkarp
Copy link
Contributor

The SDK currently requires the socket path to be set in two places, and for it to be set to the same thing:

firecracker.VMCommandBuilder is used for building the command to start the firecracker binary. Specifying the socket path here adds the --api-sock command-line argument that tells Firecracker to open the socket in a particular location.

firecracker.Config is used in the API client so that the client knows how to talk to the Firecracker API.

In order to both start a VM and communicate with it, a user of this SDK is required to specify the socket path in both places. If the user is intending to start a VM and immediately control it (which I think is the most common case), we should make it simpler to do so and allow the field to be specified only once.

samuelkarp added a commit to samuelkarp/firecracker-containerd that referenced this issue Mar 19, 2019
The Firecracker API socket is important for the runtime, as it uses the
socket to communicate with Firecracker.  Allowing the runtime to fully
control the API socket (including its path) removes a potential failure
condition from misconfiguration.

The API socket is hard-coded to exist within the current working
directory.  Part of the contract that containerd exposes for runtimes is
that they are started with the current working directory changed to be
that of the OCI bundle.  Using an API socket in the OCI bundle makes its
location well-known and predictable to the runtime.

Related: firecracker-microvm#59
Related: firecracker-microvm/firecracker-go-sdk#88

Signed-off-by: Samuel Karp <skarp@amazon.com>
samuelkarp added a commit to samuelkarp/firecracker-containerd that referenced this issue Mar 19, 2019
The Firecracker API socket is important for the runtime, as it uses the
socket to communicate with Firecracker.  Allowing the runtime to fully
control the API socket (including its path) removes a potential failure
condition from misconfiguration.

The API socket is hard-coded to exist within the current working
directory.  Part of the contract that containerd exposes for runtimes is
that they are started with the current working directory changed to be
that of the OCI bundle.  Using an API socket in the OCI bundle makes its
location well-known and predictable to the runtime.

Related: firecracker-microvm#59
Related: firecracker-microvm/firecracker-go-sdk#88

Signed-off-by: Samuel Karp <skarp@amazon.com>
@kzys
Copy link
Contributor

kzys commented Mar 18, 2020

If a customer is using NewMachine, he/she doesn't have to specify the socket path twice, because of

WithSocketPath(cfg.SocketPath).

I think that covers most of use cases.

Do we want to improve UX even further for customers who use NewClient() directly?

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

No branches or pull requests

2 participants