thi-startup / spitfire

cli tool for creating, running, tampering with firecracker microvms
MIT License
4 stars 1 forks source link

Update/streamline operations #10

Open bxffour opened 1 year ago

bxffour commented 1 year ago

Addresses: #9

This commit refactors most of the codebase, isolation several core logic into their own packages. The init command was added to ensure all dependencies are available. The create command was added to create microvms and the run command was added to run microvms.

New commands

Init

This ensures all the dependencies are available

# spitfire init

output:

2023/08/12 17:06:27 INFO init binary is ready to go
2023/08/12 17:06:27 INFO vmlinux binary is ready to go
2023/08/12 17:06:27 INFO firectl binary is ready to go

init

Create

This command creates the microvm image from a container image

create

Run

This command runs the microvm

# spitfire run <image name>

run

blackprince001 commented 1 year ago

LGTM. great work bro

buabaj commented 1 year ago

LGTM bro however, it would be much better if you made smaller PRs to ensure effective reviews as large PRs are pretty clumsy and make thorough reviewing a pain. Also, could you consider modularizing most of your code into reusable functions to prevent duplication and ensure consistency? i also believe you can do more effective validation and error-handling for certain things like parsing in flag values, etc(make sure you don't get any expected behaviour if users don't parse flags you're expecting). Logging and messaging also don't appear to be enough, it's fine if you want to do those later but please don't overlook it.

otherwise, everything else lgtm as I mentioned. you should as well look into Joe's comments. I think he makes a good argument about whether or not having an image burned on the drive is optional (unless I'm missing some context)