From ebf8547c38756fa51f95dcac110d8008019bef01 Mon Sep 17 00:00:00 2001 From: Samuel Ortiz Date: Sun, 9 Dec 2018 12:35:07 +0100 Subject: [PATCH 1/3] virtcontainers: Remove useless startSandbox wrapper startSandbox() wraps a single operation (sandbox.Start()), so we can remove it and make the code easier to read/follow. Signed-off-by: Samuel Ortiz --- virtcontainers/api.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/virtcontainers/api.go b/virtcontainers/api.go index c5162488c9..d0de395bea 100644 --- a/virtcontainers/api.go +++ b/virtcontainers/api.go @@ -234,12 +234,8 @@ func StartSandbox(ctx context.Context, sandboxID string) (VCSandbox, error) { } defer s.releaseStatelessSandbox() - return startSandbox(s) -} - -func startSandbox(s *Sandbox) (*Sandbox, error) { // Start it - err := s.Start() + err = s.Start() if err != nil { return nil, err } @@ -285,6 +281,7 @@ func RunSandbox(ctx context.Context, sandboxConfig SandboxConfig, factory Factor span, ctx := trace(ctx, "RunSandbox") defer span.Finish() + // Create the sandbox s, err := createSandboxFromConfig(ctx, sandboxConfig, factory) if err != nil { return nil, err @@ -297,7 +294,13 @@ func RunSandbox(ctx context.Context, sandboxConfig SandboxConfig, factory Factor } defer unlockSandbox(lockFile) - return startSandbox(s) + // Start the sandbox + err = s.Start() + if err != nil { + return nil, err + } + + return s, nil } // ListSandbox is the virtcontainers sandbox listing entry point. From acf833cb4a6772a087ac5529c81e05374ea6b41a Mon Sep 17 00:00:00 2001 From: Samuel Ortiz Date: Sun, 9 Dec 2018 19:24:33 +0100 Subject: [PATCH 2/3] virtcontainers: Call agent startSandbox from startVM We always ask the agent to start the sandbox when we start the VM, so we should simply call agent.startSandbox from startVM instead of open coding those. This slightly simplifies the complex createSandboxFromConfig routine. Fixes: #1011 Signed-off-by: Samuel Ortiz Signed-off-by: Eric Ernst --- virtcontainers/api.go | 16 +--------------- virtcontainers/sandbox.go | 15 ++++++++++++++- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/virtcontainers/api.go b/virtcontainers/api.go index d0de395bea..d83eee9cb7 100644 --- a/virtcontainers/api.go +++ b/virtcontainers/api.go @@ -105,21 +105,7 @@ func createSandboxFromConfig(ctx context.Context, sandboxConfig SandboxConfig, f } }() - // Once startVM is done, we want to guarantee - // that the sandbox is manageable. For that we need - // to start the sandbox inside the VM. - if err = s.agent.startSandbox(s); err != nil { - return nil, err - } - - // rollback to stop sandbox in VM - defer func() { - if err != nil { - s.agent.stopSandbox(s) - } - }() - - if err = s.getAndStoreGuestDetails(); err != nil { + if err := s.getAndStoreGuestDetails(); err != nil { return nil, err } diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index 589ef578e1..27343bbcae 100644 --- a/virtcontainers/sandbox.go +++ b/virtcontainers/sandbox.go @@ -1234,9 +1234,18 @@ func (s *Sandbox) startVM() error { } } - // Store the network s.Logger().Info("VM started") + // Once the hypervisor is done starting the sandbox, + // we want to guarantee that it is manageable. + // For that we need to ask the agent to start the + // sandbox inside the VM. + if err := s.agent.startSandbox(s); err != nil { + return err + } + + s.Logger().Info("Agent started in the sandbox") + return nil } @@ -1245,6 +1254,10 @@ func (s *Sandbox) stopVM() error { span, _ := s.trace("stopVM") defer span.Finish() + if err := s.agent.stopSandbox(s); err != nil { + s.Logger().WithError(err).WithField("sandboxid", s.id).Warning("Agent did not stop sandbox") + } + return s.hypervisor.stopSandbox() } From 09168ccda763fdc5a13fdd2888e581a638d05749 Mon Sep 17 00:00:00 2001 From: Samuel Ortiz Date: Tue, 18 Dec 2018 17:37:11 +0100 Subject: [PATCH 3/3] virtcontainers: Call stopVM() from sandbox.Stop() Now that stopVM() also calls agent.stopSandbox(), we can have the sandbox Stop() call using stopVM() directly and avoid code duplication. Fixes: #1011 Signed-off-by: Samuel Ortiz --- virtcontainers/sandbox.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index 27343bbcae..fb66ad119f 100644 --- a/virtcontainers/sandbox.go +++ b/virtcontainers/sandbox.go @@ -1254,10 +1254,12 @@ func (s *Sandbox) stopVM() error { span, _ := s.trace("stopVM") defer span.Finish() + s.Logger().Info("Stopping sandbox in the VM") if err := s.agent.stopSandbox(s); err != nil { s.Logger().WithError(err).WithField("sandboxid", s.id).Warning("Agent did not stop sandbox") } + s.Logger().Info("Stopping VM") return s.hypervisor.stopSandbox() } @@ -1592,12 +1594,7 @@ func (s *Sandbox) Stop() error { } } - if err := s.agent.stopSandbox(s); err != nil { - return err - } - - s.Logger().Info("Stopping VM") - if err := s.hypervisor.stopSandbox(); err != nil { + if err := s.stopVM(); err != nil { return err }