Skip to content

fix: Clarify error message for podman restore without --tcp-established #29026

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
simek-m wants to merge 1 commit into
base: main
Choose a base branch
from
+98 −6
Open

fix: Clarify error message for podman restore without --tcp-established #29026

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions libpod/oci_conmon_common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1455,6 +1455,11 @@ func readConmonPipeData(runtimeName string, pipe *os.File, ociLog string) (int,
if ociLog != "" {
ociLogData, err := os.ReadFile(ociLog)
if err == nil {
if errs, err := parseOCIErrors(ociLogData); err == nil {
if tcpErr := checkOCIErrorsForTCPEstablished(errs); tcpErr != nil {
return -1, tcpErr
}
}
var ociErr ociError
if err := json.Unmarshal(ociLogData, &ociErr); err == nil {
return -1, getOCIRuntimeError(runtimeName, ociErr.Msg)
Expand All @@ -1468,6 +1473,11 @@ func readConmonPipeData(runtimeName string, pipe *os.File, ociLog string) (int,
if ociLog != "" {
ociLogData, err := os.ReadFile(ociLog)
if err == nil {
if errs, err := parseOCIErrors(ociLogData); err == nil {
if tcpErr := checkOCIErrorsForTCPEstablished(errs); tcpErr != nil {
return ss.si.Data, tcpErr
}
}
var ociErr ociError
if err := json.Unmarshal(ociLogData, &ociErr); err == nil {
return ss.si.Data, getOCIRuntimeError(runtimeName, ociErr.Msg)
Expand Down
29 changes: 29 additions & 0 deletions libpod/oci_util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package libpod

import (
"bytes"
"errors"
"fmt"
"net"
Expand Down Expand Up @@ -30,6 +31,34 @@ type ociError struct {
Msg string `json:"msg,omitempty"`
}

func parseOCIErrors(log []byte) ([]ociError, error) {
var errs []ociError
for _, line := range bytes.Split(log, []byte("\n")) {
line = bytes.TrimSpace(line)
if len(line) == 0 {
continue
}
var e ociError
if err := json.Unmarshal(line, &e); err != nil {
return nil, err
}
errs = append(errs, e)
}
return errs, nil
}

// checkOCIErrorsForTCPEstablished tries to match
// an ociError indicating established TCP connections
// and convert it to a user-facing message.
func checkOCIErrorsForTCPEstablished(errs []ociError) error {
for _, e := range errs {
if strings.Contains(e.Msg, "Connected TCP socket in image") {
return fmt.Errorf("checkpoint contains established TCP connections, restore requires --tcp-established or --tcp-close: %w", define.ErrOCIRuntime)
}
}

@Luap99 Luap99 Jun 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that does not seem maintainable, we should not make assumptions about the error text from crun or criu, it changes often enough. Sure the tests depend on it to some extend but we should not make the code depend, i.e. this may not work for another runtime.

It would be better to work with crun and criu to produce better errors to begin with.

return nil
}

// Bind ports to keep them closed on the host
func bindPorts(ports []types.PortMapping) ([]*os.File, error) {
var files []*os.File
Expand Down
53 changes: 53 additions & 0 deletions libpod/oci_util_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
//go:build !remote && (linux || freebsd)

package libpod

import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

// TestParseOCIErrors verifies parsing of multiple ociErrors
// from a log file.
func TestParseOCIErrors(t *testing.T) {
log := []byte(`{"msg":"(00.022100) Error (criu/sk-inet.c:654): Connected TCP socket in image","level":"error","time":"2025-01-01T00:00:00Z"}
{"msg":"(00.022200) Error (criu/cr-restore.c:2522): Restoring FAILED.","level":"error","time":"2025-01-01T00:00:01Z"}
{"msg":"(00.022418) Error (criu/cgroup.c:1970): cg: cgroupd: recv req error: No such file or directory","level":"error","time":"2025-01-01T00:00:02Z"}
`)

errs, err := parseOCIErrors(log)
require.NoError(t, err)

// Verify that all 3 messages are parsed.
assert.Equal(t, []ociError{
{Msg: "(00.022100) Error (criu/sk-inet.c:654): Connected TCP socket in image", Level: "error", Time: "2025-01-01T00:00:00Z"},
{Msg: "(00.022200) Error (criu/cr-restore.c:2522): Restoring FAILED.", Level: "error", Time: "2025-01-01T00:00:01Z"},
{Msg: "(00.022418) Error (criu/cgroup.c:1970): cg: cgroupd: recv req error: No such file or directory", Level: "error", Time: "2025-01-01T00:00:02Z"},
}, errs)
}

// TestCheckOCIErrorsForTCPEstablished tests matching
// of "Connected TCP socket in image" errors in a log file.
func TestCheckOCIErrorsForTCPEstablished(t *testing.T) {
withTCP := []ociError{
{Msg: "(00.022100) Error (criu/sk-inet.c:654): Connected TCP socket in image", Level: "error"},
{Msg: "(00.022200) Error (criu/cr-restore.c:2522): Restoring FAILED.", Level: "error"},
{Msg: "(00.022418) Error (criu/cgroup.c:1970): cg: cgroupd: recv req error: No such file or directory", Level: "error"},
}

err := checkOCIErrorsForTCPEstablished(withTCP)

// Verify that the log message is matched and expected error converted.
require.Error(t, err)
assert.Contains(t, err.Error(), "--tcp-established")

withoutTCP := []ociError{
{Msg: "some other OCI runtime error", Level: "error"},
}

// Verify that different message is not matched.
err = checkOCIErrorsForTCPEstablished(withoutTCP)
assert.NoError(t, err)
}
12 changes: 6 additions & 6 deletions test/e2e/checkpoint_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,9 +347,9 @@ var _ = Describe("Podman checkpoint", func() {
result = podmanTest.Podman([]string{"container", "restore", cid})
result.WaitWithDefaultTimeout()

// Some older versions print "CRIU restoring failed: -52" while others
// "Error: crun: (00.054135) Error (criu/cgroup.c:1998): cg: cgroupd: recv req error: No such file or directory: OCI runtime attempted to invoke a command that was not found"
expectStderr := "cg: cgroupd: recv req error|CRIU restoring failed: -52"
// Some older versions print "CRIU restoring failed: -52",
// for newer versions the error message is converted.
expectStderr := "checkpoint contains established TCP connections, restore requires --tcp-established or --tcp-close|CRIU restoring failed: -52"
if podmanTest.OCIRuntime == "runc" {
expectStderr = "runc: criu failed: type NOTIFY errno 0"
}
Expand Down Expand Up @@ -391,9 +391,9 @@ var _ = Describe("Podman checkpoint", func() {
result := podmanTest.Podman([]string{"container", "restore", cid})
result.WaitWithDefaultTimeout()

// Some older versions print "CRIU restoring failed: -52" while others
// "Error: crun: (00.054135) Error (criu/cgroup.c:1998): cg: cgroupd: recv req error: No such file or directory: OCI runtime attempted to invoke a command that was not found"
expectStderr := "cg: cgroupd: recv req error|CRIU restoring failed: -52"
// Some older versions print "CRIU restoring failed: -52",
// for newer versions the error message is converted.
expectStderr := "checkpoint contains established TCP connections, restore requires --tcp-established or --tcp-close|CRIU restoring failed: -52"
if podmanTest.OCIRuntime == "runc" {
expectStderr = "runc: criu failed: type NOTIFY errno 0"
}
Expand Down