From df02cb51ee69c8c92fc0e4e1efd86467a9087a91 Mon Sep 17 00:00:00 2001 From: Jake Correnti Date: Sat, 10 Dec 2022 19:17:50 -0500 Subject: [PATCH] Add container error message to ContainerState This change aims to store an error message to the ContainerState struct with the last known error from the Start, StartAndAttach, and Stop OCI Runtime functions. The goal was to act in accordance with Docker's behavior. Fixes: #13729 Signed-off-by: Jake Correnti --- libpod/container.go | 2 ++ libpod/container_api.go | 35 ++++++++++++++++++++++++++++++++--- libpod/container_inspect.go | 2 +- test/e2e/inspect_test.go | 20 ++++++++++++++++++++ 4 files changed, 55 insertions(+), 4 deletions(-) diff --git a/libpod/container.go b/libpod/container.go index 091fa3340d..b92255d7ed 100644 --- a/libpod/container.go +++ b/libpod/container.go @@ -152,6 +152,8 @@ type ContainerState struct { ExitCode int32 `json:"exitCode,omitempty"` // Exited is whether the container has exited Exited bool `json:"exited,omitempty"` + // Error holds the last known error message during start, stop, or remove + Error string `json:"error,omitempty"` // OOMKilled indicates that the container was killed as it ran out of // memory OOMKilled bool `json:"oomKilled,omitempty"` diff --git a/libpod/container_api.go b/libpod/container_api.go index 71f97a240b..d9bd4ea8b4 100644 --- a/libpod/container_api.go +++ b/libpod/container_api.go @@ -82,7 +82,15 @@ func (c *Container) Init(ctx context.Context, recursive bool) error { // Start requites that all dependency containers (e.g. pod infra containers) be // running before being run. The recursive parameter, if set, will start all // dependencies before starting this container. -func (c *Container) Start(ctx context.Context, recursive bool) error { +func (c *Container) Start(ctx context.Context, recursive bool) (finalErr error) { + defer func() { + if finalErr != nil { + if err := saveContainerError(c, finalErr); err != nil { + logrus.Debug(err) + } + } + }() + if !c.batched { c.lock.Lock() defer c.lock.Unlock() @@ -114,7 +122,15 @@ func (c *Container) Update(res *spec.LinuxResources) error { // Attach call occurs before Start). // In overall functionality, it is identical to the Start call, with the added // side effect that an attach session will also be started. -func (c *Container) StartAndAttach(ctx context.Context, streams *define.AttachStreams, keys string, resize <-chan resize.TerminalSize, recursive bool) (<-chan error, error) { +func (c *Container) StartAndAttach(ctx context.Context, streams *define.AttachStreams, keys string, resize <-chan resize.TerminalSize, recursive bool) (retChan <-chan error, finalErr error) { + defer func() { + if finalErr != nil { + if err := saveContainerError(c, finalErr); err != nil { + logrus.Debug(err) + } + } + }() + if !c.batched { c.lock.Lock() defer c.lock.Unlock() @@ -193,7 +209,15 @@ func (c *Container) Stop() error { // StopWithTimeout is a version of Stop that allows a timeout to be specified // manually. If timeout is 0, SIGKILL will be used immediately to kill the // container. -func (c *Container) StopWithTimeout(timeout uint) error { +func (c *Container) StopWithTimeout(timeout uint) (finalErr error) { + defer func() { + if finalErr != nil { + if err := saveContainerError(c, finalErr); err != nil { + logrus.Debug(err) + } + } + }() + if !c.batched { c.lock.Lock() defer c.lock.Unlock() @@ -1037,3 +1061,8 @@ func (c *Container) Stat(ctx context.Context, containerPath string) (*define.Fil info, _, _, err := c.stat(mountPoint, containerPath) return info, err } + +func saveContainerError(c *Container, err error) error { + c.state.Error = err.Error() + return c.save() +} diff --git a/libpod/container_inspect.go b/libpod/container_inspect.go index e9e0a8bc2d..1e7dbcde99 100644 --- a/libpod/container_inspect.go +++ b/libpod/container_inspect.go @@ -129,7 +129,7 @@ func (c *Container) getContainerInspectData(size bool, driverData *define.Driver Pid: runtimeInfo.PID, ConmonPid: runtimeInfo.ConmonPID, ExitCode: runtimeInfo.ExitCode, - Error: "", // can't get yet + Error: runtimeInfo.Error, StartedAt: runtimeInfo.StartedTime, FinishedAt: runtimeInfo.FinishedTime, Checkpointed: runtimeInfo.Checkpointed, diff --git a/test/e2e/inspect_test.go b/test/e2e/inspect_test.go index f2ebb605b5..4851737c5a 100644 --- a/test/e2e/inspect_test.go +++ b/test/e2e/inspect_test.go @@ -557,4 +557,24 @@ var _ = Describe("Podman inspect", func() { session.WaitWithDefaultTimeout() Expect(session).Should(Exit(0)) }) + + It("podman inspect container with bad create args", func() { + session := podmanTest.Podman([]string{"container", "create", ALPINE, "efcho", "Hello World"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + cid := session.OutputToString() + session = podmanTest.Podman([]string{"container", "inspect", cid, "-f", "{{ .State.Error }}"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + Expect(session.OutputToString()).To(HaveLen(0)) + + session = podmanTest.Podman([]string{"start", cid}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(125)) + session = podmanTest.Podman([]string{"container", "inspect", cid, "-f", "'{{ .State.Error }}"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + Expect(session.OutputToString()).To(Not(HaveLen(0))) + }) + })