container.Destroy should clean up container metadata even if other cleanups fail

If the sandbox process is dead (because of a panic or some other problem),
container.Destroy will never remove the container metadata file, since it will
always fail when calling container.stop().

This CL changes container.Destroy() to always perform the three necessary
cleanup operations:
* Stop the sandbox and gofer processes.
* Remove the container fs on the host.
* Delete the container metadata directory.

Errors from these three operations will be concatenated and returned from
Destroy().

PiperOrigin-RevId: 225448164
Change-Id: I99c6311b2e4fe5f6e2ca991424edf1ebeae9df32
This commit is contained in:
Nicolas Lacasse 2018-12-13 15:36:59 -08:00 committed by Shentubot
parent a0c8aeb73d
commit 1775a0e11e
1 changed files with 25 additions and 5 deletions

View File

@ -626,20 +626,36 @@ func (c *Container) Processes() ([]*control.Process, error) {
} }
// Destroy stops all processes and frees all resources associated with the // Destroy stops all processes and frees all resources associated with the
// container. It fails fast and is idempotent. // container.
func (c *Container) Destroy() error { func (c *Container) Destroy() error {
log.Debugf("Destroy container %q", c.ID) log.Debugf("Destroy container %q", c.ID)
// We must perform the following cleanup steps:
// * stop the container and gofer processes,
// * remove the container filesystem on the host, and
// * delete the container metadata directory.
//
// It's possible for one or more of these steps to fail, but we should
// do our best to perform all of the cleanups. Hence, we keep a slice
// of errors return their concatenation.
var errs []string
if err := c.stop(); err != nil { if err := c.stop(); err != nil {
return fmt.Errorf("error stopping container: %v", err) err = fmt.Errorf("error stopping container: %v", err)
log.Warningf("%v", err)
errs = append(errs, err.Error())
} }
if err := destroyFS(c.Spec); err != nil { if err := destroyFS(c.Spec); err != nil {
return fmt.Errorf("error destroying container fs: %v", err) err = fmt.Errorf("error destroying container fs: %v", err)
log.Warningf("%v", err)
errs = append(errs, err.Error())
} }
if err := os.RemoveAll(c.Root); err != nil && !os.IsNotExist(err) { if err := os.RemoveAll(c.Root); err != nil && !os.IsNotExist(err) {
return fmt.Errorf("error deleting container root directory %q: %v", c.Root, err) err = fmt.Errorf("error deleting container root directory %q: %v", c.Root, err)
log.Warningf("%v", err)
errs = append(errs, err.Error())
} }
// "If any poststop hook fails, the runtime MUST log a warning, but the // "If any poststop hook fails, the runtime MUST log a warning, but the
@ -655,8 +671,12 @@ func (c *Container) Destroy() error {
} }
c.changeStatus(Stopped) c.changeStatus(Stopped)
if len(errs) == 0 {
return nil return nil
} }
return fmt.Errorf(strings.Join(errs, "\n"))
}
// save saves the container metadata to a file. // save saves the container metadata to a file.
// //