firecracker-microvm / firecracker-go-sdk

An SDK in Go for the Firecracker microVM API
Apache License 2.0
485 stars 123 forks source link

Support more than one IP per interface and IPv6 for results returned by CNI #478

Open th0m opened 1 year ago

th0m commented 1 year ago

Our use case is to:

  1. call SetupNetworkHandler ourselves to get the network configuration generated using a CNI plugin
  2. remove firecracker.SetupNetworkHandlerName and firecracker.SetupKernelArgsHandlerName from the list of default FcInit handlers
  3. use our own VM network interface configuration method instead of the default boot param based one
  4. call machine.Start

With that in mind we want to be able to have more than one IP on interfaces and to support IPv6, which is what this change intends on achieving while keeping backwards compatibility.

th0m commented 1 year ago

@vvejell1 can you please help review this PR when you have a moment? Thank you.

ginglis13 commented 1 year ago

Hi @th0m , looks like buildkite tests (example) have not run:

Missing DCO on d88daf38557b97d2922471b123748fd6aa2480cc

could you please sign your commit ? if you've already got your name and email configured in git, you can git commit --amend -s, then force push to this branch to update the PR

th0m commented 1 year ago

@ginglis13 I am getting build errors that seem unrelated to my change https://buildkite.com/firecracker-microvm/firecracker-go-sdk/builds/3007#01869a5d-b0c1-4d19-ad6f-156ddcc120f3/17-1015

th0m commented 1 year ago

@ginglis13 can you please have a look? Thanks

ginglis13 commented 1 year ago

@th0m we've got an infrastructure bug we're addressing. As soon as it's resolved we can re-run and continue the review

fangn2 commented 1 year ago

@th0m I have a fix on CI failure merged into main. You can rebase, push and trigger the build again.

th0m commented 1 year ago

Thanks, build failures are on me now, I will fix them.

th0m commented 1 year ago

Ready for review @fangn2 thank you.

th0m commented 1 year ago

@ginglis13 I believe this is ready to go.