diff --git a/virtcontainers/fc.go b/virtcontainers/fc.go index 573ad88d40..7942da9582 100644 --- a/virtcontainers/fc.go +++ b/virtcontainers/fc.go @@ -126,15 +126,12 @@ func (fc *firecracker) trace(name string) (opentracing.Span, context.Context) { return span, ctx } -// -// init: initialize the firecracker hypervisor's structure. Doesn't -// actually do anything with firecracker itself, rather it just parses -// through and provides necessary details for its structs... -// -func (fc *firecracker) init(ctx context.Context, id string, hypervisorConfig *HypervisorConfig, storage resourceStorage) error { +// For firecracker this call only sets the internal structure up. +// The sandbox will be created and started through startSandbox(). +func (fc *firecracker) createSandbox(ctx context.Context, id string, hypervisorConfig *HypervisorConfig, storage resourceStorage) error { fc.ctx = ctx - span, _ := fc.trace("init") + span, _ := fc.trace("createSandbox") defer span.Finish() //TODO: check validity of the hypervisor config provided @@ -154,14 +151,6 @@ func (fc *firecracker) init(ctx context.Context, id string, hypervisorConfig *Hy return nil } -// for firecracker this call isn't necessary -func (fc *firecracker) createSandbox() error { - span, _ := fc.trace("createSandbox") - defer span.Finish() - - return nil -} - func (fc *firecracker) newFireClient() *client.Firecracker { span, _ := fc.trace("newFireClient") defer span.Finish() @@ -346,7 +335,7 @@ func (fc *firecracker) fcStartVM() error { // startSandbox will start the hypervisor for the given sandbox. // In the context of firecracker, this will start the hypervisor, // for configuration, but not yet start the actual virtual machine -func (fc *firecracker) startSandbox() error { +func (fc *firecracker) startSandbox(timeout int) error { span, _ := fc.trace("startSandbox") defer span.Finish() @@ -386,7 +375,11 @@ func (fc *firecracker) startSandbox() error { } } - return fc.fcStartVM() + if err := fc.fcStartVM(); err != nil { + return err + } + + return fc.waitVMM(timeout) } func (fc *firecracker) createDiskPool() error { @@ -422,14 +415,6 @@ func (fc *firecracker) createDiskPool() error { return nil } -// waitSandbox will wait for the Sandbox's VM to be up and running. -func (fc *firecracker) waitSandbox(timeout int) error { - span, _ := fc.trace("waitSandbox") - defer span.Finish() - - return fc.waitVMM(timeout) -} - // stopSandbox will stop the Sandbox's VM. func (fc *firecracker) stopSandbox() (err error) { span, _ := fc.trace("stopSandbox") diff --git a/virtcontainers/hypervisor.go b/virtcontainers/hypervisor.go index bff95df32f..6d5335917c 100644 --- a/virtcontainers/hypervisor.go +++ b/virtcontainers/hypervisor.go @@ -590,11 +590,8 @@ func RunningOnVMM(cpuInfoPath string) (bool, error) { // hypervisor is the virtcontainers hypervisor interface. // The default hypervisor implementation is Qemu. type hypervisor interface { - init(ctx context.Context, id string, hypervisorConfig *HypervisorConfig, storage resourceStorage) error - - createSandbox() error - startSandbox() error - waitSandbox(timeout int) error + createSandbox(ctx context.Context, id string, hypervisorConfig *HypervisorConfig, storage resourceStorage) error + startSandbox(timeout int) error stopSandbox() error pauseSandbox() error saveSandbox() error diff --git a/virtcontainers/mock_hypervisor.go b/virtcontainers/mock_hypervisor.go index 89b3687a8c..f23b3b8762 100644 --- a/virtcontainers/mock_hypervisor.go +++ b/virtcontainers/mock_hypervisor.go @@ -13,15 +13,6 @@ import ( type mockHypervisor struct { } -func (m *mockHypervisor) init(ctx context.Context, id string, hypervisorConfig *HypervisorConfig, storage resourceStorage) error { - err := hypervisorConfig.valid() - if err != nil { - return err - } - - return nil -} - func (m *mockHypervisor) capabilities() capabilities { return capabilities{} } @@ -30,15 +21,16 @@ func (m *mockHypervisor) hypervisorConfig() HypervisorConfig { return HypervisorConfig{} } -func (m *mockHypervisor) createSandbox() error { +func (m *mockHypervisor) createSandbox(ctx context.Context, id string, hypervisorConfig *HypervisorConfig, storage resourceStorage) error { + err := hypervisorConfig.valid() + if err != nil { + return err + } + return nil } -func (m *mockHypervisor) startSandbox() error { - return nil -} - -func (m *mockHypervisor) waitSandbox(timeout int) error { +func (m *mockHypervisor) startSandbox(timeout int) error { return nil } diff --git a/virtcontainers/mock_hypervisor_test.go b/virtcontainers/mock_hypervisor_test.go index 2be8d30d16..75fd59c801 100644 --- a/virtcontainers/mock_hypervisor_test.go +++ b/virtcontainers/mock_hypervisor_test.go @@ -11,7 +11,7 @@ import ( "testing" ) -func TestMockHypervisorInit(t *testing.T) { +func TestMockHypervisorCreateSandbox(t *testing.T) { var m *mockHypervisor sandbox := &Sandbox{ @@ -29,7 +29,7 @@ func TestMockHypervisorInit(t *testing.T) { ctx := context.Background() // wrong config - if err := m.init(ctx, sandbox.config.ID, &sandbox.config.HypervisorConfig, sandbox.storage); err == nil { + if err := m.createSandbox(ctx, sandbox.config.ID, &sandbox.config.HypervisorConfig, sandbox.storage); err == nil { t.Fatal() } @@ -39,16 +39,7 @@ func TestMockHypervisorInit(t *testing.T) { HypervisorPath: fmt.Sprintf("%s/%s", testDir, testHypervisor), } - // right config - if err := m.init(ctx, sandbox.config.ID, &sandbox.config.HypervisorConfig, sandbox.storage); err != nil { - t.Fatal(err) - } -} - -func TestMockHypervisorCreateSandbox(t *testing.T) { - var m *mockHypervisor - - if err := m.createSandbox(); err != nil { + if err := m.createSandbox(ctx, sandbox.config.ID, &sandbox.config.HypervisorConfig, sandbox.storage); err != nil { t.Fatal(err) } } @@ -56,15 +47,7 @@ func TestMockHypervisorCreateSandbox(t *testing.T) { func TestMockHypervisorStartSandbox(t *testing.T) { var m *mockHypervisor - if err := m.startSandbox(); err != nil { - t.Fatal(err) - } -} - -func TestMockHypervisorWaitSandbox(t *testing.T) { - var m *mockHypervisor - - if err := m.waitSandbox(0); err != nil { + if err := m.startSandbox(vmStartTimeout); err != nil { t.Fatal(err) } } diff --git a/virtcontainers/qemu.go b/virtcontainers/qemu.go index 60b6476890..59d9dbbb17 100644 --- a/virtcontainers/qemu.go +++ b/virtcontainers/qemu.go @@ -208,12 +208,9 @@ func (q *qemu) trace(name string) (opentracing.Span, context.Context) { return span, ctx } -// init intializes the Qemu structure. -func (q *qemu) init(ctx context.Context, id string, hypervisorConfig *HypervisorConfig, storage resourceStorage) error { - // save - q.ctx = ctx - - span, _ := q.trace("init") +// setup sets the Qemu structure up. +func (q *qemu) setup(id string, hypervisorConfig *HypervisorConfig, storage resourceStorage) error { + span, _ := q.trace("setup") defer span.Finish() err := hypervisorConfig.valid() @@ -418,10 +415,17 @@ func (q *qemu) setupTemplate(knobs *govmmQemu.Knobs, memory *govmmQemu.Memory) g } // createSandbox is the Hypervisor sandbox creation implementation for govmmQemu. -func (q *qemu) createSandbox() error { +func (q *qemu) createSandbox(ctx context.Context, id string, hypervisorConfig *HypervisorConfig, storage resourceStorage) error { + // Save the tracing context + q.ctx = ctx + span, _ := q.trace("createSandbox") defer span.Finish() + if err := q.setup(id, hypervisorConfig, storage); err != nil { + return err + } + machine, err := q.getQemuMachine() if err != nil { return err @@ -530,7 +534,7 @@ func (q *qemu) createSandbox() error { } // startSandbox will start the Sandbox's VM. -func (q *qemu) startSandbox() error { +func (q *qemu) startSandbox(timeout int) error { span, _ := q.trace("startSandbox") defer span.Finish() @@ -574,7 +578,7 @@ func (q *qemu) startSandbox() error { return fmt.Errorf("%s", strErr) } - return nil + return q.waitSandbox(timeout) } // waitSandbox will wait for the Sandbox's VM to be up and running. diff --git a/virtcontainers/qemu_test.go b/virtcontainers/qemu_test.go index 3bd09e3cd4..73ad6adfc9 100644 --- a/virtcontainers/qemu_test.go +++ b/virtcontainers/qemu_test.go @@ -70,7 +70,7 @@ func TestQemuKernelParameters(t *testing.T) { testQemuKernelParameters(t, params, expectedOut, false) } -func TestQemuInit(t *testing.T) { +func TestQemuCreateSandbox(t *testing.T) { qemuConfig := newQemuConfig() q := &qemu{} @@ -82,13 +82,20 @@ func TestQemuInit(t *testing.T) { }, } + // Create the hypervisor fake binary + testQemuPath := filepath.Join(testDir, testHypervisor) + _, err := os.Create(testQemuPath) + if err != nil { + t.Fatalf("Could not create hypervisor file %s: %v", testQemuPath, err) + } + // Create parent dir path for hypervisor.json parentDir := filepath.Join(runStoragePath, sandbox.id) if err := os.MkdirAll(parentDir, dirMode); err != nil { t.Fatalf("Could not create parent directory %s: %v", parentDir, err) } - if err := q.init(context.Background(), sandbox.id, &sandbox.config.HypervisorConfig, sandbox.storage); err != nil { + if err := q.createSandbox(context.Background(), sandbox.id, &sandbox.config.HypervisorConfig, sandbox.storage); err != nil { t.Fatal(err) } @@ -101,7 +108,7 @@ func TestQemuInit(t *testing.T) { } } -func TestQemuInitMissingParentDirFail(t *testing.T) { +func TestQemuCreateSandboxMissingParentDirFail(t *testing.T) { qemuConfig := newQemuConfig() q := &qemu{} @@ -113,14 +120,21 @@ func TestQemuInitMissingParentDirFail(t *testing.T) { }, } + // Create the hypervisor fake binary + testQemuPath := filepath.Join(testDir, testHypervisor) + _, err := os.Create(testQemuPath) + if err != nil { + t.Fatalf("Could not create hypervisor file %s: %v", testQemuPath, err) + } + // Ensure parent dir path for hypervisor.json does not exist. parentDir := filepath.Join(runStoragePath, sandbox.id) if err := os.RemoveAll(parentDir); err != nil { t.Fatal(err) } - if err := q.init(context.Background(), sandbox.id, &sandbox.config.HypervisorConfig, sandbox.storage); err != nil { - t.Fatalf("Qemu init() is not expected to fail because of missing parent directory for storage: %v", err) + if err := q.createSandbox(context.Background(), sandbox.id, &sandbox.config.HypervisorConfig, sandbox.storage); err != nil { + t.Fatalf("Qemu createSandbox() is not expected to fail because of missing parent directory for storage: %v", err) } } diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index a34203b128..51a3a7f745 100644 --- a/virtcontainers/sandbox.go +++ b/virtcontainers/sandbox.go @@ -597,11 +597,7 @@ func newSandbox(ctx context.Context, sandboxConfig SandboxConfig, factory Factor } }() - if err = s.hypervisor.init(ctx, s.id, &sandboxConfig.HypervisorConfig, s.storage); err != nil { - return nil, err - } - - if err = s.hypervisor.createSandbox(); err != nil { + if err = s.hypervisor.createSandbox(ctx, s.id, &sandboxConfig.HypervisorConfig, s.storage); err != nil { return nil, err } @@ -951,15 +947,11 @@ func (s *Sandbox) startVM() error { return nil } - return s.hypervisor.startSandbox() + return s.hypervisor.startSandbox(vmStartTimeout) }); err != nil { return err } - if err := s.hypervisor.waitSandbox(vmStartTimeout); err != nil { - return err - } - // In case of vm factory, network interfaces are hotplugged // after vm is started. if s.factory != nil { diff --git a/virtcontainers/virtcontainers_test.go b/virtcontainers/virtcontainers_test.go index d752f92f7d..adf143b0bf 100644 --- a/virtcontainers/virtcontainers_test.go +++ b/virtcontainers/virtcontainers_test.go @@ -81,27 +81,36 @@ func TestMain(m *testing.M) { panic(err) } + fmt.Printf("INFO: Creating virtcontainers test directory %s\n", testDir) err = os.MkdirAll(testDir, dirMode) if err != nil { fmt.Println("Could not create test directories:", err) os.Exit(1) } - _, err = os.Create(filepath.Join(testDir, testKernel)) + testQemuKernelPath = filepath.Join(testDir, testKernel) + testQemuInitrdPath = filepath.Join(testDir, testInitrd) + testQemuImagePath = filepath.Join(testDir, testImage) + testQemuPath = filepath.Join(testDir, testHypervisor) + + fmt.Printf("INFO: Creating virtcontainers test kernel %s\n", testQemuKernelPath) + _, err = os.Create(testQemuKernelPath) if err != nil { fmt.Println("Could not create test kernel:", err) os.RemoveAll(testDir) os.Exit(1) } - _, err = os.Create(filepath.Join(testDir, testImage)) + fmt.Printf("INFO: Creating virtcontainers test image %s\n", testQemuImagePath) + _, err = os.Create(testQemuImagePath) if err != nil { fmt.Println("Could not create test image:", err) os.RemoveAll(testDir) os.Exit(1) } - _, err = os.Create(filepath.Join(testDir, testHypervisor)) + fmt.Printf("INFO: Creating virtcontainers test hypervisor %s\n", testQemuPath) + _, err = os.Create(testQemuPath) if err != nil { fmt.Println("Could not create test hypervisor:", err) os.RemoveAll(testDir) @@ -127,11 +136,6 @@ func TestMain(m *testing.M) { sandboxFileState = filepath.Join(runStoragePath, testSandboxID, stateFile) sandboxFileLock = filepath.Join(runStoragePath, testSandboxID, lockFileName) - testQemuKernelPath = filepath.Join(testDir, testKernel) - testQemuInitrdPath = filepath.Join(testDir, testInitrd) - testQemuImagePath = filepath.Join(testDir, testImage) - testQemuPath = filepath.Join(testDir, testHypervisor) - testHyperstartCtlSocket = filepath.Join(testDir, "test_hyper.sock") testHyperstartTtySocket = filepath.Join(testDir, "test_tty.sock") diff --git a/virtcontainers/vm.go b/virtcontainers/vm.go index 147a876d5f..43bffffe6c 100644 --- a/virtcontainers/vm.go +++ b/virtcontainers/vm.go @@ -118,11 +118,7 @@ func NewVM(ctx context.Context, config VMConfig) (*VM, error) { } }() - if err = hypervisor.init(ctx, id, &config.HypervisorConfig, &filesystem{}); err != nil { - return nil, err - } - - if err = hypervisor.createSandbox(); err != nil { + if err = hypervisor.createSandbox(ctx, id, &config.HypervisorConfig, &filesystem{}); err != nil { return nil, err } @@ -135,10 +131,7 @@ func NewVM(ctx context.Context, config VMConfig) (*VM, error) { } // 3. boot up guest vm - if err = hypervisor.startSandbox(); err != nil { - return nil, err - } - if err = hypervisor.waitSandbox(vmStartTimeout); err != nil { + if err = hypervisor.startSandbox(vmStartTimeout); err != nil { return nil, err } @@ -215,7 +208,7 @@ func (v *VM) Resume() error { // Start kicks off a configured VM. func (v *VM) Start() error { v.logger().Info("start vm") - return v.hypervisor.startSandbox() + return v.hypervisor.startSandbox(vmStartTimeout) } // Disconnect agent and proxy connections to a VM