-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Allow microvm configuration from a single json passed as command line parameter #1212
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
Conversation
388270a
to
51aa257
Compare
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.
Changes LGTM for supporting the single json config from command line.
I'm a bit worried that we're missing a good opportunity to share the relevant parts with our API path so that we also support single API call config.
I believe clients that still need the API (e.g. for post-boot operations) would be highly grateful if they could do the single API call using their existing path (swagger-generated API client) instead of having to manually create JSONs and pass them as command-line args.
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 forgot about the changelog again! This is a big change, so please update all the relevant docs:
- add an entry in
CHANGELOG.md
- add an entry in
FAQ.md
- update
jailer.md
with the new parameter - add a section in
getting-started.md#running-firecracker
I also totally forgot about the swagger spec and wouldn't have remembered if @acatangiu hadn't mentioned it. Customers can use it to autogenerate clients, so it should support the new payload format.
That's a good point, but a bit outside the scope of this PR. The opportunity to build on top of this remains, and we can prioritize it if there's a real customer need, or pick it up as follow-up work at some point. |
I agree that we needn't target the single API call config in this PR, but I think that @lauralt has a good opportunity to do that as well (in subsequent PR/PRs) - it has real customer value. |
2fc1712
to
dd1c9ed
Compare
914fe19
to
8795fb2
Compare
b2d29c3
to
270dcb8
Compare
405664f
to
c2c3178
Compare
Signed-off-by: Laura Loghin <[email protected]>
Issue #923 , if available:
Description of changes:
Added extra parameter 'config-file' to Firecracker, which allows starting a microvm without sending API requests. User has to send in the start command a file which stores a single json, that contains the configuration for all of the resources used by the microvm. Also, added an integration test which successfully starts a microvm with a configuration file sent as command line parameter and unit tests.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.