Skip to content

Simplify the runtime configuration #59

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

Closed
5 tasks done
samuelkarp opened this issue Dec 27, 2018 · 9 comments
Closed
5 tasks done

Simplify the runtime configuration #59

samuelkarp opened this issue Dec 27, 2018 · 9 comments
Assignees
Labels

Comments

@samuelkarp
Copy link
Contributor

samuelkarp commented Dec 27, 2018

The runtime configuration is fairly complicated and has a number of fields that need reasonable defaults.

  • firecracker_binary_path - the relative path default is unreasonable, since the working directory changes every time containerd executes a new container (working directory is inside the container bundle)
  • socket_path - needs a reasonable default, probably relative inside the working directory/bundle
  • kernel_image_path - could use a default lookup path
  • kernel_args - needs a reasonable default
  • root_drive - could use a default lookup path
  • cpu_count - might want to pick this dynamically from the OCI spec, with perhaps a reasonable default
  • console - should it default to "stdio"?

Related to #57

@IRCody
Copy link
Contributor

IRCody commented Dec 27, 2018

socket_path - needs a reasonable default, probably relative inside the working directory/bundle

Not sure about all the others but I think ./firecracker.sock makes sense as a default for socket_path.

@samuelkarp
Copy link
Contributor Author

Not sure about all the others but I think ./firecracker.sock makes sense as a default for socket_path.

Yep, I think so too.

@xibz
Copy link
Contributor

xibz commented Dec 27, 2018

I believe the jailer socket path (not settable) is api.socket and in firecracker's docs they use firecracker.socket in their getting started guide. Should we try to be consistent? If the jailer is on by default, may be best to go with api.socket?

@samuelkarp
Copy link
Contributor Author

cpu_count - might want to pick this dynamically from the OCI spec, with perhaps a reasonable default

Opened #76 to track the use-case for separate VM sizing parameters for different VMs

@samuelkarp
Copy link
Contributor Author

firecracker_binary_path - the relative path default is unreasonable, since the working directory changes every time containerd executes a new container (working directory is inside the container bundle)

This has been changed in firecracker-go-sdk and we've now inherited that change such that it defaults to firecracker on $PATH. This is a much more reasonable default. I'll remove this item from the overview list.

@samuelkarp
Copy link
Contributor Author

I believe the jailer socket path (not settable) is api.socket and in firecracker's docs they use firecracker.socket in their getting started guide. Should we try to be consistent? If the jailer is on by default, may be best to go with api.socket?

That looks accurate. I think for now I'm going to hard-code the socket (instead of making it configurable) as ./firecracker.sock, but when we take a look at jailer integration (#122) we'll need to revisit this.

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 <[email protected]>
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 <[email protected]>
@samuelkarp
Copy link
Contributor Author

kernel_args - needs a reasonable default

Per discussion with @nmeyerhans, my plan is to use "console=ttyS0 noapic reboot=k panic=1 pci=off nomodules rw" as the default for now.

samuelkarp added a commit to samuelkarp/firecracker-containerd that referenced this issue Mar 20, 2019
samuelkarp added a commit to samuelkarp/firecracker-containerd that referenced this issue Mar 20, 2019
samuelkarp added a commit to samuelkarp/firecracker-containerd that referenced this issue Mar 21, 2019
The console field was already removed, but stale documentation continued
to reference it.  This commit removes the stale documentation.

Related: firecracker-microvm#59

Signed-off-by: Samuel Karp <[email protected]>
@samuelkarp
Copy link
Contributor Author

console - should it default to "stdio"?

This field was removed, I'll remove it from this list.

samuelkarp added a commit to samuelkarp/firecracker-containerd that referenced this issue Mar 21, 2019
The console field was already removed, but stale documentation continued
to reference it.  This commit removes the stale documentation.

Related: firecracker-microvm#59

Signed-off-by: Samuel Karp <[email protected]>
samuelkarp added a commit to samuelkarp/firecracker-containerd that referenced this issue Mar 21, 2019
@samuelkarp
Copy link
Contributor Author

Everything that I had outlined doing in the overview is now done, so resolving.

Any further runtime configuration changes should be a separate issue.

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

No branches or pull requests

3 participants