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: containers#13729 Signed-off-by: Jake Correnti <[email protected]>
jakecorrenti · Jan 3, 2023 · df02cb5 · df02cb5
1 parent 3fbf62e
commit df02cb5
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 4 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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)))
+	})
+
 })