117 lines
3.6 KiB
Diff
117 lines
3.6 KiB
Diff
From 489e5bfbed5faff99d1fa48c146bd5a4f17b9c67 Mon Sep 17 00:00:00 2001
|
|
From: Kir Kolyshkin <kolyshkin@gmail.com>
|
|
Date: Mon, 6 Nov 2023 15:38:11 -0800
|
|
Subject: [PATCH] runc delete: do not ignore error from destroy
|
|
|
|
If container.Destroy() has failed, runc destroy still return 0, which is
|
|
wrong and can result in other issues down the line.
|
|
|
|
Let's always return error from destroy in runc delete.
|
|
|
|
For runc checkpoint and runc run, we still treat it as a warning.
|
|
|
|
Co-authored-by: Zhang Tianyang <burning9699@gmail.com>
|
|
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
|
|
---
|
|
checkpoint.go | 6 +++++-
|
|
delete.go | 7 ++-----
|
|
libcontainer/container_linux.go | 5 ++++-
|
|
utils_linux.go | 10 +++-------
|
|
4 files changed, 14 insertions(+), 14 deletions(-)
|
|
|
|
diff --git a/checkpoint.go b/checkpoint.go
|
|
index 32a62a8b..897564c1 100644
|
|
--- a/checkpoint.go
|
|
+++ b/checkpoint.go
|
|
@@ -65,7 +65,11 @@ checkpointed.`,
|
|
options := criuOptions(context)
|
|
if !(options.LeaveRunning || options.PreDump) {
|
|
// destroy container unless we tell CRIU to keep it
|
|
- defer destroy(container)
|
|
+ defer func() {
|
|
+ if err := container.Destroy(); err != nil {
|
|
+ logrus.Warn(err)
|
|
+ }
|
|
+ }()
|
|
}
|
|
// these are the mandatory criu options for a container
|
|
setPageServer(context, options)
|
|
diff --git a/delete.go b/delete.go
|
|
index 799c2a77..6fe776d0 100644
|
|
--- a/delete.go
|
|
+++ b/delete.go
|
|
@@ -18,8 +18,7 @@ func killContainer(container libcontainer.Container) error {
|
|
for i := 0; i < 10; i++ {
|
|
time.Sleep(100 * time.Millisecond)
|
|
if err := container.Signal(unix.Signal(0), false); err != nil {
|
|
- destroy(container)
|
|
- return nil
|
|
+ return container.Destroy()
|
|
}
|
|
}
|
|
return errors.New("container init still running")
|
|
@@ -72,7 +71,7 @@ status of "ubuntu01" as "stopped" the following will delete resources held for
|
|
}
|
|
switch s {
|
|
case libcontainer.Stopped:
|
|
- destroy(container)
|
|
+ return container.Destroy()
|
|
case libcontainer.Created:
|
|
return killContainer(container)
|
|
default:
|
|
@@ -81,7 +80,5 @@ status of "ubuntu01" as "stopped" the following will delete resources held for
|
|
}
|
|
return fmt.Errorf("cannot delete container %s that is not stopped: %s", id, s)
|
|
}
|
|
-
|
|
- return nil
|
|
},
|
|
}
|
|
diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go
|
|
index f56d73e6..4f9433b8 100644
|
|
--- a/libcontainer/container_linux.go
|
|
+++ b/libcontainer/container_linux.go
|
|
@@ -708,7 +708,10 @@ func (c *linuxContainer) newInitConfig(process *Process) *initConfig {
|
|
func (c *linuxContainer) Destroy() error {
|
|
c.m.Lock()
|
|
defer c.m.Unlock()
|
|
- return c.state.destroy()
|
|
+ if err := c.state.destroy(); err != nil {
|
|
+ return fmt.Errorf("unable to destroy container: %w", err)
|
|
+ }
|
|
+ return nil
|
|
}
|
|
|
|
func (c *linuxContainer) Pause() error {
|
|
diff --git a/utils_linux.go b/utils_linux.go
|
|
index a9badf20..20a949a1 100644
|
|
--- a/utils_linux.go
|
|
+++ b/utils_linux.go
|
|
@@ -114,12 +114,6 @@ func newProcess(p specs.Process) (*libcontainer.Process, error) {
|
|
return lp, nil
|
|
}
|
|
|
|
-func destroy(container libcontainer.Container) {
|
|
- if err := container.Destroy(); err != nil {
|
|
- logrus.Error(err)
|
|
- }
|
|
-}
|
|
-
|
|
// setupIO modifies the given process config according to the options.
|
|
func setupIO(process *libcontainer.Process, rootuid, rootgid int, createTTY, detach bool, sockpath string) (*tty, error) {
|
|
if createTTY {
|
|
@@ -323,7 +317,9 @@ func (r *runner) run(config *specs.Process) (int, error) {
|
|
|
|
func (r *runner) destroy() {
|
|
if r.shouldDestroy {
|
|
- destroy(r.container)
|
|
+ if err := r.container.Destroy(); err != nil {
|
|
+ logrus.Warn(err)
|
|
+ }
|
|
}
|
|
}
|
|
|
|
--
|
|
2.33.0
|
|
|